argocd-notifications icon indicating copy to clipboard operation
argocd-notifications copied to clipboard

Controller and Catalog manifests awkward to use in a gitops workflow

Open ZymoticB opened this issue 4 years ago • 8 comments

We've just deployed this and are very happy for the functionality offered. It was a bit awkward getting it integrated into our currently k8s manifest deployment and I thought I would walk through that a bit and see if we're doing something suboptimally, or there are improvements that could be made here that would be good for others as well.

Currently we have a single argocd application resource that deploys the manifests for an entire cluster via kustomize.

As a result of using kustomize, we're unable to deploy both the controller and catalog manifests as they define the same configmap (controller catalog).

Usually, we keep vendor manifests up to date with a make target and have a kustomization.yaml that uses all vendors manifests as resources, for example:

namespace: argocd
resources:
  - argocd-notifications-v1.0.1.yaml
  - argocd-notifications-catalog-v1.0.1.yaml

Unfortunately, kustomize is unable to build this due to the shared configmap ID.

$ kustomize build .
Error: accumulating resources: 2 errors occurred:
	* accumulateFile error: "merging resources from 'argocd-notifications-catalog-v1.0.1.yaml': may not add resource with an already registered id: ~G_v1_ConfigMap|~X|argocd-notifications-cm"
	* loader.New error: "error loading argocd-notifications-catalog-v1.0.1.yaml with git: url lacks orgRepo: argocd-notifications-catalog-v1.0.1.yaml, dir: got file 'argocd-notifications-catalog-v1.0.1.yaml', but '......./argocd-notifications/argocd-notifications-catalog-v1.0.1.yaml' must be a directory to be a root, get: invalid source string: argocd-notifications-catalog-v1.0.1.yaml"

We've worked around this by deleting the empty configmap definition from argocd-notifications-${VERSION}.yaml but that breaks our convention for leaving vendor manifests untouched.

A second issue we encountered is related to secrets management. We're using bitnami/sealed-secrets to manage secrets, which will not overwrite a secret by default. The controller manifest creates an empty secret so the first time we applied our PR to add argocd-notifications (after we got kustomize happy) the secret failed to unseal. We are able to work around this as sealed-secrets supports overwriting existing secrets, however, it took a bit of manual deletion of objects to recover this correctly.

We're most concerned with a strategy that avoids hand-editing the source manifests, the secrets management flow seems reasonable but we figured it was worth mentioning at the same time.

ZymoticB avatar Feb 02 '21 15:02 ZymoticB

Hi @ZymoticB

At first, If you want to use catalog, you use patchesStrategicMerge to be able to avoid errors.

namespace: argocd
resources:
- argocd-notifications-v1.0.1.yaml
patchesStrategicMerge
- argocd-notifications-catalog-v1.0.1.yaml

ryota-sakamoto avatar Feb 02 '21 15:02 ryota-sakamoto

Ah, that seems reasonable, though it's not something we've had to do with any other vendor manifests.

Maybe it's worth publishing a manifest that does this so a user can opt into installing the controller and catalog? I'm assuming that is why they are kept separate, because some users may skip the catalog and only use custom triggers?

I'm not sure what kind of change could make that sort of merge on the client side become incorrect, but I think I would expect that the vendor manifests would be apply-able as resources. Then this merge logic would be managed by the vendor and kept up to date with any changes they would like to make in the future.

ZymoticB avatar Feb 02 '21 15:02 ZymoticB

hi @ZymoticB sorry for being late. I guess that if it takes all in one pattern that is more convenient. I think catalog is optional, so the user would choice using it or not.

What do you think?

ryota-sakamoto avatar Mar 25 '21 16:03 ryota-sakamoto

I think the suggestion I'm making, is that like manifests/install.yaml vs manifests/install-bot.yaml there could be a version or each with the catalog included. I think the main concern here would be the number of combinations continuing to grow over time and becoming very confusing.

I guess having the manifests separate works just fine for a workflow that does

kubectl apply manifests/install.yaml
kubectl apply catalog/install.yaml

In a kustomize based workflow, you can do a strategic merge. Maybe the answer here is to add a Kustomize Getting Started section to https://argocd-notifications.readthedocs.io/en/stable/

ZymoticB avatar Mar 25 '21 17:03 ZymoticB

I understand. It is good solution that is to add to Getting Started.

ryota-sakamoto avatar Mar 26 '21 13:03 ryota-sakamoto

@ZymoticB , @ryota-sakamoto ,

I'm trying to make notifications even more GitOps friendly in this PR: https://github.com/argoproj-labs/argocd-notifications/pull/262 . What do you think about this idea?

alexmt avatar Apr 23 '21 02:04 alexmt

Seems reasonable to me!

ZymoticB avatar Apr 23 '21 12:04 ZymoticB

We have catalog under patchesStrategicMerge like the documentation says, but we are getting this error for the main install

`kustomize build .argocd` failed exit status 1: Error: accumulating resources: accumulation err='accumulating resources from 'argocd-notifications-v1.2.1-install.yaml': yaml: line 27: mapping values are not allowed in this context': got file 'argocd-notifications-v1.2.1-install.yaml', but '.argocd/argocd-notifications-v1.2.1-install.yaml' must be a directory to be a root

our kustomize:

apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
namespace: argocd

resources:
  - base/argocd-notifications.yaml
  - argocd-notifications-v1.2.1-install.yaml

patchesStrategicMerge:
  - argocd-notifications-catalog-v1.2.1-install.yaml
  - overlays/argocd-notifications-secret.yaml
  - overlays/argocd-notifications-cm.yaml

OliverLeighC avatar May 18 '22 18:05 OliverLeighC