feat: support enabling default compute class in cluster autoscaler

[romancin]

This PR enables Default Compute Class support for Cluster Autoscaler.

[apeabody]

Thanks for the contribution @romancin!

Please resolve the linter error:

╷
│ Error: Unsupported argument
│ 
│   on ../../modules/beta-private-cluster/cluster.tf line 179, in resource "google_container_cluster" "primary":
│  179:     default_compute_class_enabled = lookup(var.cluster_autoscaling, "enable_default_compute_class", false)
│ 
│ An argument named "default_compute_class_enabled" is not expected here.

[romancin]

Thanks for the contribution @romancin!

Please resolve the linter error:

╷
│ Error: Unsupported argument
│ 
│   on ../../modules/beta-private-cluster/cluster.tf line 179, in resource "google_container_cluster" "primary":
│  179:     default_compute_class_enabled = lookup(var.cluster_autoscaling, "enable_default_compute_class", false)
│ 
│ An argument named "default_compute_class_enabled" is not expected here.

Which google provider version is the linter using? Because this argument is available only since v7.

[apeabody]

Thanks for the contribution @romancin!
Please resolve the linter error:

╷
│ Error: Unsupported argument
│ 
│   on ../../modules/beta-private-cluster/cluster.tf line 179, in resource "google_container_cluster" "primary":
│  179:     default_compute_class_enabled = lookup(var.cluster_autoscaling, "enable_default_compute_class", false)
│ 
│ An argument named "default_compute_class_enabled" is not expected here.

Which google provider version is the linter using? Because this argument is available only since v7.

Got it - It's not the linter per say, but all the relevant versions constraints in the Terraform code as the highest allowed is selected. In this case it's the most recent v6.x.

I recommend you start by bumping the relevant minimum versions in https://github.com/terraform-google-modules/terraform-google-kubernetes-engine/blob/main/autogen/main/versions.tf.tmpl#L27, as that will more clearly illustrate the dependencies. I know at a minimum we are waiting on #2456 for some examples.

[apeabody]

Thanks for the contribution @romancin!

The code base is now TPGv7 compatible, and initial support was added in #2434. However this implementation might be preferable?

[stdmje]

Any news on this one? 1.33 is the now in the STABLE channel and ComputeClasses are such a nice feature!

[NeckBeardPrince]

Can we please get an update on this? The last release enabled this feature, but it doesn't work without this PR.

[romancin]

This one #2434 included the same change. I suppose this one is not needed anymore, right @apeabody ?

[apeabody]

initial support was added in #2434. However this implementation might be preferable?

Hi @romancin - Initial support was added in #2434. However this more complete implementation might be preferable?

[apeabody]

This one #2434 included the same change. I suppose this one is not needed anymore, right @apeabody ?

Hi @romancin - Initial support was added in #2434. However this more complete implementation might be preferable?

[NeckBeardPrince]

Bump

[romancin]

This one #2434 included the same change. I suppose this one is not needed anymore, right @apeabody ?

Hi @romancin - Initial support was added in #2434. However this more complete implementation might be preferable?

@apeabody Done the changes. Can you execute the linter now please? Thanks!

[romancin]

Ok, I see it is failing because in autopilot mode, no cluster_autoscaling variable is defined... So we want to maintain both cases? Define an independent variable called default_compute_class_enabled for using it only on autopilot clusters, and for standard ones keep that as an attribute of cluster_autoscaling variable?

[apeabody]

Ok, I see it is failing because in autopilot mode, no cluster_autoscaling variable is defined... So we want to maintain both cases? Define an independent variable called default_compute_class_enabled for using it only on autopilot clusters, and for standard ones keep that as an attribute of cluster_autoscaling variable?

Hi @romancin - I haven't looked through the PR in depth, but wanted to pass along that the templates can have different variables/behavior/etc for standard vs autopilot clusters. For example {% if autopilot_cluster != true %} {% else %} {% endif %}

[apeabody]

/gemini review

[gemini-code-assist]

Code Review

This pull request refactors the configuration for enabling the default compute class in the cluster autoscaler. The default_compute_class_enabled variable is moved into the cluster_autoscaling object, which improves configuration structure. However, this change introduces a critical issue in the autopilot cluster modules (beta-autopilot-private-cluster and beta-autopilot-public-cluster). These modules now reference var.cluster_autoscaling, which is not defined for them, and this will cause Terraform deployments to fail. The template files need to be adjusted to correctly handle the configuration for autopilot clusters.

[romancin]

Kept the variable for autopilot. This can be reviewed again.

[apeabody]

Kept the variable for autopilot. This can be reviewed again.

Hi @romancin - From the linter:

terraform_validate ./test/fixtures/simple_autopilot_private
╷
│ Error: Reference to undeclared input variable
│ 
│   on ../../../modules/beta-autopilot-private-cluster/cluster.tf line 106, in resource "google_container_cluster" "primary":
│  106:     default_compute_class_enabled = var.default_compute_class_enabled
│ 
│ An input variable with the name "default_compute_class_enabled" has not
│ been declared. This variable can be declared with a variable
│ "default_compute_class_enabled" {} block.
╵
terraform_validate ./test/fixtures/simple_autopilot_public
╷
│ Error: Reference to undeclared input variable
│ 
│   on ../../../modules/beta-autopilot-public-cluster/cluster.tf line 106, in resource "google_container_cluster" "primary":
│  106:     default_compute_class_enabled = var.default_compute_class_enabled
│ 
│ An input variable with the name "default_compute_class_enabled" has not
│ been declared. This variable can be declared with a variable
│ "default_compute_class_enabled" {} block.

[romancin]

Kept the variable for autopilot. This can be reviewed again.

Hi @romancin - From the linter:

terraform_validate ./test/fixtures/simple_autopilot_private
╷
│ Error: Reference to undeclared input variable
│ 
│   on ../../../modules/beta-autopilot-private-cluster/cluster.tf line 106, in resource "google_container_cluster" "primary":
│  106:     default_compute_class_enabled = var.default_compute_class_enabled
│ 
│ An input variable with the name "default_compute_class_enabled" has not
│ been declared. This variable can be declared with a variable
│ "default_compute_class_enabled" {} block.
╵
terraform_validate ./test/fixtures/simple_autopilot_public
╷
│ Error: Reference to undeclared input variable
│ 
│   on ../../../modules/beta-autopilot-public-cluster/cluster.tf line 106, in resource "google_container_cluster" "primary":
│  106:     default_compute_class_enabled = var.default_compute_class_enabled
│ 
│ An input variable with the name "default_compute_class_enabled" has not
│ been declared. This variable can be declared with a variable
│ "default_compute_class_enabled" {} block.

Sorry @apeabody , I moved the variable incorrectly for autopilot clusters (inside an incorrect conditional). Now it is fine, and all the commits squashed so the PR is cleaner. Could you execute the linter again please? Thank you very much!