Skip to content

Conversation

DrFaust92
Copy link
Contributor

@DrFaust92 DrFaust92 commented Jul 19, 2024

Fix #1838
Supersedes #1891

seeing if i can push it over the finish line

@DrFaust92
Copy link
Contributor Author

there is probably some other issue here based on the previous PR but lets check and see. ill fix as needed

@DrFaust92 DrFaust92 marked this pull request as ready for review July 19, 2024 16:25
@DrFaust92 DrFaust92 requested review from a team, ericyz and gtsorbo as code owners July 19, 2024 16:25
@apeabody
Copy link
Collaborator

/gcbrun

2 similar comments
@apeabody
Copy link
Collaborator

/gcbrun

@apeabody
Copy link
Collaborator

/gcbrun

@apeabody apeabody self-assigned this Jul 22, 2024
@apeabody
Copy link
Collaborator

/gcbrun

@apeabody
Copy link
Collaborator

apeabody commented Jul 23, 2024

From INT:

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 with module.example.module.gke.google_container_node_pool.pools["pool-03"], on ../../../modules/beta-public-cluster/cluster.tf line 637, in resource "google_container_node_pool" "pools": 637: node_config { 

Probably need to default to null rather than an empty string.

var.node_pools_linux_node_configs_sysctls
)
node_pools_cgroup_mode = merge(
{ all = "" },
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@apeabody i wonder if this needs to be CGROUP_MODE_UNSPECIFIED to make it work

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hi @DrFaust92 - I checked the provider code and cgroup_mode is an (optional) string, but will not accept "". So an option for https://github.com/terraform-google-modules/terraform-google-kubernetes-engine/pull/2001/files#diff-a3a07be3819af6f1a65ed243a613e69b544b7274d248a4b4d7d4d127aec40aeeR1045 would be to check the locals value and if "" instead pass 'null'.

@DrFaust92
Copy link
Contributor Author

DrFaust92 commented Jul 25, 2024 via email

@apeabody
Copy link
Collaborator

apeabody commented Aug 1, 2024

/gcbrun

@apeabody
Copy link
Collaborator

apeabody commented Aug 1, 2024

/gcbrun

Copy link
Collaborator

@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 @DrFaust92!

Confirm cgroup_mode in TPG v5.25.0

@apeabody
Copy link
Collaborator

apeabody commented Aug 2, 2024

/gcbrun

@apeabody
Copy link
Collaborator

apeabody commented Aug 5, 2024

/gcbrun

@apeabody
Copy link
Collaborator

apeabody commented Aug 6, 2024

/gcbrun

@apeabody apeabody merged commit 3fc4db4 into terraform-google-modules:master Aug 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants