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

adding auto-pilot support #850

Closed

Conversation

cmcga1125
Copy link
Contributor

autopilot support

hey friends,
Here's my PR addressing issue #835 enabling GKE Autopilot Support. right now it is blocked by an upstream PR to the provider, but this should work and i can test a cluster once the upstream PR is merged. Its a pretty straight forward change, however, be aware that enabling the feature kills sections of needed code like node pools. Additionally it has some more cluster limitations

@cmcga1125
Copy link
Contributor Author

also, your make commands are 🔥

@comment-bot-dev
Copy link

comment-bot-dev commented Mar 16, 2021

Thanks for the PR! 🚀
Unfortunately it looks like some of our CI checks failed. See the Contributing Guide for details.

  • ⚠️check_generate_modules
    The modules need to be regenerated. Please run make_build.
Checking submodule's files generation
diff -r '--exclude=.terraform' '--exclude=.kitchen' '--exclude=.git' /workspace/modules/beta-autopilot-private-cluster/README.md /tmp/tmp.OYcRAC4kjr/workspace/modules/beta-autopilot-private-cluster/README.md
31d30
<   load_config_file       = false
94a94
> | enable\_l4\_ilb\_subsetting | Enable L4 ILB Subsetting on the cluster | `bool` | `false` | no |
158a159
> | cluster\_id | Cluster ID |
diff -r '--exclude=.terraform' '--exclude=.kitchen' '--exclude=.git' /workspace/modules/beta-autopilot-private-cluster/main.tf /tmp/tmp.OYcRAC4kjr/workspace/modules/beta-autopilot-private-cluster/main.tf
34a35,37
>   // ID of the cluster
>   cluster_id = google_container_cluster.primary.id
> 
diff -r '--exclude=.terraform' '--exclude=.kitchen' '--exclude=.git' /workspace/modules/beta-autopilot-private-cluster/outputs.tf /tmp/tmp.OYcRAC4kjr/workspace/modules/beta-autopilot-private-cluster/outputs.tf
18a19,23
> output "cluster_id" {
>   description = "Cluster ID"
>   value       = local.cluster_id
> }
> 
diff -r '--exclude=.terraform' '--exclude=.kitchen' '--exclude=.git' /workspace/modules/beta-autopilot-private-cluster/variables.tf /tmp/tmp.OYcRAC4kjr/workspace/modules/beta-autopilot-private-cluster/variables.tf
400a401,406
> variable "enable_l4_ilb_subsetting" {
>   type        = bool
>   description = "Enable L4 ILB Subsetting on the cluster"
>   default     = false
> }
> 
diff -r '--exclude=.terraform' '--exclude=.kitchen' '--exclude=.git' /workspace/modules/beta-autopilot-private-cluster/versions.tf /tmp/tmp.OYcRAC4kjr/workspace/modules/beta-autopilot-private-cluster/versions.tf
24c24
<       version = ">= 3.49.0, <4.0.0"
---
>       version = ">= 3.63.0, <4.0.0"
28c28
<       version = "~> 1.10, != 1.11.0"
---
>       version = "~> 2.0"
32c32
<     module_name = "blueprints/terraform/terraform-google-kubernetes-engine:beta-autopilot-private-cluster/v14.2.0"
---
>     module_name = "blueprints/terraform/terraform-google-kubernetes-engine:beta-autopilot-private-cluster/v14.3.0"
diff -r '--exclude=.terraform' '--exclude=.kitchen' '--exclude=.git' /workspace/modules/beta-autopilot-public-cluster/README.md /tmp/tmp.OYcRAC4kjr/workspace/modules/beta-autopilot-public-cluster/README.md
28d27
<   load_config_file       = false
87a87
> | enable\_l4\_ilb\_subsetting | Enable L4 ILB Subsetting on the cluster | `bool` | `false` | no |
147a148
> | cluster\_id | Cluster ID |
diff -r '--exclude=.terraform' '--exclude=.kitchen' '--exclude=.git' /workspace/modules/beta-autopilot-public-cluster/main.tf /tmp/tmp.OYcRAC4kjr/workspace/modules/beta-autopilot-public-cluster/main.tf
34a35,37
>   // ID of the cluster
>   cluster_id = google_container_cluster.primary.id
> 
diff -r '--exclude=.terraform' '--exclude=.kitchen' '--exclude=.git' /workspace/modules/beta-autopilot-public-cluster/outputs.tf /tmp/tmp.OYcRAC4kjr/workspace/modules/beta-autopilot-public-cluster/outputs.tf
18a19,23
> output "cluster_id" {
>   description = "Cluster ID"
>   value       = local.cluster_id
> }
> 
diff -r '--exclude=.terraform' '--exclude=.kitchen' '--exclude=.git' /workspace/modules/beta-autopilot-public-cluster/variables.tf /tmp/tmp.OYcRAC4kjr/workspace/modules/beta-autopilot-public-cluster/variables.tf
369a370,375
> variable "enable_l4_ilb_subsetting" {
>   type        = bool
>   description = "Enable L4 ILB Subsetting on the cluster"
>   default     = false
> }
> 
diff -r '--exclude=.terraform' '--exclude=.kitchen' '--exclude=.git' /workspace/modules/beta-autopilot-public-cluster/versions.tf /tmp/tmp.OYcRAC4kjr/workspace/modules/beta-autopilot-public-cluster/versions.tf
24c24
<       version = ">= 3.49.0, <4.0.0"
---
>       version = ">= 3.63.0, <4.0.0"
28c28
<       version = "~> 1.10, != 1.11.0"
---
>       version = "~> 2.0"
32c32
<     module_name = "blueprints/terraform/terraform-google-kubernetes-engine:beta-autopilot-public-cluster/v14.2.0"
---
>     module_name = "blueprints/terraform/terraform-google-kubernetes-engine:beta-autopilot-public-cluster/v14.3.0"
Error: submodule's files generation has not been run, please run the
'make build' command and commit changes
  • ⚠️check_terraform
    Failed Terraform check. More details below.
