fix(commander): echo registry_mode on cluster update - #59
Open
duckhawk wants to merge 1 commit into
Open
Conversation
duckhawk
force-pushed
the
fix/commander-registry-mode-on-update
branch
from
July 29, 2026 08:50
4b35dd4 to
0decdc7
Compare
duckhawk
added a commit
to deckhouse/sds-object
that referenced
this pull request
Jul 29, 2026
The pin bumped in the previous commit does not decide which harness code runs: the reusable pipeline checks storage-e2e out into _storage-e2e and the run script applies `go mod edit -replace` onto that checkout, so the suite always ran storage-e2e main — which is why the run on the bumped pin failed the master-count specs with the same Commander 500 as before. Point storage_e2e_ref at the branch of deckhouse/storage-e2e#59 so the run actually exercises the fix. Back to main once it is merged; the go.mod pin follows. Signed-off-by: v.oleynikov <vasily.oleynikov@flant.com>
Every SetMasterCount call in the sds-object suite failed with a 500 whose body was
a raw PostgreSQL error:
PG::NotNullViolation: null value in column "registry_mode"
of relation "cluster_configs" violates not-null constraint
Confirmed by the Commander team: PUT /clusters/:id is a wholesale replace, so a
field the request omits is written as NULL — and registry_mode is NOT NULL. Create
does not need it (the API defaults it there), which is why bootstrap always worked
and only updates failed.
The client now reads registry_mode with the rest of the cluster and sends it back.
The cluster's own value is preserved rather than a constant: a cluster can
legitimately be Unmanaged, and sending Direct would repoint its registry. When the
API reports no mode at all, Direct is sent — the default for a cluster pulling from
the configured registry — because omitting it fails the update outright.
Tested: the PUT body carries the mode from the by-id read, and the fallback applies
only when the API reports none.
Signed-off-by: v.oleynikov <vasily.oleynikov@flant.com>
duckhawk
force-pushed
the
fix/commander-registry-mode-on-update
branch
from
July 29, 2026 13:30
0decdc7 to
7fa963a
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
1 participant
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.
Symptom
Every
SetMasterCountcall in thesds-objectsuite failed — 5 runs, different clusters, never once succeeded — with a500whose body was a raw PostgreSQL error:It fails immediately, before any convergence wait, so the two specs that resize the control plane never got to test anything.
Cause
Confirmed by the Commander team:
PUT /clusters/:idis a wholesale replace, so a field the request omits is written as NULL — andregistry_modeisNOT NULLincluster_configs.POST /clustersdoes not need it (the API defaults it there), which is why cluster bootstrap always worked and only updates failed.Our client never sent the field, and did not even model it:
ClusterResponsecarriedregistry_idbut noregistry_mode, so the value was discarded on the GET we build the PUT from.Fix
registry_modeis read with the rest of the cluster and echoed back on update.The cluster's own value is preserved rather than a constant: a cluster can legitimately be
Unmanaged, and unconditionally sendingDirectwould repoint its registry. When the API reports no mode at all,Directis sent — Commander's default for a cluster pulling from the configured registry — because omitting the field fails the update outright.Tests
TestUpdateClusterValues_EchoesRegistryMode: the PUT body carries the mode returned by the by-id read (UnmanagedstaysUnmanaged), and theDirectfallback applies only when the API reports none.