Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable overriding create_cmd_triggers completely #39

Closed
bharathkkb opened this issue Apr 5, 2020 · 9 comments · Fixed by #62
Closed

Enable overriding create_cmd_triggers completely #39

bharathkkb opened this issue Apr 5, 2020 · 9 comments · Fixed by #62
Assignees

Comments

@bharathkkb
Copy link
Member

bharathkkb commented Apr 5, 2020

I am using this module to deploy a Kubernetes resource foo like so

create_cmd_entrypoint = "${path.module}/scripts/kubectl_wrapper.sh"
create_cmd_body       = "https://${module.cluster.endpoint} ${data.google_client_config.default.access_token} ${module.cluster.ca_certificate} kubectl apply -k foo"

Problem:
However due to data.google_client_config.default.access_token changing between Terraform plans this causes noise in the diff as well as blows away and recreates the foo resource

Proposal:
Either let

  • create_cmd_triggers override triggers completely

  • new var create_cmd_triggers_override if set completely overrides the triggers

Open to other suggestions or if I am missing something here

@morgante
Copy link
Contributor

morgante commented Apr 5, 2020

I assume what you're trying to do is have the create command body excluded from the destroy triggers.

This will not work, because Terraform requires that all values accessed in the provisioner are referenced from triggers.

I don't like it either, but there's no workaround possible besides fixing Terraform core.

Background here: hashicorp/terraform#23679

@morgante morgante closed this as completed Apr 5, 2020
@bharathkkb
Copy link
Member Author

@morgante I have been investigating this for a bit. Is it reasonable for the kubectl submodule to generate kubeconfig on the fly using gcloud container get-credentials as opposed to passing in data.google_client_config.default.access_token which will help us bypass the permadiff issue?

@bharathkkb bharathkkb reopened this Jul 12, 2020
@morgante
Copy link
Contributor

I'd like to avoid using gcloud commands if we can avoid it. What if instead we generated a kubeconfig using the auth module and referenced that?

@bharathkkb
Copy link
Member Author

@morgante even with the auth module, it references the access token so I presume any steps dependent on this would be rerun?

@morgante
Copy link
Contributor

@bharathkkb We'd use the auth module to generate a kubeconfig file (with a static path). The gcloud module would then reference the kubeconfig file and not need to return each time.

@bharathkkb
Copy link
Member Author

Cool I can give that a try. My thought was since the kubeconfig is a templated file, TF will want to recreate that kubeconfig each time the data.google_client_config.provider.access_token changes, however with a static path the dependent gcloud module should not recreate anything else.

@bharathkkb
Copy link
Member Author

I have been testing this out

auth module to generate a kubeconfig file (with a static path)

This had two drawbacks

  • permadiff on local_file due to the static file always needing to be updated due to access_token
  • inability to set most latest kubeconfig before running destroy time provisioner
module.example.module.asm.module.asm_install.null_resource.run_command[0]: Destroying... [id=536953105127865122]
module.example.module.asm.module.asm_install.null_resource.run_command[0]: Provisioning with 'local-exec'...
module.example.module.asm.module.asm_install.null_resource.run_command[0] (local-exec): Executing: ["/bin/sh" "-c" "PATH=/google-cloud-sdk/bin:$PATH\nkubectl create ns foo --kubeconfig ../../../modules/asm/kubeconfig\n"]
module.example.module.asm.module.asm_install.null_resource.run_command[0] (local-exec): namespace/foo created
module.example.module.asm.module.asm_install.null_resource.run_command[0]: Destruction complete after 0s
module.example.module.asm.local_file.kubeconfig: Destroying... [id=06f9bffb6f12f194a80f83673f210b18a5981f5b]
module.example.module.asm.local_file.kubeconfig: Destruction complete after 0s
module.example.module.asm.local_file.kubeconfig: Creating...
module.example.module.asm.local_file.kubeconfig: Creation complete after 0s [id=6efa25c5c6a00ec6e7c862b4fccdf7689d6bbf51]
module.example.module.asm.module.asm_install.null_resource.run_command[0]: Creating...
module.example.module.asm.module.asm_install.null_resource.run_command[0]: Provisioning with 'local-exec'...

In this case, my destroy command was kubectl create ns foo --kubeconfig ../../../modules/asm/kubeconfig and that was run before generating the new kubeconfig. irl this case fail, if the token is expired by the time, this destroy command runs. This is also happening with the current implementation with access token inline and I believe that is the reason for terraform-google-modules/terraform-google-kubernetes-engine#604

If you have other ideas I would be happy to test them out.

@morgante
Copy link
Contributor

morgante commented Jul 20, 2020

permadiff on local_file due to the static file always needing to be updated due to access_token

This could probably be fixed by using ignore_changes on the file. That being said, doing so is likely more complicated than required.

How about we just add gcloud container get-credentials into the wrapper script for the kubectl submodule?

@bharathkkb
Copy link
Member Author

This could probably be fixed by using ignore_changes on the file.

I did try with ignore_changes but then its hard to determine when to actually generate a new kubeconfig and when not to.

How about we just add gcloud container get-credentials into the wrapper script for the kubectl submodule?

Sounds good, I will give that a go.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants