argo-helm icon indicating copy to clipboard operation
argo-helm copied to clipboard

feat(argo-cd): manage CRD by Helm

Open yu-croco opened this issue 2 years ago • 5 comments

This PR resolves https://github.com/argoproj/argo-helm/issues/1336 and https://github.com/argoproj/argo-helm/issues/1259 . 🙋


Signed-off-by: yu-croco [email protected]

Note on DCO:

If the DCO action in the integration test fails, one or more of your commits are not signed off. Please click on the Details link next to the DCO action for instructions on how to resolve this.

Checklist:

  • [x] I have bumped the chart version according to versioning
  • [x] I have updated the documentation according to documentation
  • [x] I have updated the chart changelog with all the changes that come with this pull request according to changelog.
  • [x] Any new values are backwards compatible and/or have sensible default.
  • [x] I have signed off all my commits as required by DCO.
  • [x] My build is green (troubleshooting builds).

Changes are automatically published when merged to main. They are not published on branches.

yu-croco avatar Jun 24 '22 14:06 yu-croco

I tested on my local environment if I could adopt existing CRDs successfully, as below. 📝


  1. initial install with chart v4.16.0
$ helm upgrade argo-cd --install --namespace argo-cd --create-namespace .
Release "argo-cd" does not exist. Installing it now.
NAME: argo-cd
LAST DEPLOYED: Sat Jun 25 01:02:52 2022
NAMESPACE: argo-cd
STATUS: deployed
REVISION: 1
TEST SUITE: None
  1. move CRD files from argo-cd/crds dir to argo-cd/template/crds dir, and helm upgrade (failed as expected)
$ helm upgrade argo-cd --install --namespace argo-cd --create-namespace .
Error: UPGRADE FAILED: rendered manifests contain a resource that already exists. Unable to continue with update: CustomResourceDefinition "applications.argoproj.io" in namespace "" exists and cannot be imported into the current release: invalid ownership metadata; label validation error: missing key "app.kubernetes.io/managed-by": must be set to "Helm"; annotation validation error: missing key "meta.helm.sh/release-name": must be set to "argo-cd"; annotation validation error: missing key "meta.helm.sh/release-namespace": must be set to "argo-cd"
  1. adopt to existing CRDs
$ YOUR_NAMESPACE="argo-cd"
$ YOUR_RELEASENAME="argo-cd"
$ for crd in "applications.argoproj.io" "applicationsets.argoproj.io" "argocdextensions.argoproj.io" "appprojects.argoproj.io"; do
for>    kubectl label --overwrite crd $crd app.kubernetes.io/managed-by=Helm
for>   kubectl annotate --overwrite crd $crd meta.helm.sh/release-namespace="$YOUR_NAMESPACE"
for>   kubectl annotate --overwrite crd $crd meta.helm.sh/release-name="$YOUR_RELEASENAME"
for> done
customresourcedefinition.apiextensions.k8s.io/applications.argoproj.io labeled
customresourcedefinition.apiextensions.k8s.io/applications.argoproj.io annotated
customresourcedefinition.apiextensions.k8s.io/applications.argoproj.io annotated
customresourcedefinition.apiextensions.k8s.io/applicationsets.argoproj.io labeled
customresourcedefinition.apiextensions.k8s.io/applicationsets.argoproj.io annotated
customresourcedefinition.apiextensions.k8s.io/applicationsets.argoproj.io annotated
customresourcedefinition.apiextensions.k8s.io/argocdextensions.argoproj.io labeled
customresourcedefinition.apiextensions.k8s.io/argocdextensions.argoproj.io annotated
customresourcedefinition.apiextensions.k8s.io/argocdextensions.argoproj.io annotated
customresourcedefinition.apiextensions.k8s.io/appprojects.argoproj.io labeled
customresourcedefinition.apiextensions.k8s.io/appprojects.argoproj.io annotated
customresourcedefinition.apiextensions.k8s.io/appprojects.argoproj.io annotated
  1. helm upgrade successfully 🎉
$ helm upgrade argo-cd --install --namespace argo-cd --create-namespace .
Release "argo-cd" has been upgraded. Happy Helming!
NAME: argo-cd
LAST DEPLOYED: Sat Jun 25 01:04:58 2022
NAMESPACE: argo-cd
STATUS: deployed
REVISION: 2
TEST SUITE: None

yu-croco avatar Jun 24 '22 16:06 yu-croco

Thank you for explaining!

