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

feat: Add support for gVisor per node pool #1001

Merged

Conversation

LukaszCzarnotaSabre
Copy link
Contributor

Issue https://github.com/terraform-google-modules/terraform-google-kubernetes-engine/issues/240 was resolved but now all node pools needs to be with gVisor or non of them.

With this PR you are able to define sandbox_enabled variable per node pool, code is backward compatible.

@google-cla
Copy link

google-cla bot commented Sep 16, 2021

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@LukaszCzarnotaSabre
Copy link
Contributor Author

@googlebot I signed it!

@comment-bot-dev
Copy link

comment-bot-dev commented Sep 16, 2021

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

@Jberlinsky
Copy link
Contributor

@LukaszCzarnotaSabre Looks like this change is causing test failures:

Step #48 - "converge node-pool-local":        Error: Invalid index
Step #48 - "converge node-pool-local":        
Step #48 - "converge node-pool-local":          on ../../../modules/beta-public-cluster/cluster.tf line 423, in resource "google_container_node_pool" "pools":
Step #48 - "converge node-pool-local":         423:       for_each = tobool((lookup(each.value, "sandbox_enabled", ((local.cluster_sandbox_enabled[0] == "gvisor") ? true : false)))) ? ["gvisor"] : []
Step #48 - "converge node-pool-local":            ├────────────────
Step #48 - "converge node-pool-local":            │ local.cluster_sandbox_enabled is empty list of string
Step #48 - "converge node-pool-local":        
Step #48 - "converge node-pool-local":        The given key does not identify an element in this collection value: the
Step #48 - "converge node-pool-local":        collection has no elements.
Step #9 - "converge safer-cluster-local":        
Step #9 - "converge safer-cluster-local":        Error: Invalid index
Step #9 - "converge safer-cluster-local":        
Step #9 - "converge safer-cluster-local":          on ../../../modules/beta-private-cluster/cluster.tf line 442, in resource "google_container_node_pool" "pools":
Step #9 - "converge safer-cluster-local":         442:       for_each = tobool((lookup(each.value, "sandbox_enabled", ((local.cluster_sandbox_enabled[0] == "gvisor") ? true : false)))) ? ["gvisor"] : []
Step #9 - "converge safer-cluster-local":            ├────────────────
Step #9 - "converge safer-cluster-local":            │ local.cluster_sandbox_enabled is empty list of string
Step #9 - "converge safer-cluster-local":        
Step #9 - "converge safer-cluster-local":        The given key does not identify an element in this collection value: the
Step #9 - "converge safer-cluster-local":        collection has no elements.
Step #48 - "converge node-pool-local":        
Step #48 - "converge node-pool-local":        Error: Invalid index
Step #48 - "converge node-pool-local":        
Step #48 - "converge node-pool-local":          on ../../../modules/beta-public-cluster/cluster.tf line 423, in resource "google_container_node_pool" "pools":
Step #48 - "converge node-pool-local":         423:       for_each = tobool((lookup(each.value, "sandbox_enabled", ((local.cluster_sandbox_enabled[0] == "gvisor") ? true : false)))) ? ["gvisor"] : []
Step #48 - "converge node-pool-local":            ├────────────────
Step #48 - "converge node-pool-local":            │ local.cluster_sandbox_enabled is empty list of string
Step #48 - "converge node-pool-local":        
Step #48 - "converge node-pool-local":        The given key does not identify an element in this collection value: the
Step #48 - "converge node-pool-local":        collection has no elements.
Step #30 - "converge stub-domains-local":        google_compute_network.main: Creating...
Step #9 - "converge safer-cluster-local": >>>>>> Running the command `terraform apply -auto-approve -lock=true -lock-timeout=0s -input=false -no-color -parallelism=10 -refresh=true  ` failed due to a non-zero exit code of 1.
Step #9 - "converge safer-cluster-local": >>>>>> ------Exception-------
Step #9 - "converge safer-cluster-local": >>>>>> Class: Kitchen::ActionFailed
Step #9 - "converge safer-cluster-local": >>>>>> Message: 1 actions failed.
Step #9 - "converge safer-cluster-local": >>>>>>     Converge failed on instance <safer-cluster-local>.  Please see .kitchen/logs/safer-cluster-local.log for more details
Step #9 - "converge safer-cluster-local": >>>>>> ----------------------
Step #9 - "converge safer-cluster-local": >>>>>> Please see .kitchen/logs/kitchen.log for more details
Step #9 - "converge safer-cluster-local": >>>>>> Also try running `kitchen diagnose --all` for configuration
Step #48 - "converge node-pool-local":        
Step #48 - "converge node-pool-local":        Error: Invalid index
Step #48 - "converge node-pool-local":        
Step #48 - "converge node-pool-local":          on ../../../modules/beta-public-cluster/cluster.tf line 423, in resource "google_container_node_pool" "pools":
Step #48 - "converge node-pool-local":         423:       for_each = tobool((lookup(each.value, "sandbox_enabled", ((local.cluster_sandbox_enabled[0] == "gvisor") ? true : false)))) ? ["gvisor"] : []
Step #48 - "converge node-pool-local":            ├────────────────
Step #48 - "converge node-pool-local":            │ local.cluster_sandbox_enabled is empty list of string
Step #48 - "converge node-pool-local":        

Could you please update tests to make sure they're compliant with this change?

@bharathkkb bharathkkb mentioned this pull request Sep 16, 2021
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! It would also be great to add this to one of our tests to make sure its WAI

autogen/main/cluster.tf.tmpl Outdated Show resolved Hide resolved
@google-cla
Copy link

google-cla bot commented Sep 27, 2021

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@bharathkkb
Copy link
Member

@googlebot I consent.

@bharathkkb
Copy link
Member

/cc @morgante for another pass since I have a commit in here

@morgante morgante merged commit 850c418 into terraform-google-modules:master Sep 28, 2021
bharathkkb added a commit that referenced this pull request Oct 18, 2021
* feat: Add support for gVisor per node pool

* fix image type, add test

Co-authored-by: Bharath KKB <bharathkrishnakb@gmail.com>
CPL-markus pushed a commit to WALTER-GROUP/terraform-google-kubernetes-engine that referenced this pull request Jul 15, 2024
…1001)

* feat: Add support for gVisor per node pool

* fix image type, add test

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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants