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(cluster.tf): add support for setting cgroup mode #1891

Conversation

chicocvenancio
Copy link
Contributor

Fix #1838

@chicocvenancio chicocvenancio requested review from ericyz, gtsorbo and a team as code owners February 29, 2024 12:23
@chicocvenancio
Copy link
Contributor Author

I do think it would be better to expose linux_node_config directly instead of splitting it into node_pools_linux_node_configs_sysctls and node_pools_cgroup_mode but did not want to make that big of a change without prior discussion.

@bharathkkb
Copy link
Member

/gcbrun

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 @chicocvenancio and I agree exposing linux_node_config would be ideal. I think this looks good given current API.

Copy link

This PR is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 7 days

@github-actions github-actions bot added the Stale label May 13, 2024
@apeabody apeabody removed the Stale label May 13, 2024
Copy link
Contributor

@apeabody apeabody 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 contribution @chicocvenancio!

From the CI tests:

Step #67 - "converge node-pool-local": STDERR: Error: expected node_config.0.linux_node_config.0.cgroup_mode to be one of ["CGROUP_MODE_UNSPECIFIED" "CGROUP_MODE_V1" "CGROUP_MODE_V2"], got 
Step #67 - "converge node-pool-local": 
Step #67 - "converge node-pool-local":   with module.example.module.gke.google_container_node_pool.pools["pool-01"],
Step #67 - "converge node-pool-local":   on ../../../modules/beta-public-cluster/cluster.tf line 565, in resource "google_container_node_pool" "pools":
Step #67 - "converge node-pool-local":  565:   node_config {
Step #67 - "converge node-pool-local": 
Step #67 - "converge node-pool-local": 
Step #67 - "converge node-pool-local": Error: expected node_config.0.linux_node_config.0.cgroup_mode to be one of ["CGROUP_MODE_UNSPECIFIED" "CGROUP_MODE_V1" "CGROUP_MODE_V2"], got 
Step #67 - "converge node-pool-local": 
Step #67 - "converge node-pool-local":   with module.example.module.gke.google_container_node_pool.pools["pool-02"],
Step #67 - "converge node-pool-local":   on ../../../modules/beta-public-cluster/cluster.tf line 565, in resource "google_container_node_pool" "pools":
Step #67 - "converge node-pool-local":  565:   node_config {
Step #67 - "converge node-pool-local": 
Step #67 - "converge node-pool-local": 
Step #67 - "converge node-pool-local": Error: expected node_config.0.linux_node_config.0.cgroup_mode to be one of ["CGROUP_MODE_UNSPECIFIED" "CGROUP_MODE_V1" "CGROUP_MODE_V2"], got 
Step #67 - "converge node-pool-local": 
Step #67 - "converge node-pool-local":   with module.example.module.gke.google_container_node_pool.pools["pool-03"],
Step #67 - "converge node-pool-local":   on ../../../modules/beta-public-cluster/cluster.tf line 565, in resource "google_container_node_pool" "pools":
Step #67 - "converge node-pool-local":  565:   node_config {

Can you also please rebase the PR, and we can do a fresh run of the CI tests.

Copy link

This PR is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 7 days

@github-actions github-actions bot added the Stale label Jul 29, 2024
@github-actions github-actions bot closed this Aug 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

support cgroup mode
3 participants