fix(gke): resolve version aliases on UpdateCluster desired versions - #198
Conversation
desiredMasterVersion and desiredNodeVersion were stored verbatim, so gcloud `container clusters upgrade` with no --cluster-version, which sends the literal "-", left the cluster reporting version "-". Route desiredMasterVersion through the resolver UpdateMaster already uses and resolve desiredNodeVersion the same way, except that "-" on the node side picks the cluster's current master version, as the proto documents. Closes floci-io#196
|
| Filename | Overview |
|---|---|
| src/main/java/io/floci/gcp/services/gke/GkeService.java | Resolves UpdateCluster master and node aliases before updating cluster and node-pool state. |
| src/test/java/io/floci/gcp/services/gke/GkeServiceTest.java | Adds focused coverage for master aliases, node aliases, explicit versions, and independent control-plane and node versions. |
| src/test/java/io/floci/gcp/services/gke/GkeUpdateClusterRestIntegrationTest.java | Verifies the gcloud-style REST request bodies produce concrete master and node versions. |
| docs/services/gke.md | Documents UpdateCluster alias behavior and the node-side meaning of -. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[UpdateCluster request] --> B{Version field}
B -->|desiredMasterVersion| C[resolveMasterVersion]
C --> D[Update currentMasterVersion]
B -->|desiredNodeVersion| E{Requested value is dash?}
E -->|Yes| F[Use current master version]
E -->|No| G[resolveMasterVersion]
F --> H[Update cluster node version]
G --> H
H --> I[Update targeted node pools]
Reviews (1): Last reviewed commit: "fix(gke): resolve version aliases on Upd..." | Re-trigger Greptile
|
Thank you, and particularly for checking the one-field-per-request rule rather than assuming it, since the whole design rests on it. I confirmed all three:
No blockers from my side. |
hectorvent
left a comment
There was a problem hiding this comment.
All three contract claims confirmed verbatim, including the one-field-per-request rule the design rests on; tests 49/0 locally.
Summary
UpdateClusterstoreddesiredMasterVersionanddesiredNodeVersionverbatim. Both fields document the same aliases asUpdateMasterRequest.master_version(latest,-,1.X,1.X.Y), and gcloud relies on them:gcloud container clusters upgrade C --masterand... --node-pool Pboth callUpdateCluster, and when no--cluster-versionis given gcloud sends the literal-(api_adapter.py,UpdateClusterCommon). Against the emulator the cluster then reportedcurrentMasterVersion: "-". Closes #196.This is the gap noted in #192, where
UpdateMastergained alias resolution andUpdateClusterwas deliberately left for its own PR.What changed
desiredMasterVersiongoes through the existingresolveMasterVersion, so it behaves exactly likeUpdateMaster.desiredNodeVersiongoes through a new one-lineresolveNodeVersion: identical rules, except-picks the cluster's current master version rather than the server default, which is the one documented difference between the two fields (cluster_service.protoL3542: "picks the Kubernetes master version"). The resolved value is applied to both the cluster aggregate and each targeted pool.updateClusteris unchanged: the node block still resolves its targets before mutating anything, so a rejecteddesiredNodePoolIdleaves no partial state. The proto allows oneClusterUpdatefield per request, so a node-reads the stored master version, not one the same request might also be changing.docs/services/gke.mdupdated.Independent of #195 (different lines of the same file); either can merge first.
Type of change
fix:)feat:)feat!:orfix!:)GCP Compatibility
Alias semantics from
ClusterUpdate.desired_master_version(L3706-L3716) anddesired_node_version(L3532-L3543) incluster_service.proto@aa87617d67. Request shapes in the REST test are the ones gcloud sends forcontainer clusters upgradewith and without--master.Tests
GkeServiceTest.updateClusterResolvesDesiredMasterVersionAliases(new):-andlatestresolve to the advertised version; explicit stays verbatim; node version does not move.GkeServiceTest.updateClusterResolvesDesiredNodeVersionAliasesAgainstTheMaster(new): on a cluster whose master is not the advertised version, nodelatestresolves to the advertised version and node-resolves to the cluster's master, on both the aggregate and the pool.GkeUpdateClusterRestIntegrationTest(new): the two gcloud request bodies overPUT, asserting the read-back is a real version and never-.but was: <->/but was: <latest>.Full suite: baseline on
main@c534f6bis 1043 run / 0 failures / 0 errors / 0 skipped; with this change 1046 / 0 / 0 / 0 (the 3 new tests, nothing else changed).Checklist
./mvnw testpasses locally🤖 Generated with Claude Code
https://claude.ai/code/session_01VTYz9WSVDx9vDqBoy4K96j