Attempting to download /workspace/test/bundle.hcl bundle.
/tmp/bundler /workspace
Fetching Terraform 0.13.6 core package...
Local plugin directory ".plugins" found; scanning for provider binaries.
No ".plugins" directory found, skipping local provider discovery.
Creating terraform_0.13.6-bundle2021091923_linux_amd64.zip ...
All done!
Archive:  terraform_0.13.6-bundle2021091923_linux_amd64.zip
/workspace
Running terraform fmt
Running terraform validate
terraform_validate . 
Success!
The configuration is valid.
terraform_validate ./examples/deploy_service 
Success!
The configuration is valid.
terraform_validate ./examples/disable_client_cert 
Success!
The configuration is valid.
terraform_validate ./examples/node_pool 
Success!
The configuration is valid.
terraform_validate ./examples/node_pool_update_variant 
Success!
The configuration is valid.
terraform_validate ./examples/node_pool_update_variant_beta 
Success!
The configuration is valid.
terraform_validate ./examples/node_pool_update_variant_public_beta 
Success!
The configuration is valid.
terraform_validate ./examples/private_zonal_with_networking 
Success!
The configuration is valid.
terraform_validate ./examples/regional_private_node_pool_oauth_scopes 
Success!
The configuration is valid.
terraform_validate ./examples/safer_cluster 
Success!
The configuration is valid.
terraform_validate ./examples/safer_cluster_iap_bastion 
Success!
The configuration is valid.
terraform_validate ./examples/shared_vpc 
Success!
The configuration is valid.
terraform_validate ./examples/simple_autopilot_private 
Error: 
Unsupported argument
 on main.tf line 49, in module "gke":
 49:   
enable_autopilot
      = var.enable_autopilot
An argument named "enable_autopilot" is not expected here.
terraform_validate ./examples/simple_autopilot_public 
Error: 
Unsupported argument
 on main.tf line 49, in module "gke":
 49:   
enable_autopilot
      = var.enable_autopilot
