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: fix ControlPlane managed resource labels during migration from 1.2 and older #369

Merged
merged 7 commits into from
Jul 10, 2024

Conversation

pmalek
Copy link
Member

@pmalek pmalek commented Jun 24, 2024

What this PR does / why we need it:

#259 introduced a new set of labels that are used on ControlPlane's

  • ClusterRoles
  • ClusterRoleBindingss
  • ValidatingWebhookConfigurations

This omitted 1 important detail which is migrating on operator upgrade (specifically from 1.2.x and older to 1.3.0).

This PR fixes this by changing the listing and deleting logic for the aforementioned resources so that the old label and owner reference are also taken into account.

This also makes other small changes

  • make operator leave the newest ClusterRole or ValidatingWebhookConfiguration in place when there's more in the cluster;
    • this ensures that when for some reason there are more than 1 of these, the newest with newest rules, stays
  • when ControlPlane fails to get provisioned - this can happen when operator's webhook doesn't respond - requeue, without this, provisioned Gateway gets stuck with ControlPlaneReady condition set to False.

Which issue this PR fixes

Fixes #373

Special notes for your reviewer:

PR Readiness Checklist:

Complete these before marking the PR as ready to review:

  • the CHANGELOG.md release notes have been updated to reflect significant changes

@pmalek pmalek self-assigned this Jun 24, 2024
@pmalek pmalek changed the title fix: fix ControlPlane managed resource labels during migration from o… fix: fix ControlPlane managed resource labels during migration from older operator versions Jun 24, 2024
@pmalek pmalek force-pushed the fix-controlplane-managed-resources-labels-migration branch 2 times, most recently from b0dd45f to ec1a893 Compare July 5, 2024 14:59
@pmalek pmalek force-pushed the fix-controlplane-managed-resources-labels-migration branch from ec1a893 to f9dd643 Compare July 5, 2024 15:09
@pmalek pmalek added this to the KGO v1.4.x milestone Jul 5, 2024
@pmalek pmalek force-pushed the fix-controlplane-managed-resources-labels-migration branch 5 times, most recently from a6650b1 to 007d337 Compare July 5, 2024 17:34
@pmalek pmalek marked this pull request as ready for review July 5, 2024 18:12
@pmalek pmalek requested a review from a team as a code owner July 5, 2024 18:12
@pmalek pmalek requested a review from programmer04 July 8, 2024 09:09
Copy link
Member

@programmer04 programmer04 left a comment

Choose a reason for hiding this comment

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

LGTM, but since it will be dealing with existing users configs I think someone else should take a look too, to double check

@pmalek pmalek changed the title fix: fix ControlPlane managed resource labels during migration from older operator versions fix: fix ControlPlane managed resource labels during migration from 1.2 and older Jul 8, 2024
@pmalek pmalek requested a review from mlavacca July 8, 2024 16:38
controller/controlplane/controller_reconciler_utils.go Outdated Show resolved Hide resolved
controller/controlplane/controller_reconciler_utils.go Outdated Show resolved Hide resolved
controller/controlplane/controller_reconciler_utils.go Outdated Show resolved Hide resolved
controller/controlplane/controller_reconciler_utils.go Outdated Show resolved Hide resolved
controller/controlplane/controller_reconciler_utils.go Outdated Show resolved Hide resolved
controller/controlplane/controller_reconciler_utils.go Outdated Show resolved Hide resolved
controller/controlplane/controller_watch.go Show resolved Hide resolved
controller/controlplane/controller_watch.go Outdated Show resolved Hide resolved
controller/controlplane/controller_watch.go Outdated Show resolved Hide resolved
controller/gateway/controller.go Show resolved Hide resolved
@pmalek pmalek enabled auto-merge (squash) July 9, 2024 13:18
@pmalek pmalek force-pushed the fix-controlplane-managed-resources-labels-migration branch from a1d5f7b to 18aa0d2 Compare July 10, 2024 10:53
pmalek and others added 3 commits July 10, 2024 13:05
@pmalek pmalek force-pushed the fix-controlplane-managed-resources-labels-migration branch from 18aa0d2 to a91bffd Compare July 10, 2024 11:05
@pmalek pmalek merged commit 508daaf into main Jul 10, 2024
20 checks passed
@pmalek pmalek deleted the fix-controlplane-managed-resources-labels-migration branch July 10, 2024 11:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants