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

Fixed tenant operator target label behavior #430

Merged
merged 1 commit into from
Mar 17, 2021
Merged

Conversation

GabriFila
Copy link
Contributor

Description

This PR fixes a little bug in the tenant operator related to the targetLabel behavior. The bug is that the filter performed at the event-level in the SetupWithManager function does not apply when entering the reconcile after returning from the previous reconcile with a non-nil RequeeAfter. This causes the resource to be reconciled by contrasting operators.

How it has been discovered

The issue appeared after changing the label on a tenant from production to pre-production. Doing this, the tenant will be later reconciled by the pre-production operator. But on the last reconcile performed by the production operator, a RequeAfter set the next reconcile in 1-2 hours, which will not be filtered by the event-filter mentioned before and will change the label on all resources related to the tenant (namespaces, roleBindings,..). This behavior leaves the label on the tenant with the value pre-production but effectively brings back the tenant from pre-production to production through the change on every related resource.

Note

The fix is not the best solution. The best solution would be probably trying to update the controller-runtime dependencies to the latest version and see if this has been fixed., if not, we could write to the kubeBuilder guys.

How Has This Been Tested?

This has been tested manually using kind locally

@GabriFila GabriFila added the kind/bug Something isn't working label Mar 16, 2021
@kingmakerbot
Copy link
Collaborator

Hi @GabriFila. Thanks for your PR.

I am @kingmakerbot.
You can interact with me issuing a slash command in the first line of a comment.
Currently, I understand the following commands:

  • /rebase: Rebase this PR onto the master branch
  • /merge: Merge this PR into the master branch
  • /hold: Adds hold label to prevent merging with /merge
  • /unhold: Removes the hold label to allow merging with /merge
  • /deploy-staging: Deploy a staging environment to test this PR
  • /undeploy-staging: Manually undeploy the staging environment

Make sure this PR appears in the CrownLabs changelog, adding one of the following labels:

  • kind/breaking: 💥 Breaking Change
  • kind/feature: 🚀 New Feature
  • kind/bug: 🐛 Bug Fix
  • kind/cleanup: 🧹 Code Refactoring
  • kind/docs: 📝 Documentation

@GabriFila GabriFila marked this pull request as ready for review March 16, 2021 17:00
@GabriFila GabriFila requested a review from a team as a code owner March 16, 2021 17:00
@giorio94 giorio94 requested a review from palexster March 16, 2021 17:20
@GabriFila
Copy link
Contributor Author

/rebase

@GabriFila
Copy link
Contributor Author

/merge

@kingmakerbot kingmakerbot merged commit bba761f into master Mar 17, 2021
@kingmakerbot kingmakerbot deleted the gbf/fix_tenant branch March 17, 2021 10:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working sig/api size/S
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants