From ec42a18d307e7dd3d9b35ebae6440c51ab3bb1c4 Mon Sep 17 00:00:00 2001 From: Andrew Peabody Date: Tue, 20 Aug 2024 15:14:54 -0700 Subject: [PATCH] fix: gcfs AP diff and general cleanup (#2043) --- autogen/main/cluster.tf.tmpl | 24 ++++++++++--------- autogen/main/variables.tf.tmpl | 6 ++--- .../beta-autopilot-private-cluster/cluster.tf | 3 +++ .../beta-autopilot-public-cluster/cluster.tf | 3 +++ .../README.md | 2 +- .../variables.tf | 2 +- modules/beta-private-cluster/README.md | 2 +- modules/beta-private-cluster/variables.tf | 2 +- .../README.md | 2 +- .../variables.tf | 2 +- modules/beta-public-cluster/README.md | 2 +- modules/beta-public-cluster/variables.tf | 2 +- .../simple_autopiliot_public_test.go | 1 + .../testdata/TestSimpleAutopilotPublic.json | 3 +++ 14 files changed, 34 insertions(+), 22 deletions(-) diff --git a/autogen/main/cluster.tf.tmpl b/autogen/main/cluster.tf.tmpl index 6fe68c5d2..7d0cafc07 100644 --- a/autogen/main/cluster.tf.tmpl +++ b/autogen/main/cluster.tf.tmpl @@ -364,9 +364,9 @@ resource "google_container_cluster" "primary" { dynamic "ray_operator_config" { for_each = local.ray_operator_config - + content { - + enabled = ray_operator_config.value.enabled ray_cluster_logging_config { @@ -480,6 +480,11 @@ resource "google_container_cluster" "primary" { ignore_changes = [node_pool, initial_node_count, resource_labels["asmv"]] } {% endif %} + {% if autopilot_cluster == true %} + lifecycle { + ignore_changes = [node_pool_defaults[0].node_config_defaults[0].gcfs_config[0].enabled] + } + {% endif %} {% if autopilot_cluster != true %} dynamic "dns_config" { @@ -660,21 +665,18 @@ resource "google_container_cluster" "primary" { } } } - {% if beta_cluster and autopilot_cluster != true %} + {% if beta_cluster %} node_pool_defaults { node_config_defaults { + {% if autopilot_cluster %} + logging_variant = var.logging_variant + {% endif %} + {% if autopilot_cluster != true %} gcfs_config { enabled = var.enable_gcfs } - } - } - {% endif %} - {% if beta_cluster and autopilot_cluster %} - - node_pool_defaults { - node_config_defaults { - logging_variant = var.logging_variant + {% endif %} } } {% endif %} diff --git a/autogen/main/variables.tf.tmpl b/autogen/main/variables.tf.tmpl index da6fb485b..01a3142ce 100644 --- a/autogen/main/variables.tf.tmpl +++ b/autogen/main/variables.tf.tmpl @@ -818,13 +818,13 @@ variable "ray_operator_config" { type = object({ enabled = bool logging_enabled = optional(bool, false) - monitoring_enabled = optional(bool, false) + monitoring_enabled = optional(bool, false) }) description = "The Ray Operator Addon configuration for this cluster." default = { enabled = false logging_enabled = false - monitoring_enabled = false + monitoring_enabled = false } } @@ -944,7 +944,7 @@ variable "sandbox_enabled" { variable "enable_gcfs" { type = bool - description = "Enable image streaming on cluster level." + description = "(Beta) Enable image streaming on cluster level." default = false } {% endif %} diff --git a/modules/beta-autopilot-private-cluster/cluster.tf b/modules/beta-autopilot-private-cluster/cluster.tf index 20604496e..06e707465 100644 --- a/modules/beta-autopilot-private-cluster/cluster.tf +++ b/modules/beta-autopilot-private-cluster/cluster.tf @@ -261,6 +261,9 @@ resource "google_container_cluster" "primary" { } } + lifecycle { + ignore_changes = [node_pool_defaults[0].node_config_defaults[0].gcfs_config[0].enabled] + } timeouts { create = lookup(var.timeouts, "create", "45m") diff --git a/modules/beta-autopilot-public-cluster/cluster.tf b/modules/beta-autopilot-public-cluster/cluster.tf index 7af63a96d..d68eae0db 100644 --- a/modules/beta-autopilot-public-cluster/cluster.tf +++ b/modules/beta-autopilot-public-cluster/cluster.tf @@ -261,6 +261,9 @@ resource "google_container_cluster" "primary" { } } + lifecycle { + ignore_changes = [node_pool_defaults[0].node_config_defaults[0].gcfs_config[0].enabled] + } timeouts { create = lookup(var.timeouts, "create", "45m") diff --git a/modules/beta-private-cluster-update-variant/README.md b/modules/beta-private-cluster-update-variant/README.md index 640a04de1..291c5ac55 100644 --- a/modules/beta-private-cluster-update-variant/README.md +++ b/modules/beta-private-cluster-update-variant/README.md @@ -203,7 +203,7 @@ Then perform the following commands on the root folder: | enable\_cost\_allocation | Enables Cost Allocation Feature and the cluster name and namespace of your GKE workloads appear in the labels field of the billing export to BigQuery | `bool` | `false` | no | | enable\_default\_node\_pools\_metadata | Whether to enable the default node pools metadata key-value pairs such as `cluster_name` and `node_pool` | `bool` | `true` | no | | enable\_fqdn\_network\_policy | Enable FQDN Network Policies on the cluster | `bool` | `null` | no | -| enable\_gcfs | Enable image streaming on cluster level. | `bool` | `false` | no | +| enable\_gcfs | (Beta) Enable image streaming on cluster level. | `bool` | `false` | no | | enable\_identity\_service | Enable the Identity Service component, which allows customers to use external identity providers with the K8S API. | `bool` | `false` | no | | enable\_intranode\_visibility | Whether Intra-node visibility is enabled for this cluster. This makes same node pod to pod traffic visible for VPC network | `bool` | `false` | no | | enable\_kubernetes\_alpha | Whether to enable Kubernetes Alpha features for this cluster. Note that when this option is enabled, the cluster cannot be upgraded and will be automatically deleted after 30 days. | `bool` | `false` | no | diff --git a/modules/beta-private-cluster-update-variant/variables.tf b/modules/beta-private-cluster-update-variant/variables.tf index c721313d9..fa7670edf 100644 --- a/modules/beta-private-cluster-update-variant/variables.tf +++ b/modules/beta-private-cluster-update-variant/variables.tf @@ -902,7 +902,7 @@ variable "sandbox_enabled" { variable "enable_gcfs" { type = bool - description = "Enable image streaming on cluster level." + description = "(Beta) Enable image streaming on cluster level." default = false } diff --git a/modules/beta-private-cluster/README.md b/modules/beta-private-cluster/README.md index 6c3bb8a4d..5db9fc0dd 100644 --- a/modules/beta-private-cluster/README.md +++ b/modules/beta-private-cluster/README.md @@ -181,7 +181,7 @@ Then perform the following commands on the root folder: | enable\_cost\_allocation | Enables Cost Allocation Feature and the cluster name and namespace of your GKE workloads appear in the labels field of the billing export to BigQuery | `bool` | `false` | no | | enable\_default\_node\_pools\_metadata | Whether to enable the default node pools metadata key-value pairs such as `cluster_name` and `node_pool` | `bool` | `true` | no | | enable\_fqdn\_network\_policy | Enable FQDN Network Policies on the cluster | `bool` | `null` | no | -| enable\_gcfs | Enable image streaming on cluster level. | `bool` | `false` | no | +| enable\_gcfs | (Beta) Enable image streaming on cluster level. | `bool` | `false` | no | | enable\_identity\_service | Enable the Identity Service component, which allows customers to use external identity providers with the K8S API. | `bool` | `false` | no | | enable\_intranode\_visibility | Whether Intra-node visibility is enabled for this cluster. This makes same node pod to pod traffic visible for VPC network | `bool` | `false` | no | | enable\_kubernetes\_alpha | Whether to enable Kubernetes Alpha features for this cluster. Note that when this option is enabled, the cluster cannot be upgraded and will be automatically deleted after 30 days. | `bool` | `false` | no | diff --git a/modules/beta-private-cluster/variables.tf b/modules/beta-private-cluster/variables.tf index c721313d9..fa7670edf 100644 --- a/modules/beta-private-cluster/variables.tf +++ b/modules/beta-private-cluster/variables.tf @@ -902,7 +902,7 @@ variable "sandbox_enabled" { variable "enable_gcfs" { type = bool - description = "Enable image streaming on cluster level." + description = "(Beta) Enable image streaming on cluster level." default = false } diff --git a/modules/beta-public-cluster-update-variant/README.md b/modules/beta-public-cluster-update-variant/README.md index 1b30408a6..a5417cdab 100644 --- a/modules/beta-public-cluster-update-variant/README.md +++ b/modules/beta-public-cluster-update-variant/README.md @@ -196,7 +196,7 @@ Then perform the following commands on the root folder: | enable\_cost\_allocation | Enables Cost Allocation Feature and the cluster name and namespace of your GKE workloads appear in the labels field of the billing export to BigQuery | `bool` | `false` | no | | enable\_default\_node\_pools\_metadata | Whether to enable the default node pools metadata key-value pairs such as `cluster_name` and `node_pool` | `bool` | `true` | no | | enable\_fqdn\_network\_policy | Enable FQDN Network Policies on the cluster | `bool` | `null` | no | -| enable\_gcfs | Enable image streaming on cluster level. | `bool` | `false` | no | +| enable\_gcfs | (Beta) Enable image streaming on cluster level. | `bool` | `false` | no | | enable\_identity\_service | Enable the Identity Service component, which allows customers to use external identity providers with the K8S API. | `bool` | `false` | no | | enable\_intranode\_visibility | Whether Intra-node visibility is enabled for this cluster. This makes same node pod to pod traffic visible for VPC network | `bool` | `false` | no | | enable\_kubernetes\_alpha | Whether to enable Kubernetes Alpha features for this cluster. Note that when this option is enabled, the cluster cannot be upgraded and will be automatically deleted after 30 days. | `bool` | `false` | no | diff --git a/modules/beta-public-cluster-update-variant/variables.tf b/modules/beta-public-cluster-update-variant/variables.tf index 86afb87da..6f335b00f 100644 --- a/modules/beta-public-cluster-update-variant/variables.tf +++ b/modules/beta-public-cluster-update-variant/variables.tf @@ -866,7 +866,7 @@ variable "sandbox_enabled" { variable "enable_gcfs" { type = bool - description = "Enable image streaming on cluster level." + description = "(Beta) Enable image streaming on cluster level." default = false } diff --git a/modules/beta-public-cluster/README.md b/modules/beta-public-cluster/README.md index d72c116ac..00654e1b3 100644 --- a/modules/beta-public-cluster/README.md +++ b/modules/beta-public-cluster/README.md @@ -174,7 +174,7 @@ Then perform the following commands on the root folder: | enable\_cost\_allocation | Enables Cost Allocation Feature and the cluster name and namespace of your GKE workloads appear in the labels field of the billing export to BigQuery | `bool` | `false` | no | | enable\_default\_node\_pools\_metadata | Whether to enable the default node pools metadata key-value pairs such as `cluster_name` and `node_pool` | `bool` | `true` | no | | enable\_fqdn\_network\_policy | Enable FQDN Network Policies on the cluster | `bool` | `null` | no | -| enable\_gcfs | Enable image streaming on cluster level. | `bool` | `false` | no | +| enable\_gcfs | (Beta) Enable image streaming on cluster level. | `bool` | `false` | no | | enable\_identity\_service | Enable the Identity Service component, which allows customers to use external identity providers with the K8S API. | `bool` | `false` | no | | enable\_intranode\_visibility | Whether Intra-node visibility is enabled for this cluster. This makes same node pod to pod traffic visible for VPC network | `bool` | `false` | no | | enable\_kubernetes\_alpha | Whether to enable Kubernetes Alpha features for this cluster. Note that when this option is enabled, the cluster cannot be upgraded and will be automatically deleted after 30 days. | `bool` | `false` | no | diff --git a/modules/beta-public-cluster/variables.tf b/modules/beta-public-cluster/variables.tf index 86afb87da..6f335b00f 100644 --- a/modules/beta-public-cluster/variables.tf +++ b/modules/beta-public-cluster/variables.tf @@ -866,7 +866,7 @@ variable "sandbox_enabled" { variable "enable_gcfs" { type = bool - description = "Enable image streaming on cluster level." + description = "(Beta) Enable image streaming on cluster level." default = false } diff --git a/test/integration/simple_autopilot_public/simple_autopiliot_public_test.go b/test/integration/simple_autopilot_public/simple_autopiliot_public_test.go index dad92ca5f..1e0221e0c 100644 --- a/test/integration/simple_autopilot_public/simple_autopiliot_public_test.go +++ b/test/integration/simple_autopilot_public/simple_autopiliot_public_test.go @@ -56,6 +56,7 @@ func TestSimpleAutopilotPublic(t *testing.T) { "addonsConfig.rayOperatorConfig.enabled", "addonsConfig.rayOperatorConfig.rayClusterLoggingConfig.enabled", "addonsConfig.rayOperatorConfig.rayClusterMonitoringConfig.enabled", + "nodePoolDefaults.nodeConfigDefaults.gcfsConfig.enabled", } for _, pth := range validateJSONPaths { g.JSONEq(assert, op, pth) diff --git a/test/integration/simple_autopilot_public/testdata/TestSimpleAutopilotPublic.json b/test/integration/simple_autopilot_public/testdata/TestSimpleAutopilotPublic.json index 76d70b63e..e0a279443 100644 --- a/test/integration/simple_autopilot_public/testdata/TestSimpleAutopilotPublic.json +++ b/test/integration/simple_autopilot_public/testdata/TestSimpleAutopilotPublic.json @@ -188,6 +188,9 @@ "variantConfig": { "variant": "DEFAULT" } + }, + "gcfsConfig": { + "enabled": true } } },