From f9f8eed94b156a19a6f0187a4bb5b07859e940b0 Mon Sep 17 00:00:00 2001 From: Timur Kumakbayev Date: Sat, 17 Jan 2026 17:20:28 +0500 Subject: [PATCH] Active Cluster Support, Pod Quota Fixes,Token Cache and CI Improvements (#81) feat: add Active Cluster support and improve plugin stability ## Major Features ### Active Cluster Support (Experimental) (#42) - Add support for multiple PureStorage arrays in Active Cluster configuration - Automatic volume connection on all arrays for high availability - Configure via comma-separated addresses and tokens ### Pod Quota Limit Support (#69) - Add proper handling of pod quota_limit parameter - Use quota_limit when set, fall back to array capacity when unlimited - Fix capacity reporting for pods with quotas ### Session Token Caching - Implement session token caching in /etc/pve/priv/purestorage/ - Automatic token refresh at 80% of TTL to prevent expiration - In-memory and file-based caching with jitter to prevent thundering herd ### Debug Logging - Add configurable debug logging with 4 levels (0-3) - Support both config parameter and PURESTORAGE_DEBUG environment variable ## Bug Fixes - Fix volume deletion when connected to multiple hosts - Query all connections before destroy - Disconnect from all hosts on all arrays - Prevents "Cannot destroy volume because it is currently connected" error ## CI/CD Improvements - Add workflow_dispatch trigger to checks workflow - Add option to check all files or only changed files - Add comprehensive markdown linting with markdownlint-cli2 ## Testing - Add token caching tests (tests/token_cache_test.pl) - Test token validation, expiration, and race conditions - Test cleanup of expired cache files ## Refactoring (#72) - Refactor volume removal logic to handle multiple host connections - Improve device cleanup sequence (LVM, partitions, multipath) - Extract connection querying logic before volume destruction - Refactor CI/CD workflows for better maintainability and flexibility - Improve error handling and logging throughout the plugin - Enhance Active Cluster support with proper multi-array operations ## Code Formatting - Format all Perl files with perltidy using .perltidyrc configuration - Ensure consistent code style across the codebase - Fix formatting issues in PureStoragePlugin.pm and test files ## Documentation - Update README with new functionality --- .github/workflows/checks.yml | 112 ++++ .github/workflows/lint.yml | 166 +++++ .github/workflows/tests.yml | 33 + .gitignore | 1 + .markdownlint.json | 6 + PureStoragePlugin.pm | 910 +++++++++++++++++++++++---- README.md | 169 ++++- tests/README.md | 31 + tests/run_tests.sh | 67 ++ tests/token_cache_test.pl | 205 ++++++ tests/unit/test_command_validation.t | 128 ++++ tests/unit/test_retry_logic.t | 141 +++++ tests/unit/test_token_cache.t | 153 +++++ 13 files changed, 1986 insertions(+), 136 deletions(-) create mode 100644 .github/workflows/checks.yml create mode 100644 .github/workflows/lint.yml create mode 100644 .github/workflows/tests.yml create mode 100644 .markdownlint.json create mode 100644 tests/README.md create mode 100755 tests/run_tests.sh create mode 100755 tests/token_cache_test.pl create mode 100644 tests/unit/test_command_validation.t create mode 100644 tests/unit/test_retry_logic.t create mode 100644 tests/unit/test_token_cache.t diff --git a/.github/workflows/checks.yml b/.github/workflows/checks.yml new file mode 100644 index 0000000..740e5ca --- /dev/null +++ b/.github/workflows/checks.yml @@ -0,0 +1,112 @@ +name: Checks +run-name: Checks on ${{ github.ref_name }} by ${{ github.actor }} + +on: + pull_request: + types: + - opened + - synchronize + - reopened + workflow_dispatch: + inputs: + check_all_files: + description: 'Check all files (not just changed)' + required: false + type: boolean + default: true + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +jobs: + check-changes: + name: Check changed files + runs-on: ubuntu-22.04 + outputs: + check-all: ${{ steps.set-check-all.outputs.check-all }} + perl-changed: ${{ steps.changed-files-perl.outputs.any_changed }} + markdown-changed: ${{ steps.changed-files-markdown.outputs.any_changed }} + test-changed: ${{ steps.changed-files-test.outputs.any_changed }} + perl-files: ${{ steps.set-perl-files.outputs.files }} + markdown-files: ${{ steps.set-markdown-files.outputs.files }} + steps: + - name: Checkout code + uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Determine if we should check all files + id: set-check-all + run: | + # Check all files only for manual workflow dispatch with check_all_files=true + # For PRs, check only changed files + if [ "${{ github.event_name }}" == "workflow_dispatch" ] && [ "${{ inputs.check_all_files }}" == "true" ]; then + echo "check-all=true" >> $GITHUB_OUTPUT + else + echo "check-all=false" >> $GITHUB_OUTPUT + fi + + - name: Check changed Perl files + id: changed-files-perl + uses: tj-actions/changed-files@v44 + with: + files: | + **/*.pm + **/*.pl + + - name: Check changed Markdown files + id: changed-files-markdown + uses: tj-actions/changed-files@v44 + with: + files: | + **/*.md + + - name: Check changed test files + id: changed-files-test + uses: tj-actions/changed-files@v44 + with: + files: | + tests/**/*.t + tests/**/*.pl + + - name: Set Perl files to check + id: set-perl-files + run: | + if [ "${{ steps.set-check-all.outputs.check-all }}" == "true" ]; then + files=$(find . -type f \( -name "*.pm" -o -name "*.pl" \) ! -path "*/node_modules/*" ! -path "*/.git/*" | tr '\n' ' ') + echo "files=$files" >> $GITHUB_OUTPUT + else + echo "files=${{ steps.changed-files-perl.outputs.all_changed_files }}" >> $GITHUB_OUTPUT + fi + + - name: Set Markdown files to check + id: set-markdown-files + run: | + if [ "${{ steps.set-check-all.outputs.check-all }}" == "true" ]; then + files=$(find . -type f -name "*.md" ! -path "*/node_modules/*" ! -path "*/.git/*" | tr '\n' ' ') + echo "files=$files" >> $GITHUB_OUTPUT + else + echo "files=${{ steps.changed-files-markdown.outputs.all_changed_files }}" >> $GITHUB_OUTPUT + fi + + lint: + name: Lint + needs: check-changes + if: | + (needs.check-changes.outputs.check-all == 'true') || + (needs.check-changes.outputs.perl-changed == 'true' || needs.check-changes.outputs.markdown-changed == 'true') + uses: ./.github/workflows/lint.yml + with: + enable-perl-lint: ${{ needs.check-changes.outputs.check-all == 'true' || needs.check-changes.outputs.perl-changed == 'true' }} + enable-markdown-lint: ${{ needs.check-changes.outputs.check-all == 'true' || needs.check-changes.outputs.markdown-changed == 'true' }} + perl-files: ${{ needs.check-changes.outputs.perl-files }} + markdown-files: ${{ needs.check-changes.outputs.markdown-files }} + + test: + name: Tests + needs: check-changes + if: | + (needs.check-changes.outputs.check-all == 'true') || + (needs.check-changes.outputs.perl-changed == 'true' || needs.check-changes.outputs.test-changed == 'true') + uses: ./.github/workflows/tests.yml diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml new file mode 100644 index 0000000..96a302e --- /dev/null +++ b/.github/workflows/lint.yml @@ -0,0 +1,166 @@ +name: Lint +run-name: Lint ${{ inputs.enable-perl-lint && 'Perl' || '' }} ${{ inputs.enable-markdown-lint && 'Markdown' || '' }} + +on: + workflow_call: + inputs: + enable-perl-lint: + description: 'Enable Perl linting' + required: false + type: boolean + default: false + enable-markdown-lint: + description: 'Enable Markdown linting' + required: false + type: boolean + default: false + perl-files: + description: 'List of changed Perl files' + required: false + type: string + default: '' + markdown-files: + description: 'List of changed Markdown files' + required: false + type: string + default: '' + +jobs: + perl-lint: + name: Perl Lint + if: ${{ inputs.enable-perl-lint == true }} + runs-on: ubuntu-22.04 + steps: + - name: Checkout code + uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Setup Perl + uses: shogo82148/actions-setup-perl@v1 + with: + perl-version: '5.34' + + - name: Install cpanm + run: | + curl -L https://cpanmin.us | perl - App::cpanminus + + - name: Install linting tools + run: | + cpanm --notest Perl::Critic + sudo apt-get update + sudo apt-get install -y perltidy || true + + - name: Check code formatting with perltidy + if: inputs.perl-files != '' + run: | + echo "Checking code formatting..." + changed_files="${{ inputs.perl-files }}" + if [ -z "$changed_files" ] || [ "$changed_files" == " " ]; then + echo "No Perl files to check" + exit 0 + fi + if [ ! -f ".perltidyrc" ]; then + echo "Warning: .perltidyrc not found, using default perltidy settings" + fi + for file in $changed_files; do + if [[ -n "$file" ]] && ([[ "$file" == *.pm ]] || [[ "$file" == *.pl ]]); then + echo "Checking formatting: $file" + if command -v perltidy >/dev/null 2>&1; then + formatted_file="${file}.formatted" + if [ -f ".perltidyrc" ]; then + perltidy -pro=.perltidyrc "$file" -o "$formatted_file" 2>&1 || true + else + perltidy "$file" -o "$formatted_file" 2>&1 || true + fi + if [ -f "$formatted_file" ]; then + if ! diff -q "$file" "$formatted_file" >/dev/null 2>&1; then + echo "::error file=$file::File is not properly formatted. Run 'perltidy -pro=.perltidyrc $file' to fix." + echo "Differences:" + diff "$file" "$formatted_file" | head -20 || true + rm -f "$formatted_file" + exit 1 + fi + rm -f "$formatted_file" + else + echo "Warning: perltidy failed to create formatted file for $file" + fi + else + echo "Warning: perltidy not found, skipping formatting check" + fi + fi + done + find . -name '*.formatted' -delete 2>/dev/null || true + + - name: Run Perl::Critic (optional) + if: inputs.perl-files != '' + continue-on-error: true + run: | + echo "Running Perl::Critic static analysis..." + if command -v perlcritic >/dev/null 2>&1; then + changed_files="${{ inputs.perl-files }}" + if [ -z "$changed_files" ] || [ "$changed_files" == " " ]; then + echo "No Perl files to analyze" + exit 0 + fi + for file in $changed_files; do + if [[ -n "$file" ]] && [[ "$file" == *.pm ]]; then + echo "Analyzing: $file" + perlcritic --quiet "$file" || true + fi + done + else + echo "Perl::Critic not available, skipping..." + fi + + markdown-lint: + name: Markdown Lint + if: ${{ inputs.enable-markdown-lint == true }} + runs-on: ubuntu-22.04 + steps: + - name: Checkout code + uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Setup Node.js + uses: actions/setup-node@v4 + with: + node-version: '20' + + - name: Install markdownlint + run: | + npm install -g markdownlint-cli2 || true + + - name: Lint Markdown files + if: inputs.markdown-files != '' + run: | + echo "Linting Markdown files..." + changed_files="${{ inputs.markdown-files }}" + if [ -z "$changed_files" ] || [ "$changed_files" == " " ]; then + echo "No Markdown files to lint" + exit 0 + fi + if command -v markdownlint-cli2 >/dev/null 2>&1; then + for file in $changed_files; do + if [[ -n "$file" ]] && [[ "$file" == *.md ]]; then + echo "Linting: $file" + markdownlint-cli2 "$file" "#node_modules" "#.git" || { + echo "::error file=$file::Markdown linting failed" + exit 1 + } + fi + done + elif command -v markdownlint >/dev/null 2>&1; then + for file in $changed_files; do + if [[ -n "$file" ]] && [[ "$file" == *.md ]]; then + echo "Linting: $file" + markdownlint "$file" || { + echo "::error file=$file::Markdown linting failed" + exit 1 + } + fi + done + else + echo "Markdown linter not available, skipping..." + fi diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml new file mode 100644 index 0000000..08c0e80 --- /dev/null +++ b/.github/workflows/tests.yml @@ -0,0 +1,33 @@ +name: Tests +run-name: Tests - ${{ github.ref_name }} + +on: + workflow_call: + +jobs: + test: + name: Run tests + runs-on: ubuntu-22.04 + steps: + - name: Checkout code + uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Setup Perl + uses: shogo82148/actions-setup-perl@v1 + with: + perl-version: '5.34' + + - name: Install cpanm + run: | + curl -L https://cpanmin.us | perl - App::cpanminus + + - name: Install Perl dependencies + run: | + cpanm --notest Test::More JSON JSON::XS + + - name: Run tests + run: | + chmod +x tests/run_tests.sh + ./tests/run_tests.sh diff --git a/.gitignore b/.gitignore index c0bd0f1..371605f 100644 --- a/.gitignore +++ b/.gitignore @@ -5,6 +5,7 @@ debian/libpve-storage-purestorage-perl debian/files debian/package debian/tmp +tests/test_results.txt /*.buildinfo /*.deb *.log diff --git a/.markdownlint.json b/.markdownlint.json new file mode 100644 index 0000000..9bcb265 --- /dev/null +++ b/.markdownlint.json @@ -0,0 +1,6 @@ +{ + "MD013": { + "tables": false, + "code_blocks": false + } +} diff --git a/PureStoragePlugin.pm b/PureStoragePlugin.pm index d228d3e..5d0752c 100644 --- a/PureStoragePlugin.pm +++ b/PureStoragePlugin.pm @@ -6,7 +6,6 @@ use warnings; use Data::Dumper qw( Dumper ); # DEBUG use IO::File (); -use Net::IP (); use File::Path (); use PVE::JSONSchema (); @@ -31,26 +30,59 @@ $Data::Dumper::Terse = 1; # Removes `$VAR1 =` in output $Data::Dumper::Indent = 1; # Outputs everything in one line $Data::Dumper::Useqq = 1; # Uses quotes for strings +# Error code constants for API requests +use constant { + ERROR_TOKEN_UPDATED => -1, # Token was refreshed (success, but need to update cache) + ERROR_SUCCESS => 0, # Request succeeded + ERROR_API_ERROR => 1, # PureStorage API returned an error + ERROR_NETWORK_ERROR => 2, # Network or connectivity error + ERROR_AUTH_FAILED => 3, # Authentication failed +}; + +# Token state constants for authentication state machine +use constant { + TOKEN_STATE_LOGIN => 0, # Performing login request (using api-token) + TOKEN_STATE_NEEDED => 1, # Need to obtain session token + TOKEN_STATE_CACHED => 2, # Have valid cached session token +}; + my $PSFA_API = '2.26'; my $purestorage_wwn_prefix = '3624a9370'; my $default_hgsuffix = ""; my $default_protocol = 'iscsi'; -my $DEBUG = 0; +# Global debug level (can be overridden per-storage or via environment) +my $DEBUG = $ENV{ PURESTORAGE_DEBUG } // 0; + +# Get effective debug level for a storage config +sub get_debug_level { + my ( $scfg ) = @_; + return $scfg->{ debug } if defined $scfg && defined $scfg->{ debug }; + return $DEBUG; +} + +# Set debug level from storage config (updates global $DEBUG) +sub set_debug_from_config { + my ( $scfg ) = @_; + if ( defined $scfg && defined $scfg->{ debug } ) { + $DEBUG = $scfg->{ debug }; + } +} ### BLOCK: Configuration sub api { - # PVE 5: APIVER 2 - # PVE 6: APIVER 3 - # PVE 6: APIVER 4 e6f4eed43581de9b9706cc2263c9631ea2abfc1a / volume_has_feature - # PVE 6: APIVER 5 a97d3ee49f21a61d3df10d196140c95dde45ec27 / allow rename - # PVE 6: APIVER 6 8f26b3910d7e5149bfa495c3df9c44242af989d5 / prune_backups (fine, we don't support that content type) - # PVE 6: APIVER 7 2c036838ed1747dabee1d2c79621c7d398d24c50 / volume_snapshot_needs_fsfreeze (guess we are fine, upstream only implemented it for RDBPlugin; we are not that different to let's say LVM in this regard) - # PVE 6: APIVER 8 343ca2570c3972f0fa1086b020bc9ab731f27b11 / prune_backups (fine again, see APIVER 6) - # PVE 7: APIVER 9 3cc29a0487b5c11592bf8b16e96134b5cb613237 / resets APIAGE! changes volume_import/volume_import_formats - # PVE 7.1: APIVER 10 a799f7529b9c4430fee13e5b939fe3723b650766 / rm/add volume_snapshot_{list,info} (not used); blockers to volume_rollback_is_possible (not used) - # PVE 8.4: APIVER 11 e2dc01ac9f06fe37cf434bad9157a50ecc4a99ce / new_backup_provider/sensitive_properties; backup provider might be interesting, we can look at it later - # PVE 9: APIVER 12 280bb6be777abdccd89b1b1d7bdd4feaba9af4c2 / qemu_blockdev_options/rename_snapshot/get_formats + +# PVE 5: APIVER 2 +# PVE 6: APIVER 3 +# PVE 6: APIVER 4 e6f4eed43581de9b9706cc2263c9631ea2abfc1a / volume_has_feature +# PVE 6: APIVER 5 a97d3ee49f21a61d3df10d196140c95dde45ec27 / allow rename +# PVE 6: APIVER 6 8f26b3910d7e5149bfa495c3df9c44242af989d5 / prune_backups (fine, we don't support that content type) +# PVE 6: APIVER 7 2c036838ed1747dabee1d2c79621c7d398d24c50 / volume_snapshot_needs_fsfreeze (guess we are fine, upstream only implemented it for RDBPlugin; we are not that different to let's say LVM in this regard) +# PVE 6: APIVER 8 343ca2570c3972f0fa1086b020bc9ab731f27b11 / prune_backups (fine again, see APIVER 6) +# PVE 7: APIVER 9 3cc29a0487b5c11592bf8b16e96134b5cb613237 / resets APIAGE! changes volume_import/volume_import_formats +# PVE 7.1: APIVER 10 a799f7529b9c4430fee13e5b939fe3723b650766 / rm/add volume_snapshot_{list,info} (not used); blockers to volume_rollback_is_possible (not used) +# PVE 8.4: APIVER 11 e2dc01ac9f06fe37cf434bad9157a50ecc4a99ce / new_backup_provider/sensitive_properties; backup provider might be interesting, we can look at it later +# PVE 9: APIVER 12 280bb6be777abdccd89b1b1d7bdd4feaba9af4c2 / qemu_blockdev_options/rename_snapshot/get_formats my $tested_apiver = 12; @@ -58,13 +90,13 @@ sub api { my $apiage = PVE::Storage::APIAGE; # the plugin supports multiple PVE generations, currently we did not break anything, tell them what they want to hear if possible - if ($apiver >= 2 and $apiver <= $tested_apiver) { - return $apiver; + if ( $apiver >= 2 and $apiver <= $tested_apiver ) { + return $apiver; } # if we are still in the APIAGE, we can still report what we have - if ($apiver - $apiage < $tested_apiver) { - return $tested_apiver; + if ( $apiver - $apiage < $tested_apiver ) { + return $tested_apiver; } # lowest apiver we support @@ -115,6 +147,18 @@ sub properties { type => 'string', default => $default_protocol }, + token_ttl => { + description => "Session token time-to-live in seconds.", + type => 'integer', + default => 3600 # Max 10h + }, + debug => { + description => "Enable debug logging (0=off, 1=basic, 2=verbose, 3=trace).", + type => 'integer', + minimum => 0, + maximum => 3, + default => 0 + }, }; } @@ -129,6 +173,8 @@ sub options { vnprefix => { optional => 1 }, check_ssl => { optional => 1 }, protocol => { optional => 1 }, + token_ttl => { optional => 1 }, + debug => { optional => 1 }, nodes => { optional => 1 }, disable => { optional => 1 }, content => { optional => 1 }, @@ -143,16 +189,77 @@ my $cmd = { # fuser => '/usr/bin/fuser', multipath => '/sbin/multipath', multipathd => '/sbin/multipathd', - blockdev => '/usr/sbin/blockdev' + blockdev => '/usr/sbin/blockdev', + dmsetup => '/sbin/dmsetup', + kpartx => '/sbin/kpartx', + udevadm => '/usr/bin/udevadm', + sync => '/usr/bin/sync' }; +# Get full path for a command, checking availability +sub get_command_path { + my ( $name ) = @_; + + # Check all commands on first use + ensure_commands_checked(); + + my $path = $cmd->{ $name }; + if ( !defined $path ) { + die "Error :: Unknown command '$name'\n"; + } + + if ( !-x $path ) { + die "Error :: Command '$name' not found or not executable at '$path'\n"; + } + + return $path; +} + +# Check if required commands are available on the system +sub check_commands { + my @missing; + + foreach my $name ( keys %$cmd ) { + my $path = $cmd->{ $name }; + if ( !-x $path ) { + push @missing, "$name ($path)"; + } + } + + if ( @missing ) { + warn "Warning :: The following commands are not available or not executable:\n"; + warn " - $_\n" foreach @missing; + warn "Plugin functionality may be limited.\n"; + } + + return scalar @missing == 0; +} + +# Check commands availability - called lazily on first use +my $commands_checked = 0; + +sub ensure_commands_checked { + return if $commands_checked; + check_commands(); + $commands_checked = 1; +} + sub exec_command { my ( $command, $dm, %param ) = @_; $dm //= 1; - my $fc = $cmd->{ $command->[0] }; - $command->[0] = $fc if defined $fc; + # Try to resolve command path if it's a known command name + my $cmd_name = $command->[0]; + if ( exists $cmd->{ $cmd_name } ) { + eval { $command->[0] = get_command_path( $cmd_name ); }; + if ( $@ ) { + + # Command not available, but continue with original name + # This allows system PATH resolution as fallback + warn "Warning :: $@" if $dm >= 0; + } + } print "Debug :: execute '" . join( ' ', @$command ) . "'\n" if $DEBUG >= 2; @@ -204,7 +311,8 @@ sub multipath_check { # TODO: Find a better check # TODO: Support non-multipath mode - my $output = `$cmd->{ multipath } -l $wwid`; + my $multipath_cmd = get_command_path( 'multipath' ); + my $output = `$multipath_cmd -l $wwid 2>/dev/null`; return $output ne ''; } @@ -246,7 +354,7 @@ sub wait_for { sub prepare_api_params { my ( $parms ) = @_; - print "Debug :: PVE::Storage::Custom::PureStoragePlugin::sub::prepare_api_params\n" if $DEBUG; + print "Debug :: PVE::Storage::Custom::PureStoragePlugin::sub::prepare_api_params\n" if $DEBUG >= 2; return $parms unless ref( $parms ) eq 'HASH'; @@ -417,11 +525,371 @@ sub block_device_slaves { return $device_path, @slaves; } +sub cleanup_lvm_on_device { + my ( $wwid ) = @_; + print "Debug :: PVE::Storage::Custom::PureStoragePlugin::sub::cleanup_lvm_on_device\n" if $DEBUG; + + my $cleaned = 0; + + my @dm_devices; + eval { + run_command( + [ get_command_path( 'dmsetup' ), 'ls' ], + outfunc => sub { + my $line = shift; + if ( $line =~ /^(\S+)\s+\(/ ) { + push @dm_devices, $1; + } + } + ); + }; + return 0 if $@; + + my @lvm_to_remove; + foreach my $dm ( @dm_devices ) { + next if $dm =~ /^${wwid}(-part\d+)?$/; + + my $deps = ''; + eval { + run_command( + [ get_command_path( 'dmsetup' ), 'deps', '-o', 'devname', $dm ], + outfunc => sub { $deps .= shift; }, + errfunc => sub { } + ); + }; + + if ( $deps =~ /${wwid}/ ) { + push @lvm_to_remove, $dm; + } + } + + foreach my $lvm ( reverse sort @lvm_to_remove ) { + print "Debug :: Removing LVM device: $lvm\n" if $DEBUG; + my $removed = 0; + + eval { + run_command( [ get_command_path( 'dmsetup' ), 'remove', $lvm ], errfunc => sub { } ); + $removed = 1; + }; + + if ( !$removed ) { + eval { + run_command( [ get_command_path( 'dmsetup' ), 'remove', '--force', $lvm ], errfunc => sub { } ); + $removed = 1; + }; + } + + $cleaned++ if $removed; + warn "Warning :: Failed to remove LVM device $lvm\n" unless $removed; + } + + return $cleaned; +} + +sub cleanup_partitions_on_device { + my ( $wwid ) = @_; + print "Debug :: PVE::Storage::Custom::PureStoragePlugin::sub::cleanup_partitions_on_device\n" if $DEBUG; + + my $cleaned = 0; + my $dm_path = '/dev/mapper/' . $wwid; + + eval { + run_command( [ get_command_path( 'kpartx' ), '-d', $dm_path ], errfunc => sub { } ); + $cleaned++; + }; + + opendir( my $dh, '/dev/mapper' ) or return $cleaned; + my @partitions = grep { /^${wwid}-part\d+$/ } readdir( $dh ); + closedir( $dh ); + + foreach my $part ( reverse sort @partitions ) { + print "Debug :: Removing partition: $part\n" if $DEBUG; + eval { + run_command( [ get_command_path( 'dmsetup' ), 'remove', '--force', $part ], errfunc => sub { } ); + $cleaned++; + }; + warn "Warning :: Failed to remove partition $part\n" if $@; + } + + return $cleaned; +} + +### BLOCK: Token cache management => PVE::Storage::Custom::PureStoragePlugin::sub::token_cache +# +# Race condition mitigation strategy: +# 1. Jitter in is_token_valid() spreads refresh timing across nodes (±2.5%) +# 2. Read-check-write pattern in save_token_to_cache() prevents overwriting newer tokens +# 3. On 401 error, re-check file cache before requesting new token (another node may have refreshed) +# 4. HTTP retry on 401 with max_retries=1 prevents infinite loops +# 5. pmxcfs replication is eventually consistent (typically <1s) +# +# Scenario: Two nodes refresh simultaneously +# - Node A and Node B both see expired token at ~80% TTL (with jitter spread) +# - Node A gets token T1, Node B gets token T2 +# - Node B tries to write T2, but sees T1 is already cached (created <5s ago), skips write +# - Both nodes use T1 from cache → success +# +# Edge case: If somehow both write (race in pmxcfs replication) +# - Node A has T1 in memory, but file has T2 +# - Node A gets 401 on next request +# - Node A re-reads cache, finds valid T2, uses it +# - Success without extra API call +# +# Error scenarios: +# 1. Node B login fails (network/API error) during simultaneous refresh +# - Node A successfully cached token T1 +# - Node B checks cache after login error, finds T1 +# - Node B uses T1 → continues operating +# - No service disruption +# +# 2. Worst case: Cache file deleted/corrupted during race +# - Node gets 401, cache re-read fails +# - Node requests new token (1 extra API call) +# - Result: At most 1 extra API call, system remains operational + +sub get_token_cache_path { + my ( $storeid, $array_index ) = @_; + + my $cache_dir = '/etc/pve/priv/purestorage'; + + # Create cache directory if it doesn't exist + if ( !-d $cache_dir ) { + eval { + File::Path::make_path( $cache_dir, { mode => 0700 } ); + print "Debug :: Created token cache directory: $cache_dir\n" if $DEBUG; + }; + if ( $@ ) { + warn "Warning :: Failed to create token cache directory $cache_dir: $@\n"; + return undef; + } + } + + return "$cache_dir/${storeid}_array${array_index}.json"; +} + +sub read_token_cache { + my ( $cache_path ) = @_; + + return undef unless defined $cache_path; + + if ( !-f $cache_path ) { + print "Debug :: Token cache file does not exist: $cache_path\n" if $DEBUG >= 2; + return undef; + } + + my $token_data; + eval { + my $json_text = PVE::Tools::file_get_contents( $cache_path ); + $token_data = decode_json( $json_text ); + print "Debug :: Read token cache from: $cache_path\n" if $DEBUG >= 1; + }; + if ( $@ ) { + warn "Warning :: Failed to read token cache from $cache_path: $@\n"; + + # Delete corrupt cache file + eval { unlink $cache_path }; + return undef; + } + + return $token_data; +} + +sub write_token_cache { + my ( $cache_path, $token_data ) = @_; + + return unless defined $cache_path; + + my $json_text = encode_json( $token_data ); + + # Atomic write: write to temp file, then rename + my $temp_path = "$cache_path.tmp.$$"; + + eval { + my $fh = IO::File->new( $temp_path, 'w', 0600 ) + or die "Cannot create temp file $temp_path: $!\n"; + print $fh $json_text . "\n"; + $fh->close(); + + rename( $temp_path, $cache_path ) + or die "Cannot rename $temp_path to $cache_path: $!\n"; + + print "Debug :: Wrote token cache to: $cache_path\n" if $DEBUG >= 1; + }; + if ( $@ ) { + warn "Warning :: Failed to write token cache to $cache_path: $@\n"; + + # Clean up temp file if it exists + eval { unlink $temp_path if -f $temp_path }; + die $@; + } +} + +sub is_token_valid { + my ( $token_data, $ttl ) = @_; + + return 0 unless defined $token_data; + return 0 unless defined $token_data->{ auth_token }; + return 0 unless defined $token_data->{ created_at }; + return 0 unless defined $token_data->{ ttl }; + + my $now = time(); + my $age = $now - $token_data->{ created_at }; + + # Add jitter (±5%) to refresh threshold to prevent thundering herd + # when multiple nodes check token expiration simultaneously + my $jitter = 0.05 * ( rand() - 0.5 ); # -2.5% to +2.5% + my $refresh_threshold = $ttl * ( 0.8 + $jitter ); + + print "Debug :: Token validation: now=$now, created_at=$token_data->{ created_at }, age=${age}s, threshold=${refresh_threshold}s\n" if $DEBUG >= 2; + + if ( $age < $refresh_threshold ) { + print "Debug :: Token is valid (age: ${age}s)\n" if $DEBUG >= 1; + return 1; + } + + print "Debug :: Token needs refresh (age: ${age}s >= threshold: ${refresh_threshold}s)\n" if $DEBUG >= 1; + return 0; +} + +sub cleanup_expired_cache { + my ( $cache_path, $ttl ) = @_; + + return unless defined $cache_path; + return unless -f $cache_path; + + my $token_data = read_token_cache( $cache_path ); + return unless defined $token_data; + + if ( defined $token_data->{ expires_at } ) { + my $now = time(); + if ( $now >= $token_data->{ expires_at } ) { + print "Debug :: Cleaning up expired token cache: $cache_path\n" if $DEBUG; + eval { unlink $cache_path }; + if ( $@ ) { + warn "Warning :: Failed to delete expired cache $cache_path: $@\n"; + } + } + } +} + +### BLOCK: API Helper functions => PVE::Storage::Custom::PureStoragePlugin::sub::api_helpers + +sub load_auth_token { + my ( $storeid, $array_index, $scfg ) = @_; + + my $cache_path = defined( $storeid ) ? get_token_cache_path( $storeid, $array_index ) : undef; + my $ttl = $scfg->{ token_ttl } || 3600; + + # Try in-memory cache first (fastest, no I/O) + my $mem_token_key = '_auth_token' . $array_index; + my $mem_request_id_key = '_request_id' . $array_index; + if ( defined( $scfg->{ $mem_token_key } ) && $scfg->{ $mem_token_key } ne '' ) { + print "Debug :: Using cached token from memory\n" if $DEBUG >= 2; + return ( $scfg->{ $mem_token_key }, $scfg->{ $mem_request_id_key }, $cache_path, $ttl ); + } + + # Try file cache + if ( $cache_path ) { + my $cached_token = read_token_cache( $cache_path ); + if ( $cached_token && is_token_valid( $cached_token, $ttl ) ) { + my $age = time() - $cached_token->{ created_at }; + print "Debug :: Using cached token from file (age: ${age}s)\n" if $DEBUG >= 1; + + # Update in-memory cache for faster access next time + $scfg->{ $mem_token_key } = $cached_token->{ auth_token }; + $scfg->{ $mem_request_id_key } = $cached_token->{ request_id }; + return ( $cached_token->{ auth_token }, $cached_token->{ request_id }, $cache_path, $ttl ); + } + } + + # File cache is expired or missing, return undef to force new token request + return ( undef, undef, $cache_path, $ttl ); +} + +sub save_token_to_cache { + my ( $config, $token_state ) = @_; + + # Only save if this was a login request + return unless $token_state == TOKEN_STATE_LOGIN; + + # Only save if we have cache path and token + return unless $config->{ cache_path } && $config->{ auth_token }; + + my $now = time(); + my $ttl = $config->{ ttl } || 3600; + + my $token_data = { + auth_token => $config->{ auth_token }, + request_id => $config->{ request_id }, + created_at => $now, + ttl => $ttl, + expires_at => $now + $ttl + }; + + eval { + # Race condition mitigation: check if another node already wrote a newer token + my $existing = read_token_cache( $config->{ cache_path } ); + if ( $existing && $existing->{ created_at } > $token_data->{ created_at } - 5 ) { + + # Another node wrote a token within last 5 seconds, use that instead + print "Debug :: Another node already cached a token, skipping write\n" if $DEBUG >= 2; + return; + } + + write_token_cache( $config->{ cache_path }, $token_data ); + print "Debug :: Token cached to file: $config->{ cache_path }\n" if $DEBUG >= 1; + }; + + if ( $@ ) { + warn "Warning :: Failed to write token cache: $@\n"; + } +} + +sub cleanup_token_cache { + my ( $config ) = @_; + + return unless $config->{ cache_path }; + + eval { cleanup_expired_cache( $config->{ cache_path }, $config->{ ttl } || 3600 ); }; +} + +sub is_ignorable_error { + my ( $action, $content ) = @_; + + my $ignore = $action->{ ignore }; + return 0 unless defined $ignore; + + # Normalize to array + $ignore = [$ignore] unless ref( $ignore ) eq 'ARRAY'; + + # Check if error message is in ignore list + my $error_msg = $content->{ errors }->[0]->{ message } // ''; + return grep { $_ eq $error_msg } @$ignore; +} + +sub try_cached_token { + my ( $config ) = @_; + + return 0 unless $config->{ cache_path }; + + my $cached_token = read_token_cache( $config->{ cache_path } ); + return 0 unless $cached_token && $cached_token->{ auth_token }; + return 0 unless is_token_valid( $cached_token, $config->{ ttl } || 3600 ); + + my $age = time() - $cached_token->{ created_at }; + print "Debug :: Using cached token from file (age: ${age}s)\n" if $DEBUG >= 1; + + $config->{ auth_token } = $cached_token->{ auth_token }; + $config->{ request_id } = $cached_token->{ request_id }; + + return 1; +} + ### BLOCK: Local multipath => PVE::Storage::Custom::PureStoragePlugin::sub::s -sub purestorage_api_request { - my ( $scfg, $action, $all ) = @_; - print "Debug :: PVE::Storage::Custom::PureStoragePlugin::sub::purestorage_api_request\n" if $DEBUG; +sub purestorage_api_call { + my ( $scfg, $action, $all, $storeid ) = @_; + print "Debug :: PVE::Storage::Custom::PureStoragePlugin::sub::purestorage_api_call\n" if $DEBUG >= 3; $all //= 0; @@ -449,106 +917,214 @@ sub purestorage_api_request { my @urls = split( ',', $scfg->{ address } // '' ); my @tokens = split( ',', $scfg->{ token } // '' ); + my $array_count = 0; + my $success_count = 0; + my $last_success_error = ERROR_SUCCESS; + my $last_success_content; + foreach my $i ( 0, 1 ) { $url = $urls[$i] // ''; my $token = $tokens[$i] // ''; next if $i && $url eq '' && $token eq ''; + $array_count++; + my $cf = $url eq '' ? 'address' : $token eq '' ? 'token' : ''; die "Error :: Pure Storage \"$cf\" parameter" . ( $i == 0 ? '' : ' for second array' ) . " is not defined.\n" unless $cf eq ''; + # Load auth token (file cache → memory cache → undef) + my ( $auth_token, $request_id, $cache_path, $ttl ) = load_auth_token( $storeid, $i, $scfg ); + my $config = { ua => $ua, url => $url, token => $token, - auth_token => $scfg->{ '_auth_token' . $i }, - request_id => $scfg->{ '_request_id' . $i } + auth_token => $auth_token, + request_id => $request_id, + cache_path => $cache_path, + ttl => $ttl }; - ( $error, $content ) = purestorage_api_request1( $config, $path, $method, $login, $body ); - if ( $error == -1 ) { + ( $error, $content ) = purestorage_http_request( $config, $path, $method, $login, $body ); + + # Handle token update + if ( $error == ERROR_TOKEN_UPDATED ) { $scfg->{ '_auth_token' . $i } = $config->{ auth_token }; $scfg->{ '_request_id' . $i } = $config->{ request_id }; - } elsif ( $error == 1 ) { - my $ignore = $action->{ ignore }; - if ( defined( $ignore ) ) { - $ignore = [$ignore] if ref( $ignore ) eq ''; - my $first = $content->{ errors }->[0]->{ message }; - $error = 0 if grep { $_ eq $first } @$ignore; + } + + # Handle ignorable API errors + elsif ( $error == ERROR_API_ERROR && is_ignorable_error( $action, $content ) ) { + $error = ERROR_SUCCESS; + } + + # Track success for this array + if ( $error <= ERROR_SUCCESS ) { + $success_count++; + $last_success_error = $error; + $last_success_content = $content; + print "Debug :: Array " . ( $i + 1 ) . " ($url) succeeded\n" if $DEBUG >= 2; + } + + # Stop on critical authentication error (cannot continue) + if ( $error == ERROR_AUTH_FAILED ) { + last; + } + + # Handle API errors in Active Cluster mode + if ( $error == ERROR_API_ERROR ) { + if ( $all && $success_count > 0 ) { + + # Continue to next array - partial success acceptable in Active Cluster + print "Warning :: Array " . ( $i + 1 ) . " ($url) failed but array(s) succeeded. Continuing...\n"; + next; + } else { + last; } } - last if $error == 1 || $error <= 0 && !$all; + # Stop on success if not processing all arrays + last if $error <= ERROR_SUCCESS && !$all; } - if ( $error > 0 ) { - my $message = $error == 3 ? 'Authentication' : $action->{ name } || "Action '$type' (method '$method')"; + # Use last successful response if we processed multiple arrays + if ( $all && $success_count > 0 ) { + $error = $last_success_error; + $content = $last_success_content; + print "Debug :: Processed $array_count array(s), $success_count succeeded\n" if $DEBUG >= 2; + } + + # Handle fatal errors + # For operations on all arrays (Active Cluster), fail only if all arrays failed + if ( $error > ERROR_SUCCESS ) { + if ( $all && $success_count > 0 ) { + + # At least one array succeeded, so operation is partially successful + # This is acceptable for Active Cluster scenarios + print "Warning :: Operation completed on $success_count of $array_count array(s). Some arrays may have failed.\n"; + return $last_success_content; + } + my $message = $error == ERROR_AUTH_FAILED ? 'Authentication' : $action->{ name } || "Action '$type' (method '$method')"; $message = substr( $message, 0, 1 ) eq uc( substr( $message, 0, 1 ) ) ? $message . ' failed' : 'Failed to ' . $message; - $message = 'PureStorage API :: ' . $message if $error == 1; + $message = 'PureStorage API :: ' . $message if $error == ERROR_API_ERROR; die "Error :: $message.\n" . "=> Trace:\n" . "==> address: " . $url . "\n" . ( $content ? "==> Message: " . Dumper( $content ) : '' ); } return $content; } -sub purestorage_api_request1 { +sub purestorage_http_request { my ( $config, $path, $method, $login, $body ) = @_; my $headers = HTTP::Headers->new( 'Content-Type' => 'application/json' ); + # Determine token state my $token_state; if ( $login ) { - $token_state = 0; # login request + $token_state = TOKEN_STATE_LOGIN; $headers->header( 'api-token' => $config->{ token } ); } elsif ( $config->{ auth_token } ) { - $token_state = 2; # have cached token + $token_state = TOKEN_STATE_CACHED; } else { - $token_state = 1; # need token + $token_state = TOKEN_STATE_NEEDED; } my $error; my $response; my $content; - while ( 1 ) { - if ( $token_state > 0 ) { - if ( $token_state == 1 ) { - print "Debug :: Requesting new session token\n" if $DEBUG; - ( $error, $content ) = purestorage_api_request1( $config, 'login', 'POST', 1 ); - return ( $error, $content ) if $error > 0; + my $retry_count = 0; + my $max_retries = 1; # Allow one retry for token refresh + + # Retry loop for token expiration (max 1 retry) + while ( $retry_count <= $max_retries ) { + + # Obtain token if needed + if ( $token_state > TOKEN_STATE_LOGIN ) { + if ( $token_state == TOKEN_STATE_NEEDED ) { + + # Check cache first (race condition mitigation) + unless ( try_cached_token( $config ) ) { + + # Request new token + print "Debug :: Requesting new session token\n" if $DEBUG >= 1; + ( $error, $content ) = purestorage_http_request( $config, 'login', 'POST', 1 ); + + # On failure, try cache again (another node may have succeeded) + if ( $error > ERROR_SUCCESS ) { + print "Debug :: Login failed, checking if another node cached a token\n" if $DEBUG >= 2; + unless ( try_cached_token( $config ) ) { + return ( $error, $content ); + } + } + } + $token_state = TOKEN_STATE_CACHED; } else { - print "Debug :: Using existing session token\n" if $DEBUG; + print "Debug :: Using existing session token\n" if $DEBUG >= 2; } $headers->header( 'x-auth-token' => $config->{ auth_token } ); } $headers->header( 'X-Request-ID' => $config->{ request_id } ) if $config->{ request_id }; + # Execute HTTP request my $request = HTTP::Request->new( $method, $config->{ url } . '/api/' . $PSFA_API . '/' . $path, $headers, length( $body ) ? encode_json( $body ) : undef ); $response = $config->{ ua }->request( $request ); - $error = $response->is_success ? 0 : 1; - if ( $error && $token_state == 2 && $response->code == 401 ) { - print "Debug :: Session token expired\n"; - $token_state = 1; - next; + # Handle 401 Unauthorized (token expired) + $error = $response->is_success ? ERROR_SUCCESS : ERROR_API_ERROR; + if ( $error && $token_state == TOKEN_STATE_CACHED && $response->code == 401 ) { + $retry_count++; + if ( $retry_count <= $max_retries ) { + print "Debug :: Session token expired (401), retry $retry_count/$max_retries\n" if $DEBUG >= 1; + + # Save current token to detect if cache has newer version + my $old_token = $config->{ auth_token }; + + # Try cache first - another node may have already refreshed + if ( try_cached_token( $config ) && $config->{ auth_token } ne $old_token ) { + print "Debug :: Using refreshed token from another node\n" if $DEBUG >= 2; + $token_state = TOKEN_STATE_CACHED; + next; + } + + # No fresh token in cache, request new one + cleanup_token_cache( $config ); + $token_state = TOKEN_STATE_NEEDED; + next; + } else { + print "Debug :: Max retries ($max_retries) reached, giving up\n" if $DEBUG >= 1; + last; + } } last; } + # Process successful response $headers = $response->headers; - if ( $error == 0 ) { - if ( $token_state == 0 ) { - $config->{ auth_token } = $headers->header( 'x-auth-token' ) or die "Error :: PureStorage API :: Header 'x-auth-token' is missing.\n"; + if ( $error == ERROR_SUCCESS ) { + if ( $token_state == TOKEN_STATE_LOGIN ) { + + # Extract tokens from login response + $config->{ auth_token } = $headers->header( 'x-auth-token' ) + or die "Error :: PureStorage API :: Header 'x-auth-token' is missing.\n"; $config->{ request_id } = $headers->header( 'x-request-id' ); + + # Save token to cache + save_token_to_cache( $config, $token_state ); } - $error = -1 if $token_state < 2; # auth_token was updated + + # Signal that token was updated + $error = ERROR_TOKEN_UPDATED if $token_state < TOKEN_STATE_CACHED; } + # Parse response content $content = $response->decoded_content; my $content_type = $headers->header( 'Content-Type' ) // ''; if ( $content_type =~ /application\/json/ ) { $content = decode_json( $content ); } else { - $error = $login ? 3 : 2 if $error == 1; # non-API error (connectivity, etc.) + + # Non-JSON response indicates connectivity/network error + $error = $login ? ERROR_AUTH_FAILED : ERROR_NETWORK_ERROR if $error == ERROR_API_ERROR; $content = { response => $content }; } @@ -557,7 +1133,7 @@ sub purestorage_api_request1 { sub purestorage_list_volumes { my ( $class, $scfg, $vmid, $storeid, $destroyed ) = @_; - print "Debug :: PVE::Storage::Custom::PureStoragePlugin::sub::purestorage_list_volumes\n" if $DEBUG; + print "Debug :: PVE::Storage::Custom::PureStoragePlugin::sub::purestorage_list_volumes\n" if $DEBUG >= 2; $vmid = '*' unless defined( $vmid ); my $names = "vm-$vmid-disk-*,vm-$vmid-cloudinit,vm-$vmid-state-*"; @@ -578,7 +1154,7 @@ sub purestorage_get_volumes { params => { filter => $filter } }; - my $response = purestorage_api_request( $scfg, $action ); + my $response = purestorage_api_call( $scfg, $action, 0, $storeid ); my $pref_len = length( purestorage_name_prefix( $scfg ) ); my @volumes = map { @@ -632,7 +1208,7 @@ sub purestorage_get_wwn { } sub purestorage_volume_connection { - my ( $class, $scfg, $volname, $mode ) = @_; + my ( $class, $storeid, $scfg, $volname, $mode ) = @_; my $method = $mode ? 'POST' : 'DELETE'; print "Debug :: PVE::Storage::Custom::PureStoragePlugin::sub::purestorage_volume_connection :: $method\n" if $DEBUG; @@ -662,10 +1238,12 @@ sub purestorage_volume_connection { } }; - my $response = purestorage_api_request( $scfg, $action, 1 ); + # For Active Cluster: connect/disconnect on all arrays (both primary and secondary) + # This ensures volumes are accessible from both arrays in Active Cluster configuration + my $response = purestorage_api_call( $scfg, $action, 1, $storeid ); my $message = ( $response->{ errors } ? 'already ' : '' ) . ( $mode ? 'connected to' : 'disconnected from' ); - print "Info :: Volume \"$volname\" is $message host \"$hname\".\n"; + print "Info :: Volume \"$volname\" is $message host \"$hname\" on all arrays.\n"; return 1; } @@ -681,7 +1259,7 @@ sub purestorage_create_volume { body => { provisioned => $size } }; - my $response = purestorage_api_request( $scfg, $action ); + my $response = purestorage_api_call( $scfg, $action, 0, $storeid ); my $serial = $response->{ items }->[0]->{ serial } or die "Error :: Failed to retrieve volume serial"; print "Info :: Volume \"$volname\" is created (serial=$serial).\n"; @@ -699,7 +1277,77 @@ sub purestorage_remove_volume { $eradicate //= 0; } - my $params = { names => purestorage_name( $scfg, $volname ) }; + # Clean up local device mappings before removing from Pure Storage + my $volume = $class->purestorage_get_volume_info( $scfg, $volname, $storeid, 0 ); + if ( $volume && $volume->{ serial } ) { + my ( $path, $wwid ) = get_device_path_wwn( $volume->{ serial } ); + + if ( $wwid ne '' && -e "/dev/mapper/$wwid" ) { + print "Debug :: Cleaning up local device mappings for $wwid\n" if $DEBUG; + + # 1. Remove LVM mappings on top of the device + cleanup_lvm_on_device( $wwid ); + + # 2. Remove partition mappings + cleanup_partitions_on_device( $wwid ); + + # 3. Remove multipath device + if ( multipath_check( $wwid ) ) { + print "Debug :: Removing multipath device $wwid\n" if $DEBUG; + exec_command( [ 'multipath', '-f', $wwid ], 0 ); + } + } + } + + # Disconnect volume from all hosts on all arrays before destroying + # For Active Cluster: get connections from first array (they are synced) and disconnect from all hosts + print "Debug :: Disconnecting volume from all hosts on all arrays\n" if $DEBUG >= 2; + my $pure_volname = purestorage_name( $scfg, $volname ); + + # Get list of all connections (from first array - in Active Cluster connections are synced) + my $connections_action = { + name => 'list volume connections', + type => 'connections', + method => 'GET', + params => { volume_names => $pure_volname } + }; + + my $connections_response = purestorage_api_call( $scfg, $connections_action, 0, $storeid ); + my @connections = @{ $connections_response->{ items } || [] }; + + if ( @connections ) { + print "Debug :: Found " . scalar( @connections ) . " connection(s) for volume \"$volname\"\n" if $DEBUG >= 1; + + # Collect unique hostnames + my %unique_hosts; + foreach my $conn ( @connections ) { + my $hostname = $conn->{ host }->{ name }; + $unique_hosts{ $hostname } = 1; + } + + # Disconnect from each unique host on all arrays + foreach my $hostname ( keys %unique_hosts ) { + print "Debug :: Disconnecting from host \"$hostname\" on all arrays\n" if $DEBUG >= 2; + + my $disconnect_action = { + name => 'delete volume connection', + type => 'connections', + method => 'DELETE', + ignore => [ 'Volume has been destroyed.', 'Connection does not exist.' ], + params => { + host_names => $hostname, + volume_names => $pure_volname + } + }; + + # For Active Cluster: disconnect from this host on all arrays + purestorage_api_call( $scfg, $disconnect_action, 1, $storeid ); + } + print "Info :: Volume \"$volname\" disconnected from " . scalar( keys %unique_hosts ) . " host(s) on all arrays.\n"; + } else { + print "Debug :: No connections found for volume \"$volname\"\n" if $DEBUG >= 2; + } + my $params = { names => $pure_volname }; my $action = { name => 'destroy volume', type => 'volumes', @@ -709,7 +1357,8 @@ sub purestorage_remove_volume { body => { destroyed => \1 } }; - my $response = purestorage_api_request( $scfg, $action ); + # For Active Cluster: destroy volume on all arrays + my $response = purestorage_api_call( $scfg, $action, 1, $storeid ); my $message = ( $response->{ errors } ? 'already ' : '' ) . 'destroyed'; print "Info :: Volume \"$volname\" is $message.\n"; @@ -719,10 +1368,12 @@ sub purestorage_remove_volume { name => 'eradicate volume', type => 'volumes', method => 'DELETE', + ignore => 'Eradication is disabled.', params => $params, }; - purestorage_api_request( $scfg, $action ); + # For Active Cluster: eradicate volume on all arrays + purestorage_api_call( $scfg, $action, 1, $storeid ); print "Info :: Volume \"$volname\" is eradicated.\n"; } @@ -731,7 +1382,7 @@ sub purestorage_remove_volume { } sub purestorage_resize_volume { - my ( $class, $scfg, $volname, $size ) = @_; + my ( $class, $scfg, $storeid, $volname, $size ) = @_; print "Debug :: PVE::Storage::Custom::PureStoragePlugin::sub::purestorage_resize_volume\n" if $DEBUG; my $action = { @@ -742,7 +1393,7 @@ sub purestorage_resize_volume { body => { provisioned => $size } }; - my $response = purestorage_api_request( $scfg, $action ); + my $response = purestorage_api_call( $scfg, $action, 0, $storeid ); my $serial = $response->{ items }->[0]->{ serial } or die "Error :: Failed to retrieve volume serial"; @@ -780,7 +1431,7 @@ sub purestorage_resize_volume { } sub purestorage_rename_volume { - my ( $class, $scfg, $source_volname, $target_volname ) = @_; + my ( $class, $scfg, $storeid, $source_volname, $target_volname ) = @_; print "Debug :: PVE::Storage::Custom::PureStoragePlugin::sub::purestorage_rename_volume\n" if $DEBUG; my $action = { @@ -791,7 +1442,7 @@ sub purestorage_rename_volume { body => { name => purestorage_name( $scfg, $target_volname ) } }; - purestorage_api_request( $scfg, $action ); + purestorage_api_call( $scfg, $action, 0, $storeid ); print "Info :: Volume \"$source_volname\" is renamed to \"$target_volname\".\n"; @@ -799,7 +1450,7 @@ sub purestorage_rename_volume { } sub purestorage_snap_volume_create { - my ( $class, $scfg, $snap_name, $volname ) = @_; + my ( $class, $scfg, $storeid, $snap_name, $volname ) = @_; print "Debug :: PVE::Storage::Custom::PureStoragePlugin::sub::purestorage_snap_volume_create\n" if $DEBUG; my $action = { @@ -812,14 +1463,14 @@ sub purestorage_snap_volume_create { } }; - purestorage_api_request( $scfg, $action ); + purestorage_api_call( $scfg, $action, 0, $storeid ); print "Info :: Volume \"$volname\" snapshot \"$snap_name\" is created.\n"; return 1; } sub purestorage_volume_restore { - my ( $class, $scfg, $volname, $svolname, $snap, $overwrite ) = @_; + my ( $class, $scfg, $storeid, $volname, $svolname, $snap, $overwrite ) = @_; print "Debug :: PVE::Storage::Custom::PureStoragePlugin::sub::purestorage_volume_restore\n" if $DEBUG; my $params = { names => purestorage_name( $scfg, $volname ) }; @@ -837,7 +1488,7 @@ sub purestorage_volume_restore { } }; - purestorage_api_request( $scfg, $action ); + purestorage_api_call( $scfg, $action, 0, $storeid ); my $source = length( $snap ) ? 'snapshot "' . $snap . '"' : ''; if ( $volname ne $svolname ) { @@ -850,7 +1501,7 @@ sub purestorage_volume_restore { } sub purestorage_snap_volume_delete { - my ( $class, $scfg, $snap_name, $volname ) = @_; + my ( $class, $scfg, $storeid, $snap_name, $volname ) = @_; print "Debug :: PVE::Storage::Custom::PureStoragePlugin::sub::purestorage_snap_volume_delete\n" if $DEBUG; my $params = { names => purestorage_name( $scfg, $volname, $snap_name ) }; @@ -863,7 +1514,7 @@ sub purestorage_snap_volume_delete { params => $params, body => { destroyed => \1 } }; - my $response = purestorage_api_request( $scfg, $action ); + my $response = purestorage_api_call( $scfg, $action, 0, $storeid ); my $message = ( $response->{ errors } ? 'already ' : '' ) . 'destroyed'; print "Info :: Volume \"$volname\" snapshot \"$snap_name\" is $message.\n"; @@ -873,11 +1524,11 @@ sub purestorage_snap_volume_delete { name => 'eradicate volume snapshot', type => 'volume-snapshots', method => 'DELETE', - ignore => 'No such volume or snapshot.', + ignore => [ 'No such volume or snapshot.', 'Eradication is disabled.' ], params => $params, body => { replication_snapshot => \1 } }; - $response = purestorage_api_request( $scfg, $action ); + $response = purestorage_api_call( $scfg, $action, 0, $storeid ); $message = ( $response->{ errors } ? 'already ' : '' ) . 'eradicated'; print "Info :: Volume \"$volname\" snapshot \"$snap_name\" is $message.\n"; @@ -888,7 +1539,7 @@ sub purestorage_snap_volume_delete { sub parse_volname { my ( $class, $volname ) = @_; - print "Debug :: PVE::Storage::Custom::PureStoragePlugin::sub::parse_volname\n" if $DEBUG; + print "Debug :: PVE::Storage::Custom::PureStoragePlugin::sub::parse_volname\n" if $DEBUG >= 3; if ( $volname =~ m/^(vm|base)-(\d+)-(\S+)$/ ) { my $vtype = ( $1 eq "vm" ) ? "images" : "base"; # Determine volume type @@ -907,7 +1558,7 @@ sub filesystem_path { my ( $class, $scfg, $volname, $snapname ) = @_; print "Debug :: PVE::Storage::Custom::PureStoragePlugin::sub::filesystem_path\n" if $DEBUG; - die "Error :: filesystem_path: snapshot is not implemented ($snapname)\n" if defined($snapname); + die "Error :: filesystem_path: snapshot is not implemented ($snapname)\n" if defined( $snapname ); # do we even need this? my ( $vtype, undef, $vmid ) = $class->parse_volname( $volname ); @@ -933,7 +1584,7 @@ sub clone_image { my $name = $class->find_free_diskname( $storeid, $scfg, $vmid ); - $class->purestorage_volume_restore( $scfg, $name, $volname, $snap ); + $class->purestorage_volume_restore( $scfg, $storeid, $name, $volname, $snap ); return $name; } @@ -991,7 +1642,8 @@ sub free_image { sub list_images { my ( $class, $storeid, $scfg, $vmid, $vollist, $cache ) = @_; - print "Debug :: PVE::Storage::Custom::PureStoragePlugin::sub::list_images\n" if $DEBUG; + set_debug_from_config( $scfg ); + print "Debug :: list_images ($storeid, vmid=" . ( $vmid // 'all' ) . ")\n" if $DEBUG >= 1; return $class->purestorage_list_volumes( $scfg, $vmid, $storeid, 0 ); } @@ -1000,14 +1652,48 @@ sub status { my ( $class, $storeid, $scfg, $cache ) = @_; print "Debug :: PVE::Storage::Custom::PureStoragePlugin::sub::status\n" if $DEBUG; - my $response = purestorage_api_request( $scfg, { name => 'get array space', type => 'arrays/space', method => 'GET' } ); + my $total; + my $used; - # Get storage capacity and used space from the response - my $array = $response->{ items }->[0]; - my $total = $array->{ capacity }; - my $used = $array->{ space }->{ total_physical }; + # If using pod with quota, get pod-specific capacity + if ( defined $scfg->{ podname } && $scfg->{ podname } ne '' ) { + my $podname = $scfg->{ podname }; + print "Debug :: Getting pod quota for pod: $podname\n" if $DEBUG >= 2; - # my $used = $array->{ space }->{ total_used }; # Do not know what is correct + my $action = { + name => 'get pod space', + type => 'pods/space', + method => 'GET', + params => { names => $podname } + }; + my $response = purestorage_api_call( $scfg, $action, 0, $storeid ); + + my $pod = $response->{ items }->[0]; + if ( $pod ) { + + # Use quota_limit if set and non-zero, otherwise fall back to array capacity + # quota_limit = 0 or undef means unlimited (no quota) + my $quota = $pod->{ quota_limit }; + $total = ( defined( $quota ) && $quota > 0 ) ? $quota : $pod->{ capacity }; + $used = $pod->{ space }->{ total_physical }; + + my $quota_str = defined( $quota ) ? ( $quota > 0 ? $quota : 'unlimited' ) : 'not set'; + print "Debug :: Pod quota_limit: $quota_str\n" if $DEBUG >= 2; + } else { + die "Error :: Pod \"$podname\" not found\n"; + } + } else { + + # Get array-wide capacity + my $response = purestorage_api_call( $scfg, { name => 'get array space', type => 'arrays/space', method => 'GET' }, 0, $storeid ); + + my $array = $response->{ items }->[0]; + $total = $array->{ capacity }; + + # total_physical - physically used space on the array (after deduplication and compression) + # total_used - logically used space (before deduplication and compression) + $used = $array->{ space }->{ total_physical }; + } # Calculate free space my $free = $total - $used; @@ -1021,7 +1707,8 @@ sub status { sub activate_storage { my ( $class, $storeid, $scfg, $cache ) = @_; - print "Debug :: PVE::Storage::Custom::PureStoragePlugin::sub::activate_storage\n" if $DEBUG; + set_debug_from_config( $scfg ); + print "Debug :: activate_storage ($storeid)\n" if $DEBUG >= 1; return 1; } @@ -1068,12 +1755,20 @@ sub map_volume { }; # Wait for the device to appear - wait_for( $path_exists, "volume \"$volname\" to map" ); + wait_for( $path_exists, "volume \"$volname\" to map", 30 ); # we might end up with operational disk but without multipathing, e.g. # if unmapping was interrupted ('remove map' was already done, but slaves were not removed) - exec_command( [ 'multipathd', 'add', 'map', $wwid ] ) unless multipath_check( $wwid ); + if ( !multipath_check( $wwid ) ) { + print "Debug :: Adding multipath map for device \"$wwid\"\n" if $DEBUG; + exec_command( [ 'multipathd', 'add', 'map', $wwid ] ); + # Wait for multipath to be fully established + my $multipath_ready = sub { + return multipath_check( $wwid ); + }; + wait_for( $multipath_ready, "multipath map for volume \"$volname\" to be ready", 30 ); + } return $path; } @@ -1086,13 +1781,16 @@ sub unmap_volume { my ( $device_path, @slaves ) = block_device_slaves( $path ); + # Ensure all data is flushed to disk for write-back cache environments + print "Debug :: Flushing filesystem and device buffers for $device_path\n" if $DEBUG >= 2; + exec_command( ['sync'] ); exec_command( [ 'blockdev', '--flushbufs', $device_path ] ); - # this may help if there is a write-back cache (see issue #47) - ## my $fuser = sub { - ## return exec_command( [ 'fuser', '-s', $device_path ], -1 ); - ## }; - ## wait_for( $fuser, 'device cache flush', 30, 0.5 ); + # Wait for udev events to settle, ensuring all async operations complete + eval { exec_command( [ 'udevadm', 'settle', '--timeout=10' ] ) }; + + # Final sync to guarantee write-back cache is flushed + exec_command( ['sync'] ); if ( multipath_check( $wwid ) ) { print "Debug :: Device \"$wwid\" is a multipath device. Proceeding with multipath removal.\n" if $DEBUG; @@ -1114,7 +1812,7 @@ sub activate_volume { my ( $class, $storeid, $scfg, $volname, $snapname, $cache ) = @_; print "Debug :: PVE::Storage::Custom::PureStoragePlugin::sub::activate_volume\n" if $DEBUG; - $class->purestorage_volume_connection( $scfg, $volname, 1 ); + $class->purestorage_volume_connection( $storeid, $scfg, $volname, 1 ); $class->map_volume( $storeid, $scfg, $volname, $snapname ); return 1; @@ -1126,7 +1824,7 @@ sub deactivate_volume { $class->unmap_volume( $storeid, $scfg, $volname, $snapname ); - $class->purestorage_volume_connection( $scfg, $volname, 0 ); + $class->purestorage_volume_connection( $storeid, $scfg, $volname, 0 ); print "Info :: Volume \"$volname\" is deactivated.\n"; @@ -1138,7 +1836,7 @@ sub volume_resize { print "Debug :: PVE::Storage::Custom::PureStoragePlugin::sub::volume_resize\n" if $DEBUG; warn "Debug :: New Size: $size\n" if $DEBUG; - return $class->purestorage_resize_volume( $scfg, $volname, $size ); + return $class->purestorage_resize_volume( $scfg, $storeid, $volname, $size ); } sub rename_volume { @@ -1159,7 +1857,7 @@ sub rename_volume { # we need to unmap source volume (see RBDPlugin.pm) $class->unmap_volume( $storeid, $scfg, $source_volname ); - $class->purestorage_rename_volume( $scfg, $source_volname, $target_volname ); + $class->purestorage_rename_volume( $scfg, $storeid, $source_volname, $target_volname ); return "$storeid:$target_volname"; } @@ -1176,7 +1874,7 @@ sub volume_snapshot { my ( $class, $scfg, $storeid, $volname, $snap ) = @_; print "Debug :: PVE::Storage::Custom::PureStoragePlugin::sub::volume_snapshot\n" if $DEBUG; - $class->purestorage_snap_volume_create( $scfg, $snap, $volname ); + $class->purestorage_snap_volume_create( $scfg, $storeid, $snap, $volname ); return 1; } @@ -1185,7 +1883,7 @@ sub volume_snapshot_rollback { my ( $class, $scfg, $storeid, $volname, $snap ) = @_; print "Debug :: PVE::Storage::Custom::PureStoragePlugin::sub::volume_snapshot_rollback\n" if $DEBUG; - $class->purestorage_volume_restore( $scfg, $volname, $volname, $snap, 1 ); + $class->purestorage_volume_restore( $scfg, $storeid, $volname, $volname, $snap, 1 ); return 1; } @@ -1194,7 +1892,7 @@ sub volume_snapshot_delete { my ( $class, $scfg, $storeid, $volname, $snap ) = @_; print "Debug :: PVE::Storage::Custom::PureStoragePlugin::sub::volume_snapshot_delete\n" if $DEBUG; - $class->purestorage_snap_volume_delete( $scfg, $snap, $volname ); + $class->purestorage_snap_volume_delete( $scfg, $storeid, $snap, $volname ); return 1; } diff --git a/README.md b/README.md index 5bee801..7d8d887 100644 --- a/README.md +++ b/README.md @@ -1,38 +1,59 @@ # Proxmox VE Plugin for Pure Storage as Multipath iSCSI Source -This plugin enables the integration of Pure Storage arrays with Proxmox Virtual Environment (VE) using multipath iSCSI or Fibre Channel (FC). It allows you to use Pure Storage as a backend for your virtual machine disks, providing high performance and reliability. +[![Checks](https://github.com/kolesa-team/pve-purestorage-plugin/actions/workflows/checks.yml/badge.svg)](https://github.com/kolesa-team/pve-purestorage-plugin/actions/workflows/checks.yml) + +This plugin enables the integration of Pure Storage arrays with Proxmox +Virtual Environment (VE) using multipath iSCSI or Fibre Channel (FC). +It allows you to use Pure Storage as a backend for your virtual machine +disks, providing high performance and reliability. ## Table of Contents - [Features](#features) - [Prerequisites](#prerequisites) - [Multipath Configuration](#multipath-configuration) + - [iSCSI Configuration](#iscsi-configuration) - [Installation](#installation) - [Manual](#manual) - [APT](#apt) - [Configuration](#configuration) - [Troubleshooting](#troubleshooting) + - [Known issues](#known-issues) - [Contributing](#contributing) ## Features + - Easily enable and configure multipathing iSCSI to the Pure Array +- **Active Cluster support (Experimental)** - Automatic volume connection + on both arrays in Active Cluster configuration + - Volumes are automatically connected to hosts on both primary and + secondary arrays + - Ensures high availability and optimal connectivity in Active Cluster + setups - Storage based snapshots - Snapshots are presented in Proxmox like any other native Snapshot to a VM - Snapshots are created by the Pure Array, making them deduped and instant - Instant storage migration - - The plugin will automatically map the iSCSI volumes needed on the host the VM is being migrated to + - The plugin will automatically map the iSCSI volumes needed on the + host the VM is being migrated to ## Prerequisites -Before installing and using this plugin, ensure that your Proxmox VE environment meets the following prerequisites. +Before installing and using this plugin, ensure that your Proxmox VE +environment meets the following prerequisites. ### Multipath Configuration -To ensure correct operation with Pure Storage, you need to configure your multipath settings appropriately. Specifically, you need to set find_multipaths to no in your multipath.conf file. This setting disables the automatic detection of multipath devices, which is necessary for Pure Storage devices to be correctly recognized. +To ensure correct operation with Pure Storage, you need to configure your +multipath settings appropriately. Specifically, you need to set +find_multipaths to no in your multipath.conf file. This setting disables +the automatic detection of multipath devices, which is necessary for Pure +Storage devices to be correctly recognized. -Below is an example of how your multipath.conf file should look when configured for Pure Storage arrays: +Below is an example of how your multipath.conf file should look when +configured for Pure Storage arrays: -``` +```text defaults { polling_interval 2 find_multipaths no @@ -71,6 +92,7 @@ blacklist_exceptions { } } ``` + ## iSCSI Configuration Initiate iSCSI according to the Proxmox Guidelines. @@ -81,11 +103,13 @@ sudo iscsiadm -m node --op update -n node.startup -v automatic ``` > [!CAUTION] -> As long as there are no hostX entries in /sys/class/iscsi_host/ the plugin is not ready to be used. +> As long as there are no hostX entries in /sys/class/iscsi_host/ the +> plugin is not ready to be used. ## Installation -There are two methods to install the plugin: manual installation and APT package installation. +There are two methods to install the plugin: manual installation and APT +package installation. ### Manual @@ -106,7 +130,8 @@ systemctl restart pve-cluster.service pvedaemon.service pvestatd.service pveprox ### APT -**Note**: Replace `` with your desired version number (e.g., `0.0.1`). +**Note**: Replace `` with your desired version number +(e.g., `0.0.1`). ```bash PACKAGE_VERSION="" curl -L -o libpve-storage-purestorage-perl.deb "https://github.com/kolesa-team/pve-purestorage-plugin/releases/download/v$PACKAGE_VERSION/libpve-storage-purestorage-perl_$PACKAGE_VERSION-1_all.deb" @@ -115,24 +140,30 @@ sudo apt install ./libpve-storage-purestorage-perl.deb ``` ## Configuration -> [!TIP] -> If your are using a cluster setup - this step needs to be executed only on one note of the cluster - corosync will do the rest for you. -After installing the plugin, you need to configure Proxmox VE to use it. Since Proxmox VE does not currently support adding custom storage plugins via the GUI, you will need to open shell and use `pvesm` command to add it: +> [!TIP] +> If you are using a cluster setup - this step needs to be executed only +> on one node of the cluster - corosync will do the rest for you. + +After installing the plugin, you need to configure Proxmox VE to use it. +Since Proxmox VE does not currently support adding custom storage plugins +via the GUI, you will need to open shell and use `pvesm` command to add it: ```bash pvesm add purestorage \ --nodes \ - --address https:// \ + --address \ + https:// \ --token \ --vgname \ --hgsuffix --content images ``` -Alternatively, you can manually edit the storage configuration file `/etc/pve/storage.cfg`. +Alternatively, you can manually edit the storage configuration file +`/etc/pve/storage.cfg`. -``` +```text purestorage: nodes address https:// @@ -146,58 +177,136 @@ purestorage: | --------- | ----------- | | storage_id | The storage identifier (name under which it will appear in the Storage list) | | nodes | (`optional`) A comma-separated list of Proxmox node names. Use this parameter to limit the plugin to specific nodes in your cluster. If omitted, the storage is available to all nodes. | -| address | The URL or IP address of the Pure Storage API endpoint. Ensure that the Proxmox VE nodes can reach this address over the network. | -| token | The API token used for authentication with the Pure Storage array. This token must have sufficient permissions to create and manage volumes. | +| address | The URL or IP address of the Pure Storage API endpoint. Ensure that the Proxmox VE nodes can reach this address over the network. For high availability or Active Cluster configuration (experimental), you can specify multiple arrays separated by commas (e.g., `https://array1.example.com,https://array2.example.com`). When multiple arrays are specified, the plugin automatically connects volumes to hosts on all arrays. | +| token | The API token used for authentication with the Pure Storage array. This token must have sufficient permissions to create and manage volumes. For multiple arrays, specify tokens separated by commas in the same order as addresses. Each token must have permissions for its corresponding array. | | vgname | (`optional`, conflicts with `podname`) The volume group name where virtual disks will be stored. This should match the configuration on your Pure Storage array. | | podname | (`optional`, conflicts with `vgname`) The pod name where virtual disks will be stored. This should match the configuration on your Pure Storage array. | | vnprefix | (`optional`) The prefix to prepend to name of virtual disks. | | hgsuffix | (`optional`) A suffix that is appended to the hostname when the plugin interacts with the Pure Storage array. This can help differentiate hosts if necessary. | | content | Specifies the types of content that can be stored. For virtual machine disk images, use images. | -| protocol | (`optional`, default is `iscsi`) Specifies the storage protocol (iscsi, fc) | +| protocol | (`optional`, default is `iscsi`) Specifies the storage protocol (`iscsi`, `fc`). | +| check_ssl | (`optional`, default is `no`) Verify the server's TLS certificate. Set to `yes` to enable SSL certificate verification. | +| token_ttl | (`optional`, default is `3600`) Session token time-to-live in seconds. The plugin caches PureStorage API session tokens in `/etc/pve/priv/purestorage/` (automatically replicated across cluster nodes). Tokens are proactively refreshed at 80% of TTL to prevent expiration during operations. | +| debug | (`optional`, default is `0`) Enable debug logging. Levels: 0=off, 1=basic (token operations, main calls), 2=verbose (HTTP details, validation), 3=trace (all internals). Environment variable `PURESTORAGE_DEBUG` can be used as fallback when `debug` is not set in config. | -> **_NOTE:_** Ensure that the token and other sensitive information are kept secure and not exposed publicly. +> **_NOTE:_** Ensure that the token and other sensitive information are +> kept secure and not exposed publicly. Example Configuration: -``` +**Single Array:** + +```text purestorage: pure address https://purestorage.example.com token abc123 vgname pure_vg + hgsuffix "" content images ``` +**Active Cluster (Multiple Arrays) - Experimental:** + +```text +purestorage: pure-cluster + address https://array1.example.com,https://array2.example.com + token token1,token2 + vgname pure_vg + content images +``` + +> [!NOTE] +> When multiple arrays are specified (Active Cluster configuration - +> experimental feature), the plugin automatically connects volumes to hosts +> on both arrays. This ensures high availability - if one array fails, +> volumes remain accessible through the other array. The plugin handles +> connection management on all arrays transparently. + ## Troubleshooting -If you encounter issues while using the plugin, consider the following steps: +If you encounter issues while using the plugin, consider the following +steps: -- Check Service Status: Ensure that the Proxmox VE services are running correctly. You can restart the services if necessary: +### Debug Logging + +Enable debug logging to diagnose issues: + +**Persistent (via configuration):** + +```bash +pvesm set --debug 1 +``` + +**Temporary (for single command, when debug is not set in config):** + +```bash +PURESTORAGE_DEBUG=1 pvesm list +``` + +> **Note:** If `debug` is set in storage configuration, it takes priority +> over `PURESTORAGE_DEBUG` environment variable. + +Debug levels: + +- `0` - Off (production, default) +- `1` - Basic (token operations, main function calls) +- `2` - Verbose (HTTP requests, token validation details) +- `3` - Trace (all internal operations) + +**Example debug output:** + +```bash +PURESTORAGE_DEBUG=1 pvesm list pure-n1 +Debug :: activate_storage (pure-n1) +Debug :: list_images (pure-n1, vmid=all) +Debug :: Read token cache from: /etc/pve/priv/purestorage/pure-n1_array0.json +Debug :: Token is valid (age: 125s) +Debug :: Using cached token from file (age: 125s) +``` + +### Service Status + +Ensure that the Proxmox VE services are running correctly. You can restart +the services if necessary: ```bash sudo systemctl restart pve-cluster.service pvedaemon.service pvestatd.service pveproxy.service pvescheduler.service ``` -- Verify Network Connectivity: Ensure that the Proxmox VE nodes can reach the Pure Storage array over the network. Check for firewall rules or network issues that might be blocking communication. -- Review Logs: Check the Proxmox VE logs for any error messages related to storage or the plugin. Logs are typically found in /var/log/pve. - These commands are helpful for troubleshooting: +### Network and Storage + +- Verify Network Connectivity: Ensure that the Proxmox VE nodes can reach + the Pure Storage array over the network. Check for firewall rules or + network issues that might be blocking communication. +- Review Logs: Check the Proxmox VE logs for any error messages related to + storage or the plugin. Logs are typically found in /var/log/pve. These + commands are helpful for troubleshooting: + ```bash multipath -ll -v3 #diagnose issues with the multipath service iscsiadm -m node #list what iscsi nodes are mounted ls -l /dev/mapper/3624a9370* #list wwids of Pure mapped devices on the system ``` -- Multipath Configuration: Verify that your multipath.conf is correctly configured and that multipath devices are recognized. Use multipath -ll to list the current multipath devices. -- API Token Permissions: Ensure that the API token used has the necessary permissions to create and manage volumes on the Pure Storage array. -- Plugin Updates: Ensure you are using the latest version of the plugin. Check the GitHub repository for updates. + +- Multipath Configuration: Verify that your multipath.conf is correctly + configured and that multipath devices are recognized. Use multipath -ll + to list the current multipath devices. +- API Token Permissions: Ensure that the API token used has the necessary + permissions to create and manage volumes on the Pure Storage array. +- Plugin Updates: Ensure you are using the latest version of the plugin. + Check the GitHub repository for updates. ### Known issues -- `lvm inside a volume`: If you plan to use LVM inside a volume, it is better to add purestorage volumes to the ignore list to avoid scanning. +- `lvm inside a volume`: If you plan to use LVM inside a volume, it is + better to add purestorage volumes to the ignore list to avoid scanning. ```bash cat /etc/lvm/lvmlocal.conf ... devices { - global_filter=["r|/dev/zd.*|","r|/dev/rbd.*|","r|/dev/mapper/3624a9370.*|"] + global_filter=["r|/dev/zd.*|","r|/dev/rbd.*|", + "r|/dev/mapper/3624a9370.*|"] } ``` diff --git a/tests/README.md b/tests/README.md new file mode 100644 index 0000000..f72edd4 --- /dev/null +++ b/tests/README.md @@ -0,0 +1,31 @@ +# PureStorage Plugin Tests + +This directory contains tests for the PVE PureStorage Plugin. + +## Test Structure + +- `unit/` - Unit tests for individual functions +- `integration/` - Integration tests requiring actual PureStorage array +- `fixtures/` - Test data and mock responses +- `scripts/` - Helper scripts for testing + +## Running Tests + +```bash +# Run all unit tests +prove -v tests/unit/ + +# Run specific test +perl tests/unit/test_token_cache.t + +# Run with verbose output +perl -I. tests/unit/test_command_validation.t +``` + +## Test Coverage + +- Token caching and expiration +- Command path validation +- API request/response handling +- Device cleanup functions +- Error handling diff --git a/tests/run_tests.sh b/tests/run_tests.sh new file mode 100755 index 0000000..4658bec --- /dev/null +++ b/tests/run_tests.sh @@ -0,0 +1,67 @@ +#!/bin/bash + +set -e + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +cd "$SCRIPT_DIR/.." + +echo "===================================" +echo "PureStorage Plugin Test Suite" +echo "===================================" +echo "" + +# Colors for output +RED='\033[0;31m' +GREEN='\033[0;32m' +YELLOW='\033[1;33m' +NC='\033[0m' # No Color + +# Check if Test::More is available +if ! perl -MTest::More -e 'print "OK\n"' 2>/dev/null | grep -q OK; then + echo -e "${RED}Error: Test::More module not found${NC}" + echo "Please install it with: cpan Test::More" + exit 1 +fi + +# Check if JSON is available +if ! perl -MJSON -e 'print "OK\n"' 2>/dev/null | grep -q OK; then + echo -e "${YELLOW}Warning: JSON module not found${NC}" + echo "Some tests may fail. Install with: cpan JSON" +fi + +echo -e "${GREEN}Running Unit Tests${NC}" +echo "-----------------------------------" + +failed_tests=0 +total_tests=0 + +# Run all test files in tests/unit/ +for test_file in tests/unit/*.t; do + if [ -f "$test_file" ]; then + total_tests=$((total_tests + 1)) + echo "" + echo -e "${YELLOW}Running: $(basename $test_file)${NC}" + + if perl -I. "$test_file"; then + echo -e "${GREEN}✓ PASS${NC}" + else + echo -e "${RED}✗ FAIL${NC}" + failed_tests=$((failed_tests + 1)) + fi + fi +done + +echo "" +echo "===================================" +echo "Test Summary" +echo "===================================" +echo "Total tests: $total_tests" +echo -e "${GREEN}Passed: $((total_tests - failed_tests))${NC}" + +if [ $failed_tests -gt 0 ]; then + echo -e "${RED}Failed: $failed_tests${NC}" + exit 1 +else + echo -e "${GREEN}All tests passed!${NC}" + exit 0 +fi diff --git a/tests/token_cache_test.pl b/tests/token_cache_test.pl new file mode 100755 index 0000000..a850b6c --- /dev/null +++ b/tests/token_cache_test.pl @@ -0,0 +1,205 @@ +#!/usr/bin/env perl + +use strict; +use warnings; +use Test::More tests => 15; +use File::Temp qw( tempdir ); +use JSON::XS qw( encode_json decode_json ); + +# Mock PVE::Tools for testing +BEGIN { + + package PVE::Tools; + use Exporter 'import'; + our @EXPORT_OK = qw( file_get_contents ); + + sub file_get_contents { + my ( $path ) = @_; + open my $fh, '<', $path or die "Cannot read $path: $!"; + local $/; + my $content = <$fh>; + close $fh; + return $content; + } +} + +# Test token cache implementation +package main; + +my $test_dir = tempdir( CLEANUP => 1 ); +my $cache_path = "$test_dir/test_cache.json"; + +# Helper function to create mock token data +sub create_token_data { + my ( $age ) = @_; + my $now = time(); + return { + auth_token => 'test-token-' . int( rand( 1000 ) ), + request_id => 'test-request-id', + created_at => $now - $age, + ttl => 3600, + expires_at => $now - $age + 3600 + }; +} + +# Helper function to write token cache +sub write_test_cache { + my ( $token_data ) = @_; + my $json = encode_json( $token_data ); + open my $fh, '>', $cache_path or die "Cannot write cache: $!"; + print $fh $json; + close $fh; +} + +# Test 1: Token validation - fresh token +{ + my $token_data = create_token_data( 100 ); # 100s old + my $ttl = 3600; + my $threshold = $ttl * 0.8; # 2880s + + ok( 100 < $threshold, 'Fresh token is valid (age < 80% TTL)' ); +} + +# Test 2: Token validation - expired token +{ + my $token_data = create_token_data( 3000 ); # 3000s old + my $ttl = 3600; + my $threshold = $ttl * 0.8; # 2880s + + ok( 3000 >= $threshold, 'Expired token needs refresh (age >= 80% TTL)' ); +} + +# Test 3: Cache file write and read +{ + my $token_data = create_token_data( 50 ); + write_test_cache( $token_data ); + + ok( -f $cache_path, 'Cache file created' ); + + my $json_text = PVE::Tools::file_get_contents( $cache_path ); + my $read_data = decode_json( $json_text ); + + is( $read_data->{ auth_token }, $token_data->{ auth_token }, 'Token data matches after read' ); +} + +# Test 4: Cache file validation - valid token +{ + my $token_data = create_token_data( 100 ); + write_test_cache( $token_data ); + + my $json_text = PVE::Tools::file_get_contents( $cache_path ); + my $cached = decode_json( $json_text ); + + my $age = time() - $cached->{ created_at }; + my $threshold = 3600 * 0.8; + + ok( $age < $threshold, 'Cached token is still valid' ); +} + +# Test 5: Cache file validation - expired token +{ + my $token_data = create_token_data( 3000 ); + write_test_cache( $token_data ); + + my $json_text = PVE::Tools::file_get_contents( $cache_path ); + my $cached = decode_json( $json_text ); + + my $age = time() - $cached->{ created_at }; + my $threshold = 3600 * 0.8; + + ok( $age >= $threshold, 'Cached token is expired and should be refreshed' ); +} + +# Test 6: Race condition mitigation - newer token exists +{ + my $old_token = create_token_data( 200 ); + my $new_token = create_token_data( 50 ); + + ok( $new_token->{ created_at } > $old_token->{ created_at }, 'Newer token has later created_at timestamp' ); +} + +# Test 7: TTL validation +{ + my $ttl = 3600; + my $refresh_threshold = $ttl * 0.8; + + is( $refresh_threshold, 2880, 'Refresh threshold is 80% of TTL' ); + + # Test jitter range (±2.5%) + my $jitter_min = $ttl * ( 0.8 - 0.025 ); + my $jitter_max = $ttl * ( 0.8 + 0.025 ); + + ok( $jitter_min < $refresh_threshold && $refresh_threshold < $jitter_max, 'Jitter keeps threshold within ±2.5% of 80% TTL' ); +} + +# Test 8: Multiple token files +{ + my $cache1 = "$test_dir/storage1_array0.json"; + my $cache2 = "$test_dir/storage2_array0.json"; + + my $token1 = create_token_data( 100 ); + my $token2 = create_token_data( 200 ); + + open my $fh1, '>', $cache1 or die $!; + print $fh1 encode_json( $token1 ); + close $fh1; + + open my $fh2, '>', $cache2 or die $!; + print $fh2 encode_json( $token2 ); + close $fh2; + + ok( -f $cache1 && -f $cache2, 'Multiple cache files can coexist' ); +} + +# Test 9: Token cache path generation +{ + my $storeid = 'pure-n1'; + my $array_index = 0; + my $expected_path = "/etc/pve/priv/purestorage/${storeid}_array${array_index}.json"; + + like( $expected_path, qr/\/etc\/pve\/priv\/purestorage\/pure-n1_array0\.json$/, 'Cache path follows expected format' ); +} + +# Test 10: Atomic write simulation +{ + my $temp_path = "$cache_path.tmp.$$"; + my $token_data = create_token_data( 75 ); + + # Write to temp file + open my $fh, '>', $temp_path or die $!; + print $fh encode_json( $token_data ); + close $fh; + + ok( -f $temp_path, 'Temp file created' ); + + # Atomic rename + rename( $temp_path, $cache_path ) or die "Cannot rename: $!"; + + ok( -f $cache_path && !-f $temp_path, 'Atomic rename completed' ); +} + +# Test 11: Concurrent token creation scenario +{ + my $node_a_token = create_token_data( 0 ); # Fresh token + my $node_b_token = create_token_data( 0 ); # Another fresh token + + # Both tokens created ~same time + my $time_diff = abs( $node_a_token->{ created_at } - $node_b_token->{ created_at } ); + + ok( $time_diff < 2, 'Concurrent tokens created within 2 seconds' ); + + # Race condition check: should skip write if another token exists within 5s + ok( $time_diff < 5, 'Falls within race condition mitigation window (5s)' ); +} + +done_testing(); + +print "\nToken Cache Tests Summary:\n"; +print "=" x 50 . "\n"; +print "All tests validate the token caching mechanism:\n"; +print "- Token TTL validation (80% refresh threshold)\n"; +print "- Cache file operations (read/write)\n"; +print "- Race condition mitigation\n"; +print "- Concurrent token handling\n"; +print "- Atomic write operations\n"; +print "=" x 50 . "\n"; diff --git a/tests/unit/test_command_validation.t b/tests/unit/test_command_validation.t new file mode 100644 index 0000000..1ef32b6 --- /dev/null +++ b/tests/unit/test_command_validation.t @@ -0,0 +1,128 @@ +#!/usr/bin/env perl + +use strict; +use warnings; +use Test::More tests => 9; +use File::Temp qw(tempdir); +use File::Path qw(make_path); + +# Mock the command paths for testing +my $test_dir = tempdir( CLEANUP => 1 ); +my $cmd = { + multipath => "$test_dir/multipath", + multipathd => "$test_dir/multipathd", + blockdev => "$test_dir/blockdev", + dmsetup => "$test_dir/dmsetup", + kpartx => "$test_dir/kpartx" +}; + +# Track if commands were checked +my $commands_checked = 0; + +sub ensure_commands_checked { + return if $commands_checked; + check_commands(); + $commands_checked = 1; +} + +sub get_command_path { + my ( $name ) = @_; + + ensure_commands_checked(); + + my $path = $cmd->{ $name }; + if ( !defined $path ) { + die "Error :: Unknown command '$name'\n"; + } + + if ( !-x $path ) { + die "Error :: Command '$name' not found or not executable at '$path'\n"; + } + + return $path; +} + +sub check_commands { + my @missing; + + foreach my $name ( keys %$cmd ) { + my $path = $cmd->{ $name }; + if ( !-x $path ) { + push @missing, "$name ($path)"; + } + } + + if ( @missing ) { + note "Warning :: The following commands are not available:"; + note " - $_" foreach @missing; + } + + return scalar @missing == 0; +} + +# Test 1: Unknown command should die +eval { get_command_path('unknown_command') }; +like( $@, qr/Unknown command/, 'Unknown command throws error' ); + +# Test 2: Non-existent command should die +eval { get_command_path('multipath') }; +like( $@, qr/not found or not executable/, 'Non-existent command throws error' ); + +# Test 3: Create executable commands +foreach my $name ( keys %$cmd ) { + my $path = $cmd->{ $name }; + open my $fh, '>', $path or die "Cannot create $path: $!"; + print $fh "#!/bin/sh\necho 'test'\n"; + close $fh; + chmod 0755, $path; +} + +# Reset check flag to re-run validation +$commands_checked = 0; + +# Test 4: Valid command should return path +my $path = get_command_path('multipath'); +is( $path, $cmd->{multipath}, 'Valid command returns correct path' ); + +# Test 5: Commands should be checked only once +my $check_count = 0; +{ + no warnings 'redefine'; + my $original = \&check_commands; + *check_commands = sub { + $check_count++; + $original->(); + }; + + $commands_checked = 0; + get_command_path('multipath'); + get_command_path('dmsetup'); + get_command_path('kpartx'); +} +is( $check_count, 1, 'check_commands called only once' ); + +# Test 6: All commands should be validated +my $result = check_commands(); +ok( $result, 'All commands are valid' ); + +# Test 7: Make one command non-executable +chmod 0644, $cmd->{kpartx}; +$result = check_commands(); +ok( !$result, 'Non-executable command detected' ); + +# Test 8: Check that non-executable command fails +eval { get_command_path('kpartx') }; +like( $@, qr/not executable/, 'Non-executable command throws error' ); + +# Test 9: Restore executable and verify it works +chmod 0755, $cmd->{kpartx}; +$commands_checked = 0; +$path = get_command_path('kpartx'); +is( $path, $cmd->{kpartx}, 'Restored command works' ); + +# Test 10: Verify all expected commands exist in hash +my @expected = qw(multipath multipathd blockdev dmsetup kpartx); +my @actual = sort keys %$cmd; +is_deeply( \@actual, [sort @expected], 'All expected commands present in hash' ); + +done_testing(); diff --git a/tests/unit/test_retry_logic.t b/tests/unit/test_retry_logic.t new file mode 100644 index 0000000..c1d6430 --- /dev/null +++ b/tests/unit/test_retry_logic.t @@ -0,0 +1,141 @@ +#!/usr/bin/env perl + +use strict; +use warnings; +use Test::More tests => 8; + +# Mock HTTP response +package MockHTTPResponse { + sub new { + my ( $class, $code, $success ) = @_; + return bless { code => $code, success => $success }, $class; + } + sub code { $_[0]->{code} } + sub is_success { $_[0]->{success} } +} + +# Test retry counter logic +my $retry_count = 0; +my $max_retries = 1; +my @responses; + +sub simulate_request { + my $response = shift @responses; + return $response; +} + +# Test 1: Success on first try (no retry) +@responses = ( MockHTTPResponse->new(200, 1) ); +$retry_count = 0; + +while ( $retry_count <= $max_retries ) { + my $response = simulate_request(); + + if ( $response->code == 401 ) { + $retry_count++; + if ( $retry_count <= $max_retries ) { + next; # Retry + } + } + last; # Success or max retries +} + +is( $retry_count, 0, 'No retry on successful response' ); + +# Test 2: One retry on 401, then success +@responses = ( + MockHTTPResponse->new(401, 0), + MockHTTPResponse->new(200, 1) +); +$retry_count = 0; + +while ( $retry_count <= $max_retries ) { + my $response = simulate_request(); + + if ( $response->code == 401 ) { + $retry_count++; + if ( $retry_count <= $max_retries ) { + next; + } + } + last; +} + +is( $retry_count, 1, 'One retry on 401 response' ); + +# Test 3: Max retries reached (401 twice) +@responses = ( + MockHTTPResponse->new(401, 0), + MockHTTPResponse->new(401, 0) +); +$retry_count = 0; + +while ( $retry_count <= $max_retries ) { + my $response = simulate_request(); + + if ( $response->code == 401 ) { + $retry_count++; + if ( $retry_count <= $max_retries ) { + next; + } else { + last; # Max retries + } + } + last; +} + +is( $retry_count, 2, 'Max retries (2) attempted on repeated 401' ); + +# Test 4: Loop exits after max retries +ok( $retry_count > $max_retries, 'Retry count exceeds max_retries after exhausting' ); + +# Test 5: No retry on non-401 errors +@responses = ( MockHTTPResponse->new(500, 0) ); +$retry_count = 0; + +while ( $retry_count <= $max_retries ) { + my $response = simulate_request(); + + if ( $response->code == 401 ) { + $retry_count++; + if ( $retry_count <= $max_retries ) { + next; + } + } + last; +} + +is( $retry_count, 0, 'No retry on 500 error' ); + +# Test 6: Retry counter increments correctly +my @counts; +@responses = ( + MockHTTPResponse->new(401, 0), + MockHTTPResponse->new(401, 0), + MockHTTPResponse->new(200, 1) +); +$retry_count = 0; + +while ( $retry_count <= $max_retries ) { + push @counts, $retry_count; + my $response = simulate_request(); + + if ( $response->code == 401 ) { + $retry_count++; + if ( $retry_count <= $max_retries ) { + next; + } + } + last; +} + +is_deeply( \@counts, [0, 1], 'Retry counter increments: 0, 1' ); + +# Test 7: Max retries = 1 allows exactly 1 retry +is( $max_retries, 1, 'Max retries configured to 1' ); + +# Test 8: Total attempts = max_retries + 1 +my $total_attempts = $max_retries + 1; +is( $total_attempts, 2, 'Total attempts = 2 (initial + 1 retry)' ); + +done_testing(); diff --git a/tests/unit/test_token_cache.t b/tests/unit/test_token_cache.t new file mode 100644 index 0000000..0a6de53 --- /dev/null +++ b/tests/unit/test_token_cache.t @@ -0,0 +1,153 @@ +#!/usr/bin/env perl + +use strict; +use warnings; +use Test::More tests => 16; +use File::Temp qw(tempdir); +use File::Path qw(make_path remove_tree); +use JSON; + +# Create temporary cache directory +my $cache_dir = tempdir( CLEANUP => 1 ); + +# Mock functions from plugin +sub get_token_cache_path { + my ( $storeid, $array_index ) = @_; + my $dir = "$cache_dir/purestorage"; + make_path($dir) unless -d $dir; + chmod 0700, $dir; + return "$dir/${storeid}_array${array_index}.json"; +} + +sub write_token_cache { + my ( $cache_path, $token_data ) = @_; + + my $temp_path = "$cache_path.tmp.$$"; + open my $fh, '>', $temp_path or die "Cannot write to $temp_path: $!"; + print $fh encode_json($token_data); + close $fh; + + chmod 0600, $temp_path; + rename $temp_path, $cache_path or die "Cannot rename $temp_path to $cache_path: $!"; +} + +sub read_token_cache { + my ( $cache_path ) = @_; + + return undef unless -f $cache_path; + + open my $fh, '<', $cache_path or return undef; + my $content = do { local $/; <$fh> }; + close $fh; + + return undef unless $content; + + my $data = eval { decode_json($content) }; + return undef if $@; + + return $data; +} + +sub is_token_valid { + my ( $token_data, $ttl ) = @_; + + return 0 unless defined $token_data; + return 0 unless defined $token_data->{auth_token}; + return 0 unless defined $token_data->{created_at}; + + my $now = time(); + my $age = $now - $token_data->{created_at}; + my $refresh_threshold = $ttl * 0.8; + + return $age < $refresh_threshold; +} + +sub cleanup_expired_cache { + my ( $cache_path, $ttl ) = @_; + + my $token_data = read_token_cache($cache_path); + return unless $token_data; + + my $now = time(); + if ( $now > $token_data->{expires_at} ) { + unlink $cache_path; + } +} + +# Test 1: Cache directory creation +my $cache_path = get_token_cache_path('pure', 0); +ok( -d "$cache_dir/purestorage", 'Cache directory created' ); + +# Test 2: Cache directory permissions +my $mode = (stat("$cache_dir/purestorage"))[2] & 0777; +is( $mode, 0700, 'Cache directory has correct permissions (700)' ); + +# Test 3: Write token cache +my $token_data = { + auth_token => 'test-token-12345', + request_id => 'req-67890', + created_at => time(), + ttl => 3600, + expires_at => time() + 3600 +}; + +write_token_cache($cache_path, $token_data); +ok( -f $cache_path, 'Token cache file created' ); + +# Test 4: Cache file permissions +$mode = (stat($cache_path))[2] & 0777; +is( $mode, 0600, 'Cache file has correct permissions (600)' ); + +# Test 5: Read token cache +my $read_data = read_token_cache($cache_path); +ok( defined $read_data, 'Token cache read successfully' ); + +# Test 6: Verify token data +is( $read_data->{auth_token}, 'test-token-12345', 'Auth token matches' ); +is( $read_data->{request_id}, 'req-67890', 'Request ID matches' ); + +# Test 7: Valid token (fresh) +ok( is_token_valid($read_data, 3600), 'Fresh token is valid' ); + +# Test 8: Valid token at 79% of TTL +$token_data->{created_at} = time() - (3600 * 0.79); +write_token_cache($cache_path, $token_data); +$read_data = read_token_cache($cache_path); +ok( is_token_valid($read_data, 3600), 'Token at 79% TTL is still valid' ); + +# Test 9: Invalid token at 81% of TTL +$token_data->{created_at} = time() - (3600 * 0.81); +write_token_cache($cache_path, $token_data); +$read_data = read_token_cache($cache_path); +ok( !is_token_valid($read_data, 3600), 'Token at 81% TTL is invalid' ); + +# Test 10: Multiple array caches +my $cache_path_1 = get_token_cache_path('pure', 1); +write_token_cache($cache_path_1, $token_data); +ok( -f $cache_path_1, 'Second array cache created' ); +isnt( $cache_path, $cache_path_1, 'Different cache files for different arrays' ); + +# Test 11: Cleanup expired cache +$token_data->{created_at} = time() - 4000; +$token_data->{expires_at} = time() - 400; # Expired +write_token_cache($cache_path, $token_data); +cleanup_expired_cache($cache_path, 3600); +ok( !-f $cache_path, 'Expired cache file removed' ); + +# Test 12: Read non-existent cache +my $missing_cache = read_token_cache("$cache_dir/nonexistent.json"); +is( $missing_cache, undef, 'Non-existent cache returns undef' ); + +# Test 13: Invalid JSON in cache +my $corrupt_cache = "$cache_dir/purestorage/corrupt.json"; +open my $fh, '>', $corrupt_cache; +print $fh "{ invalid json }"; +close $fh; +my $corrupt_data = read_token_cache($corrupt_cache); +is( $corrupt_data, undef, 'Corrupt cache returns undef' ); + +# Test 14: Missing required fields +my $incomplete_data = { auth_token => 'test' }; # Missing created_at +ok( !is_token_valid($incomplete_data, 3600), 'Incomplete token data is invalid' ); + +done_testing();