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

feat: Add option to customize local cluster name #1154

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ItsKev
Copy link
Contributor

@ItsKev ItsKev commented Jan 12, 2024

What type of PR is this?

Uncomment only one /kind line, and delete the rest.
For example, > /kind bug would simply become: /kind bug

/kind bug
/kind chore
/kind cleanup
/kind failing-test

/kind enhancement

/kind documentation
/kind code-refactoring

What does this PR do / why we need it:

This PR introduces a method to specify the cluster name for Argo CD deployments. Previously, it defaulted to in-cluster.

Our team creates templates for other DevOps teams in our company. One such template deploys the primary Argo CD instance in the production cluster, along with mock instances in testing and external clusters. These instances are labeled "in-cluster," "testing," and "external." This labeling has caused confusion due to the absence of a "production" label. Renaming "in-cluster" to "production" will resolve this issue.

Have you updated the necessary documentation?

  • Documentation update is required by this PR.
  • Documentation has been updated.

Which issue(s) this PR fixes:

Fixes #1093

How to test changes / Special notes to the reviewer:

Deploy the ArgoCD custom resource below and confirm that example-cluster is set as the name in the example-argocd-default-cluster-config secret.

apiVersion: argoproj.io/v1beta1
kind: ArgoCD
metadata:
  name: argocd
spec:
  clusters:
    local:
      name: example-cluster

@ItsKev ItsKev force-pushed the feature/customizable-cluster-name branch 2 times, most recently from 83c39c6 to ea64f57 Compare January 12, 2024 09:37
@ItsKev
Copy link
Contributor Author

ItsKev commented Jan 21, 2024

@svghadi Could you take a look at my PR when you get a chance? Thanks!

@svghadi
Copy link
Collaborator

svghadi commented Feb 9, 2024

Hi @ItsKev, Thanks for the PR. I will take a look at it this weekend.

@jaideepr97
Copy link
Collaborator

Hi @ItsKev, not sure I follow the pain point you're hitting - in-cluster is an internal name that is used within argocd-secret and is not really public facing. I am not sure why that would preclude you from being able to label your argo-cd instances however you want

@ItsKev
Copy link
Contributor Author

ItsKev commented Feb 15, 2024

Hi @jaideepr97, thanks for the reply. This in-cluster name is indeed "public" facing. It's visible in the Clusters Overview in the Settings page of Argo CD and in the Applications overview. You can even use this name in the spec.destination.name of an Application.

image
image

application.yaml:

apiVersion: argoproj.io/v1alpha1
kind: Application
metadata:
  name: guestbook
  namespace: default
spec:
  destination:
    namespace: default
    name: example-cluster
  project: default
  source:
    path: helm-guestbook
    repoURL: https://github.com/argoproj/argocd-example-apps.git
    targetRevision: HEAD

@ItsKev
Copy link
Contributor Author

ItsKev commented Sep 27, 2024

@svghadi @jaideepr97 any update on this?

@svghadi
Copy link
Collaborator

svghadi commented Sep 30, 2024

Hi @ItsKev, sorry the PR went unnoticed. Allow me some time. I am discussing internally on the crd structure for this. I will get back to you soon.

@svghadi
Copy link
Collaborator

svghadi commented Oct 3, 2024

Hi @ItsKev , could you update the field
from

spec:
  inClusterName: example-cluster

to

spec:
  clusters:
    local:
      name: example-cluster

This structure will be beneficial for future enhancements related to clusters. For example, if we decide to support configuring remote clusters during installation, this approach allows for easy expansion by introducing a new field like .spec.clusters.remote.

@ItsKev ItsKev force-pushed the feature/customizable-cluster-name branch 4 times, most recently from ecfc3df to a0e8dd7 Compare October 4, 2024 06:48
@ItsKev ItsKev force-pushed the feature/customizable-cluster-name branch from a0e8dd7 to 92127a8 Compare October 4, 2024 06:52
@ItsKev ItsKev changed the title feat: Add option to customize in-cluster name feat: Add option to customize local cluster name Oct 4, 2024
@ItsKev
Copy link
Contributor Author

ItsKev commented Oct 4, 2024

Hi @svghadi

I've changed the crd structure according to your inputs.

apiVersion: argoproj.io/v1beta1
kind: ArgoCD
metadata:
  name: argocd
spec:
  clusters:
    local:
      name: example-cluster

@svghadi
Copy link
Collaborator

svghadi commented Oct 16, 2024

Hi @ItsKev,
We discussed internally about this. There are some concerns regarding a potential breaking change for existing users. The update logic in the code could result into renaming of clusters back to in-cluster for existing users if they have already modified the name from UI/CLI. This could cause issues to existing deployments on those clusters.

I have few ideas(mainly scripting) which could address your use-case without needing a code change. The in-cluster name is store in name key in <argocd-name>-default-cluster-config secret. Maybe a script can patch this secret with new name or use argocd-cli to update the name. Do you think, it is feasible?

@ItsKev
Copy link
Contributor Author

ItsKev commented Oct 19, 2024

@svghadi we are doing this internally already. I get the fear about a potential breaking change for existing users. Not sure though how we could achieve implementing this feature without those..

I'm happy to implement any potential solution, otherwise we probably have to close this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature Request: Customizable Cluster Name in ArgoCD Operator
3 participants