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

helm mode: add additional deprecated secret logic, and fix clustermesh connect for helm mode #1551

Merged
merged 1 commit into from May 8, 2023

Conversation

asauber
Copy link
Member

@asauber asauber commented Apr 27, 2023

Motivation

Previously, we had logic to look for deprecated names like clustermesh-apiserver-client-certs when clustermesh-apiserver-client-cert was expected. With helm-based installations, we now need to look for clustermesh-apiserver-client-cert when clustermesh-apiserver-remote-cert is expected.

Implementation

This implements that concpet recursively, looking for the "even more deprecated" name for secrets if applicable.

Example Runs

Two example runs that exercise the logic:

Using one level of deprecated names to find the secret

✅ ClusterMesh enabled!
Trying to get secret clustermesh-apiserver-remote-cert by deprecated name clustermesh-apiserver-client-cert
⚠️ Deprecated secret name "clustermesh-apiserver-client-cert", should be changed to "clustermesh-apiserver-remote-cert"
⚠️  Service type NodePort detected! Service may fail when nodes are removed from the cluster!
✅ Cluster access information is available:
  - 172.18.0.4:30212

Using two levels of deprecated names to find the secret

✅ ClusterMesh enabled!
Trying to get secret clustermesh-apiserver-remote-cert by deprecated name clustermesh-apiserver-client-cert
Trying to get secret clustermesh-apiserver-client-cert by deprecated name clustermesh-apiserver-client-certs
⚠️ Deprecated secret name "clustermesh-apiserver-client-certs", should be changed to "clustermesh-apiserver-remote-cert"
⚠️  Service type NodePort detected! Service may fail when nodes are removed from the cluster!
✅ Cluster access information is available:
  - 172.18.0.4:30652

@asauber asauber temporarily deployed to ci April 27, 2023 15:32 — with GitHub Actions Inactive
@asauber asauber force-pushed the pr/asauber/clustermesh-secretname branch from b8c8fa1 to 380b63d Compare April 27, 2023 15:38
@asauber asauber temporarily deployed to ci April 27, 2023 15:38 — with GitHub Actions Inactive
@asauber asauber marked this pull request as ready for review April 27, 2023 15:49
@asauber asauber requested review from a team as code owners April 27, 2023 15:49
@asauber asauber changed the title helm mode: add additional deprecated secret logic helm mode: add additional deprecated secret logic, and fix "clustermesh connect" for helm mode May 2, 2023
@asauber asauber changed the title helm mode: add additional deprecated secret logic, and fix "clustermesh connect" for helm mode helm mode: add additional deprecated secret logic, and fix clustermesh connect for helm mode May 2, 2023
@michi-covalent
Copy link
Contributor

@joamaki ping!

@joestringer
Copy link
Member

How do we trigger the required test helm-upgrade-clustermesh?

Previously, we had logic to look for deprecated names like
clustermesh-apiserver-client-certs when
clustermesh-apiserver-client-cert was expected. With helm-based
installations, we now need to also look for
clustermesh-apiserver-client-cert when clustermesh-apiserver-remote-cert
is expected, and do this recursively.

Signed-off-by: Andrew Sauber <andrew.sauber@isovalent.com>
@asauber asauber force-pushed the pr/asauber/clustermesh-secretname branch from 380b63d to 127d6f9 Compare May 5, 2023 22:09
@asauber asauber temporarily deployed to ci May 5, 2023 22:10 — with GitHub Actions Inactive
@asauber
Copy link
Member Author

asauber commented May 5, 2023

@joestringer That workflow was merged since this PR was opened. I'll try a rebase.

@joamaki joamaki removed their request for review May 8, 2023 09:00
@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label May 8, 2023
@tklauser tklauser merged commit e16cbaa into main May 8, 2023
15 checks passed
@tklauser tklauser deleted the pr/asauber/clustermesh-secretname branch May 8, 2023 15:07
@asauber asauber self-assigned this May 23, 2023
asauber added a commit that referenced this pull request May 31, 2023
As of #1551, the clustermesh
connect subcommand looks for all three possible names for the
clustermesh-apiserver client certificate in the local cluster. This
removes the logic needed by the GKE-based CI workflow prior to that
change.

Signed-off-by: Andrew Sauber <andrew.sauber@isovalent.com>
tklauser pushed a commit that referenced this pull request Jun 1, 2023
As of #1551, the clustermesh
connect subcommand looks for all three possible names for the
clustermesh-apiserver client certificate in the local cluster. This
removes the logic needed by the GKE-based CI workflow prior to that
change.

Signed-off-by: Andrew Sauber <andrew.sauber@isovalent.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge This PR has passed all tests and received consensus from code owners to merge.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants