Conversation
jan-g
requested review from
RafalKorepta,
andrewstucki,
chrisseto and
gene-redpanda
as code owners
jan-g
force-pushed
the
v1-drop-drift-controller
branch
2 times, most recently
from
7c2f311 to
048fe2e
Compare
Contributor
Author
|
To-do here: also stash the hash of the full last-applied-configuration. Use that to short-circuit the periodic reassertion of configuration, so that actual config changes get pushed when they're seen. Edited to add: we stash the last-applied-critical-configuration hash - that's the hash of the cluster config, considering only specified settings that require a cluster restart. |
jan-g
force-pushed
the
v1-drop-drift-controller
branch
from
048fe2e to
22602d3
Compare
jan-g
force-pushed
the
v1-drop-drift-controller
branch
10 times, most recently
from
e5f4ee6 to
a861a84
Compare
This the first step to dropping the drift controller.
Drift detection is replaced by a periodic reassertion of desired config.
Use the generation / observed generation to trigger an immediate configuration push, if required.
We'll have to update some of these, since the annotations applied will have changed.
We're just using a declarative-mode assert in v1 now.
jan-g
force-pushed
the
v1-drop-drift-controller
branch
from
4cf44f8 to
90c8d4c
Compare
operator/pkg/resources/secret.go
Outdated
| ) error { | ||
| // Do not touch existing last-applied-configuration (it's not reconciled in the main loop) | ||
| if val, ok := current.Annotations[LastAppliedConfigurationAnnotationKey]; ok { | ||
| // TODO: what's this doing on here? |
Contributor
There was a problem hiding this comment.
This check can be removed. Secrets are not annotated.
The update method had a spurious copy of an annotation that's never applied to it.
jan-g
added a commit
that referenced
this pull request
Apr 4, 2025
* Periodically cycle to reassert cluster configuration * Drop drift controller * Fix up unit test failures * Drop three-way merge code * Drop annotation copy in secret (cherry picked from commit d9feceb) # Conflicts: # operator/internal/controller/vectorized/cluster_controller_configuration.go
Contributor
Author
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
alenkacz
pushed a commit
that referenced
this pull request
Apr 7, 2025
* Periodically cycle to reassert cluster configuration * Drop drift controller * Fix up unit test failures * Drop three-way merge code * Drop annotation copy in secret
alenkacz
pushed a commit
that referenced
this pull request
Apr 7, 2025
* Periodically cycle to reassert cluster configuration * Drop drift controller * Fix up unit test failures * Drop three-way merge code * Drop annotation copy in secret
RafalKorepta
pushed a commit
that referenced
this pull request
Apr 7, 2025
* Periodically cycle to reassert cluster configuration * Drop drift controller * Fix up unit test failures * Drop three-way merge code * Drop annotation copy in secret
alenkacz
added a commit
that referenced
this pull request
Apr 8, 2025
* Testing: one MockAdminAPI per cluster (#561) Given the desire to merge the drift controller with the general cluster reconciler, we need to track the mock admin api state on a per-cluster basis - otherwise the rapid reassertion of state will cause flapping as the mock API is poked on behalf of multiple clusters. * V1 drop drift controller (#543) * Periodically cycle to reassert cluster configuration * Drop drift controller * Fix up unit test failures * Drop three-way merge code * Drop annotation copy in secret --------- Co-authored-by: jan grant <3430517+jan-g@users.noreply.github.com>
jan-g
added a commit
that referenced
this pull request
Apr 29, 2025
* Periodically cycle to reassert cluster configuration * Drop drift controller * Fix up unit test failures * Drop three-way merge code * Drop annotation copy in secret (cherry picked from commit d9feceb) # Conflicts: # operator/internal/controller/vectorized/test/cluster_controller_configuration_test.go
Contributor
Author
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
RafalKorepta
pushed a commit
that referenced
this pull request
Apr 30, 2025
* Periodically cycle to reassert cluster configuration * Drop drift controller * Fix up unit test failures * Drop three-way merge code * Drop annotation copy in secret (cherry picked from commit d9feceb) # Conflicts: # operator/internal/controller/vectorized/test/cluster_controller_configuration_test.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This drops the drift controller and replaces it with a regular cycle of the main v1 controller. That now uses the "declarative"-style Syncer - so the practice of leaving ad hoc changes in place, that aren't mentioned in the CR, will no longer apply.
An earlier PR fixed up the Ginkgo tests to support this - should tests end up leaving multiple clusters around at the same time, there are no longer races around the ownership of a single testAdminAPI.