Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix tags not being applied on AWS EBS Snapshots #1007

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions barman/cloud_providers/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,7 @@ def get_snapshot_interface(config):
config.aws_profile,
config.aws_region,
config.aws_await_snapshots_timeout,
config.tags,
]
return AwsCloudSnapshotInterface(*args)
else:
Expand Down Expand Up @@ -253,6 +254,7 @@ def get_snapshot_interface_from_server_config(server_config):
server_config.aws_profile,
server_config.aws_region,
server_config.aws_await_snapshots_timeout,
server_config.tags,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be removed. This is the server config object and there's currently no tags option available in the server parameters. This function is used in the context of snapshot backups via the barman standard server (when backup_method = snapshot) and there's currently no way to pass tags in there.

You indeed get an error when attempting a snapshot backup via barman server with your code:

$ barman backup snapshot-server
WARNING: No backup strategy set for server 'snapshot-server' (using default 'concurrent_backup').
ERROR: Error initialising snapshot provider aws: 'ServerConfig' object has no attribute 'tags' 

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
server_config.tags,

)
else:
raise CloudProviderUnsupported(
Expand Down
16 changes: 12 additions & 4 deletions barman/cloud_providers/aws_s3.py
Original file line number Diff line number Diff line change
Expand Up @@ -463,7 +463,7 @@ class AwsCloudSnapshotInterface(CloudSnapshotInterface):
https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/ebs-creating-snapshot.html
"""

def __init__(self, profile_name=None, region=None, await_snapshots_timeout=3600):
def __init__(self, profile_name=None, region=None, await_snapshots_timeout=3600, tags=None):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's also add a description for tags in the docstring.

"""
Creates the client necessary for creating and managing snapshots.

Expand All @@ -478,6 +478,7 @@ def __init__(self, profile_name=None, region=None, await_snapshots_timeout=3600)
self.region = region or self.session.region_name
self.ec2_client = self.session.client("ec2", region_name=self.region)
self.await_snapshots_timeout = await_snapshots_timeout
self.tags = tags

def _get_waiter_config(self):
delay = 15
Expand Down Expand Up @@ -710,13 +711,20 @@ def _create_snapshot(self, backup_info, volume_name, volume_id):
volume_name,
volume_id,
)
tags = [
{"Key": "Name", "Value": snapshot_name},
]

if self.tags is not None:
for tag in self.tags:
key, value = tag
tags.append({"Key": key, "Value": value})

resp = self.ec2_client.create_snapshot(
TagSpecifications=[
{
"ResourceType": "snapshot",
"Tags": [
{"Key": "Name", "Value": snapshot_name},
],
"Tags": tags,
}
],
VolumeId=volume_id,
Expand Down
45 changes: 45 additions & 0 deletions tests/test_cloud_snapshot_interface.py
Original file line number Diff line number Diff line change
Expand Up @@ -2740,6 +2740,51 @@ def test_create_snapshot(self, caplog):
in caplog.text
)

def test_create_snapshot_with_tags(self, caplog):
"""
Verify that _create_snapshot calls boto3 with defined tags and returns the expected values.
"""
# GIVEN a new AwsCloudInterface with tags defined
snapshot_interface = AwsCloudSnapshotInterface(tags=[
("environment", "production"),
("project", "my-project"),
("service", "barman")
])
# AND a backup_info for a given server name and backup ID
backup_info = mock.Mock(backup_id=self.backup_id, server_name=self.server_name)
# AND a mock create_snapshot function which returns a successful response
mock_ec2_client = self._mock_boto3.Session.return_value.client.return_value
mock_resp = mock_ec2_client.create_snapshot.return_value
mock_resp["State"] = "pending"
# AND log level is INFO
caplog.set_level(logging.INFO)

# WHEN _create_snapshot is called
volume_name = "my-pgdata-volume"
volume_id = "vol-0123456789abcdef01"
snapshot_name, _ = snapshot_interface._create_snapshot(
backup_info,
volume_name,
volume_id,
)

# THEN create_snapshot is called on the EC2 client with the expected args
mock_ec2_client.create_snapshot.assert_called_once()
mock_ec2_client.create_snapshot.assert_called_once_with(
TagSpecifications=[
{
"ResourceType": "snapshot",
"Tags": [
{"Key": "Name", "Value": snapshot_name},
{"Key": "environment", "Value": "production"},
{"Key": "project", "Value": "my-project"},
{"Key": "service", "Value": "barman"}
],
}
],
VolumeId=volume_id,
)

def test_create_snapshot_failed(self):
"""
Verify that _create_snapshot calls boto3 and returns the expected values.
Expand Down