feat(TF>=1.1)!: Configure ASM management mode #1702

ferrarimarco · 2023-08-04T11:22:19Z

This PR implements the ability of configuring ASM management mode.

Without this, users have to manually configure the feature membership.

ferrarimarco · 2023-08-04T11:36:10Z

Ah! But we configure the CPR already, so this may not actually be needed...

ericyz · 2023-08-15T00:38:31Z

/gcbrun

ferrarimarco · 2023-08-16T07:35:47Z

Also, it looks like that the module.cpr and kubernetes_config_map.asm_options aren't necessary anymore because the fleet API takes care of that, and actually conflict with the Managed ASM control plane installation. Worth removing in this PR, or a separate one?

ferrarimarco · 2023-08-16T07:36:28Z

PS: the build failure doesn't seem to be related to this PR.

apeabody · 2023-09-13T04:46:36Z

/gcbrun

apeabody · 2023-09-26T20:16:54Z

/gcbrun

apeabody · 2023-09-27T00:00:47Z

Thanks for the pull request @ferrarimarco

From the linter

Checking submodule's files generation
diff -r '--exclude=.terraform' '--exclude=.kitchen' '--exclude=.git' /workspace/modules/asm/README.md /tmp/tmp.eM1y9hxVzy/workspace/modules/asm/README.md
62c62
< | enable\_vpc\_sc | Determines whether to enable VPC-SC for this ASM installation. For more information read <https://cloud.google.com/service-mesh/docs/managed/vpc-sc> | `bool` | `false` | no |
---
> | enable\_vpc\_sc | Determines whether to enable VPC-SC for this ASM installation. For more information read https://cloud.google.com/service-mesh/docs/managed/vpc-sc | `bool` | `false` | no |
65c65
< | mesh\_management | ASM management mode. For more information, see the [gke_hub_feature_membership resource documentation](https://registry.terraform.io/providers/hashicorp/google/latest/docs/resources/gke_hub_feature_membership#nested_mesh) | `string` | `""` | no |
---
> | mesh\_management | ASM Management mode. | `string` | `""` | no |
Error: submodule's files generation has not been run, please run the
'make build' command and commit changes

Checking for documentation generation
diff -r '--exclude=.terraform' '--exclude=.kitchen' '--exclude=autogen' '--exclude=*.tfvars' '--exclude=*metadata.yaml' /workspace/modules/asm/README.md /tmp/tmp.3rgofwWMtd/generate_docs/workspace/modules/asm/README.md
62c62
< | enable\_vpc\_sc | Determines whether to enable VPC-SC for this ASM installation. For more information read <https://cloud.google.com/service-mesh/docs/managed/vpc-sc> | `bool` | `false` | no |
---
> | enable\_vpc\_sc | Determines whether to enable VPC-SC for this ASM installation. For more information read https://cloud.google.com/service-mesh/docs/managed/vpc-sc | `bool` | `false` | no |
65c65
< | mesh\_management | ASM management mode. For more information, see the [gke_hub_feature_membership resource documentation](https://registry.terraform.io/providers/hashicorp/google/latest/docs/resources/gke_hub_feature_membership#nested_mesh) | `string` | `""` | no |
---
> | mesh\_management | ASM Management mode. | `string` | `""` | no |
Error: Documentation generation has not been run, please run the
'make docker_generate_docs' command and commit the above changes.

ferrarimarco · 2023-09-29T07:42:16Z

@apeabody thanks for the update. I'll look into that.

ferrarimarco · 2023-09-29T09:03:17Z

@apeabody hopefully fixed!

apeabody · 2023-09-29T15:50:18Z

@apeabody hopefully fixed!

Thanks @ferrarimarco - With the addition of count to the module, you will also need to add the index for the output value:

│ Error: Unsupported attribute
│ 
│   on ../../../modules/asm/outputs.tf line 23, in output "wait":
│   23:   value       = module.cpr.wait
│     ├────────────────
│     │ module.cpr is a list of object
│ 
│ Can't access attributes on a list of objects. Did you mean to access
│ attribute "wait" for a specific element of the list, or across all elements
│ of the list?

ferrarimarco · 2023-10-03T12:08:33Z

@apeabody thanks for your review.

I've modified the wait output variable initialization to fix the issue reported in #1702 (comment).

Also, the module now accordingly sets its value considering both MANAGEMENT_AUTOMATIC and MANAGEMENT_MANUAL (or unset management mode).

apeabody · 2023-10-03T15:04:46Z

/gcbrun

modules/asm/variables.tf

apeabody · 2023-10-04T15:21:38Z

/gcbrun

ferrarimarco · 2023-10-06T10:43:25Z

Build failures seem related to quota issues.

apeabody · 2023-10-06T15:41:30Z

/gcbrun

ferrarimarco · 2023-10-09T09:32:54Z

This failure seems to be related to an unsupported GKE version. I don't think it's related to this PR :)

apeabody · 2023-10-09T20:30:09Z

Also, it looks like that the module.cpr and kubernetes_config_map.asm_options aren't necessary anymore because the fleet API takes care of that, and actually conflict with the Managed ASM control plane installation. Worth removing in this PR, or a separate one?

Hi @ferrarimarco - Is there a recommend migration path? While reviewing this PR its clear that supporting both adds extra complexity. Ideally we would want to minimze wrapping imperative commands like in module.cpr.

ferrarimarco · 2023-10-10T07:40:16Z

Currently, Anthos Service Mesh supports the following installation methods:

Managed ASM: managed control plane that runs outside the cluster.
- using Fleet API: fully automated, recommended by ASM docs.
- using asmcli.
In-cluster ASM: control plan runs inside the cluster. Install using asmcli.

In all cases, there's no explicit creation of ControlPlaneRevision resources. Either the Fleet API does it, or asmcli takes care of creating it.

My opinion is that we should:

Remove the module.cpr resource because the Fleet API and asmcli take care of creating it.
Remove the kubernetes_namespace.system resource because the Fleet API and asmcli take care of creating it.
Clarify that we install the "managed ASM control plane" version using the Fleet API.
~~Refactor optional features according to Enable optional features on managed ASM. This probably requires dropping unsupported features, such as var.enable_cni.~~ See the update.

Installation methods that involve using asmcli will likely result in having to wrap calls to asmcli using null_resources, which are hard to maintain and to implement with the right triggers. That's why I suggest sticking with provisioning and configuring managed ASM using the Fleet API only.

I'm happy to talk about this over Meet if you prefer :)

/cc @jbrook

UPDATE: I think we should also drop the dependency from the Kubernetes provider because we wouldn't need to connect to individual clusters in the fleet and avoid configuring optional features at all Enable optional features on managed ASM. As pointed out by folks I aligned with, it may be unlikely that we want users to manage individual objects inside clusters with Terraform. We might want to defer optional features configuration to a later PR, provided that we see demand.

UPDATE 2: By dropping the dependency from the Kubernetes provider, we'd need to find an alternative way to check if the ControlPlaneRevision is there, which we use as a proxy to check if ASM mutating web hooks are in place.

apeabody · 2023-10-10T15:39:22Z

Currently, Anthos Service Mesh supports the following installation methods:

Managed ASM: managed control plane that runs outside the cluster.

using Fleet API: fully automated, recommended by ASM docs.

using asmcli.

In-cluster ASM: control plan runs inside the cluster. Install using asmcli.

In all cases, there's no explicit creation of ControlPlaneRevision resources. Either the Fleet API does it, or asmcli takes care of creating it.

My opinion is that we should:

Remove the module.cpr resource because the Fleet API and asmcli take care of creating it.

Remove the kubernetes_namespace.system resource because the Fleet API and asmcli take care of creating it.

Clarify that we install the "managed ASM control plane" version using the Fleet API.

Refactor optional features according to Enable optional features on managed ASM. This probably requires dropping unsupported features, such as var.enable_cni.

Installation methods that involve using asmcli will likely result in having to wrap calls to asmcli using null_resources, which are hard to maintain and to implement with the right triggers. That's why I suggest sticking with provisioning and configuring managed ASM using the Fleet API only.

I'm happy to talk about this over Meet if you prefer :)

/cc @jbrook

Thanks @ferrarimarco - Given the scale of the change, let me ping you directly.

apeabody · 2023-10-24T19:48:11Z

/gcbrun

apeabody · 2023-10-27T17:18:10Z

Currently, Anthos Service Mesh supports the following installation methods:

Managed ASM: managed control plane that runs outside the cluster.

using Fleet API: fully automated, recommended by ASM docs.

using asmcli.

In-cluster ASM: control plan runs inside the cluster. Install using asmcli.

In all cases, there's no explicit creation of ControlPlaneRevision resources. Either the Fleet API does it, or asmcli takes care of creating it.
My opinion is that we should:

Remove the module.cpr resource because the Fleet API and asmcli take care of creating it.

Remove the kubernetes_namespace.system resource because the Fleet API and asmcli take care of creating it.

Clarify that we install the "managed ASM control plane" version using the Fleet API.

Refactor optional features according to Enable optional features on managed ASM. This probably requires dropping unsupported features, such as var.enable_cni.

Installation methods that involve using asmcli will likely result in having to wrap calls to asmcli using null_resources, which are hard to maintain and to implement with the right triggers. That's why I suggest sticking with provisioning and configuring managed ASM using the Fleet API only.
I'm happy to talk about this over Meet if you prefer :)
/cc @jbrook

Thanks @ferrarimarco - Given the scale of the change, let me ping you directly.

Hi @ferrarimarco - Did we want to move forward with this change for now? Cheers!

apeabody · 2023-10-27T17:18:28Z

/gcbrun

ferrarimarco · 2023-10-30T10:28:05Z

Hi Andrew! I'll align internally and get back to you ASAP. Thanks for following up!

apeabody · 2023-11-10T15:58:29Z

/gcbrun

apeabody · 2023-11-10T21:06:15Z

/gcbrun

apeabody

Thanks @ferrarimarco!

modules/asm/hub.tf

modules/asm/main.tf

…es#1702) Co-authored-by: Andrew Peabody <[email protected]>

Configure ASM management mode

272b5d9

ferrarimarco requested review from Jberlinsky, ericyz and a team as code owners August 4, 2023 11:22

ferrarimarco changed the title ~~Configure ASM management mode~~ feat: Configure ASM management mode Aug 4, 2023

Merge branch 'master' into mesh-feature-configuration

07eb11c

Merge branch 'master' into mesh-feature-configuration

c415864

Don't create resources that are automatically managed

f298bfc

Merge branch 'master' into mesh-feature-configuration

4e0865a

ferrarimarco added 2 commits October 3, 2023 12:00

Don't set the wait output variable with auto management

4620333

Wait for ASM to be ready in auto mode

69e176c

ferrarimarco and others added 3 commits October 3, 2023 12:22

Fix depends

bf69b06

Fix depends

0f6f501

Merge branch 'master' into mesh-feature-configuration

a62e1c0

apeabody reviewed Oct 3, 2023

View reviewed changes

modules/asm/variables.tf Show resolved Hide resolved

ferrarimarco added 2 commits October 4, 2023 08:03

Allow null values

b5e10fb

Update min Terraform version to handle null values

da4df2b

apeabody changed the title ~~feat: Configure ASM management mode~~ feat(TF>=1.1)!: Configure ASM management mode Oct 4, 2023

Merge branch 'master' into mesh-feature-configuration

8e01e15

apeabody requested review from g-awmalik and removed request for Jberlinsky October 18, 2023 16:37

g-awmalik and others added 2 commits October 19, 2023 10:49

Merge branch 'master' into mesh-feature-configuration

60d787b

Merge branch 'master' into mesh-feature-configuration

5d9007a

Merge branch 'master' into mesh-feature-configuration

b0243a4

Merge branch 'master' into mesh-feature-configuration

578ce48

Merge branch 'master' into mesh-feature-configuration

6bdb793

apeabody approved these changes Nov 10, 2023

View reviewed changes

modules/asm/hub.tf Show resolved Hide resolved

modules/asm/main.tf Show resolved Hide resolved

modules/asm/main.tf Show resolved Hide resolved

apeabody merged commit a9de2d7 into terraform-google-modules:master Nov 10, 2023
4 checks passed

release-please bot mentioned this pull request Nov 10, 2023

chore(master): release 30.0.0 #1794

Merged

ferrarimarco deleted the mesh-feature-configuration branch November 13, 2023 09:44

michaellzc mentioned this pull request Nov 23, 2023

fix: avoid auto_provisioning_defaults drift #1806

Merged

legal90 mentioned this pull request Dec 28, 2023

fix: Add project ID to the fleet feature membership for ASM #1832

Merged

yorik mentioned this pull request May 14, 2024

Sync upstream matter-labs/terraform-google-kubernetes-engine#4

Closed

CPL-markus pushed a commit to WALTER-GROUP/terraform-google-kubernetes-engine that referenced this pull request Jul 15, 2024

feat(TF>=1.1)!: Configure ASM management mode (terraform-google-modul…

81a3a32

…es#1702) Co-authored-by: Andrew Peabody <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(TF>=1.1)!: Configure ASM management mode #1702

feat(TF>=1.1)!: Configure ASM management mode #1702

ferrarimarco commented Aug 4, 2023

ferrarimarco commented Aug 4, 2023

ericyz commented Aug 15, 2023

ferrarimarco commented Aug 16, 2023

ferrarimarco commented Aug 16, 2023

apeabody commented Sep 13, 2023

apeabody commented Sep 26, 2023

apeabody commented Sep 27, 2023

ferrarimarco commented Sep 29, 2023

ferrarimarco commented Sep 29, 2023

apeabody commented Sep 29, 2023

ferrarimarco commented Oct 3, 2023

apeabody commented Oct 3, 2023

apeabody commented Oct 4, 2023

ferrarimarco commented Oct 6, 2023

apeabody commented Oct 6, 2023

ferrarimarco commented Oct 9, 2023 •

edited

Loading

apeabody commented Oct 9, 2023

ferrarimarco commented Oct 10, 2023 •

edited

Loading

apeabody commented Oct 10, 2023

apeabody commented Oct 24, 2023

apeabody commented Oct 27, 2023

apeabody commented Oct 27, 2023

ferrarimarco commented Oct 30, 2023 •

edited

Loading

apeabody commented Nov 10, 2023

apeabody commented Nov 10, 2023

apeabody left a comment

feat(TF>=1.1)!: Configure ASM management mode #1702

feat(TF>=1.1)!: Configure ASM management mode #1702

Conversation

ferrarimarco commented Aug 4, 2023

ferrarimarco commented Aug 4, 2023

ericyz commented Aug 15, 2023

ferrarimarco commented Aug 16, 2023

ferrarimarco commented Aug 16, 2023

apeabody commented Sep 13, 2023

apeabody commented Sep 26, 2023

apeabody commented Sep 27, 2023

ferrarimarco commented Sep 29, 2023

ferrarimarco commented Sep 29, 2023

apeabody commented Sep 29, 2023

ferrarimarco commented Oct 3, 2023

apeabody commented Oct 3, 2023

apeabody commented Oct 4, 2023

ferrarimarco commented Oct 6, 2023

apeabody commented Oct 6, 2023

ferrarimarco commented Oct 9, 2023 • edited Loading

apeabody commented Oct 9, 2023

ferrarimarco commented Oct 10, 2023 • edited Loading

apeabody commented Oct 10, 2023

apeabody commented Oct 24, 2023

apeabody commented Oct 27, 2023

apeabody commented Oct 27, 2023

ferrarimarco commented Oct 30, 2023 • edited Loading

apeabody commented Nov 10, 2023

apeabody commented Nov 10, 2023

apeabody left a comment

Choose a reason for hiding this comment

ferrarimarco commented Oct 9, 2023 •

edited

Loading

ferrarimarco commented Oct 10, 2023 •

edited

Loading

ferrarimarco commented Oct 30, 2023 •

edited

Loading