-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Feature/config sync module #493
Feature/config sync module #493
Conversation
…, renamed a few vars.
…also made operator credential a lot more configurable
…, renamed a few vars.
…also made operator credential a lot more configurable
…erraform-google-kubernetes-engine into feature/config-sync-module had rebased from upstream
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just some initial feedback.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution; this is awesome :D
|
||
|
||
module "k8s_operator" { | ||
source = "terraform-google-modules/gcloud/google" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: I know we havent enabled skip_download before, but imho we should be allowing users to control skip_download for the gcloud module, just like in project factory. @morgante thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh, this is great -- happy to add this here or in a fast following PR. wish i knew this was available, working on this was very slow because of the download and local install everytime!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yup, I gave config sync a spin locally and manually set it to true for faster apply
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cool, added in 418f9ae
Left a few small comments but LGTM 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this! Just looks like one extra file might have been committed?
…ync templates to test this, so we dont need to support a copy here.
@linde Please also regenerate docs, otherwise this should be good to go. |
@morgante -- looks like the build ran into a problem. it completed ok in cloudbuild but it isnt reflected here. can you re-kick it off maybe? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, tests pass on Cloud Build.
BREAKING CHANGE: The ACM module has been refactored and resources will be recreated. This will show up in Terraform plans but is a safe no-op for Kubernetes.
this is an effort to make a config sync counterpart to the acm module.