Conversation
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
blaipr
force-pushed
the
chore/retire-the-awx-operator-leader-election-id
branch
from
September 16, 2026 23:09
3677618 to
1611c5c
Compare
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Changing the lease ID before cleanup can allow old and new controllers to reconcile concurrently.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
This PR retires the legacy awx-operator leader-election ID in both operator manifests.
Changes:
- Updates the manager manifest’s leader-election ID.
- Updates the rendered Deployment patch.
File summaries
| File | Review |
|---|---|
config/manager/manager.yaml |
Critical: retain the legacy ID until old-prefixed Deployments are removed. |
config/default/manager_auth_proxy_patch.yaml |
Critical: retain the legacy ID until migration cleanup is complete. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| - "--metrics-bind-address=127.0.0.1:8080" | ||
| - "--leader-elect" | ||
| - "--leader-election-id=awx-operator" | ||
| - "--leader-election-id=ascender-operator" |
| - args: | ||
| - --leader-elect | ||
| - --leader-election-id=awx-operator | ||
| - --leader-election-id=ascender-operator |
The last awx in the operator's own install, and the one that had to go last. While the prefix rename is rolling out, an old awx-operator-* Deployment and a new ascender-operator-* one can be in a namespace at the same time, until hack/rename-operator-objects.sh removes the first. They pass the same --leader-election-id, so they contend for one lease and only one is ever active: an upgrade that has not been cleaned up is an idle standby rather than two controllers reconciling the same objects against different specs. Changing the id gives them separate leases and lets both act. So this waits until no operator carrying the old prefix is left anywhere, which means at least one release after the rename has been out and the cleanup has been run. The value is set twice, in config/manager/manager.yaml and again in the auth proxy patch, and the patch is the one that reaches the rendered Deployment. Changing only the first leaves the old id in the output, which is what rendering the manifests rather than trusting the edit showed. Nothing else references it. It is not a name any object carries, so there is nothing to migrate: the lease is recreated under the new id and the old one is garbage.
blaipr
force-pushed
the
chore/retire-the-awx-operator-leader-election-id
branch
from
September 17, 2026 19:54
1611c5c to
8b41d59
Compare
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
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.
Draft on purpose. Do not merge yet. It is the last
awxin the operator's own install, and the one that has to go last.Why it waits
While #54's prefix rename rolls out, a namespace can hold an old
awx-operator-*Deployment and a newascender-operator-*one at the same time, untilhack/rename-operator-objects.shremoves the first.They pass the same
--leader-election-id, so they contend for one lease and only one is ever active. An upgrade that has not been cleaned up yet is an idle standby, not two controllers reconciling the same objects against different specs. That property is what makes a half-finished migration safe, and it is bought entirely by leaving this value alone.Changing it gives the two operators separate leases and lets both act. That is the one genuinely dangerous state in this whole sequence.
When it can merge
Once no operator carrying the old prefix is left anywhere: at least one release after #54 has shipped and the cleanup script has been run. Until then this sits here so the last item is tracked rather than forgotten.
What it costs to change
Nothing to migrate. The id is not a name any object carries; the lease is simply recreated under the new one and the old lease becomes garbage. No other file references it.
A detail found by rendering rather than trusting the edit
The value is set twice:
config/manager/manager.yamland again inconfig/default/manager_auth_proxy_patch.yaml. The patch is the one that reaches the rendered Deployment, so changing only the first leaves the old id in the output. Both move here, andkubectl kustomize config/defaultconfirms it.