Introduce Istio/ASM Kustomize component

[gbrayut]

Background

The current service mesh instructions only include the OSS Istio installation instructions and does not mention the preferred approach for GKE, which is ASM via Fleet API. The instruction also are not using Kustomize components, which is now the preferred approach for demo variants.

Fixes

Fixes #1203

Change Summary

1. Move istio-manifests to kustomize/components/service-mesh-istio
2. Create test for Istio kustomize component
3. Update hack/make-release-artifacts.sh and kustomize/kustomization.yaml
4. Move instructions into the README on kustomize component

Additional Notes

Remaining Tasks for this PR:

• update kustomize/components/service-mesh-istio/README.md
• update microservices-demo README.md and /docs/cloudshell-tutorial.md
• incorporate any PR feedback

Testing Procedure

Install Anthos Service Mesh or Istio OSS, setup an Istio Ingress Gateway, and make sure sidecar injection is enabled in target namespace.

Then from the kustomize/ folder at the root level of this repository:

kustomize edit add component components/service-accounts
kustomize edit add component components/service-mesh-istio
kubectl apply -k .

[gbrayut]

@NimJay I finished the first draft of the new README (see rendered version here) and this should now be ready for review.

I forgot that changing from istio-ingressgateway to Kubernetes Gateway API also means changing from Istio CRDs to the Gateway CRDs. Those are still "The Future" of ASM/Istio, but may be more disruptive if other demos still rely on the Istio way of configuring things.

But it does eliminate a lot of steps like having to deploy the istio-ingress namespace/deployment/service resources. Take a look at the frontend-gateway.yaml and if you think that is too much change right now I can try separating the Gateway API stuff into another component.

[NimJay]

Will Review Soon

Just so you're not left in the dark: I plan on reviewing this tomorrow.
Overall (based on my quick skim), this looks amazing!

[gbrayut]

No worries, take all the time you need. Also, the more I think about it, the more I feel it probably is not best to fully replace the Istio classic resources (Istio Gateway and VirtualService CRDs) with the new Kubernetes Gateway and HTTPRoute method of configuring the service mesh. I think a better approach would be keeping the service-mesh-istio component as it currently stands and then:

• add a new standalone service-mesh-istio-classic component that uses the previous Istio CRDs so anyone using those resources can use that component instead
• switch the hack script to use the classic component for rendering the release/istio-manifest.yaml file
• optionally could create a new istio-httproute-manifest for the other component, but since using kustomize is preferred I'm fine with omitting this and only having the static manifest for istio classic

The classic CRDs won't ever go away, and if adoption of the new gateway/HTTPRoute picks up and becomes preferred the static manifest could be updated. But removing those entirely right now is probably not great.

Happy to add that to this PR... let me know your thoughts.

[NimJay]

Thanks again, @gbrayut, for creating this pull-request!
Things are looking good.
I've left a few comments.

Kubernetes Gateway API

You mentioned

"I feel it probably is not best to fully replace the Istio classic resources (Istio Gateway and VirtualService CRDs) with the new Kubernetes Gateway and HTTPRoute"

I think Online Boutique should embrace the new "Kubernetes Gateway and HTTPRoute" way of deploying Istio resources. Reasons:

• Istio intends to make it the default API for traffic management [source].
• I want to avoid having to maintain multiple manifests doing the same thing.
• I worry it could confuse users. Online Boutique should pick a single approach and imply "this is how it should be done".

Related note:

• I eventually want to delete the /release/istio-manifests.yaml file.

[NimJay]

Praise: Thanks for thoroughly updating the surrounding documentation!

[NimJay]

Praise: Thanks for pro-actively adding this test.

[NimJay]

Question:
Should we remove the

# - components/native-grpc-health-check
# - components/without-loadgenerator

lines at the bottom (since we've just moved it up)?

Also, thanks for catching this!

[gbrayut]

I removed the duplicate without-loadgenerator and confirmed they work in the listed order. native-grpc-health-check was not duplicated and I think it needs to be at the end since it also modifies the image tags. But I could be wrong there.

[NimJay]

"confirmed they work in the listed order"

Thank you for being so thorough!

"native-grpc-health-check ... needs to be at the end since it also modifies the image tags"

Ah, that's a good thought!
If I had to guess the order, it would be:

# - components/container-images-tag
# - components/native-grpc-health-check
# - components/container-images-tag-suffix
# - components/container-images-registry

Reason for my guess: Both native-grpc-health-check and container-images-tag use the newTag field, so they should be in the same position.

But I doubt anyone using native-grpc-health-check would use any of: container-images-tags, container-images-tag-suffix, or container-images-registry. So let's let it be. :)

[NimJay]

Nitpick: For readability and consistency:

Suggested change

	mesh.cloud.google.com/proxy='{"managed":"false"}'
	mesh.cloud.google.com/proxy='{"managed":"false"}'

[NimJay]

Nitpick: For readability and consistency:

Suggested change

	--allow tcp:10250,tcp:443,tcp:15017
	--allow tcp:10250,tcp:443,tcp:15017

[NimJay]

Issue:
Please bring back these files:

• istio-manifests/frontend.yaml
• istio-manifests/frontend-gateway.yaml
• istio-manifests/allow-egress-googleapis.yaml

I realized that it's currently being used in https://github.com/GoogleCloudPlatform/cloud-ops-sandbox/blob/milestone/0.9/provisioning/kustomize/online-boutique/no-loadgenerator/with-ingress/kustomization.yaml.
I will eventually delete them from Online Boutique once I fix GoogleCloudPlatform/cloud-ops-sandbox#1012.

You made a good point about preserving git history (via git mv).
But that's okay — we can live without the git history attached to the new files.
Hopefully, anyone investigating the git history will come across this pull-request. :)

[NimJay]

Suggestion (feel free to ignore):
Let's remove this "Deploy via istio-manifests.yaml".
Ideally, we would stop supporting this method — for ease of maintenance.

[gbrayut]

Branch updated with requested changes and should be ready for final review. I assume you'll handle the "out-of-date with base" and/or do a squash commit. Also it looks like your gmail (which gets used for the recommended changes) doesn't pass the CLA check, so I had to force-push that commit out of the branch.

[NimJay]

Thank you for addressing my comments so quickly, @gbrayut!
Everything looks good.
You did an incredible job here. 🤯
Approved!

CLA issue with my personal email

Ah, yes, I noticed that my suggestion's commits use my personal email.
I'll look into fixing this issue (either force GitHub to use my Google email for suggestion commits or CLA-approve my personal email).
(Tip: we can also use go/gh-prinfo to override the CLA check.)

[NimJay]

FYI: While merging conflicts, I put components/alloydb under components/service-accounts. This is intentional. components/alloydb modifies the cartservice ServiceAccount — so it needs to come after (to avoid Kustomize errors). :)