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

Add configuration flag for #141 #160

Merged
merged 13 commits into from
Jun 7, 2019
Merged

Add configuration flag for #141 #160

merged 13 commits into from
Jun 7, 2019

Conversation

ingwarr
Copy link
Contributor

@ingwarr ingwarr commented Jun 4, 2019

added enable_binary_authorization flag

Copy link
Contributor

@alexkonkin alexkonkin left a comment

Choose a reason for hiding this comment

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

You should not do this directly in this file. Please transfer your changes to the autogen/cluster_regional.tf template. The same is related to all changes that are connected to the resources that are present in autogen folder. The changes from this folder are propagated to all places where they are used.

@alexkonkin
Copy link
Contributor

alexkonkin commented Jun 4, 2019

added enable_binary_authorization flag

Your lint tests failed because you should have executed 2 commands before commit:
make generate
make generate_docs

@alexkonkin
Copy link
Contributor

Please also don't forget to add the tests that validate that your feature is present in the newly created cluster.

@ingwarr
Copy link
Contributor Author

ingwarr commented Jun 4, 2019

@alexkonkin Thank you!

@ingwarr
Copy link
Contributor Author

ingwarr commented Jun 4, 2019

added enable_binary_authorization flag

Your lint tests failed because you should have executed 2 commands before commit:
make generate
make generate_docs
Fixed, I've added appropriate variables to autogen

autogen/cluster_regional.tf Show resolved Hide resolved
autogen/outputs.tf Outdated Show resolved Hide resolved
autogen/variables.tf Outdated Show resolved Hide resolved
autogen/variables.tf Outdated Show resolved Hide resolved
examples/deploy_service/README.md Outdated Show resolved Hide resolved
ingwarr and others added 4 commits June 4, 2019 20:14
Co-Authored-By: Aaron Lane <aarondrewlane@gmail.com>
Co-Authored-By: Aaron Lane <aarondrewlane@gmail.com>
Co-Authored-By: Aaron Lane <aarondrewlane@gmail.com>
Copy link
Contributor

@aaron-lane aaron-lane left a comment

Choose a reason for hiding this comment

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

Please apply the suggested changes, run make generate && make generate_docs, and then commit the changes.

autogen/outputs.tf Outdated Show resolved Hide resolved
autogen/variables.tf Show resolved Hide resolved
autogen/variables.tf Show resolved Hide resolved
modules/private-cluster/outputs.tf Outdated Show resolved Hide resolved
ingwarr and others added 4 commits June 4, 2019 21:24
Co-Authored-By: Aaron Lane <aarondrewlane@gmail.com>
Co-Authored-By: Aaron Lane <aarondrewlane@gmail.com>
Co-Authored-By: Aaron Lane <aarondrewlane@gmail.com>
@alexkonkin
Copy link
Contributor

alexkonkin commented Jun 5, 2019

Quoting the results of the integration-tests execution:
a)
google_compute_subnetwork.main: Error waiting to create Subnetwork: Error waiting for Creating Subnetwork: Quota 'ROUTES' exceeded. Limit: 250.0 globally.
b)
Error waiting for Deleting Network: The network resource '<...>/networks/cft-gke-test-5pwt' is already being used by <...>/regions/us-east4/subnetworks/cft-gke-test-5pwt'

It seems that some resources have not been deleted after the previous CI execution....
As far as I remember such situation can be resolved by the one who has
the administrator's permission to do this.

@aaron-lane
Copy link
Contributor

I manually verified the node pool suite; the failure is unrelated to these changes.

@aaron-lane aaron-lane merged commit 974c8e9 into terraform-google-modules:master Jun 7, 2019
aaron-lane added a commit that referenced this pull request Jun 7, 2019
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 this pull request may close these issues.

None yet

3 participants