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.

Sign up for GitHub

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

Jump to bottom

feat!: Add support for asm v1.8 to the asm module #824

Merged
merged 6 commits into from
Feb 23, 2021
Merged

feat!: Add support for asm v1.8 to the asm module #824

merged 6 commits into from
Feb 23, 2021

Conversation

kaariger
Copy link
Contributor

@kaariger kaariger commented Feb 17, 2021 •
edited by morgante

  • Modified default value for asm_version to 1.8
  • Modified install_asm.sh wrapper script to call the new install_asm script with the updated flags. Removed flag to for the temp directory, allowing the install_asm script to manage its own temp directory.
    • TODO: Need to remove the --enable_cluster_labels flag in the wrapper script once the install_asm script makes that flag optional. It is currently required.
  • Added additional IAM bindings and API enablement to the test scripts
  • Regenerated docs
  • Ran lint - only saw warnings about other files that are unmodified in this change
  • Ran integration tests
  • Fixes Terraform integration w/ASM script #707

@morgante
Copy link
Contributor

@kaariger What's the reason for a new PR instead of updating #821?

bharathkkb
bharathkkb reviewed Feb 17, 2021
View reviewed changes
Copy link
Member

@bharathkkb bharathkkb left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @kaariger

We can wait for upstream to remove the asmv validate as it will cause a diff on cluster labels if the cluster is managed by TF.

@kaariger
Copy link
Contributor Author

@morgante its a mistake on my part. I should have just updated my original PR. I closed the original PR (my mistake). Please let me know if I should update this PR to make it easier. For future, I will update existing PRs.

@bharathkkb it may be ok to wait. However, I had heard that some users were waiting for the module to support 1.8. Don't know if there is any urgent need to allow it as is with the labels added by install_asm script.

@morgante
Copy link
Contributor

morgante commented Feb 18, 2021 •
edited

@morgante its a mistake on my part. I should have just updated my original PR. I closed the original PR (my mistake). Please let me know if I should update this PR to make it easier. For future, I will update existing PRs.

No worries, thanks! Just wanted to confirm if something prompted it.

@bharathkkb it may be ok to wait. However, I had heard that some users were waiting for the module to support 1.8. Don't know if there is any urgent need to allow it as is with the labels added by install_asm script.

I'd lean towards adding this sooner rather than later. We can update later.

@abhinavrau abhinavrau mentioned this pull request Feb 19, 2021
Closed
@comment-bot-dev
Copy link

comment-bot-dev commented Feb 23, 2021 •
edited

Thanks for the PR! 🚀
✅ Lint checks have passed.

@kaariger
-updated provider version in the safer_cluster_iap_bastion example -a…
2dd1478 
…dded readme for v14.0 release
@kaariger
Merge branch 'support-asm-1.8' of github.com:kaariger/terraform-googl…
c4b8c28 
…e-kubernetes-engine into support-asm-1.8

merging back remote changes
@kaariger
Copy link
Contributor Author

I have committed two more changes:

  • updated provider version in the safer_cluster_iap_bastion example to 2.52. The previous version was causing integration tests to fail.
  • added a readme for version 14.0 since this is a breaking release. Added limitations around upgrades and installations for ASM versions.

bharathkkb
bharathkkb reviewed Feb 23, 2021
View reviewed changes
docs/upgrading_to_v14.0.md Outdated Show resolved Hide resolved
docs/upgrading_to_v14.0.md Outdated Show resolved Hide resolved
@bharathkkb bharathkkb changed the title Add support for asm v1.8 to the asm module feat!: Add support for asm v1.8 to the asm module Feb 23, 2021
@bharathkkb bharathkkb merged commit 923eff4 into terraform-google-modules:master Feb 23, 2021
@release-please release-please bot mentioned this pull request Feb 23, 2021
Merged
@bharathkkb bharathkkb mentioned this pull request Mar 8, 2021
Closed
CPL-markus pushed a commit to WALTER-GROUP/terraform-google-kubernetes-engine that referenced this pull request Jul 15, 2024
@kaariger @bharathkkb
feat!: Add support for asm v1.8 to the asm module (terraform-google-m…
d6d89c3 
…odules#824)

* Initial changes to asm module to support installing asm ver 1.8.

* Updated changes to support ASM 1.8

* -updated provider version in the safer_cluster_iap_bastion example -added readme for v14.0 release

* -updated readme for v14.0 release

Co-authored-by: kaariger <kaariger@users.noreply.github.com>
Co-authored-by: Bharath KKB <bharathkrishnakb@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bharathkkb bharathkkb bharathkkb approved these changes

@Jberlinsky Jberlinsky Awaiting requested review from Jberlinsky

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Terraform integration w/ASM script
4 participants
@kaariger @morgante @comment-bot-dev @bharathkkb