feat!: support for gateway api #1510

tuunit · 2022-12-26T10:49:00Z

Motivation and Context

As the Gateway API is now GA and part of the Terraform Google Provider (GoogleCloudPlatform/magic-modules#6875) release with version 4.47.0 (https://github.com/hashicorp/terraform-provider-google/releases/tag/v4.47.0).
Therefore I added full support for setting the gateway api channel to this module as well.

Resolves: #1462

tuunit · 2022-12-27T11:04:00Z

@bharathkkb as mentioned in the issue my change to the terraform provider is now merged and released therefore I was able to add the gateway api to this module as well.

@Jberlinsky I'm not sure completely sure about the kitchen tests scope. I just copied the "simple_regional" kitchen test files and adapted them to include the gateway api channel attribute. What I'm not sure about is if it is really necessary to keep all the attribute checks and variables from the "simple_regional" test fixture and example.

ericyz · 2022-12-28T01:10:18Z

Hi @tuunit , thanks for the contribution. Minor comments:

Could you please also update google provider version in versions.tf.tmpl to over 4.47.0?
Could you please add the integration test in build/int.cloudbuild.yaml ?

tuunit · 2022-12-28T08:33:43Z

@tuunit 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/README.md /tmp/tmp.77hqr2udQ3/workspace/README.md
304,305c304,305
< - [Terraform](https://www.terraform.io/downloads.html) 0.13+
< - [Terraform Provider for GCP][terraform-provider-google] v4.47
---
> - [Terraform](https://www.terraform.io/downloads.html) 0.12
> - [Terraform Provider for GCP][terraform-provider-google] v3.41
Error: submodule's files generation has not been run, please run the
'make build' command and commit changes

Never mind, the bot updated it's state and I had to push another fix anyway. The kitchen / cloudbuild integration and testing and how to add new test suites is not really clear. Maybe we could extend the documentation regarding adding test coverage for new features like in this PR.

@bharathkkb @Jberlinsky I don't think this is correct as I already ran make build. It might be a raise condition with two lint checks as I pushed two commits quite rapidly after another. Can you please re-run the lint check to make sure we don't have a problem 😄

tuunit · 2022-12-28T13:01:06Z

Unfortunately, I cannot access the cloud build details to check the test failure myself.

ericyz · 2022-12-28T23:26:27Z

Hi @tuunit , the failure was not relevant to your chance. I'll try to fix it and re-trigger the build after.

test/fixtures/simple_regional_with_gateway_api/example.tf

ericyz · 2022-12-30T00:58:11Z

Minor comments, but LGTM after resolving the conflicts

ericyz · 2022-12-30T00:55:39Z

examples/simple_regional_with_gateway_api/main.tf

+ */
+
+locals {
+  cluster_type = "simple-regional"


Please update the cluster type accordingly

simple-regional -> simple-regional-with-gatewayapi

Done. Can you shortly explain to me why this is necessary? Or is it just to keep everything aligned with some kind of naming convention?

Your understanding is right. In this way, it's easier to identify the testing clusters by its name.

But I guess this now leads to an issue in the integration check as the cluster_name attribute is wrongly set?
https://github.com/terraform-google-modules/terraform-google-kubernetes-engine/pull/1510/files#diff-0e66d26ba66e1418a2d27bff7f636be5977955a71c6222b5e2f9f6f586d21180R15-R25

https://github.com/terraform-google-modules/terraform-google-kubernetes-engine/pull/1510/files#diff-ab53d5a46fde1a871e35ca555d6f5dc58fcac8fa7cddd9c1fa188c125ce672d8R17

It was the length of the cluster name exceeding the limit and it should be good now.

tuunit · 2022-12-30T05:30:35Z

@ericyz Thanks a million for fixing the issue. Now I understand what the problem was! It is really unfortunate that normal contributors cannot view the issues of the integration pipeline.

comment-bot-dev · 2022-12-30T06:42:45Z

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

ericyz

LGTM. FYI - @bharathkkb

tuunit · 2023-01-03T13:13:30Z

@Jberlinsky ready for final review 😄

Happy New Year 🥳

tuunit requested review from a team and Jberlinsky as code owners December 26, 2022 10:49

tuunit mentioned this pull request Dec 26, 2022

Gateway Controller is GA #1462

Closed

tuunit force-pushed the gateway-api branch from cba6023 to 4a97f90 Compare December 26, 2022 12:49

tuunit changed the title ~~wip: feat: support for gateway api~~ feat: support for gateway api Dec 26, 2022

tuunit force-pushed the gateway-api branch from c4cdd85 to f52e95c Compare December 27, 2022 10:37

tuunit force-pushed the gateway-api branch 2 times, most recently from 91cf156 to ad52e25 Compare December 28, 2022 08:23

ericyz reviewed Dec 29, 2022

View reviewed changes

test/fixtures/simple_regional_with_gateway_api/example.tf Outdated Show resolved Hide resolved

tuunit force-pushed the gateway-api branch from 50ad504 to 5f5f36c Compare December 29, 2022 09:49

ericyz force-pushed the gateway-api branch 2 times, most recently from 381cc1a to 5f5f36c Compare December 30, 2022 00:51

ericyz reviewed Dec 30, 2022

View reviewed changes

ericyz force-pushed the gateway-api branch from 2c8d2db to acb0c21 Compare December 30, 2022 01:28

tuunit and others added 8 commits December 30, 2022 05:39

add support for gateway api

fb65b74

add example and test fixture for gateway api

8c5b0b8

finalize test integration for gateway api

1a2bc58

add gateway kitchen execution to cloud build

1353122

increase terraform gcp provider dependency version to 4.47

ef9f8f0

add backend name to gateway api kitchen execution

77215be

use a different project id for testing the gateway api

c88e19c

fix: update project id for network and project

01530c0

tuunit force-pushed the gateway-api branch from acb0c21 to 01530c0 Compare December 30, 2022 04:41

ericyz self-assigned this Dec 30, 2022

tuunit added 2 commits December 30, 2022 07:03

change to custom gateway cluster type

ced1863

change to custom gateway cluster type

7719fab

tuunit force-pushed the gateway-api branch from ced1863 to 7719fab Compare December 30, 2022 06:24

ericyz added 2 commits December 30, 2022 16:28

fix: reduce cluster name within limit

0fbaf17

fix: reduce cluster name within limit

a4e2da5

ericyz approved these changes Dec 30, 2022

View reviewed changes

bharathkkb approved these changes Jan 3, 2023

View reviewed changes

bharathkkb changed the title ~~feat: support for gateway api~~ feat!: support for gateway api Jan 3, 2023

bharathkkb merged commit 4181276 into terraform-google-modules:master Jan 3, 2023

release-please bot mentioned this pull request Jan 3, 2023

chore(master): release 25.0.0 #1512

Merged

lauraseidler mentioned this pull request Jan 6, 2023

feat!: support for gateway api for safer cluster variants #1523

Merged

michaellzc mentioned this pull request Nov 23, 2023

fix: avoid auto_provisioning_defaults drift #1806

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat!: support for gateway api #1510

feat!: support for gateway api #1510

tuunit commented Dec 26, 2022 •

edited

tuunit commented Dec 27, 2022

ericyz commented Dec 28, 2022 •

edited

tuunit commented Dec 28, 2022 •

edited

tuunit commented Dec 28, 2022

ericyz commented Dec 28, 2022

ericyz commented Dec 30, 2022

ericyz Dec 30, 2022

ericyz Dec 30, 2022

tuunit Dec 30, 2022

ericyz Dec 30, 2022

tuunit Dec 30, 2022

ericyz Dec 30, 2022 •

edited

tuunit commented Dec 30, 2022

comment-bot-dev commented Dec 30, 2022

ericyz left a comment •

edited

tuunit commented Jan 3, 2023 •

edited

feat!: support for gateway api #1510

feat!: support for gateway api #1510

Conversation

tuunit commented Dec 26, 2022 • edited

Motivation and Context

tuunit commented Dec 27, 2022

ericyz commented Dec 28, 2022 • edited

tuunit commented Dec 28, 2022 • edited

tuunit commented Dec 28, 2022

ericyz commented Dec 28, 2022

ericyz commented Dec 30, 2022

ericyz Dec 30, 2022

Choose a reason for hiding this comment

ericyz Dec 30, 2022

Choose a reason for hiding this comment

tuunit Dec 30, 2022

Choose a reason for hiding this comment

ericyz Dec 30, 2022

Choose a reason for hiding this comment

tuunit Dec 30, 2022

Choose a reason for hiding this comment

ericyz Dec 30, 2022 • edited

Choose a reason for hiding this comment

tuunit commented Dec 30, 2022

comment-bot-dev commented Dec 30, 2022

ericyz left a comment • edited

Choose a reason for hiding this comment

tuunit commented Jan 3, 2023 • edited

tuunit commented Dec 26, 2022 •

edited

ericyz commented Dec 28, 2022 •

edited

tuunit commented Dec 28, 2022 •

edited

ericyz Dec 30, 2022 •

edited

ericyz left a comment •

edited

tuunit commented Jan 3, 2023 •

edited