Fix zsh-specific syntax in retry loop within null_resource.update_config_map_aws_auth

[marcelloromani]

PR o'clock

Description

This is a fix for #238
Fixes two problems:

• kubectl apply is actually tried up to 5 times
• terraform apply fails if said command fails after 5 attempts

Checklist

• terraform fmt and terraform validate both work from the root and examples/eks_test_fixture directories (look in CI for an example)
• Tests for the changes have been added and passing (for bug fixes/features)
• Test results are pasted in this PR (in lieu of CI)
• I've added my change to CHANGELOG.md
• Any breaking changes are highlighted above

Tests

Not sure how to add automated testing to this.

I did test my code changes manually against module version 1.8.0, and noticed it took kubectl 4 times before succeeding.
I therefore thought it would be safer to increase the number of retries to 10

[elatt]

FWIW, I don't know if the {1..5} syntax is zsh-specific as it appears to work on the Mac version of bash:

erik:terraform erik.lattimore$ for i in {1..5}; do echo $i; done
1
2
3
4
5
erik:terraform erik.lattimore$ echo $SHELL
/bin/bash
erik:terraform erik.lattimore$ /bin/bash --version
GNU bash, version 3.2.57(1)-release (x86_64-apple-darwin18)
Copyright (C) 2007 Free Software Foundation, Inc.

But agree it isn't POSIX compliant so your change seems better. Most important though is the exit handling though if we reach the end of the loop.

[marcelloromani]

For the sake of curiosity, I tested that statement on a few more envs...

Alpine Linux: {1..5} fail, seq 1 5 pass
Ubuntu 18:04: {1..5} pass, seq 1 5 pass
Debian: {1..5} pass, seq 1 5 pass
Centos: {1..5} pass, seq 1 5 pass

Ouch, it seems to be (yet another) one of those Alpine quirks. As a matter of fact, we run Terraform via a docker image based on Alpine, which explains why I noticed the "bug".
Considering Alpine use is widespread I still think it's a worthwhile change though.

I agree with you the more important issue is the exit code.

[max-rocket-internet]

OK great, thanks for testing those other OSs.

[marcelloromani]

Hi @max-rocket-internet is there a chance we could see this fixed in e.g. 1.8.1 ? I am able to offer help if there's anything I can do.

[max-rocket-internet]

You mean you just want a new release? e.g. v2.1.0?

[marcelloromani]

Yes, a new release.

I was thinking about 1.8.1 because in 2.0.0 there are breaking changes, but I guess we can bear upgrading to a 2.0.1 and adjust our codebase accordingly.

[max-rocket-internet]

Yes sir!

https://github.com/terraform-aws-modules/terraform-aws-eks/releases/tag/v2.1.0

[github-actions]

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.