FrittenToni commented 2 years ago

Summary

ApplicationSets automatically override manual Application parameter overrides, e.g. performed with argocd client (argocd app set command). The only option to avoid it seems to be to start the applicationset controller with "create-only" policy but this is not always what one wants. Especially in combination with a pull request generator since the pull request apps will never be deleted again once a pull request has been closed.

Motivation

I agree that manual parameter overrides should never be used in production but there is a dev use case where we need them:

Inside our pull request pipeline we're creating a temporary staging docker repository on the fly and we push our PR docker image into it
The application generated by PR generator should consume images from that prev. created staging repository
The name of the staging repository including the credentials to access it are not predictable and therefore we use argocd client to override these parameters

Inside our main pipeline basically the same mechanism happens. We create a staging repository and push our docker image into it. Then, we want to do artifact validation by running a bunch of tests and only if they are successful we promote our docker image from staging into our final release repository.

Proposal

How do you think this should be implemented?

There should be a switch on applicationset level to express that manual parameter overrides should be preserved. If that switch is true and applicationset controller updates an Application object it should merge the parameter overrides instead of just replacing them with whatever is inside the repository.

jdomag commented 2 years ago

I would love to see this feature as well. My use case: When we build a new image we don't do commit/push to git repo containing the helm chart - we simply do argocd app set APP_ID --helm-set image.tag=new_image The same happens when image updater is used (imperative write-back method)

LeartBeqiraj commented 2 years ago

Any update on this?

crenshaw-dev commented 2 years ago

I think it would be a nontrivial change in ApplicationSet. We'd have to do a diff, apply ignored-field rules, and apply the diff as a patch. Not terrible, but also not trivial.

I'm curious if we can just solve the problem one level higher, using an Application, so the already feature-rich Application controller can handle it.

What if you 1) converted your ApplicationSet to a Helm chart, 2) deployed the ApplicationSet with a new, top-level Application, 3) set the parameter override on the new top-level Application and let it propagate down to the target Application.

The main trick would be applying the parameter override to only the target Application. I think that could probably be done with a merge generator in the ApplicationSet.

raelga commented 2 years ago

This feature is also interesting for our team, as we usually play with the branches to test changes in the development / staging Applications, before pushing them to production. We also usually manually stop the automatic syncing / self healing for non-production Applications while testing.

arturkasperek commented 2 years ago

Hi, our team needs that cause we wanna give a possibility to rollback

crenshaw-dev commented 2 years ago

@raelga would a "pause" label or annotation solve your use case?

raelga commented 2 years ago

Is not as straight forward as the SyncPolicy update, but it can work.

For our use case, we ended reproducing the ApplicationSet outcome using kustomize. So now we have a base with the applications manifests and then several overlays to create all the applications environments. This generates the same combination you can generated with a matrix ApplicationSet from a list of Components (apps from the same workload) and a list of Environments (clusters and repos).

The benefit of using kustomize is that it behaves as an App of Apps with all the benefits of using an Application instead of an ApplicationSet: it can be configured to manual synchronisation, makes really easy to detect apps Out of Sync (not pointing to HEAD or without the automated healing),... etc.

This is how the kustomized App of Apps deployment looks, not as clean as an ApplicationSet, but with all the functionalities of a regular Application.

❯ tree
.
├── application.yaml
├── base
│   ├── applications
│   │   ├── example-component.yaml
│   │   ├── example-component-2.yaml
│   │   ├── example-component-3.yaml
│   │   ├── example-component-4.yaml
│   │   └── example-component-5.yaml
│   └── kustomization.yaml
├── kustomization.yaml
└── overlays
    ├── dev
    │   ├── appproject.yaml
    │   └── kustomization.yaml
    ├── pro
    │   ├── applications
    │   │   ├── pro-only-example-component.yaml
    │   │   └── pro-only-example-component-1.yaml
    │   ├── appproject.yaml
    │   └── kustomization.yaml
    └── stg
        ├── applications
        │   └── stg-only-example-component.yaml
        ├── appproject.yaml
        └── kustomization.yaml

kustomization.yaml

resources:
  - overlays/dev
  - overlays/stg
  - overlays/pro

overlays/pro/kustomization.yaml

bases:
  - ../../base

resources:
  - appproject.yaml
  - applications/pro-only-example-component.yaml
  - applications/pro-only-example-component-1.yaml

