feat: Update modules to use new kubectl module #602

bharathkkb · 2020-07-16T03:11:20Z

fixes #604

Use kubectl module instead of gcloud wherever possible
default ACM/ConfigSync modules to skip_gcloud_download=true
TODO
- new gcloud module release with latest kubectl module

comment-bot-dev · 2020-07-27T02:46:33Z

Thanks for the PR! 🚀
Unfortunately it looks like some of our CI checks failed. See the Contributing Guide for details.

⚠️check_terraform
Failed Terraform check. More details below.

Running terraform fmt
modules/beta-private-cluster/dns.tf
--- old/modules/beta-private-cluster/dns.tf
+++ new/modules/beta-private-cluster/dns.tf
@@ -20,8 +20,8 @@
  Delete default kube-dns configmap
 *****************************************/
module "gcloud_delete_default_kube_dns_configmap" {
-  source  = "terraform-google-modules/gcloud/google//modules/kubectl-wrapper"
-  version = "~> 1.4"
+  source           = "terraform-google-modules/gcloud/google//modules/kubectl-wrapper"
+  version          = "~> 1.4"
  enabled          = (local.custom_kube_dns_config || local.upstream_nameservers_config) && ! var.skip_provisioners
  cluster_name     = google_container_cluster.primary.name
  cluster_location = google_container_cluster.primary.location
Error: terraform fmt failed with exit code 3
Check the output for diffs and correct using terraform fmt <dir>
modules/private-cluster/dns.tf
--- old/modules/private-cluster/dns.tf
+++ new/modules/private-cluster/dns.tf
@@ -20,8 +20,8 @@
  Delete default kube-dns configmap
 *****************************************/
module "gcloud_delete_default_kube_dns_configmap" {
-  source  = "terraform-google-modules/gcloud/google//modules/kubectl-wrapper"
-  version = "~> 1.4"
+  source           = "terraform-google-modules/gcloud/google//modules/kubectl-wrapper"
+  version          = "~> 1.4"
  enabled          = (local.custom_kube_dns_config || local.upstream_nameservers_config) && ! var.skip_provisioners
  cluster_name     = google_container_cluster.primary.name
  cluster_location = google_container_cluster.primary.location
Error: terraform fmt failed with exit code 3
Check the output for diffs and correct using terraform fmt <dir>
modules/beta-public-cluster/dns.tf
--- old/modules/beta-public-cluster/dns.tf
+++ new/modules/beta-public-cluster/dns.tf
@@ -20,8 +20,8 @@
  Delete default kube-dns configmap
 *****************************************/
module "gcloud_delete_default_kube_dns_configmap" {
-  source  = "terraform-google-modules/gcloud/google//modules/kubectl-wrapper"
-  version = "~> 1.4"
+  source           = "terraform-google-modules/gcloud/google//modules/kubectl-wrapper"
+  version          = "~> 1.4"
  enabled          = (local.custom_kube_dns_config || local.upstream_nameservers_config) && ! var.skip_provisioners
  cluster_name     = google_container_cluster.primary.name
  cluster_location = google_container_cluster.primary.location
Error: terraform fmt failed with exit code 3
Check the output for diffs and correct using terraform fmt <dir>
modules/beta-public-cluster-update-variant/dns.tf
--- old/modules/beta-public-cluster-update-variant/dns.tf
+++ new/modules/beta-public-cluster-update-variant/dns.tf
@@ -20,8 +20,8 @@
  Delete default kube-dns configmap
 *****************************************/
module "gcloud_delete_default_kube_dns_configmap" {
-  source  = "terraform-google-modules/gcloud/google//modules/kubectl-wrapper"
-  version = "~> 1.4"
+  source           = "terraform-google-modules/gcloud/google//modules/kubectl-wrapper"
+  version          = "~> 1.4"
  enabled          = (local.custom_kube_dns_config || local.upstream_nameservers_config) && ! var.skip_provisioners
  cluster_name     = google_container_cluster.primary.name
  cluster_location = google_container_cluster.primary.location
Error: terraform fmt failed with exit code 3
Check the output for diffs and correct using terraform fmt <dir>
modules/private-cluster-update-variant/dns.tf
--- old/modules/private-cluster-update-variant/dns.tf
+++ new/modules/private-cluster-update-variant/dns.tf
@@ -20,8 +20,8 @@
  Delete default kube-dns configmap
 *****************************************/
module "gcloud_delete_default_kube_dns_configmap" {
-  source  = "terraform-google-modules/gcloud/google//modules/kubectl-wrapper"
-  version = "~> 1.4"
+  source           = "terraform-google-modules/gcloud/google//modules/kubectl-wrapper"
+  version          = "~> 1.4"
  enabled          = (local.custom_kube_dns_config || local.upstream_nameservers_config) && ! var.skip_provisioners
  cluster_name     = google_container_cluster.primary.name
  cluster_location = google_container_cluster.primary.location
Error: terraform fmt failed with exit code 3
Check the output for diffs and correct using terraform fmt <dir>
modules/beta-private-cluster-update-variant/dns.tf
--- old/modules/beta-private-cluster-update-variant/dns.tf
+++ new/modules/beta-private-cluster-update-variant/dns.tf
@@ -20,8 +20,8 @@
  Delete default kube-dns configmap
 *****************************************/
module "gcloud_delete_default_kube_dns_configmap" {
-  source  = "terraform-google-modules/gcloud/google//modules/kubectl-wrapper"
-  version = "~> 1.4"
+  source           = "terraform-google-modules/gcloud/google//modules/kubectl-wrapper"
+  version          = "~> 1.4"
  enabled          = (local.custom_kube_dns_config || local.upstream_nameservers_config) && ! var.skip_provisioners
  cluster_name     = google_container_cluster.primary.name
  cluster_location = google_container_cluster.primary.location
Error: terraform fmt failed with exit code 3
Check the output for diffs and correct using terraform fmt <dir>
dns.tf
--- old/dns.tf
+++ new/dns.tf
@@ -20,8 +20,8 @@
  Delete default kube-dns configmap
 *****************************************/
module "gcloud_delete_default_kube_dns_configmap" {
-  source  = "terraform-google-modules/gcloud/google//modules/kubectl-wrapper"
-  version = "~> 1.4"
+  source           = "terraform-google-modules/gcloud/google//modules/kubectl-wrapper"
+  version          = "~> 1.4"
  enabled          = (local.custom_kube_dns_config || local.upstream_nameservers_config) && ! var.skip_provisioners
  cluster_name     = google_container_cluster.primary.name
  cluster_location = google_container_cluster.primary.location
Error: terraform fmt failed with exit code 3
Check the output for diffs and correct using terraform fmt <dir>

morgante · 2020-07-27T16:12:37Z

@bharathkkb Looks like the comment bot isn't updating?

bharathkkb · 2020-07-27T17:53:11Z

@morgante its actually a bug. The comment bot doesn't seem to ignore the autogen folder and tries to fmt. I will look into this.

morgante · 2020-07-27T17:59:20Z

Ok thanks for looking into it.

add beta add kustomize test latest gcloud fix for binaries pin gcloud module to >= 1.2 build all transition to kubectl module fix dependecy crd module pin versions

bharathkkb · 2020-07-28T02:53:04Z

@morgante this should be ready for review

morgante

I thought you were updating to use https://github.com/terraform-google-modules/terraform-google-gcloud/tree/master/modules/kubectl-wrapper?

modules/asm/main.tf

modules/asm/scripts/kubectl_wrapper.sh

modules/asm/main.tf

morgante

Have you had a chance to test this with existing deployments/clusters? What does the plan look like and does it work?

bharathkkb · 2020-07-28T22:02:24Z

@morgante will update once I test, but all these already had a permadiff in every plan due to token refresh.
for example

stub-domains-upstream-nameservers would always say there are two changes and reapply the shell script which would say Terraform maintained kube-dns configmap appears to have already been created in kube-system namespace
acm, asm, configSync would always want to do a destroy_cmd_body followed by create_cmd_body

morgante · 2020-07-28T23:00:24Z

@bharathkkb You mean they have a permadiff on master or just this branch?

bharathkkb · 2020-07-28T23:18:17Z

@morgante permadiff on master due to new data.google_client_config.default.access_token same issue as
terraform-google-modules/terraform-google-gcloud#39 (comment)

for instance with stub-domains-upstream-nameservers on master after an apply and then the new plan will show. This is no-op though as the script catches this.

