fix(secret): stop pulp-server Secret churn from nondeterministic map iteration#1649
fix(secret): stop pulp-server Secret churn from nondeterministic map iteration#1649mouchar wants to merge 1 commit into
Conversation
…ings
needsMigrationSetting iterated MigrationSettingsList as a map, whose
range order Go randomizes per invocation. With RedirectToObjectStorage=true
and HideGuardedDistributions=false (the default), each reconcile produced
one of two different settings.py contents depending on which key the
runtime visited first:
- if HideGuardedDistributions was visited first, the loop hit
`if !config { return }` and aborted, dropping REDIRECT_TO_OBJECT_STORAGE
entirely;
- if RedirectToObjectStorage was visited first, the line was written and
HideGuardedDistributions then triggered the early return.
The two outputs differed, so ReconcileObject kept rewriting the
pulp-server Secret and restarting pulpcore pods on every reconcile.
Fix both issues at the source:
- change MigrationSettingsList to return []MigrationSetting (a slice of
named OperatorField/PulpField pairs) so iteration order is fixed at
the data definition rather than reconstructed at each call site;
- replace the buggy `return` with `continue` so a single disabled flag
no longer skips subsequent settings.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Robert Moucha <robert.moucha@gooddata.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: mouchar The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Hi @mouchar. Thanks for your PR. I'm waiting for a pulp member to verify that this patch is reasonable to test. If it is, they should reply with Regular contributors should join the org to skip this step. Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions 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. |
1 participant
Summary
The
pulp-serverSecret containingsettings.pywas being rewritten on nearly every reconcile whenspec.redirect_to_object_storage=true. The rewrite removed and re-added theREDIRECT_TO_OBJECT_STORAGE = Trueline in rapid succession, and each change restarted all pulpcore Deployments.Observed in operator logs:
Root cause
needsMigrationSetting(incontrollers/repo_manager/secret.go) iteratedcontrollers.MigrationSettingsList()as a Gomap[string]string. Go randomizes map range order per invocation. With the default CR (RedirectToObjectStorage=true,HideGuardedDistributions=false), the generatedsettings.pydiffered depending on which key the runtime visited first:HideGuardedDistributionswas visited first,if !config { return }aborted the loop andREDIRECT_TO_OBJECT_STORAGEwas never written.RedirectToObjectStoragewas visited first, the line was written, thenHideGuardedDistributionstriggered the same earlyreturn.Each time the freshly generated string differed from the stored Secret,
ReconcileObjectupdated it and calledrestartPulpCorePods, cascading into Deployment reconciles.Fix
Two issues addressed at the source:
MigrationSettingsListnow returns[]MigrationSetting— a slice of namedOperatorField/PulpFieldpairs — so iteration order is fixed at the data definition rather than reconstructed at each call site.returninneedsMigrationSettingis replaced withcontinue, so a single disabled flag no longer skips later settings. (This latent bug would have resurfaced if both flags were evertrueand ordering swapped.)Both call sites of
MigrationSettingsListwere updated.Test plan
spec.redirect_to_object_storage=trueand observe operator logs over several reconcile cycles — the "Data from Secret pulp-server has been modified" / "Reprovisioning pulpcore pods" churn should not appear.pulp-serverSecret'ssettings.pycontainsREDIRECT_TO_OBJECT_STORAGE = Trueand is stable across reconciles.pulp-api,pulp-content,pulp-worker) are not restarted on the steady-state reconcile.redirect_to_object_storageorhide_guarded_distributionsactually changes (SettingNeedsMigrationChangedpath).