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

chore: remove Argo CD CRDs from namespaced install #6022

Merged
merged 3 commits into from
Apr 13, 2021

Conversation

alexmt
Copy link
Collaborator

@alexmt alexmt commented Apr 12, 2021

Signed-off-by: Alexander Matyushentsev [email protected]

The namespaced install is used by Argo CD admins who manage multiple Argo CD instances within one namespace. As per description, it does not require any admin permissions. CRDs are cluster lever resources so should be removed from namespaced manifests bundle.

@alexmt alexmt requested review from jannfis and sbose78 April 12, 2021 22:48
@alexmt
Copy link
Collaborator Author

alexmt commented Apr 12, 2021

We use namespaced bundle to manage multiple Argo CD instances deployed into separate namespaces. Because CRDs are bundled we cannot just reference namespace-install.yaml in kustomize.yaml and instead have to copy it and remove CRDs section.

I think other users have the same use case . @sbose78 , @jannfis what do you think?

@codecov
Copy link

codecov bot commented Apr 12, 2021

Codecov Report

Merging #6022 (18c9a65) into master (8301d39) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #6022   +/-   ##
=======================================
  Coverage   40.93%   40.93%           
=======================================
  Files         147      147           
  Lines       19661    19661           
=======================================
  Hits         8049     8049           
  Misses      10508    10508           
  Partials     1104     1104           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8301d39...18c9a65. Read the comment docs.

Copy link
Member

@jannfis jannfis left a comment

Choose a reason for hiding this comment

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

Indeed, the namespaced install probably should not include the CRDs. I have just a minor comment, no formal request for change. Please see below.

Comment on lines 20 to 21
> Note: Argo CD CRDs are not included into [namespace-install.yaml](namespace-install.yaml) and have to be installed
> separately.
Copy link
Member

Choose a reason for hiding this comment

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

Can we give instructions on how to install the CRDs manually? E.g. where to find them in the code base?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

good point. added

Signed-off-by: Alexander Matyushentsev <[email protected]>
@sbose78
Copy link
Contributor

sbose78 commented Apr 13, 2021 via email

@kshamajain99
Copy link
Contributor

@alexmt we should make respective changes in manifests/ha/namespace-install as well
https://github.com/argoproj/argo-cd/tree/master/manifests/ha/namespace-install

Signed-off-by: Alexander Matyushentsev <[email protected]>
@alexmt
Copy link
Collaborator Author

alexmt commented Apr 13, 2021

Thank you @kshamajain99 ! Fixed.

@alexmt
Copy link
Collaborator Author

alexmt commented Apr 13, 2021

@sbose78, CRDs have to be applied by cluster-admin, not namespace admin. For example, in our case, CRDs and all other cluster-level resources are managed by a platform team.

Our team is responsible for managing Argo CD instances (each instance in a separate namespace). If we removing CRD from namespace-install.yaml simplifies the process for us. We can just reference namespace-install.yaml in kustomization.yaml instead of copying to modifying it.

@alexmt alexmt merged commit c847bd9 into argoproj:master Apr 13, 2021
@alexmt alexmt deleted the namespaced-install-crd branch April 13, 2021 18:41
yujunz added a commit to abcue/argo-cd that referenced this pull request Apr 15, 2021
5bc7297 fix: bitbucket server failing diagnostics:ping (argoproj#6029) (argoproj#6034)
8f53bd5 fix: add helm dependencies with custom CA (argoproj#6003)
8fd6f13 docs: Custom resource actions (argoproj#5838)
8a2897d docs: update delete policy verbiage (argoproj#6025)
c847bd9 chore: remove Argo CD CRDs from namespaced install (argoproj#6022)
61080b3 docs: improve Orphaned Resources Monitoring with more examples and correct grammar (argoproj#6006)
8301d39 Adding explicit bind to redis and sentinel for IPv4 clusters argoproj#5957 (argoproj#6005)
12cabdf fix: adding tests for helm OCI registry (argoproj#5978)
9da9514 docs: Add Ant Group to the list of users (argoproj#6011)
5e34a8a add Polarpoint.io (argoproj#6010)
2f92777 chore: move access checks from api server to repo server (argoproj#5940)
ae2d0ff fix(ui): Unscheduled pods in node view are now visible. Fixes argoproj#5981 (argoproj#5988)
b003f70 docs: SealedSecret status missing on k8s 1.16+ (argoproj#5846)
445872f fix: use correct field for evaluating whether or not GitHub Enterprise is selected (argoproj#5987)
9afa833 chore: Make e2e tests runnable against remote cluster (argoproj#5895)
shubhamagarwal19 pushed a commit to shubhamagarwal19/argo-cd that referenced this pull request Apr 15, 2021
* chore: remove Argo CD CRDs from namespaced install

Signed-off-by: Alexander Matyushentsev <[email protected]>
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.

4 participants