  # module.example.module.gke.module.gcloud_delete_default_kube_dns_configmap.null_resource.additional_components[0] must be replaced
-/+ resource "null_resource" "additional_components" {
      ~ id       = "4447235248369243877" -> (known after apply)
      ~ triggers = { # forces replacement
            "additional_components_command" = ".terraform/modules/example.gke.gcloud_delete_default_kube_dns_configmap/terraform-google-gcloud-1.3.0/scripts/check_components.sh gcloud kubectl"
          ~ "arguments"                     = "9f79c50b614be193be9fffe2e3188ca6" -> "c78e810c7855c28edf0ccb2d2e764095"
            "md5"                           = "48135f08d58ee02069d9787f607b6904"
        }
    }

  # module.example.module.gke.module.gcloud_delete_default_kube_dns_configmap.null_resource.run_command[0] must be replaced
-/+ resource "null_resource" "run_command" {
      ~ id       = "1402122050957459867" -> (known after apply)
      ~ triggers = { # forces replacement
          ~ "arguments"              = "9f79c50b614be193be9fffe2e3188ca6" -> "c78e810c7855c28edf0ccb2d2e764095"
          ~ "create_cmd_body"        = "https://123.123.123.123 OLD_TOKEN../../../scripts/delete-default-resource.sh kube-system configmap kube-dns" -> "https://123.123.123.123 ya29.c.NEW_TOKEN../../../scripts/delete-default-resource.sh kube-system configmap kube-dns"
            "create_cmd_entrypoint"  = "../../../scripts/kubectl_wrapper.sh"
            "destroy_cmd_body"       = "info"
            "destroy_cmd_entrypoint" = "gcloud"
            "gcloud_bin_abs_path"    = "/google-cloud-sdk/bin"
            "md5"                    = "48135f08d58ee02069d9787f607b6904"
        }
    }

morgante · 2020-07-28T23:22:22Z

@bharathkkb Thanks. Can you test what the upgrade path is too?

bharathkkb · 2020-07-29T01:10:52Z

@morgante i have tested upgrading some of our examples and documented findings below. I have also confirmed that with this implementation the permadiff issue has been resolved.

Upgrade path for main module when stub_domains or upstream_nameservers. Tested with stub_domains_upstream_nameservers.
- No-op as there is no destroy cmd and script checks if kube-dns already deleted.

 # module.example.module.gke.module.gcloud_delete_default_kube_dns_configmap.null_resource.additional_components[0] will be destroyed
  # module.example.module.gke.module.gcloud_delete_default_kube_dns_configmap.null_resource.additional_components_destroy[0] will be destroyed
  # module.example.module.gke.module.gcloud_delete_default_kube_dns_configmap.null_resource.module_depends_on[0] will be destroyed
  # module.example.module.gke.module.gcloud_delete_default_kube_dns_configmap.null_resource.run_command[0] will be destroyed
  # module.example.module.gke.module.gcloud_delete_default_kube_dns_configmap.module.gcloud_kubectl.null_resource.additional_components[0] will be created
  # module.example.module.gke.module.gcloud_delete_default_kube_dns_configmap.module.gcloud_kubectl.null_resource.additional_components_destroy[0] will be created
  # module.example.module.gke.module.gcloud_delete_default_kube_dns_configmap.module.gcloud_kubectl.null_resource.module_depends_on[0] will be created
  # module.example.module.gke.module.gcloud_delete_default_kube_dns_configmap.module.gcloud_kubectl.null_resource.run_command[0] will be created

Upgrade path for acm. Tested with simple_zonal_with_acm.
- Destroys and reapplies CRDs and manifests (this is current behavior during every apply due to permadiff on master)
- Not pasting whole diff but 13 to add, 0 to change, 37 to destroy.
- we also change the default value for skip_gcloud_download from false to true
Upgrade path for asm. Tested with simple_zonal_with_asm.
- Destroys and reapplies CRDs, manifests (this is current behavior during every apply due to permadiff on master)
- Destroys and recreates hub membership (this is our example specific as we use had overridden use_tf_google_credentials_env_var in the example and use_tf_google_credentials_env_var is no longer supported. This can be a breaking change.)

# module.example.module.asm.module.asm_install.null_resource.additional_components[0] will be destroyed
  # module.example.module.asm.module.asm_install.null_resource.additional_components_destroy[0] will be destroyed
  # module.example.module.asm.module.asm_install.null_resource.gcloud_auth_google_credentials[0] will be destroyed
  # module.example.module.asm.module.asm_install.null_resource.gcloud_auth_google_credentials_destroy[0] will be destroyed
  # module.example.module.asm.module.asm_install.null_resource.module_depends_on[0] will be destroyed
  # module.example.module.asm.module.asm_install.null_resource.run_command[0] will be destroyed
  # module.example.module.asm.module.gke_hub_registration.null_resource.gcloud_auth_google_credentials[0] will be destroyed
  # module.example.module.asm.module.gke_hub_registration.null_resource.gcloud_auth_google_credentials_destroy[0] will be destroyed
  # module.example.module.asm.module.gke_hub_registration.null_resource.run_command[0] must be replaced
-/+ resource "null_resource" "run_command" {
  # module.example.module.asm.module.asm_install.module.gcloud_kubectl.null_resource.additional_components[0] will be created
  # module.example.module.asm.module.asm_install.module.gcloud_kubectl.null_resource.additional_components_destroy[0] will be created
  # module.example.module.asm.module.asm_install.module.gcloud_kubectl.null_resource.module_depends_on[0] will be created
  # module.example.module.asm.module.asm_install.module.gcloud_kubectl.null_resource.run_command[0] will be created

bharathkkb · 2020-07-29T01:59:06Z

An update for the acm example, after swapping to the new module, it did fail initially as the destroy hook was not triggered during the upgrade and hence did not clean up git-creds secret.
I had to do .terraform/modules/example.acm.acm_operator.k8sop_creds_secret/terraform-google-gcloud-1.4.0/modules/kubectl-wrapper/scripts/kubectl_wrapper.sh cluster-name us-central1-a project false false kubectl delete secret git-creds -n=config-management-system to purge it.

morgante · 2020-07-29T02:02:48Z

@bharathkkb Thanks for investigating. Do you think these changes merit a major release?

bharathkkb · 2020-07-29T02:14:27Z

@morgante yeah I believe so because

we are removing use_tf_google_credentials_env_var option from asm
skip_gcloud_download defaults for acm/configSync are being changed so TF cloud users maybe affected

morgante · 2020-07-29T02:16:13Z

@bharathkkb Sounds good, can you start a small upgrade guide (in separate PR). It should also cover the commands needed to recover/force delete if gcloud has issues.

…les#602) BREAKING CHANGE: In-cluster resources have been updated to use the [kubectl wrapper](https://github.com/terraform-google-modules/terraform-google-gcloud/tree/master/modules/kubectl-wrapper) module. See the upgrade guide for details.

bharathkkb requested review from Jberlinsky and a team as code owners July 16, 2020 03:11

bharathkkb marked this pull request as draft July 16, 2020 03:11

bharathkkb mentioned this pull request Jul 16, 2020

fix: Check for binaries in additional_components terraform-google-modules/terraform-google-gcloud#57

Merged

bharathkkb changed the title ~~[DRAFT] Update gcloud module~~ feat: Update gcloud module Jul 16, 2020

bharathkkb marked this pull request as ready for review July 16, 2020 18:33

bharathkkb mentioned this pull request Jul 24, 2020

ASM Module fails #609

Closed

bharathkkb changed the title ~~feat: Update gcloud module~~ feat: Update modules to use new kubectl module Jul 27, 2020

bharathkkb marked this pull request as draft July 27, 2020 02:48

test gcloud main

639ecb2

add beta add kustomize test latest gcloud fix for binaries pin gcloud module to >= 1.2 build all transition to kubectl module fix dependecy crd module pin versions

bharathkkb force-pushed the fix-gcloud-install branch from e4f79f5 to 639ecb2 Compare July 28, 2020 02:06

bharathkkb mentioned this pull request Jul 28, 2020

fix: CFT bot ignore autogen fmt GoogleCloudPlatform/cloud-foundation-toolkit#750

Merged

bharathkkb marked this pull request as ready for review July 28, 2020 02:51

morgante suggested changes Jul 28, 2020

View reviewed changes

modules/asm/main.tf Outdated Show resolved Hide resolved

morgante suggested changes Jul 28, 2020

View reviewed changes

modules/asm/scripts/kubectl_wrapper.sh Outdated Show resolved Hide resolved

modules/asm/main.tf Outdated Show resolved Hide resolved

bharathkkb added 3 commits July 28, 2020 01:05

use kubectl mod with asm

f62485b

use kubectl mod for kube-dns delete

3860b61

fmt

faae9e9

bharathkkb requested a review from morgante July 28, 2020 16:51

morgante approved these changes Jul 28, 2020

View reviewed changes

morgante merged commit 794da61 into master Jul 29, 2020

morgante deleted the fix-gcloud-install branch July 29, 2020 02:16

bharathkkb mentioned this pull request Aug 10, 2020

[workload-identity] Options to disable annotating existing service account #622

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Update modules to use new kubectl module #602

feat: Update modules to use new kubectl module #602

bharathkkb commented Jul 16, 2020 •

edited

Loading

comment-bot-dev commented Jul 27, 2020 •

edited

Loading

morgante commented Jul 27, 2020

bharathkkb commented Jul 27, 2020

morgante commented Jul 27, 2020

bharathkkb commented Jul 28, 2020

morgante left a comment

morgante left a comment

bharathkkb commented Jul 28, 2020

morgante commented Jul 28, 2020

bharathkkb commented Jul 28, 2020 •

edited

Loading

morgante commented Jul 28, 2020

bharathkkb commented Jul 29, 2020

bharathkkb commented Jul 29, 2020

morgante commented Jul 29, 2020

bharathkkb commented Jul 29, 2020 •

edited

Loading

morgante commented Jul 29, 2020

feat: Update modules to use new kubectl module #602

feat: Update modules to use new kubectl module #602

Conversation

bharathkkb commented Jul 16, 2020 • edited Loading

comment-bot-dev commented Jul 27, 2020 • edited Loading

morgante commented Jul 27, 2020

bharathkkb commented Jul 27, 2020

morgante commented Jul 27, 2020

bharathkkb commented Jul 28, 2020

morgante left a comment

Choose a reason for hiding this comment

morgante left a comment

Choose a reason for hiding this comment

bharathkkb commented Jul 28, 2020

morgante commented Jul 28, 2020

bharathkkb commented Jul 28, 2020 • edited Loading

morgante commented Jul 28, 2020

bharathkkb commented Jul 29, 2020

bharathkkb commented Jul 29, 2020

morgante commented Jul 29, 2020

bharathkkb commented Jul 29, 2020 • edited Loading

morgante commented Jul 29, 2020

bharathkkb commented Jul 16, 2020 •

edited

Loading

comment-bot-dev commented Jul 27, 2020 •

edited

Loading

bharathkkb commented Jul 28, 2020 •

edited

Loading

bharathkkb commented Jul 29, 2020 •

edited

Loading