helm needs to apply CRDs first and wait until the API extension is ready before we can deploy a instance of that CRD, otherwise the release is failed. And IMHO we should not implement a feature which rely on an edge case which might work for some but not all cases and doesn't respect the architecture how helm is designed.

You're right. It's better to consider as a whole including the design of helm, instead of patching partially. 🤔

In my opition we should

  1. remove the two options listed above
  2. inform our customers/users (at least via README and a major version bump)
  3. provide a dedicated chart for the removed features like argocd-apps or argocd-resources
  4. and then we could talk about implementing that feature :)

Another approach would be to externalize all CRDs which was recommended here: #550

I agree with it. May I create PR(s) to resolve the above if there were not any other opinions/objections? 🙋

yu-croco avatar Jun 27 '22 13:06 yu-croco

@yu-croco - If you are getting inspiration from Argo Events please note the keep option that is not in your PR. If you uninstall the chart CRDs will be also removed together with all user-defined applications.

pdrastil avatar Jun 27 '22 21:06 pdrastil

@pdrastil Thank you for more information! Oh that is important. :eyes: https://github.com/argoproj/argo-helm/blob/main/charts/argo-events/values.yaml#L22-L23

yu-croco avatar Jun 27 '22 23:06 yu-croco

*Whichever method we adopt, we don't have to keep open this PR for now, so I've put it back in draft.

yu-croco avatar Jun 27 '22 23:06 yu-croco

How can I avoid this... 👀 🤔 https://github.com/argoproj/argo-helm/runs/8005988745?check_suite_focus=true

Creating namespace 'argo-cd-ot5rdosmp3'...
namespace/argo-cd-ot5rdosmp3 created
Error: INSTALLATION FAILED: rendered manifests contain a resource that already exists. Unable to continue with install: CustomResourceDefinition "applications.argoproj.io" in namespace "" exists and cannot be imported into the current release: invalid ownership metadata; annotation validation error: key "meta.helm.sh/release-name" must equal "argo-cd-ot5rdosmp3": current value is "argo-cd-8eonx6zuj5"; annotation validation error: key "meta.helm.sh/release-namespace" must equal "argo-cd-ot5rdosmp3": current value is "argo-cd-8eonx6zuj5"
========================================================================================================================
........................................................................................................................
==> Events of namespace argo-cd-ot5rdosmp3
........................................................................................................................
No resources found in argo-cd-ot5rdosmp3 namespace.
........................................................................................................................
<== Events of namespace argo-cd-ot5rdosmp3
........................................................................................................................
========================================================================================================================
Deleting release 'argo-cd-ot5rdosmp3'...
Error: uninstall: Release not loaded: argo-cd-ot5rdosmp3: release: not found
Error deleting Helm release: Error waiting for process: exit status 1
Deleting namespace 'argo-cd-ot5rdosmp3'...
namespace "argo-cd-ot5rdosmp3" deleted
Namespace 'argo-cd-ot5rdosmp3' terminated.
Error: Error installing charts: Error processing charts
------------------------------------------------------------------------------------------------------------------------
 ✖︎ argo-cd => (version: "5.1.0", path: "charts/argo-cd") > Error waiting for process: exit status 1
------------------------------------------------------------------------------------------------------------------------
Error installing charts: Error processing charts
Error: Process completed with exit code 1.

yu-croco avatar Aug 25 '22 00:08 yu-croco

How can I avoid this... 👀 🤔 https://github.com/argoproj/argo-helm/runs/8005988745?check_suite_focus=true

Creating namespace 'argo-cd-ot5rdosmp3'...
namespace/argo-cd-ot5rdosmp3 created
Error: INSTALLATION FAILED: rendered manifests contain a resource that already exists. Unable to continue with install: CustomResourceDefinition "applications.argoproj.io" in namespace "" exists and cannot be imported into the current release: invalid ownership metadata; annotation validation error: key "meta.helm.sh/release-name" must equal "argo-cd-ot5rdosmp3": current value is "argo-cd-8eonx6zuj5"; annotation validation error: key "meta.helm.sh/release-namespace" must equal "argo-cd-ot5rdosmp3": current value is "argo-cd-8eonx6zuj5"
(..)

Fixed it like this a24cf37

mkilchhofer avatar Aug 25 '22 07:08 mkilchhofer

@mkilchhofer Thank you for fixing! 👍 🙇 I turned it into ready.

yu-croco avatar Aug 25 '22 07:08 yu-croco