Separating GKE Standard and Autopilot Modules

[avinashkumar1289]

This Pull request contains changes for separating GKE autopilot and Standard Modules.
I have commented out changes in the autopilot module, so it becomes easy to find what blocks I have removed before merging.

[avinashkumar1289]

@ludoo @juliocc I am facing a lot of error because of the naming convention of the module. There are a lot of reference for gke-cluster and since I have divided them to gke-autopilot-cluster and gke-standard-cluster, few of the checks are not passing ?
Please advice

[juliocc]

@ludoo @juliocc I am facing a lot of error because of the naming convention of the module. There are a lot of reference for gke-cluster and since I have divided them to gke-autopilot-cluster and gke-standard-cluster, few of the checks are not passing ? Please advice

Hey @avinashkumar1289, it's not a naming convention issue. You're actually removing a module that is a dependency elsewhere in the repo. The tests are failing because they depend on that module. This is exactly what should happen.

The PR should update those other parts of the codebase and ensure all the references to the existing module are updated and work as expected. The failing tests should actually help you figure out what is broken by the changes you're introducing.

If you want we can show you how to go about fixing some of those failures.

[avinashkumar1289]

Thanks @juliocc .Yeah i had a sync with ludo. I am going to replace all the dependencies from gke-cluster to gke-standard-cluster.
Thanks !!

[ludoo]

Super job thanks a lot for tackling this! Can you remove the commented code in the autpilot mode before merging?

[avinashkumar1289]

@ludoo Thanks .Pushed the changes !! I will go ahead and merge the code.

[ludoo]

It's good to go for me, let's wait this morning so Julio can also give it a look, then you should be able to merge (do you see the green "Squash and merge" button?)

[avinashkumar1289]

@ludoo Thanks !! Yes lets wait for @juliocc to confirm !!
Yes I see the "Squash and Merge" option under "Merge Pull request".

[intotecho]

Hi, What is the reason why var.monitoring_config was removed from the autopilot version? Is it not supported in GKE?

[avinashkumar1289]

@intotecho monitoring config is enabled by default in GKE Autopilot and cannot be disabled. Hence we have removed the configuration.

[intotecho]

Thanks, makes sense. Are all the components enabled, like SYSTEM_COMPONENTS, APISERVER, CONTROLLER_MANAGER, and SCHEDULER?

[juliocc]

These are the details: https://cloud.google.com/kubernetes-engine/docs/resources/autopilot-standard-feature-comparison#:~:text=Logging%20and%20monitoring

[intotecho]

Thanks for the link! The new modules are very clean and a good improvement. Note that deleting the old module instead of deprecating it forces users to upgrade to either autopilot or standard when they upgrade the cloud foundation fabric version.

…

On Tue, 23 May 2023, 5:03 pm Julio Castillo, ***@***.***> wrote: These are the details: https://cloud.google.com/kubernetes-engine/docs/resources/autopilot-standard-feature-comparison#:~:text=Logging%20and%20monitoring — Reply to this email directly, view it on GitHub <#1330 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAFBBWAETIBXNE5XVWANPMLXHR4PPANCNFSM6AAAAAAXA7FZOM> . You are receiving this because you were mentioned.Message ID: ***@***.*** com>

[intotecho]

I am having trouble getting the gke autopilot cluster into BALANCED mode.

Although the provider says BALANCED is the default. The console is reporting something else.

Autoscaling profile | Optimize utilization

  cluster_autoscaling {

    autoscaling_profile = "BALANCED"
    dynamic "auto_provisioning_defaults" {
      for_each = var.service_account != null ? [""] : []
      content {
        service_account = var.service_account
      }
    }
  }

I tried adding autoscaling_profile = "BALANCED" in modules/gke-cluster-autopilot/main.tf
But it didn't change the infrastructure.

I am not sure if this is user error, an issue with the google beta provider version "4.60" or the way it is called.

[wiktorn]

@intotecho I can confirm your findings. As far as I can tell, there are two bugs lurking here:

1. Providing autoscaling_profile in google_container_cluster does not change anything. As this is not editable on the cluster, it should destroy cluster and create new one. Nothing happens, because there is DiffSuppressFunc defined which for autopilot clusters ignores any changes to field. It needs to be ForceNew to force cluster recreation.
2. When cluster is created without providing autoscaling_profile, clusters are created as OPTIMIZE_UTILIZATION which is against API documentation

So as temporary workaround, after modifying gke-cluster-autopilot module force cluster recreation and it should recreate with proper profile:

terraform plan -out tf.plan -replace 'module.cluster-1.google_container_cluster.cluster'

(your module address may differ)

[wiktorn]

@intotecho Sorry, my bad. For autopilot clusters you can't modify this value.

[intotecho]

Ok, thanks for looking into it. Regards Chris

…

On Tue, 13 June 2023, 6:41 am Wiktor Niesiobędzki, ***@***.***> wrote: @intotecho <https://github.com/intotecho> Sorry, my bad. For autopilot clusters you can't modify this value. — Reply to this email directly, view it on GitHub <#1330 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAFBBWHP4VTPQYMVV6FK7SDXLB32NANCNFSM6AAAAAAXA7FZOM> . You are receiving this because you were mentioned.Message ID: ***@***.*** com>