An argument named "enable_autopilot" is not expected here.
terraform_validate ./examples/simple_regional 
Success!
The configuration is valid.
terraform_validate ./examples/simple_regional_beta 
Success!
The configuration is valid.
terraform_validate ./examples/simple_regional_private 
Success!
The configuration is valid.
terraform_validate ./examples/simple_regional_private_beta 
Success!
The configuration is valid.
terraform_validate ./examples/simple_regional_with_kubeconfig 
Success!
The configuration is valid.
terraform_validate ./examples/simple_regional_with_networking 
Success!
The configuration is valid.
terraform_validate ./examples/simple_zonal_private 
Success!
The configuration is valid.
terraform_validate ./examples/simple_zonal_with_acm 
Success!
The configuration is valid.
terraform_validate ./examples/simple_zonal_with_asm 
Success!
The configuration is valid.
terraform_validate ./examples/simple_zonal_with_hub 
Success!
The configuration is valid.
terraform_validate ./examples/simple_zonal_with_hub_kubeconfig 
Success!
The configuration is valid.
terraform_validate ./examples/stub_domains 
Success!
The configuration is valid.
terraform_validate ./examples/stub_domains_private 
Success!
The configuration is valid.
terraform_validate ./examples/stub_domains_upstream_nameservers 
Success!
The configuration is valid.
terraform_validate ./examples/upstream_nameservers 
Success!
The configuration is valid.
terraform_validate ./examples/workload_identity 
Success!
The configuration is valid.
terraform_validate ./examples/workload_metadata_config 
Success!
The configuration is valid.
terraform_validate ./modules/acm 
Success!
The configuration is valid.
terraform_validate ./modules/asm 
Success!
The configuration is valid.
terraform_validate ./modules/auth 
Success!
The configuration is valid.
terraform_validate ./modules/beta-autopilot-private-cluster 
Success!
The configuration is valid.
terraform_validate ./modules/beta-autopilot-public-cluster 
Success!
The configuration is valid.
terraform_validate ./modules/beta-private-cluster 
Success!
The configuration is valid.
terraform_validate ./modules/beta-private-cluster-update-variant 
Success!
The configuration is valid.
terraform_validate ./modules/beta-public-cluster 
Success!
The configuration is valid.
terraform_validate ./modules/beta-public-cluster-update-variant 
Success!
The configuration is valid.
terraform_validate ./modules/binary-authorization 
Success!
The configuration is valid.
terraform_validate ./modules/config-sync 
Success!
The configuration is valid.
terraform_validate ./modules/hub 
Success!
The configuration is valid.
terraform_validate ./modules/k8s-operator-crd-support 
Success!
The configuration is valid.
terraform_validate ./modules/private-cluster 
Success!
The configuration is valid.
terraform_validate ./modules/private-cluster-update-variant 
Success!
The configuration is valid.
terraform_validate ./modules/safer-cluster 
Success!
The configuration is valid.
terraform_validate ./modules/safer-cluster-update-variant 
Success!
The configuration is valid.
terraform_validate ./modules/services 
Success!
The configuration is valid.
terraform_validate ./modules/workload-identity 
Success!
The configuration is valid.
terraform_validate ./test/fixtures/beta_cluster 
Success!
The configuration is valid.
terraform_validate ./test/fixtures/deploy_service 
Success!
The configuration is valid.
terraform_validate ./test/fixtures/disable_client_cert 
Success!
The configuration is valid.
terraform_validate ./test/fixtures/node_pool 
Success!
The configuration is valid.
terraform_validate ./test/fixtures/node_pool_update_variant 
Success!
The configuration is valid.
terraform_validate ./test/fixtures/private_zonal_with_networking 
Success!
The configuration is valid.
terraform_validate ./test/fixtures/safer_cluster 
Success!
The configuration is valid.
terraform_validate ./test/fixtures/safer_cluster_iap_bastion 
Success!
The configuration is valid.
terraform_validate ./test/fixtures/sandbox_enabled 
Success!
The configuration is valid.
terraform_validate ./test/fixtures/simple_autopilot_private 
Error: 
Unsupported argument
 on ../../../examples/simple_autopilot_private/main.tf line 49, in module "gke":
 49:   
enable_autopilot
      = var.enable_autopilot
