-
Notifications
You must be signed in to change notification settings - Fork 637
✨ Addons: Add functionality for resolveConflicts: PRESERVE #5330
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
✨ Addons: Add functionality for resolveConflicts: PRESERVE #5330
Conversation
|
Welcome @edmondceausu! |
|
Hi @edmondceausu. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/ok-to-test |
b599143 to
ef40f66
Compare
|
Thanks for this! Overall, this looks good to me. The tests are currently failing because Once those are fixed (and it passes e2es), I think we're good! |
|
/retest |
|
/test pull-cluster-api-provider-aws-test |
|
/test pull-cluster-api-provider-aws-e2e |
richardcase
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this @edmondceausu . Just the one small comment.
924b7be to
925b5d4
Compare
|
/test pull-cluster-api-provider-aws-e2e |
1 similar comment
|
/test pull-cluster-api-provider-aws-e2e |
|
/test ? |
|
@damdo code has been rebased. Apologies for the delay. |
damdo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
LGTM label has been added. Git tree hash: 67a9048a28dc02ac811bb4bb8ed38d116c7c8a44
|
|
This would fix #5524 🎉 |
|
@AndiDog what do I need to do to get this merged? |
|
/approve @edmondceausu Thank you for your patience! |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: nrb The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
fixes for test to pass removed v1beta1 support
632280c to
738123f
Compare
|
/test pull-cluster-api-provider-aws-e2e-blocking |
damdo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
|
LGTM label has been added. Git tree hash: 4e49ac1edd5756335888c8ed245ee09ad2efbc03
|
…s-sigs#5330) * Add functionality for resolveConflicts: PRESERVE fixes for test to pass removed v1beta1 support * make generate * updated documentation and code to reflect kubebuilder behavior * fixed rebase errors * make generate after rebase
What type of PR is this?
/kind feature
What this PR does / why we need it:
Added support for resolveConflicts.PRESERVE (API Reference). This is helpful if users are migrating an existing cluster to Cluster API and do not have their existing Addon Configs specified, or if an out of band change is made to an Addon (eg. configMap change) and the user wants to preserve it.
The schema for conflictResolution had listed
noneas the default behavior. I updated the docs and code to reflect that.*
Special notes for your reviewer*:
Checklist:
Release note: