Aws irsa auth barman cloud #1103
Draft
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit includes necessary shared logic which will be used by recovery operations in order to achieve remote execution. This includes: - `_get_command_interface`: a shared method created in the `RecoveryOperation` class to retrieve the appropriate command interface for each operation. The appropriate command interface is based on whether the operation will be executing locally (`staging_location` as `local`) or remotely (`staging_location` as `remote`). References: BAR-795 Signed-off-by: Gustavo William <gustavo.oliveira@enterprisedb.com>
Added tests for all the shared logic created in the previous commit which allows the recovery operations to be able to execute remotely. References: BAR-795 Signed-off-by: Gustavo William <gustavo.oliveira@enterprisedb.com>
Added a new `_post_recovery_cleanup` method to the `RecoveryOperation` class. This is a helper method used to remove unwanted files from the destination directory after the operation is completed, such as contents inside the `pg_wal`, `pg_log`, the `postmaster.pid` file, etc. This method is useful for certain operations that direct its output directly to the final destination, without the means to ignore specific files. References: BAR-792 Signed-off-by: Gustavo William <gustavo.oliveira@enterprisedb.com>
Added a unit test to the new `post_recovery_cleanup` method added to the `RecoveryOperation` class in the previous commit. References: BAR-792 Signed-off-by: Gustavo William <gustavo.oliveira@enterprisedb.com>
Rename misleading WAL number label in barman show-backup output to `Number of WALs`. This commit changes the output, tests and docs related to this label. References: BAR-791 Signed-off-by: Andre Marchesini <andre.marchesini@enterprisedb.com>
Adds a release note for BAR-791 which changes the label `WAL Number` to `Number of WALs` from the `barman show-backup` command. References: BAR-791 Signed-off-by: Andre Marchesini <andre.marchesini@enterprisedb.com>
When the archiving check fails, users are advised to manually switch a WAL file and run `barman cron` (as per docs). This is because Barman relies primarily on the contents of the `xlog.db`. If no WALs are present there, it assumes the archiving is not working correctly and advises users to switch a WAL and run cron to see if content arrives. This is a process that can be automated, as Barman has the ability to identify an empty `xlog.db` and automatically switch and archive WAL files internally. That said, this commit implements the following logic: If Barman notices there's nothing in the `xlog.db` and no previous backups, instead of giving an error it will: 1. Check if there are WALs in the incoming/streaming directories. If there are, run internal `barman archive-wal` command to archive them. 2. If there are no WALs in the incoming/streaming directories, it will run the internal `barman switch-wal` command to switch to a new WAL file and try to archive it as soon as it arrives. The timeout for the WAL file to arrive is <check_timeout> / 3. 3. If Barman still wasn't able to archive anything, then we consider the check as failed. The idea is to provide a better user experience by not making users have to run such commands manually whenever configuring a new server or when Barman has `xlog.db` empty for some reason. References: BAR-718 Signed-off-by: Gustavo William <gustavo.oliveira@enterprisedb.com>
After changing the logic of the check_archive method to automatically switch WALs for the user when the check fails (previous commit), the tests needed to be updated to reflect this new behavior. This commit updates the tests to ensure they correctly verify the new logic. References: BAR-718 Signed-off-by: Gustavo William <gustavo.oliveira@enterprisedb.com>
References: BAR-718 Signed-off-by: Gustavo William <gustavo.oliveira@enterprisedb.com>
Added a `cmd` attribute to the `RecoveryOperation` class to hold the command interface used for executing Unix commands. This is so that we don't have to keep calling `_get_command_interface` every time we need to run a command, which can be inefficient. References: BAR-834 Signed-off-by: Gustavo William <gustavo.oliveira@enterprisedb.com>
Implements the `RsyncCopyOperation` to handle the copy of backups and tablespaces. The main logic is split into the following methods: - `_rsync_backup_copy`: the entry point of the copy operation. It is run by the `_execute` method. This method creates the rsync copy controller and calls `_copy_backup_dir` or `_copy_pgdata_and_tablespaces` accordinggly, depending if this is the last operation or not. - `copy_backup_dir`: called when this is not the last operation. It copies the whole backup directory (as in the Barman catalog) to the destination (which in this case will be in the staging directory). - `_copy_pgdata_and_tablespaces`: called when this is the last operation. It copies PGDATA to the root of the destination directory and tablespaces to their final destination, honoring relocation, if requested. References: BAR-614 Signed-off-by: Gustavo William <gustavo.oliveira@enterprisedb.com>
Added tests for the `RsyncCopyOperation` class implemented in the previous commit. References: BAR-614 Signed-off-by: Gustavo William <gustavo.oliveira@enterprisedb.com>
directory Previously, Barman did not verify whether tablespace destination directories were empty before restoring, which could result in their contents being silently overwritten—potentially causing data loss or corruption. Now, the restore process ensures that all destination directories, including those for data and tablespaces, are empty before proceeding. If any are not, Barman will halt the operation and display an appropriate error message, significantly improving safety and data integrity for both local and remote restores. References: BAR-787 Signed-off-by: Andre Marchesini <andre.marchesini@enterprisedb.com>
Adds test cases for the restore operation verifying that the destination for the data and tablespaces directories are not empty, preventing possible issues with data corruption or unintended behavior when restoring. References: BAR-787 Signed-off-by: Andre Marchesini <andre.marchesini@enterprisedb.com>
Adds a release note for BAR-787 which verifies if the data and tablespace destination directories are non-empty before restoring. References: BAR-787 Signed-off-by: Andre Marchesini <andre.marchesini@enterprisedb.com>
`pg_combinebackup` requires the symbolic links of the backups tablespaces to be correct previous to combining them, otherwise tablespaces are ignored. That said, we need to ensure that the symbolic links are created, mainly after operations such as `RsyncCopyOperation` and `DecompressOperation`, which are the only ones that can preceed a `CombineOperation` in the recovery process. This new method is intened to replace the old `_prepare_tablespaces` method, which performed a similar function but created the links before the copy process and was only called once per recovery process, essentially dealing only with the final destination of the tablespaces, not with the case where they are in the staging area. That said, this commit adds a new method `_link_tablespaces` to the `RecoveryOperation` class, which will handle that and can be reused by operations when needed. The method creates a symbolic link from `PGDATA/pg_tblspc/<oid>` to `<tablespace_path>`. `<tablespace_path>` will either point to: 1. The final tablespace location, if this is the last operation; 2. The tablespace location in the staging area, if more operations will follow. Intended usage: - After the `RsyncCopyOperation` or `DecompressOperation` is done, call `_link_tablespaces` to create the symbolic links for the tablespaces, as a `CombineOperation` might come next. If it's the last operation, it will do the same as the old `_prepare_tablespaces`. - `DecryptOperation` does not need it because it will always be followed by a `DecompressOperation` and will never be the last operation. - `CombineOperation` does not need to call it either, as it `pg_combinebackup` already handles the symbolic links when relocating. References: BAR-800 Signed-off-by: Gustavo William <gustavo.oliveira@enterprisedb.com>
Added some tests to the `_link_tablespaces` method to ensure it creates the symbolic links correctly and handles errors properly. References: BAR-800 Signed-off-by: Gustavo William <gustavo.oliveira@enterprisedb.com>
Boto3 >= 1.46 switched the default value for Data Integrity Protections, breaking compatibility with some S3 providers who haven't implemented this yet. This commit adds a note to the docs so users know how to workaround the issue. References: BAR-762 Reported by: Sylvain Desbureaux Closes #1086 Signed-off-by: Martín Marqués <martin.marques@enterprisedb.com>
References: BAR-799 Signed-off-by: Anhui-tqhuang <tianqiu.huang@enterprisedb.com>
Changes "No of files" to "Number of files" in the show-backup command. References: BAR-798 Signed-off-by: Andre Marchesini <andre.marchesini@enterprisedb.com>
Adds a release note for BAR-798 which changes the label `No of files` of the Wal Information section to `Number of files` from the `barman show-backup` command output. References: BAR-798 Signed-off-by: Andre Marchesini <andre.marchesini@enterprisedb.com>
Implements the `CombineOperation` class to handle the combination of incremental backups. The main logic is split into the following methods: - `_combine_backups`: the main method, receives the restoring incremental backup, prepare all arguments an call `_run_pg_combinebackup`. - `_run_pg_combinebackup`: invokes the `PgCombineBackup` command wrapper class to combine the incremental backups. It handles the command execution, retries, and error handling. - `_get_tablespace_mapping`: retrieves the tablespace mapping which will be passed to the `PgCombineBackup` command wrapper class. If it is the last operation, tablespaces are mapped to their final destination, honoring relocation, if requested. Otherwise, they are mapped to their directory in the volatile backup's directory. - `_get_backup_chain_paths`: retrieves the paths of the backup chain that will be combined, from parent to child. - `_fetch_remote_status`: a helper method to retrieve information about the `pg_combinebackup` utility. - `_retry_handler`: a helper method to handle clean up directories between retries. References: BAR-612 Signed-off-by: Gustavo William <gustavo.oliveira@enterprisedb.com>
Added tests for the new `CombineOperation` class implemented in the previous commit. References: BAR-612 Signed-off-by: Gustavo William <gustavo.oliveira@enterprisedb.com>
…ing` This adds a regex pattern to the `EXCLUDE_LIST` in `postgres_plumbing` to exclude temporary relation files from the backup, similar to the exclusion performed by `pg_basebackup`. This is needed to avoid errors when running Barman in rsync-concurrent mode against a Postgres instance that creates and drops many temporary tables, since rsync might try to copy a temp file that had already been removed, causing Barman to hang until unblocked. Closes #293 References: BAR-266 Signed-off-by: Barbara Leidens <barbara.leidens@enterprisedb.com> Co-authored-by: Israel <israel.barth@enterprisedb.com>
References: BAR-266 Signed-off-by: Barbara Leidens <barbara.leidens@enterprisedb.com> Co-authored-by: Israel <israel.barth@enterprisedb.com>
Updated method names and documentation to use "decompress" instead of "uncompress" for greater clarity and correctness when referring to the removal of file compression. All incorrect uses of “uncompress” in the code and documentation have been replaced. We use “decompress” to describe the action of removing compression from a file. “uncompressed” should refer only to the state of a file that is no longer compressed. References: BAR-770 Signed-off-by: Andre Marchesini <andre.marchesini@enterprisedb.com>
This commit replaces the placeholder `_decompress_backup` method in `DecompressOperation` with a complete implementation that handles the decompression of the base tarball and tablespace files using the appropriate compression class (gzip, lz4, zstd, or none). Key changes: - Implemented `_decompress_backup` to call the correct decompressor based on the `backup_info.compression` value. - Added logic to determine the correct target paths for tablespaces, including support for relocation when specified. - Enhanced logging with `output.debug` to aid debugging and traceability. - Added UnsupportedCompressionFormat custom exception to exceptions.py. These changes are part of the ongoing recovery code refactoring to modularize and simplify the recovery pipeline. References: BAR-770 Signed-off-by: Andre Marchesini <andre.marchesini@enterprisedb.com>
Adds test cases for the implementation of DecompressOperation. References: BAR-770 Signed-off-by: Andre Marchesini <andre.marchesini@enterprisedb.com>
When testing rsync in combination with other operations following it, I noticed a few problems: - In the scenario where we copy the whole backup directory to the staging area, we were missing the passing the `item_class` parameter when calling `add_directory` in the copy controller. While this is not really that relevant for the copyt itself, it is a required parameter and it raises an error if not passed. I created a new `VOLATILE_BACKUP_CLASS` option and it's what I'm passing there. - We need to call `_link_tablespaces` after every rsync operation so that it creates the symlinks for the tablespaces. - Reimplemented the `save` method of `VolatileBackupInfo` class. While it's true that we should not save a volatile backup to a file, we need to to save it to a file-like object in the `_create_volatile_backup_info` method, so it was failing whenever the `backup_info` received was a `VolatileBackupInfo` instance. References: BAR-768 Signed-off-by: Gustavo William <gustavo.oliveira@enterprisedb.com>
… with other operations Fixed the tests that were broken by the previous commit, which fixed the `RsyncCopyOperation` class to run in conjunction with other operations. References: BAR-768 Signed-off-by: Gustavo William <gustavo.oliveira@enterprisedb.com>
restoring It removes the danger notes about overwriting pgdata and tablespaces when restoring a backup to a node where there is already a postgres cluster with the same pgdata and tablespaces locations Or when restoring a backup locally. In BAR-787 #478, we have updated this behavior to raise an error and quit if the recovery target is an existing, non-empty directory. References: BAR-845 References: BAR-787 Signed-off-by: Andre Marchesini <andre.marchesini@enterprisedb.com>
The retry handler method passed to `PgCombineBackup` had the wrong signature, which caused errors whenever called. Instead of fixing the handler, after internal discussion, it was decided to remove the retry mechanism for `pg_combinebackup` entirely, as: 1. There are not that many variables involved in combining backups, so if something failed it is likely because it was supposed to and a retry is not going to produce a different result. 2. Similar operations, e.g. decompression and decryption, which are also part of the recovery process do not have any retry mechanism at all. 4. The general rule throughout the code seems to be to only have retry mechanisms for things that are network bound, such as `rsync` and `pg_basebackup`. Due to that, this commit removes the whole logic of retrying `pg_combinebackup` from the code. References: BAR-852 Signed-off-by: Gustavo William <gustavo.oliveira@enterprisedb.com>
Fixed an issue in the `DecompressOperation` where the backup manifest
was not being copied to the next staging directory. The manifest file is
required by subsequent a `CombineOperation` that might come later.
This is only required when `DecompressOperation` is not the last
operation in the recovery process.
The copy of the manifest file itself was implemented using a new `copy`
method added to the `fs` module, which uses the `cp` command. Using rsync
here was avoided because it doesn't support copying files between two
locations on the same remote server. E.g. there might be a case where
`DecompressOperation` is executed on a remote server and has to copy the
manifest file from `/remote/server/staging/barman-decryption/backup_manifest` to
`/remote/server/staging/barman-decompress/backup_manifest`. Doing that via
rsync results in errors like the one below:
EXCEPTION: {'ret': 1, 'out': '', 'err': 'The source and destination cannot both be remote.\nrsync error: syntax or usage error (code 1) at main.c(1420) [Receiver=3.2.3]\n'}
To avoid that, a simple `cp` command turned out to be easier and more reliable.
References: BAR-853
Signed-off-by: Gustavo William <gustavo.oliveira@enterprisedb.com>
Fixed the tests that were broken when changing the `DecompressOperation` to copy the backup manifest to the next staging directory. Also added a test to the new `copy` method created in the `fs` module. References: BAR-853 Signed-off-by: Gustavo William <gustavo.oliveira@enterprisedb.com>
This reverts commit 5de6b7ac07748095d4dcbcb8648e50894d4bc9a2. We are reverting this as we realised that this change would increase cloud costs at least in 10x. References: BAR-855
This reverts commit 0693bf74dfeb629d9cd926b318ef0fae4968b3c6. Reverting this as this change would increase cloud cost bills at least in 10x. References: BAR-855
Adds a new flag `--if-not-exists` to `barman receive-wal` command. This flag prevents `receive-wal` from exiting with an error when a slot with the same name already exists. This is the same behavior as calling `pg_receivewal` with the same flag. This flag will have an effect only with `--create-slot`, otherwise it is just ignored. References: BAR-527 Special thanks to @crazybolillo <antonio@zoftko.com> a.k.a @antonag32 for creating this PR and contributing to Barman. Signed-off-by: Andre Marchesini <andre.marchesini@enterprisedb.com>
Adds a release note for BAR-527 which adds a new flag `--if-not-exists` to `barman receive-wal`. References: BAR-527 Signed-off-by: Andre Marchesini <andre.marchesini@enterprisedb.com>
This commit sets a default value of "local" for the `staging_location` configuration option. While testing local recovery of compressed backups, we identified an issue: for such backups, `staging_location` is unnecessary (the recovery is already local), and `staging_path` is also not needed since decompression is performed directly into the final destination. After reviewing the recovery logic, we determined that assigning a default value of "local" to `staging_location` aligns better with user expectations. It’s generally preferable for Barman to handle decompression, decryption, and/or backup combination locally before transferring the result to the remote node, rather than performing those operations on the remote system. We also set a default for `staging_path='/tmp'` at runtime so we do not break compatibility with deprecated options. References: BAR-850 Signed-off-by: Andre Marchesini <andre.marchesini@enterprisedb.com>
Modifies unit tests based on the changes that we introduced in BAR-850, which assign default values to staging_path and staging_location options. References: BAR-850 Signed-off-by: Andre Marchesini <andre.marchesini@enterprisedb.com>
…ation Adds info to documention in the configuration section informing the user about the default values of `staging_path` and `staging_location`. References: BAR-850 Signed-off-by: Andre Marchesini <andre.marchesini@enterprisedb.com>
This commit adds some important information implemented in BAR-850 to the compiled release note of the new recovery features so we avoid creating a separate release note as this info is better suited to be already part of the compiled information. References: BAR-850 References: BAR-846 Signed-off-by: Andre Marchesini <andre.marchesini@enterprisedb.com>
Barman restore fails with an unhandled exception when there are no custom tablespaces in Postgres. This fixes this bug by adding a check in the method _link_tablespaces, after creating the `pg_tblspc` directory and before actually linking tablespace, so it returns when there are no custom tablespaces in the backup. References: BAR-861 Signed-off-by: Andre Marchesini <andre.marchesini@enterprisedb.com>
Previously, Barman attempted to map the deprecated options `recovery_staging_path` and `local_staging_path` to equivalent new options using `staging_path` and `staging_location`. However, this mapping can sometimes generate weird combinations, e.g. having staging area as "local" for some operations and then "remote" for others, and then "local" again later. This commit fixes this issue by ensuring that, whenever the deprecated options are used, `staging_location` is always set to "local". This guarantees a consistent behavior across all operations. References: BAR-864 Signed-off-by: Gustavo William <gustavo.oliveira@enterprisedb.com>
barman-cloud-backup and barman-cloud-wal-archive now support multiple --tag flags (e.g. --tag key1,val1 --tag key2,val2) instead of a single --tags flag with multiple values. This change resolves a known issue where the --tags flag was not correctly parsed when placed after positional arguments. References: BAR-441 Signed-off-by: Andre Marchesini <andre.marchesini@enterprisedb.com>
It adds unit tests for the new way Barman will use tags with barman-cloud commands that have support for it. References: BAR-441 Signed-off-by: Andre Marchesini <andre.marchesini@enterprisedb.com>
Changed docs that reference the tags flags in the command section. Note: Fix docs related to `--snapshot-disk NAME` as a side change as it was incorrectly documented how to use it. References: BAR-441 Signed-off-by: Andre Marchesini <andre.marchesini@enterprisedb.com>
Adds a release note for BAR-441 which changes how tags are used in barman-cloud-backup and barman-cloud-wal-archive. References: BAR-441 Signed-off-by: Andre Marchesini <andre.marchesini@enterprisedb.com>
This commit makes more visible some recommendations regarding the use of NFS as the storage for backups and WALs. First we make sure the word NFS appears in the configuration option for barman_lock_directory so it's easier to spot. Second, we revive a section from the old docs that wasn't migrated regarding recommendations on Backups. This shares information on how the development team tests and certain recommendations on how to use Barman together with an NFS storage. References: BAR-862 Signed-off-by: Martín Marqués <martin.marques@enterprisedb.com>
Previously, the `MainRecoveryExecutor` was not able to relocate tablespaces inside the PGDATA directory depending on the operations being performed. The problem was mainly in two operations: `DecompressOperation` and `CombineOperation`. 1. In the `DecompressOperation`, the problem was that the tablespaces were decompressed to the final destination first (e.g. /path/to/destination/tbs1) and then later PGDATA was decompressed (e.g. /path/to/destination). But every destination is cleaned before decompressing so decompressing PGDATA essentially meant deleting the tablespaces copied previously. This problem is fixed by cleaning PGDATA destination first, then decompressing the tablespaces, and finally decompressing PGDATA. 2. In the `CombineOperation`, the main problem was that the tablespaces directories were created inside the PGDATA destination directory, and `pg_combinebackup` raises an error if the destination directory is not empty. This is fixed by avoiding creating the tablespace directories if they are subdirectories of the PGDATA destination directory. Other than that, the `RsyncCopyOperation` deals with tablespaces inside PGDATA just fine. And the `DecryptOperation` does not need to handle this because it is never the last operation, hence it is never the responsible for relocating tablespaces. References: BAR-860 Signed-off-by: Gustavo William <gustavo.oliveira@enterprisedb.com>
…erations
When restoring an incremental compressed backup where its parent
was not compressed, the following error was thrown out:
Traceback (most recent call last):
File "/home/vagrant/Repositories/edb/barman-internal/barman/recovery_executor.py", line 2592, in _decompress_backup
compressor = compressors[compression](self.cmd)
KeyError: None
This commit addresses the bug by introducing an additional
`_should_execute` check within the `_execute_on_chain` method for
each backup in the chain. This ensures that the operation is
only performed on backups that actually require it.
The method signatures have been slightly changed because destination
is a common argument for all operation methods that inherit from the
parent class and it seems reasonable to add it in the front after
backup_info so we have this distinction of what is required and what
is optional. Also, as we need the destination in the
`_execute_on_chain`, this seems a good approach.
It also adds a logic to prepare directories, move/copy backups that
were not operated on and link tablespaces, so we can safely cleanup
the staging paths in between operations, instead of having to move
the cleanup of all to the end.
References: BAR-863
Signed-off-by: Andre Marchesini <andre.marchesini@enterprisedb.com>
Users have complained that Barman doesn't copy all the transaction logs it has when restoring with no-get-wal, particularly, the .partial file in the streaming directory. In this commit we document how to workaround the limitation by copying the partial WAL to the staging directory for WALs on the destination node. References: BAR-858 Signed-off-by: Martín Marqués <martin.marques@enterprisedb.com>
Manual fixes on compiled release notes Signed-off-by: Andre Marchesini <andre.marchesini@enterprisedb.com>
References: BAR-874 Signed-off-by: Andre Marchesini <andre.marchesini@enterprisedb.com>
Signed-off-by: Andre Marchesini <andre.marchesini@enterprisedb.com>
adding aws_irsa to cloud_ingerface kwargs adding aws_irsa to S3CloudInterface and AwsCloudSnapshotInterface relocating arg to preserve signature adding aws_irs to serverConfig reposition aws_ira arg on AwsCloudSnapshotInterface handle aws_profile or aws_irsa on get_snapshot_interface_from_backup_info adding aws_irsa to test barman cloud backup adding aws_irsa to test barman cloud restore adding aws_irsa to test barman cloud wal archive adding aws_irsa to test cloud, config and helpers updating docs with aws_irsa updating comment with aws_irsa context adding more context for --aws-irsa flag adding more context to flag, as this is eks specific References: BAR-407
This commit performs some fixes to ensure correctness when using the new aws_irsa flag. - Adds `False` as the default value for the config `aws_irsa` - Refactors `get_snapshot_interface_from_backup_info` from cloud_providers.__init__. The code would never reach the else clause if `aws_irsa` was not passed never reaching the aws_profile config. - Relocated the `aws_irsa` config when passed as a list. This would cause issues when unpacking the values to the Interface. - Fix all tests broken by this implementation - Amend docs adding additional information. Add additional information requiring specific env vars to be used with aws_irsa. References: BAR-407 Signed-off-by: Andre Marchesini <andre.marchesini@enterprisedb.com>
Contributor
Author
|
Hey @smcaine, how are you doing? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PR to test AWS IRSA auth