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!: Set the provided SA when creating autopilot clusters #1495

Merged
merged 11 commits into from
Dec 28, 2022
Merged

feat!: Set the provided SA when creating autopilot clusters #1495

merged 11 commits into from
Dec 28, 2022

Conversation

ferrarimarco
Copy link
Contributor

Fixes #1488

Hi folks!

In this PR, I'm trying to address the above issue, although I think the necessary server-side fix is still being rolled out.

Hopefully I got the autogen bits right :)

@ferrarimarco
Copy link
Contributor Author

If I read the log correctly, the failures reported in https://github.com/terraform-google-modules/terraform-google-kubernetes-engine/pull/1495/checks?check_run_id=9992487427 don't seem too related to this PR, but rather to a checksum mismatch when downloading providers/plugins.

@ferrarimarco
Copy link
Contributor Author

Ah, so now the build may be failing for that yet missing server-side fix.

@ferrarimarco
Copy link
Contributor Author

It appears to be the case. This is an extract from the Terraform INFO output running in my environment when testing this PR:

2022-12-09T13:53:19.420Z [ERROR] provider.terraform-provider-google-beta_v4.44.1_x5: Response contains error diagnostic: diagnostic_attribute=AttributeName("cluster_autoscaling").ElementKeyInt(0).AttributeName("enabled") @caller=github.com/hashicorp/[email protected]/tfprotov5/internal/diag/diagnostics.go:56 diagnostic_detail=""cluster_autoscaling.0.enabled": conflicts with enable_autopilot" diagnostic_summary="Conflicting configuration arguments" @module=sdk.proto diagnostic_severity=ERROR tf_proto_version=5.2 tf_provider_addr=provider tf_req_id=REDACTED tf_resource_type=google_container_cluster tf_rpc=ValidateResourceTypeConfig timestamp=2022-12-09T13:53:19.420Z
2022-12-09T13:53:19.420Z [ERROR] vertex "module.gke.google_container_cluster.primary" error: Conflicting configuration arguments
2022-12-09T13:53:19.420Z [ERROR] vertex "module.gke.google_container_cluster.primary (expand)" error: Conflicting configuration arguments

@ferrarimarco
Copy link
Contributor Author

Ah, I was wrongly setting cluster_autoscaling.enabled. Fixed.

@ferrarimarco
Copy link
Contributor Author

ferrarimarco commented Dec 9, 2022

Ok, now I think I'm getting the server-side error on a resource update:

│ Error: googleapi: Error 400: Overriding Autopilot autoscaling settings is not allowed.
│ Details:
│ [
│   {
│     "@type": "type.googleapis.com/google.rpc.DebugInfo",
│     "detail": "[ORIGINAL ERROR] cloud-kubernetes::3: Overriding Autopilot autoscaling settings is not allowed. [google.rpc.error_details_ext] { code: 3 message: \"Overriding Autopilot autoscaling settings is not allowed.\" }"
│   }
│ ]
│ , badRequest
│
│   with module.gke.google_container_cluster.primary,
│   on terraform-google-kubernetes-engine/modules/beta-autopilot-private-cluster/cluster.tf line 22, in resource "google_container_cluster" "primary":
│   22: resource "google_container_cluster" "primary" {

I was able to create it anew tho.

@CaliWee
Copy link

CaliWee commented Dec 13, 2022

Fixes #1488

Hi folks!

In this PR, I'm trying to address the above issue, although I think the necessary server-side fix is still being rolled out.

Hopefully I got the autogen bits right :)

Hi @ferrarimarco! Thank you for working on this. is there a GitHub issue or PR I can watch to track the server side fix?

@bharathkkb bharathkkb changed the title Set the provided SA when creating autopilot clusters feat: Set the provided SA when creating autopilot clusters Dec 14, 2022
@bharathkkb bharathkkb changed the title feat: Set the provided SA when creating autopilot clusters feat!: Set the provided SA when creating autopilot clusters Dec 14, 2022
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 @ferrarimarco.
I pushed a few changes as we are switching to a new test framework. Looks like this will be a breaking change as the conflict on the field was removed in 4.44+.

@ferrarimarco
Copy link
Contributor Author

ferrarimarco commented Dec 14, 2022

@CaliWee hi! I don't think there is a PR or issue to follow.

@bharathkkb Thanks for reviewing and for the changes!

Update: Yes, in 4.44+. I forgot to update the provider version constraint, but I see you did it. Thanks!

@comment-bot-dev
Copy link

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

@bharathkkb bharathkkb merged commit d122a55 into terraform-google-modules:master Dec 28, 2022
@ferrarimarco ferrarimarco deleted the autopilot-service-account branch December 28, 2022 09:16
mnahkies added a commit to mnahkies/terraform-google-kubernetes-engine that referenced this pull request Dec 28, 2022
mnahkies added a commit to mnahkies/terraform-google-kubernetes-engine that referenced this pull request Dec 28, 2022
CPL-markus pushed a commit to WALTER-GROUP/terraform-google-kubernetes-engine that referenced this pull request Jul 15, 2024
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.

beta-autopilot-private-cluster uses default service account
4 participants