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.

Sign up for GitHub

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

Jump to bottom

helm mode: autodetect service parameters for clustermesh #1642

Merged
merged 1 commit into from
May 23, 2023
Merged

helm mode: autodetect service parameters for clustermesh #1642

merged 1 commit into from
May 23, 2023

Conversation

asauber
Copy link
Member

@asauber asauber commented May 18, 2023 •
edited

Classic mode uses the Kubernetes flavor to automatically set the clustermesh-apiserver Service type and LoadBalancer annotations if needed. This patch reimplements this logic for Helm mode.

Fixes #1621

@asauber asauber temporarily deployed to ci May 18, 2023 12:29 — with GitHub Actions Inactive
@asauber asauber mentioned this pull request May 18, 2023
Closed
@asauber asauber marked this pull request as ready for review May 18, 2023 12:56
@asauber asauber requested review from a team as code owners May 18, 2023 12:56
@asauber asauber force-pushed the pr/asauber/clustermesh-annotations branch from d264c80 to f235e3d Compare May 18, 2023 13:37
@asauber asauber requested a review from a team as a code owner May 18, 2023 13:38
@asauber asauber requested a review from nbusseneau May 18, 2023 13:38
@asauber asauber temporarily deployed to ci May 18, 2023 13:39 — with GitHub Actions Inactive
@asauber asauber force-pushed the pr/asauber/clustermesh-annotations branch from 9ffcb76 to e8cc0b6 Compare May 18, 2023 13:46
@asauber asauber force-pushed the pr/asauber/clustermesh-annotations branch from e8cc0b6 to bf22c93 Compare May 18, 2023 13:47
@asauber asauber temporarily deployed to ci May 18, 2023 13:47 — with GitHub Actions Inactive
@asauber
Copy link
Member Author

asauber commented May 18, 2023

This workflow failure is a bug with this PR which I'm fixing locally.

2023-05-18T14:02:20.080443664Z Error: Unable to enable ClusterMesh: YAML parse error on cilium/templates/clustermesh-apiserver/service.yaml: error unmarshaling JSON: while decoding JSON: json: cannot unmarshal object into Go struct field .metadata.annotations of type string

@asauber asauber force-pushed the pr/asauber/clustermesh-annotations branch from bf22c93 to da7edf4 Compare May 19, 2023 00:15
@asauber asauber temporarily deployed to ci May 19, 2023 00:15 — with GitHub Actions Inactive
@asauber asauber force-pushed the pr/asauber/clustermesh-annotations branch from da7edf4 to 078c50f Compare May 19, 2023 14:55
@asauber asauber temporarily deployed to ci May 19, 2023 14:55 — with GitHub Actions Inactive
@asauber
Copy link
Member Author

asauber commented May 19, 2023

Successful run of Helm mode with this logic.

https://github.com/cilium/cilium-cli/actions/runs/5025536421/jobs/9012647174?pr=1642

2023-05-19T15:12:04.356141413Z + kubectl --context gke_***_us-west2-a_cilium-cilium-cli-5025536421-helm-1 get svc -n kube-system clustermesh-apiserver -o 'jsonpath={.metadata.annotations}'
2023-05-19T15:12:04.508224818Z + printf 'Service annotations for Cluster 1 %s\n' '{"cloud.google.com/load-balancer-type":"Internal","cloud.google.com/neg":"{\"ingress\":true}","meta.helm.sh/release-name":"cilium","meta.helm.sh/release-namespace":"kube-system","networking.gke.io/internal-load-balancer-allow-global-access":"true"}'
2023-05-19T15:12:04.508304633Z Service annotations for Cluster 1 {"cloud.google.com/load-balancer-type":"Internal","cloud.google.com/neg":"{\"ingress\":true}","meta.helm.sh/release-name":"cilium","meta.helm.sh/release-namespace":"kube-system","networking.gke.io/internal-load-balancer-allow-global-access":"true"}
2023-05-19T15:12:04.509240384Z + kubectl --context gke_***_us-west2-a_cilium-cilium-cli-5025536421-helm-2 get svc -n kube-system clustermesh-apiserver -o 'jsonpath={.metadata.annotations}'
2023-05-19T15:12:04.997769100Z + printf 'Service annotations for Cluster 2 %s\n' '{"cloud.google.com/load-balancer-type":"Internal","cloud.google.com/neg":"{\"ingress\":true}","meta.helm.sh/release-name":"cilium","meta.helm.sh/release-namespace":"kube-system","networking.gke.io/internal-load-balancer-allow-global-access":"true"}'
2023-05-19T15:12:04.997849526Z Service annotations for Cluster 2 {"cloud.google.com/load-balancer-type":"Internal","cloud.google.com/neg":"{\"ingress\":true}","meta.helm.sh/release-name":"cilium","meta.helm.sh/release-namespace":"kube-system","networking.gke.io/internal-load-balancer-allow-global-access":"true"}

Removing the Do Not Merge commit now which activates the test.

@asauber asauber force-pushed the pr/asauber/clustermesh-annotations branch from 078c50f to 59c2f32 Compare May 19, 2023 15:31
@asauber asauber temporarily deployed to ci May 19, 2023 15:31 — with GitHub Actions Inactive
giorio94
giorio94 approved these changes May 22, 2023
View reviewed changes
Copy link
Member

@giorio94 giorio94 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@michi-covalent
Copy link
Contributor

rebase it and ship it

@asauber
helm mode: autodetect service parameters for clustermesh
77cb128 
Classic mode uses the Kubernetes flavor to automatically set the
clustermesh-apiserver Service type and LoadBalancer annotations if
needed. This patch reimplements this logic for Helm mode.

Signed-off-by: Andrew Sauber <andrew.sauber@isovalent.com>
@asauber asauber force-pushed the pr/asauber/clustermesh-annotations branch from 59c2f32 to 77cb128 Compare May 22, 2023 23:38
@asauber asauber temporarily deployed to ci May 22, 2023 23:38 — with GitHub Actions Inactive
@aanm aanm merged commit 00c8c68 into main May 23, 2023
19 checks passed
@aanm aanm deleted the pr/asauber/clustermesh-annotations branch May 23, 2023 14:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nathanjsweet nathanjsweet nathanjsweet approved these changes

@christarazi christarazi christarazi approved these changes

@nbusseneau nbusseneau nbusseneau approved these changes

@giorio94 giorio94 giorio94 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Restore automatic LoadBalancer annotations for clustermesh enable
7 participants
@asauber @michi-covalent @nathanjsweet @christarazi @nbusseneau @giorio94 @aanm