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.

Sign up for GitHub

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

Jump to bottom

Preserve label/annotations on updated Deployments #71

Merged
merged 3 commits into from
Jul 16, 2024
Merged

Preserve label/annotations on updated Deployments #71

merged 3 commits into from
Jul 16, 2024

Conversation

jgwest
Copy link
Member

@jgwest jgwest commented Jun 18, 2024 •
edited
Loading

What does this PR do / why we need it:

  • At present, when we reconcile a Rollout on an existing Deployment, and that Deployment contains labels/annotations that were not originally present, we will update the Deployment to remove those labels.
  • However, labels/annotations can be added by other external tools in Kubernetes, and we should preserve them.
  • Likewise, we should not update a Deployment if no other fields changes besides the labels/annotations.

Have you updated the necessary documentation?

  • Documentation update is required by this PR, and has been updated.

Which issue(s) this PR fixes:
Fixes #72

How to test changes / Special notes to the reviewer:

@jgwest
Preserve label/annotations on updated Deployments
8506dbc 
Signed-off-by: Jonathan West <jonwest@redhat.com>
@jgwest
Normalize the .spec.template annotation/label map to reduce churn
f6decdc 
Signed-off-by: Jonathan West <jonwest@redhat.com>
@jgwest
Recreate Deployment when .spec.selector is updated
7c51603 
Signed-off-by: Jonathan West <jonwest@redhat.com>
@jgwest jgwest force-pushed the preserve-labels-annots-on-deployments-may-2024 branch from 5ba57ca to 7c51603 Compare June 25, 2024 06:06
@jgwest jgwest requested a review from chetan-rns July 10, 2024 09:01
chetan-rns
chetan-rns approved these changes Jul 15, 2024
View reviewed changes
Copy link
Collaborator

@chetan-rns chetan-rns left a comment

Choose a reason for hiding this comment

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

Thanks @jgwest

/lgtm

@jgwest
Copy link
Member Author

jgwest commented Jul 16, 2024

Thanks @chetan-rns!

@jgwest jgwest merged commit c771c50 into argoproj-labs:main Jul 16, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chetan-rns chetan-rns chetan-rns approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Label/annotations on updated Deployments should be preserved
2 participants
@jgwest @chetan-rns