transformers:
  - |-
    apiVersion: builtin
    kind: PrefixSuffixTransformer
    metadata:
      name: prefix
    prefix: workload-pro-
    fieldSpecs:
      - kind: Application
        path: metadata/name
  - |-
    apiVersion: builtin
    kind: PrefixSuffixTransformer
    metadata:
      name: source-path-prefix
    prefix: manifests/workload/pro
    fieldSpecs:
      - kind: Application
        path: spec/source/path
patches:
  - patch: |-
      - op: replace
        path: /spec/project
        value: workload-pro
      - op: replace
        path: /spec/destination
        value:
          server: https://api.pro-cluster-fqdn:6443
          namespace: workload-pro
    target:
      kind: Application

And the base are several Application manifests:

/bases/applications/example-component.yaml

apiVersion: argoproj.io/v1alpha1
kind: Application
metadata:
  annotations:
    argocd.argoproj.io/manifest-generate-paths: .
  name: example-component
spec:
  project: kustomize-overlay
  source:
    repoURL: https://github.com/org/workload
    targetRevision: HEAD
    path: example-component     # base path is added as prefix by kustomize with a transfomer
    directory:
      recurse: true
  destination:
    server: kustomize-overlay
    namespace: kustomize-overlay
  syncPolicy:
    automated:
      selfHeal: true

tenczardavid commented 2 years ago

Hi Everyone,

The main problem we have currently with ApplicationSet is the fact that it removes some built-in features of ArgoCD.

One of them is to disable automated sync on Application level. This also has the side-effect that ArgoCD rollbacks can't function in this setup. This causes a lot of confusion for our developers when we would like to on-board them.

As ApplicationSet became a core feature of ArgoCD, I think we should put some effort into trying to integrate it properly into its ecosystem. Was there any planning done for this feature already? How we could pitch in and help to move forward with this issue?

My main concern that in the documentation there is a sentence To some extent this is by design, was this reconsidered or is this still the case today?

https://argo-cd.readthedocs.io/en/stable/operator-manual/applicationset/Controlling-Resource-Modification/#limitation-no-support-for-manual-edits-to-individual-applications

crenshaw-dev commented 2 years ago

ArgoCD rollbacks can't function in this setup

I think we should probably create another issue to track the no-rollbacks problem (if an issue doesn't already exist for that). I'm not sure the solution will be the same as the solution to the parameter preservation issue.

@tenczardavid I'm open to the idea of trying to implement the fix as proposed - basically create a boolean field in ApplicationSet indicating that spec.source fields should be merged rather than overwritten. Precisely which fields should be merged and how they should be merged I think remains an open and important question.

Personally I'd like to see the ApplicationSet spec become fully templateable so that people can use their ApplicationSet source of truth (hopefully git or something else that's stable and has a history mechanism) to do things like disable auto-sync. That would be the more "gitops way" to do things. But I understand that one thing people like about Argo CD is its pragmatism. People like the ability to make a quick change in the UI before going back and making the change in Git. I'm open to expanding that pragmatism to ApplicationSets.

bakito commented 1 year ago

@raelga would a "pause" label or annotation solve your use case?

We are using the Image Updater https://argocd-image-updater.readthedocs.io/en/stable/ and would like to set annotations on certain applications created by the applicationset. Unless we set the policy to "create-only" the pause annotations/labels would help in our case.

takumakume commented 1 year ago

My team is having trouble because we are using the Pull Request Generator + Image Updater.

The create-only policy alleviates this problem. However, it is difficult to use because it is not deleted when the pull request is closed.

I think the root solution is to keep overwriting or 3-way-merge.

However, this problem has not been solved for a long time because it requires a major modification.

I propose adding a create-delete policy as a way to alleviate this problem quickly.

I think this would be useful for applications like the Pull request Generator, which often have a short lifecycle.

nhuray commented 1 year ago

From my perspective that feature would be useful in certain cases but it probably bring more complexity to the controller code because of the diff computation to make.

One thing I would prefer it's to be able to define the policy (e.g create-only) at ApplicationSet level and not globally at controller level.

This would allow to define different strategy per-environment, so for instance:

a Preview Environment will use an Application Set with a policy sync in order to create and destroy applications for each Pull Requests (using the Pull Request Generator).
a Production environment will use an Application Set with a policy create-only to ensure applications are not destroyed by error.

There's an issue open here for that capability: https://github.com/argoproj/argo-cd/issues/11073