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

fix: empty addons_config handling #1978

Merged

Conversation

0Delta
Copy link
Contributor

@0Delta 0Delta commented Jun 17, 2024

resolve #1716

Older clusters may have missing or missing values in google_container_cluster.primary.addons_config.
The current code assumes that the value is sufficient and cannot resolve this.

This patch will reduce dirty hacks such as pulling and modifying this repo and importing.

@0Delta 0Delta requested review from ericyz, gtsorbo and a team as code owners June 17, 2024 18:48
@apeabody
Copy link
Collaborator

/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 @0Delta!

Please see the linter output for change.

@0Delta 0Delta force-pushed the fix/empty_addons_config branch from dac779d to 2a01483 Compare June 27, 2024 06:47
@0Delta
Copy link
Contributor Author

0Delta commented Jun 27, 2024

I have fixed the points you pointed out, apologize for my bad format.

@apeabody
Copy link
Collaborator

apeabody commented Jul 1, 2024

From the linter:

Checking submodule's files generation
diff -r '--exclude=.terraform' '--exclude=.kitchen' '--exclude=.git' /workspace/main.tf /tmp/tmp.xMCHtY7AKG/workspace/main.tf
121,123c121,123
<   cluster_output_network_policy_enabled             = coalescelist(lookup(coalescelist(google_container_cluster.primary.addons_config, [{}])[0], "network_policy_config", [{}]), [{ disabled = false }])[0].disabled
<   cluster_output_http_load_balancing_enabled        = coalescelist(lookup(coalescelist(google_container_cluster.primary.addons_config, [{}])[0], "http_load_balancing", [{}]), [{ disabled = false }])[0].disabled
<   cluster_output_horizontal_pod_autoscaling_enabled = coalescelist(lookup(coalescelist(google_container_cluster.primary.addons_config, [{}])[0], "horizontal_pod_autoscaling", [{}]), [{ disabled = false }])[0].disabled
---
>   cluster_output_network_policy_enabled             = google_container_cluster.primary.addons_config[0].network_policy_config[0].disabled
>   cluster_output_http_load_balancing_enabled        = google_container_cluster.primary.addons_config[0].http_load_balancing[0].disabled
>   cluster_output_horizontal_pod_autoscaling_enabled = google_container_cluster.primary.addons_config[0].horizontal_pod_autoscaling[0].disabled
Error: submodule's files generation has not been run, please run the
'make build' command and commit changes

@0Delta
Copy link
Contributor Author

0Delta commented Jul 2, 2024

Linter indicates the previous code, indicating that existing problems are not solved.
Which is better to ask Linter ignoring only this section or design a code that makes Linter in a good mood?

@apeabody
Copy link
Collaborator

apeabody commented Jul 2, 2024

Linter indicates the previous code, indicating that existing problems are not solved. Which is better to ask Linter ignoring only this section or design a code that makes Linter in a good mood?

Hi @0Delta - It appears the templates haven't been updated. Please see here for details: https://github.com/terraform-google-modules/terraform-google-kubernetes-engine/blob/master/CONTRIBUTING.md#templating. Thanks!

@0Delta 0Delta force-pushed the fix/empty_addons_config branch from 2a01483 to 6129e83 Compare July 3, 2024 15:07
@0Delta
Copy link
Contributor Author

0Delta commented Jul 3, 2024

I finally found out what this repository was made up of.
Thank you @apeabody for the detailed guide.

@apeabody
Copy link
Collaborator

apeabody commented Jul 3, 2024

/gcbrun

@apeabody
Copy link
Collaborator

apeabody commented Jul 8, 2024

/gcbrun

@apeabody
Copy link
Collaborator

/gcbrun

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

/gcbrun

@apeabody
Copy link
Collaborator

/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 @0Delta!

@apeabody apeabody merged commit 9ae8b38 into terraform-google-modules:master Jul 17, 2024
4 checks passed
@0Delta 0Delta deleted the fix/empty_addons_config branch July 18, 2024 02:52
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.

Import of existing cluster fails due to addons response empty
2 participants