Skip to content
This repository has been archived by the owner on Oct 30, 2024. It is now read-only.

s3-propagate-tags #3177

Merged
merged 3 commits into from
Sep 29, 2021
Merged

s3-propagate-tags #3177

merged 3 commits into from
Sep 29, 2021

Conversation

calvix
Copy link
Contributor

@calvix calvix commented Sep 29, 2021

This is poor man s3 bucket tag propagation.
The creation of S3 bucket is almost the same we just append the cloud tags from cluster CR

But for the update, we did not have any logic to determine the changes, to make it super simple in each update loop we simply update the tags.

I was thinking about checking if the tags are there before updating but that would be an extra AWS API call and in the end even more expensive than just blindly set the tags to desired value on every loop.

I just wanted to get rid of this ASAP as it's just one customer who needs this.

towards https://github.com/giantswarm/giantswarm/issues/18887

@calvix calvix self-assigned this Sep 29, 2021
@calvix calvix requested a review from a team September 29, 2021 14:05
return nil, microerror.Maskf(executionFailedError, "expected 1 CR got %d", len(list.Items))
}

allTags := addCloudTags(tags, list.Items[0].GetLabels())
Copy link
Contributor Author

Choose a reason for hiding this comment

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

here we take the usual tags and append the cloud tags to the map

@calvix calvix requested review from a team and removed request for a team September 29, 2021 14:42
@calvix
Copy link
Contributor Author

calvix commented Sep 29, 2021

thoughts, ideas?

Copy link
Contributor

@anvddriesch anvddriesch left a comment

Choose a reason for hiding this comment

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

LGTM if tested

@calvix calvix merged commit a131e50 into master Sep 29, 2021
@calvix calvix deleted the s3-propagate-tags branch September 29, 2021 15:56
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants