feat(gke): implement ClusterManager.UpdateMaster - #192
Conversation
Add the POST .../clusters/{cluster}:updateMaster route, the last
unimplemented container.v1 cluster RPC after floci-io#96. The call moves
currentMasterVersion only: real GKE upgrades the control plane
independently of node pools, so currentNodeVersion and each pool's
version are left untouched. masterVersion is required (400 when
missing), the documented aliases (latest, -, 1.X, 1.X.Y) resolve
against the single version GetServerConfig advertises, and the
operation is reported as UPGRADE_MASTER, the Operation.Type real GKE
uses.
Closes floci-io#177
|
| Filename | Overview |
|---|---|
| src/main/java/io/floci/gcp/services/gke/GkeService.java | Implements control-plane-only master upgrades and alias resolution, but directly casting malformed JSON values can produce an internal server error. |
| src/main/java/io/floci/gcp/services/gke/KubernetesController.java | Adds the correctly shaped regional :updateMaster REST custom method. |
| src/main/java/io/floci/gcp/services/gke/operations/OperationType.java | Adds the GKE-compatible UPGRADE_MASTER operation enum value. |
| src/test/java/io/floci/gcp/services/gke/GkeUpdateMasterRestIntegrationTest.java | Covers successful routing, operation lookup, state changes, missing versions, and missing clusters, but not non-string version values. |
| compatibility-tests/sdk-test-java/src/test/java/io/floci/gcp/test/GkeTest.java | Verifies the new method through the Java HttpJson SDK and confirms node versions remain unchanged. |
Sequence Diagram
sequenceDiagram
participant Client
participant Controller as KubernetesController
participant Service as GkeService
participant Store as Cluster Store
participant Operations as Operation Service
Client->>Controller: "POST /clusters/{id}:updateMaster"
Controller->>Service: updateMaster(project, location, id, body)
Service->>Store: requireCluster(...)
Store-->>Service: StoredCluster
Service->>Service: validate and resolve masterVersion
Service->>Store: persist updated control-plane version
Service->>Operations: create UPGRADE_MASTER operation
Operations-->>Client: DONE operation
Reviews (1): Last reviewed commit: "feat(gke): implement ClusterManager.Upda..." | Re-trigger Greptile
| String masterVersion = body == null ? null : (String) body.get("masterVersion"); | ||
| if (masterVersion == null || masterVersion.isBlank()) { | ||
| throw GcpException.invalidArgument("masterVersion is required"); | ||
| } |
There was a problem hiding this comment.
A REST caller can send valid JSON such as {"masterVersion":123}. Jackson stores that value as a non-string object, so this direct cast throws ClassCastException. The exception is not mapped to the GCP error format, causing an internal server error instead of 400 INVALID_ARGUMENT. This violates the repository directive to preserve GCP protocol compatibility and return GCP-compatible JSON errors.
| String masterVersion = body == null ? null : (String) body.get("masterVersion"); | |
| if (masterVersion == null || masterVersion.isBlank()) { | |
| throw GcpException.invalidArgument("masterVersion is required"); | |
| } | |
| Object masterVersionValue = body == null ? null : body.get("masterVersion"); | |
| if (!(masterVersionValue instanceof String masterVersion) || masterVersion.isBlank()) { | |
| throw GcpException.invalidArgument("masterVersion is required"); | |
| } |
Context Used: AGENTS.md (source)
Summary
Implements
ClusterManager.UpdateMaster(POST .../clusters/{cluster}:updateMaster), the onecontainer.v1cluster RPC left unimplemented after #96, where it was split out during review to keep that PR focused. Closes #177.Today the route does not exist, so
ClusterManagerClient.updateMaster(...)throwsNotFoundExceptionand a raw REST call gets a bare 404, whiledocs/services/gke.mdsays the surface is complete exceptCancelOperation.One correction to the issue text: gcloud (
container clusters upgrade --master) and the Terraform provider both useUpdateClusterwithdesiredMasterVersionrather than this RPC, so the consumers here are direct SDK and REST callers. The RPC is still part of the documented surface and the SDK exposes it, hence the change.What changed
OperationType: addUPGRADE_MASTER, theOperation.Typereal GKE reports for a master upgrade (cluster_service.proto), so the SDK's enum parses it rather than returningUNRECOGNIZED.KubernetesController:POST /clusters/{clusterId: [^:/]+}:updateMaster, same regex-constrained pattern as the sibling custom methods.GkeService.updateMaster: requires the cluster; rejects a missing or blankmasterVersionwith400 INVALID_ARGUMENTbefore touching state (the proto marks it REQUIRED); setscurrentMasterVersiononly; bumps etag/fingerprint; returns a synchronousDONEoperation.currentNodeVersionand every node pool'sversionare deliberately left untouched. Real GKE upgrades the control plane independently of node pools; this was the open question in the issue and it is pinned by tests at all three levels.latest,-,1.X,1.X.Y) resolve against the single versionGetServerConfigadvertises. Any other explicit version is stored verbatim, consistent withinitialClusterVersionon create anddesiredMasterVersion/desiredNodeVersiononUpdateCluster.docs/services/gke.md:UpdateMasteradded to the supported list with a paragraph on the semantics above.Not included, on purpose: the deprecated zonal binding (
.../zones/{z}/clusters/{c}/master), since no other route here implements zonal bindings; and alias resolution onUpdateCluster.desiredMasterVersion, which is a separate pre-existing gap (gcloud sends"-"there) and better handled in its own PR.Type of change
fix:)feat:)feat!:orfix!:)GCP Compatibility
Wire shape from
googleapis/googleapis@aa87617d67google/container/v1/cluster_service.proto(UpdateMasterRequest,Operation.Type.UPGRADE_MASTER). Verified withgoogle-cloud-containerJava SDK via the HttpJson transport (compatibility-tests/sdk-test-javaGkeTest.updateMasterUpgradesOnlyTheControlPlane) against a localquarkus:devinstance in GKE mock mode: 5/5 pass.Tests
GkeServiceTest(+4): master moves and node versions do not on a two-pool cluster; alias resolution incl. the1.3vs1.30non-match; missing/blank/null version is 400 and leaves the stored cluster and etag untouched; unknown cluster is 404.GkeUpdateMasterRestIntegrationTest(new, 3): the route itself, operation body andGET /operations/{id}round trip, cluster read-back; 400 and 404 error shapes. Written first and failing 3/3 with 404 before the change.compatibility-tests/sdk-test-javaGkeTest(+1), README counts updated.Full suite:
./mvnw testbaseline onmain@0ec3a3ewas 1031 run / 0 failures / 0 errors / 0 skipped; after this change it is 1038 run / 0 failures / 0 errors / 0 skipped (the 7 new tests, nothing else changed).Checklist
./mvnw testpasses locally🤖 Generated with Claude Code
https://claude.ai/code/session_01VTYz9WSVDx9vDqBoy4K96j