An argument named "enable_autopilot" is not expected here.
terraform_validate ./test/fixtures/simple_autopilot_public 
Error: 
Unsupported argument
 on ../../../examples/simple_autopilot_public/main.tf line 49, in module "gke":
 49:   
enable_autopilot
      = var.enable_autopilot
An argument named "enable_autopilot" is not expected here.
terraform_validate ./test/fixtures/simple_regional 
Success!
The configuration is valid.
terraform_validate ./test/fixtures/simple_regional_private 
Success!
The configuration is valid.
terraform_validate ./test/fixtures/simple_regional_with_kubeconfig 
Success!
The configuration is valid.
terraform_validate ./test/fixtures/simple_regional_with_networking 
Success!
The configuration is valid.
terraform_validate ./test/fixtures/simple_zonal 
Success!
The configuration is valid.
terraform_validate ./test/fixtures/simple_zonal_private 
Success!
The configuration is valid.
terraform_validate ./test/fixtures/simple_zonal_with_asm 
Success!
The configuration is valid.
terraform_validate ./test/fixtures/stub_domains 
Success!
The configuration is valid.
terraform_validate ./test/fixtures/stub_domains_private 
Success!
The configuration is valid.
terraform_validate ./test/fixtures/stub_domains_upstream_nameservers 
Success!
The configuration is valid.
terraform_validate ./test/fixtures/upstream_nameservers 
Success!
The configuration is valid.
terraform_validate ./test/fixtures/workload_identity 
Success!
The configuration is valid.
terraform_validate ./test/fixtures/workload_metadata_config 
Success!
The configuration is valid.
terraform_validate ./test/setup 
Success!
The configuration is valid.

@cmcga1125
Copy link
Contributor Author

tried building an autopilot private cluster and got this - i'll wait until the upstream is merged and try again - it might be good to have an example as it has some caveats?

Error: Unsupported argument

  on .terraform/modules/gke/modules/beta-private-cluster-update-variant/cluster.tf line 96, in resource "google_container_cluster" "primary":
  96:   enable_autopilot            = var.enable_autopilot

An argument named "enable_autopilot" is not expected here.

@morgante
Copy link
Contributor

I'm wondering if autopilot clusters should actually be a distinct submodule, since they seem fairly different. Ex. node pool configuration isn't applicable.

@bharathkkb Thoughts?

@bharathkkb
Copy link
Member

@morgante I think autopilot should be a submodule, we can should be able to adapt the existing tmpl to support this.

@cmcga1125
Copy link
Contributor Author

cmcga1125 commented Mar 16, 2021

yep - was torn on if it should be separate, but the provider did not break from the standard google_container_cluster resource and from what i can tell you can just omit the node pool. I do get the whole make it separate argument. I can give it a go - looking for the configuration that splits out new modules 🙃

@morgante
Copy link
Contributor

morgante commented Mar 16, 2021

The modules config is here: https://github.com/terraform-google-modules/terraform-google-kubernetes-engine/blob/master/autogen_modules.json

I think it would be worthwhile to have submodules which suppressed the node pool configuration entirely so it's clear what is/isn't valid for Autopilot.

@cmcga1125
Copy link
Contributor Author

cmcga1125 commented Mar 17, 2021

i tried to mark the PR as wip or something but don't seem to have permissions 😃

Also, i think its in a decent place to test once the provider drops. i'll keep an eye out -- thanks for the opportunity, i've been wanting to commit to some public projects :)

oh yeah, sorry i'm a very iterative worker and push a lot - we can flatten it 🙃

@morgante morgante marked this pull request as draft March 17, 2021 05:15
@morgante
Copy link
Contributor

@cmcga1125 Thanks for working on it! I've gone ahead and converted your PR to a draft for now.

@cmcga1125
Copy link
Contributor Author

as an update - the provider is hoping to have this out march 29 2021

@cmcga1125
Copy link
Contributor Author

Successfully created a private cluster :)
I'll test a public cluster in a bit.

@cmcga1125
Copy link
Contributor Author

I've successfully built both cluster types with this module - but the kubernetes-engine-lint-trigger is failing. i can't see the test output to correct. Can someone provide?

@morgante
Copy link
Contributor

@cmcga1125 Are you still planning to work on this? We'd love to see autopilot support landed. :)

@cmcga1125
Copy link
Contributor Author

cmcga1125 commented May 18, 2021

hey! sorry i got very busy and am struggling to get the tests to run correctly. any guidance would be helpful :) I think the underlying TF is okay, just struggling with the tests/toolchain and lack of time the last few weeks.

@morgante
Copy link
Contributor

What issues are you having with the tests? This codelab might help to understand how they work.

@cmcga1125
Copy link
Contributor Author

i'll run through the codelab tonight and see if that helps.

@eana
Copy link
Contributor

eana commented Jun 3, 2021

@cmcga1125 Did you have any luck with the tests? It would be awesome to have autopilot support :)

@mirestrepo
Copy link

It would be so nice to see this merging. Seemed like you all were so close! 💪

@cmcga1125
Copy link
Contributor Author

Hey everyone, sorry i've gotten really busy this last month or so and havn't had extra time to look at this. If anyone wants to pick it up feel free. I hope to maybe look at it in the next few weeks.

I went through the codelab and it was helpful, just havn't had time to work on the tests.

@splieth
Copy link

splieth commented Jul 13, 2021

I wonder how the ip-mask-agent is supposed to work in autopilot mode. As far as I understood, one is not allowed to alter any resource in the kube-system namespace (according to the docs) and thus creating a config map there for the ip-mask-agent is IMHO not possible.

@github-actions
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 Sep 11, 2021
@github-actions github-actions bot closed this Sep 19, 2021
@morgante morgante reopened this Sep 19, 2021
@github-actions github-actions bot removed the Stale label Sep 20, 2021
@philippeboyd
Copy link

Any news on this?

@morgante
Copy link
Contributor

@philippeboyd We're still waiting for someone to pick it up from @cmcga1125. Feel free to take it over!

@github-actions
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 Nov 28, 2021
@github-actions github-actions bot closed this Dec 5, 2021
@odise
Copy link
Contributor

odise commented Dec 13, 2021

@morgante Would it be possible to agree on what's left to do on this PR so that it can be merged? I see some minors such as the node pool comment but also some bigger issues like var.zones to be handled. What else needs to be changed from your point of view?

@FearlessHyena
Copy link

Since a lot of work has gone into this PR maybe it would be better to reopen this?

@morgante morgante reopened this Dec 13, 2021
@morgante
Copy link
Contributor

@morgante Would it be possible to agree on what's left to do on this PR so that it can be merged? I see some minors such as the node pool comment but also some bigger issues like var.zones to be handled. What else needs to be changed from your point of view?

The last review's comments need to be addressed and someone will also have to resolve merge conflicts. I'd be very happy to take a look if someone is able to pick this up.

@odise
Copy link
Contributor

odise commented Dec 13, 2021

I'm interested to finish the PR and get it merged. However it would be extremely helpful to have a fast catch-up beforehand to get a feeling how much effort it will be. Especially because the complexity of this module is quite high and raising with the autopilot feature.

@github-actions github-actions bot removed the Stale label Dec 13, 2021
@cmcga1125
Copy link
Contributor Author

cmcga1125 commented Jan 28, 2022

I'm interested to finish the PR and get it merged. However it would be extremely helpful to have a fast catch-up beforehand to get a feeling how much effort it will be. Especially because the complexity of this module is quite high and raising with the autopilot feature.

i'd be happy to chat about it, but do not have the ability to run the testing tools

@jmymy
Copy link
Contributor

jmymy commented Feb 10, 2022

Spinning up the testing env now and will have a look... hopefully have something by tomorrow

@jmymy
Copy link
Contributor

jmymy commented Feb 10, 2022

Pull request with combined changes here -> #1148

thanks for the heavy lifting @cmcga1125 !

@jmymy
Copy link
Contributor

jmymy commented Mar 2, 2022

can close. merged in #1148

@morgante morgante closed this Mar 2, 2022
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