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

AWS: Allow setting tags to snapshots or AMIs only #38

Merged
merged 1 commit into from
Nov 10, 2024

Conversation

JAVGan
Copy link
Contributor

@JAVGan JAVGan commented Nov 8, 2024

This commit introduces two new optional properties on AWSPublishingMetadata named snapshot_tags and ami_tags, which when set allows to add tags just for snapshot imports or the registered image respectively.

Rationale: Currently our team is facing sporadic issues on community AMIs workflow when uploading the image to different billing types (access or hourly) after a retry, as the second attempt will look for the image name, fail to find, search for tags and find an AMI which concerns the other billing type. This was initially fixed by release-engineering/pubtools-marketplacesvm#74 by adding the billing type as tags. However, the fix brings another potential issue: we will tag everything with the billing tags, including the S3 object, which could being unecessarily uploaded twice, as well as having a tag which doesn't concern the S3 RAW object, but the snapshot/image only.

With this change on cloudimg we aim to provide the billing type tags just for the snapshot and AMI, leaving the S3 object with the previous ones as it shouldn't receive the billing tag.

This change also allows other teams to have a more granular control on which tags should go where.

Refers to SPSTRAT-451

@JAVGan
Copy link
Contributor Author

JAVGan commented Nov 8, 2024

@lslebodn @rohanpm PTAL

cloudimg/aws.py Outdated
@@ -558,6 +566,13 @@ def publish(self, metadata):
Returns:
An EC2 Image
"""
def add_tags(tag_name, extra_kwargs):
Copy link

Choose a reason for hiding this comment

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

tags_parameter_name might be simpler to understand the code
I have a feeling tag_name might be confusing

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

This commit introduces two new optional properties on
`AWSPublishingMetadata` named `snapshot_tags` and `ami_tags`, which when
set allows to add tags just for snapshot imports or the registered image
respectively.

Rationale: Currently our team is facing sporadic issues on community
AMIs workflow when uploading the image to different billing types
(`access` or `hourly`) after a retry, as the second attempt will look
for the image name, fail to find, search for tags and find an AMI which
concerns the other billing type. This was initially fixed by
release-engineering/pubtools-marketplacesvm#74 by adding the billing
type as tags. However, the fix brings another potential issue: we will
tag everything with the billing tags, including the S3 object, which
could being unecessarily uploaded twice, as well as having a tag which
doesn't concern the S3 RAW object, but the snapshot/image only.

With this change on `cloudimg` we aim to provide the billing type tags
just for the snapshot and AMI, leaving the S3 object with the previous
ones as it shouldn't receive the billing tag.

This change also allows other teams to have a more granular control on
which tags should go where.

Refers to SPSTRAT-451
Copy link

@lslebodn lslebodn left a comment

Choose a reason for hiding this comment

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

LGTM

@rohanpm rohanpm merged commit 143d93c into release-engineering:master Nov 10, 2024
4 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants