Skip to content
This repository has been archived by the owner on Sep 21, 2020. It is now read-only.

[GPII-3251]: Idempotence for helm-release destroy #10

Merged
merged 3 commits into from
Aug 23, 2018

Conversation

natarajaya
Copy link

@natarajaya natarajaya commented Aug 21, 2018

I know, it looks like a really complicated fix for such a trivial issue, but believe me, there is no better way :)

Terraform will always fail during Helm provider configuration (which happens during :destroy as well) if certificate files not present on disk (already removed during previous :destroy) with:

Error: module.gpii-flowmanager.provider.helm: file: open /project/live/dev/secrets/kube-system/helm-tls/ca.cert.pem: no such file or directory in:

${file("${var.client_auth}/ca.cert.pem")}

And there is no way to evaluate if local file is present on disk in Terraform: hashicorp/terraform#10878.

@natarajaya natarajaya self-assigned this Aug 21, 2018
@natarajaya natarajaya requested review from mrtyler and removed request for mrtyler August 21, 2018 22:42
program = [
"sh", "-c",
<<EOF
ca_cert=$(cat ${var.client_auth}/ca.cert.pem 2>/dev/null | awk '$1=$1' ORS=' \n')
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI I had to look up both the $1=$1 idiom and ORS. Combined with the use of jq, it might be worth a comment explaining what all this is doing and why.

If the reason for all of this is to avoid loading a non-existent file, is it possible (and would it be simpler) to create these files elsewhere in Terraform or with rake code instead?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like jq is capable of handling newlines properly even without awk treatment, so that part looks a bit simpler now.

Regarding alternative solutions: well, creating empty cert files at the end of every destroy, and then removing them at the beginning of apply definitely does not sound better than just fixing provider configuration to not fail when there is no certs!

@mrtyler
Copy link

mrtyler commented Aug 23, 2018

Yes, that looks nicer.

Can you add a comment explaining why this is here? Do that and it LGTM.

Also please offer this fix to upstream. :)

@natarajaya natarajaya merged commit c8a21bc into gpii-ops:master Aug 23, 2018
@natarajaya
Copy link
Author

exekube#100

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

Successfully merging this pull request may close these issues.

2 participants