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: GKE autopilot support #1148

Merged
merged 34 commits into from
Mar 2, 2022

Conversation

jmymy
Copy link
Contributor

@jmymy jmymy commented Feb 10, 2022

No description provided.

@comment-bot-dev
Copy link

comment-bot-dev commented Feb 10, 2022

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

@jmymy jmymy mentioned this pull request Feb 10, 2022
@jmymy jmymy marked this pull request as ready for review February 15, 2022 01:18
@jmymy jmymy requested review from a team, Jberlinsky and bharathkkb as code owners February 15, 2022 01:18
@jmymy
Copy link
Contributor Author

jmymy commented Feb 15, 2022

@cmcga1125 can you sign the CLA? If you want credit on the PR. Otherwise I'll have to remove your commits from the PR.

@jmymy
Copy link
Contributor Author

jmymy commented Feb 15, 2022

@bharathkkb any insight on what kitchen test is failing?

@bharathkkb
Copy link
Member

@jmymy It may have been a flake since it was for a different test verify beta-cluster-local

     ×  Command: `gcloud beta --project=PROJECT_ID container clusters --zone=us-central1 describe simple-regional-beta-cluster-i8nk --format=json` cluster has the expected identityServiceConfig config
     
     expected: {"enabled"=>true}
          got: nil
     
     (compared using ==)

@jmymy
Copy link
Contributor Author

jmymy commented Feb 15, 2022

@jmymy It may have been a flake since it was for a different test verify beta-cluster-local

     ×  Command: `gcloud beta --project=PROJECT_ID container clusters --zone=us-central1 describe simple-regional-beta-cluster-i8nk --format=json` cluster has the expected identityServiceConfig config
     
     expected: {"enabled"=>true}
          got: nil
     
     (compared using ==)

nope that one was me. I found an issue with the merge/rebase and the new identity config.. Should be fixed.

@jmymy
Copy link
Contributor Author

jmymy commented Feb 17, 2022

@bharathkkb next steps? I figured i'd give @cmcga1125 more time while ya'll review the PR.

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 working on this @jmymy
/cc @kaariger since you worked on the KRM autopilot blueprint

examples/simple_autopilot_private/README.md Outdated Show resolved Hide resolved
examples/simple_autopilot_private/README.md Outdated Show resolved Hide resolved
examples/simple_autopilot_private/variables.tf Outdated Show resolved Hide resolved
examples/simple_autopilot_private/variables.tf Outdated Show resolved Hide resolved
examples/simple_autopilot_private/variables.tf Outdated Show resolved Hide resolved
modules/beta-autopilot-public-cluster/versions.tf Outdated Show resolved Hide resolved
examples/simple_autopilot_private/main.tf Outdated Show resolved Hide resolved
examples/simple_autopilot_private/main.tf Outdated Show resolved Hide resolved
examples/simple_autopilot_private/outputs.tf Outdated Show resolved Hide resolved
jmymy and others added 4 commits February 18, 2022 09:59
Co-authored-by: Bharath KKB <bharathkrishnakb@gmail.com>
Co-authored-by: Bharath KKB <bharathkrishnakb@gmail.com>
Co-authored-by: Bharath KKB <bharathkrishnakb@gmail.com>
Co-authored-by: Bharath KKB <bharathkrishnakb@gmail.com>
@cmcga1125
Copy link
Contributor

cmcga1125 commented Feb 21, 2022

@cmcga1125 can you sign the CLA? If you want credit on the PR. Otherwise I'll have to remove your commits from the PR.

CLA submitted! Thanks for taking it across the finish line! Sorry I didn't have enough permissions to test on my account 🙃

@jmymy
Copy link
Contributor Author

jmymy commented Feb 23, 2022

@bharathkkb how do y'all normally handle the version file in ./test/setup ? its causing the error above.

@bharathkkb
Copy link
Member

@bharathkkb
Copy link
Member

Opened #1159 since it was affecting a few PRs

@jmymy
Copy link
Contributor Author

jmymy commented Mar 1, 2022

@bharathkkb next steps? Trying to not let this get stale

@morgante
Copy link
Contributor

morgante commented Mar 1, 2022

Tests are failing. Please fix integration tests:

$$$$$$ Selecting the kitchen-terraform-simple-autopilot-private-local Terraform workspace...
$$$$$$ Finished selecting the kitchen-terraform-simple-autopilot-private-local Terraform workspace.
$$$$$$ Downloading the modules needed for the Terraform configuration...
       - example in ../../../examples/simple_autopilot_private
       Downloading terraform-google-modules/network/google 4.1.0 for example.gcp-network...
       - example.gcp-network in .terraform/modules/example.gcp-network
       - example.gcp-network.firewall_rules in .terraform/modules/example.gcp-network/modules/firewall-rules
       - example.gcp-network.routes in .terraform/modules/example.gcp-network/modules/routes
       - example.gcp-network.subnets in .terraform/modules/example.gcp-network/modules/subnets
       - example.gcp-network.vpc in .terraform/modules/example.gcp-network/modules/vpc
       - example.gke in ../../../modules/beta-autopilot-private-cluster
       Downloading terraform-google-modules/gcloud/google 3.1.0 for example.gke.gcloud_delete_default_kube_dns_configmap...
       - example.gke.gcloud_delete_default_kube_dns_configmap in .terraform/modules/example.gke.gcloud_delete_default_kube_dns_configmap/modules/kubectl-wrapper
       - example.gke.gcloud_delete_default_kube_dns_configmap.gcloud_kubectl in .terraform/modules/example.gke.gcloud_delete_default_kube_dns_configmap
$$$$$$ Finished downloading the modules needed for the Terraform configuration.
$$$$$$ Validating the Terraform configuration files...
       Success! The configuration is valid.
       
$$$$$$ Finished validating the Terraform configuration files.
$$$$$$ Building the infrastructure based on the Terraform configuration...
       
       Error: Invalid for_each argument
       
         on .terraform/modules/example.gcp-network/modules/subnets/main.tf line 29, in resource "google_compute_subnetwork" "subnetwork":
         29:   for_each                 = local.subnets
           ├────────────────
           │ local.subnets will be known only after apply
       
       The "for_each" value depends on resource attributes that cannot be determined
       until apply, so Terraform cannot predict how many instances will be created.
       To work around this, use the -target argument to first apply only the

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.

Unrelated tests are failing due to flakes

@bharathkkb bharathkkb merged commit d5ceafb into terraform-google-modules:master Mar 2, 2022
@jmymy jmymy deleted the auto-pilot branch March 2, 2022 19:25
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

5 participants