diff --git a/docs/services/gke.md b/docs/services/gke.md index fdf9f158..d4cf4a24 100644 --- a/docs/services/gke.md +++ b/docs/services/gke.md @@ -176,6 +176,11 @@ analysis, since floci-gcp has no real infrastructure behind them to inspect: omitted; with several it is required, and a request without it is rejected as `400 INVALID_ARGUMENT` rather than silently upgrading pools the caller did not name. + `desiredMasterVersion` and `desiredNodeVersion` accept the same aliases as + `UpdateMaster` below, with the documented difference that `-` on the node side + picks the cluster's current master version rather than the server default. This + is what `gcloud container clusters upgrade` sends when no `--cluster-version` is + given, so both forms of that command leave the cluster on a real version. - `UpdateMaster` moves only the control plane: `currentMasterVersion` changes and `currentNodeVersion` and every node pool's `version` stay as they were, as in real GKE, where the master and node pools upgrade independently. `masterVersion` is diff --git a/src/main/java/io/floci/gcp/services/gke/GkeService.java b/src/main/java/io/floci/gcp/services/gke/GkeService.java index 3a110ed5..531ea894 100644 --- a/src/main/java/io/floci/gcp/services/gke/GkeService.java +++ b/src/main/java/io/floci/gcp/services/gke/GkeService.java @@ -328,19 +328,23 @@ public StoredOperation updateCluster(String project, String location, String clu // assigning first would leave a rejected request's version behind on it. List targets = nodeVersionUpdateTargets(project, location, clusterId, updateMap); - cluster.setCurrentNodeVersion(desiredNodeVersion); + String nodeVersion = resolveNodeVersion(desiredNodeVersion, cluster); + cluster.setCurrentNodeVersion(nodeVersion); // The pool carries its own `version`, and GetNodePool/ListNodePools read it from // the pool store, so moving only the cluster aggregate would report the new // version on the cluster while the pool still reported the old one. for (StoredNodePool pool : targets) { - pool.setVersion(desiredNodeVersion); + pool.setVersion(nodeVersion); pool.setEtag(newFingerprint()); nodePoolStore.put(nodePoolKey(project, location, clusterId, pool.getName()), pool); } } String desiredMasterVersion = (String) updateMap.get("desiredMasterVersion"); if (desiredMasterVersion != null) { - cluster.setCurrentMasterVersion(desiredMasterVersion); + // Same aliases as UpdateMaster; gcloud sends "-" here for `clusters upgrade + // --master` without --cluster-version, which stored verbatim left the cluster + // reporting version "-". + cluster.setCurrentMasterVersion(resolveMasterVersion(desiredMasterVersion)); } if (updateMap.get("desiredLocations") != null) { cluster.setLocations(stringListField(updateMap, "desiredLocations", cluster.getLocations())); @@ -824,6 +828,15 @@ private static String resolveMasterVersion(String requested) { return requested; } + /** {@code desired_node_version} documents the same spellings as {@code master_version} + * with one difference: {@code "-"} "picks the Kubernetes master version", the cluster's + * current control plane version, not the server default that {@code "-"} means on the master + * field. The proto allows one {@code ClusterUpdate} field per request, so this reads the + * master version as stored, not one the same request might also be changing. */ + private static String resolveNodeVersion(String requested, StoredCluster cluster) { + return "-".equals(requested) ? cluster.getCurrentMasterVersion() : resolveMasterVersion(requested); + } + /** The node pools an {@code UpdateCluster} carrying {@code desiredNodeVersion} upgrades. * *

{@code desired_node_version} upgrades the single pool named by {@code diff --git a/src/test/java/io/floci/gcp/services/gke/GkeServiceTest.java b/src/test/java/io/floci/gcp/services/gke/GkeServiceTest.java index 5d2bcf3d..cd415d6b 100644 --- a/src/test/java/io/floci/gcp/services/gke/GkeServiceTest.java +++ b/src/test/java/io/floci/gcp/services/gke/GkeServiceTest.java @@ -732,6 +732,48 @@ void desiredNodePoolIdDoesNotLeakIntoClusterState() { assertNull(service.getCluster(PROJECT, LOCATION, "no-leak").getExtraConfig().get("nodePoolId")); } + @Test + void updateClusterResolvesDesiredMasterVersionAliases() { + // gcloud `container clusters upgrade C --master` sends desiredMasterVersion "-" when no + // --cluster-version is given; stored verbatim, the cluster then reported version "-". + service.createCluster(PROJECT, LOCATION, Map.of("name", "master-alias", + "initialClusterVersion", "1.29.0-gke.1")); + String advertised = (String) service.getServerConfig().get("defaultClusterVersion"); + + service.updateCluster(PROJECT, LOCATION, "master-alias", Map.of("desiredMasterVersion", "-")); + assertEquals(advertised, service.getCluster(PROJECT, LOCATION, "master-alias").getCurrentMasterVersion()); + + service.updateCluster(PROJECT, LOCATION, "master-alias", Map.of("desiredMasterVersion", "1.29.0-gke.1")); + service.updateCluster(PROJECT, LOCATION, "master-alias", Map.of("desiredMasterVersion", "latest")); + assertEquals(advertised, service.getCluster(PROJECT, LOCATION, "master-alias").getCurrentMasterVersion()); + + // An explicit version is still stored verbatim, and node versions do not move with the master. + service.updateCluster(PROJECT, LOCATION, "master-alias", Map.of("desiredMasterVersion", "1.31.5-gke.1")); + StoredCluster cluster = service.getCluster(PROJECT, LOCATION, "master-alias"); + assertEquals("1.31.5-gke.1", cluster.getCurrentMasterVersion()); + assertEquals("1.29.0-gke.1", cluster.getCurrentNodeVersion()); + } + + @Test + void updateClusterResolvesDesiredNodeVersionAliasesAgainstTheMaster() { + // desired_node_version documents the same aliases as the master field, except that "-" + // "picks the Kubernetes master version": the cluster's control plane, not the server default. + service.createCluster(PROJECT, LOCATION, Map.of("name", "node-alias", + "initialClusterVersion", "1.29.0-gke.1")); + String advertised = (String) service.getServerConfig().get("defaultClusterVersion"); + assertNotEquals(advertised, "1.29.0-gke.1"); + + service.updateCluster(PROJECT, LOCATION, "node-alias", Map.of("desiredNodeVersion", "latest")); + assertEquals(advertised, service.getCluster(PROJECT, LOCATION, "node-alias").getCurrentNodeVersion()); + assertEquals(advertised, service.getNodePool(PROJECT, LOCATION, "node-alias", "default-pool").getVersion()); + + service.updateCluster(PROJECT, LOCATION, "node-alias", Map.of("desiredNodeVersion", "-")); + StoredCluster cluster = service.getCluster(PROJECT, LOCATION, "node-alias"); + assertEquals("1.29.0-gke.1", cluster.getCurrentMasterVersion()); + assertEquals("1.29.0-gke.1", cluster.getCurrentNodeVersion()); + assertEquals("1.29.0-gke.1", service.getNodePool(PROJECT, LOCATION, "node-alias", "default-pool").getVersion()); + } + @Test void updateClusterMergesDesiredFieldsIntoExtraConfig() { service.createCluster(PROJECT, LOCATION, Map.of("name", "updatable")); diff --git a/src/test/java/io/floci/gcp/services/gke/GkeUpdateClusterRestIntegrationTest.java b/src/test/java/io/floci/gcp/services/gke/GkeUpdateClusterRestIntegrationTest.java new file mode 100644 index 00000000..7194e9eb --- /dev/null +++ b/src/test/java/io/floci/gcp/services/gke/GkeUpdateClusterRestIntegrationTest.java @@ -0,0 +1,73 @@ +package io.floci.gcp.services.gke; + +import io.quarkus.test.junit.QuarkusTest; +import org.junit.jupiter.api.Test; + +import static io.restassured.RestAssured.given; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.not; + +/** + * {@code ClusterManager.UpdateCluster} over REST ({@code PUT .../clusters/{id}}), with the + * request shapes gcloud actually sends for {@code container clusters upgrade}. + */ +@QuarkusTest +class GkeUpdateClusterRestIntegrationTest { + + private static final String PROJECT = "gke-update-cluster-it"; + private static final String LOCATION = "us-central1"; + private static final String BASE = "/container/v1/projects/" + PROJECT + "/locations/" + LOCATION; + + @Test + void gcloudUpgradeWithoutAVersionSendsDashAndGetsARealVersionBack() { + String cluster = "gcloud-upgrade"; + String clusterPath = BASE + "/clusters/" + cluster; + + given() + .contentType("application/json") + .body("{\"cluster\":{\"name\":\"" + cluster + "\",\"initialClusterVersion\":\"1.29.0-gke.1\"}}") + .when().post(BASE + "/clusters") + .then() + .statusCode(200); + + String advertised = given() + .when().get(BASE + "/serverConfig") + .then() + .statusCode(200) + .extract().path("defaultClusterVersion"); + + // `gcloud container clusters upgrade C --master` with no --cluster-version. + given() + .contentType("application/json") + .body("{\"update\":{\"desiredMasterVersion\":\"-\"}}") + .when().put(clusterPath) + .then() + .statusCode(200) + .body("status", equalTo("DONE")); + + given() + .when().get(clusterPath) + .then() + .statusCode(200) + .body("currentMasterVersion", equalTo(advertised)) + .body("currentMasterVersion", not(equalTo("-"))) + .body("currentNodeVersion", equalTo("1.29.0-gke.1")); + + // `gcloud container clusters upgrade C --node-pool default-pool` with no --cluster-version: + // "-" on the node side means the cluster's master version. + given() + .contentType("application/json") + .body("{\"update\":{\"desiredNodeVersion\":\"-\",\"desiredNodePoolId\":\"default-pool\"}}") + .when().put(clusterPath) + .then() + .statusCode(200) + .body("status", equalTo("DONE")); + + given() + .when().get(clusterPath) + .then() + .statusCode(200) + .body("currentNodeVersion", equalTo(advertised)) + .body("nodePools[0].version", equalTo(advertised)); + } +}