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

Add explicit "dependsOn" functionality to Applications #10154

Closed
karlschriek opened this issue Aug 1, 2022 · 9 comments
Closed

Add explicit "dependsOn" functionality to Applications #10154

karlschriek opened this issue Aug 1, 2022 · 9 comments
Labels
enhancement New feature or request

Comments

@karlschriek
Copy link

karlschriek commented Aug 1, 2022

Summary

We would love to have the ability to define an explicit "dependsOn" field on Applications. For example:

App1 doesn't depend on anything:

apiVersion: argoproj.io/v1alpha1
kind: Application
metadata:
  name: App1
spec:
  project: default
  ...

App2 depends on App1:

apiVersion: argoproj.io/v1alpha1
kind: Application
metadata:
  name: App2
spec:
  project: default
  dependsOn: 
    - App1
  ...

At this stage I would like to clarify a misunderstanding that I have come across in various (now-defunc) issues. See for example:
#3454
#7978

The pattern described above is not the same as using sync-waves with the App-of-Apps pattern (which in any case is no longer directly supported, and needs to be reactivated explicitly in order to be used, see #8358, https://argo-cd.readthedocs.io/en/stable/operator-manual/upgrading/1.7-1.8/#health-assessment-of-argoprojioapplication-crd-has-been-removed).

What we are looking for is something that is self-contained on the Application itself. I.e. I want to be able to define directly on App2 that it depends on App1. If App1 doesn't exist, then App2 doesn't sync. Once App1 comes into existence, App2 will sync. If App1 re-syncs, App2 will re-sync as well after App1's sync has completed.

Motivation

There are several circumstances where the order of deployment matters. A good example are mutating webhooks, such as Istio's istio-sidecar-injector which will look for the annotation istio-injection: enabled. If it finds it, it will inject sidecars into Pods. However, if a Pod that has been thus annotated starts up before the webhook is ready, then it will not receive the sidecars. In this case it needs to be manually restarted at a later stage. It would therefore here be extremely useful to instruct ArgoCD to wait with rolling out an Application that contains that Pod until such time as the Application with the Istio webhook is ready.

To also further clarify why using sync-waves with the "App-of-Apps" is not desired here: Our Application specs do not live inside a GitOps repo. (Only the actual resources we want ArgoCD to sync for us live inside a GitOps repo). Our Application specs are dynamically composed and rolled out. As such it is not possible to do App-of-Apps. There are some other reasons as well related to the plugin-approach we use to render our manifests, but that goes quite far beyond the scope of what we are discussing here.

Proposal

Add the field dependsOn to the Application spec and then add the following to the sync logic: (I'll use App1 and App2 as listed above for example)

  • If App1 does not exist, or exists but hasn't successfully synced, report that back as a "depensdOn" sync failure on App2
  • If App1 has succesfully synced, proceed to sync App2
@karlschriek karlschriek added the enhancement New feature or request label Aug 1, 2022
@rouke-broersma
Copy link
Contributor

rouke-broersma commented Aug 1, 2022

Our Application specs do not live inside a GitOps repo. (Only the actual resources we want ArgoCD to sync for us live inside a GitOps repo). Our Application specs are dynamically composed and rolled out. As such it is not possible to do App-of-Apps.

It seems to me your whole problem rests on and is caused by this issue you're describing. Dynamic composition of app-of-apps is very possible by using helm chart templating imo so I think your 'we cannot use app-of-apps' needs clarification. Right now it reads more like you don't want to use app-of-apps, but if you would use app-of-apps the dependsOn would not be neccesary because it could be solved with sync waves.

@karlschriek
Copy link
Author

True, if that were the only reason why we want to avoid App-of-Apps then a templating approach could solve it, and you are right to point that out. In this case, brevity was not my friend, so let me try to explain it a bit more fundamentally:

App-of-Apps forces you to construct a single entrypoint for all your Applications, i.e. you have to define a single "master" App that takes care of rolling them out. But this may not be feasible or may conflict directly (as is the case for us) with other established design patterns that take precedence. For example we have an established pattern where Applications live within larger self-contained modules (together with any requisite cloud resources that it relies on, such as external storage, IAM roles, DNS zones etc.) that are rolled out together. Using an App-of-Apps (irrespective of whether you use templating tools such as helm to compose it) would require you to strip those Applications out their (terraform) modules and deal with them all together, separately. This would severely hamstring our ability to "dynamically compose" these resources.

@rouke-broersma
Copy link
Contributor

rouke-broersma commented Aug 1, 2022

We use app-of-apps to construct app-of-apps which are modules that contain applications + external resource manifests (like terraform operator and such) using templating. 🤷‍♀️

We use multiple top level app-of-apps argo applications for different purposes, those master argocd applications deploy app-of-apps argo applications which itself are then a 'master' for distinct modules that together form one deploy unit. Or, they can even contain more app-of-apps apps to further dynamically compose modules.

Fictional but completely possible setup:

root-app-of-apps-a -> module app-of-apps-a -> app-1 -> pods, ssl, ingress etc etc, db-1
                                                                                app-2 -> pods, ssl, ingress etc etc, db-2
                                                                                app-3 -> pods, ssl, ingress etc etc, db-3
                                                                                app-shared-resources -> db-shared
                    -> module app-of-apps-b -> app-4 -> pods, ssl, ingress etc etc, db-1
                                                                                app-5 -> pods, ssl, ingress etc etc, db-2
                                                                                app-6 -> pods, ssl, ingress etc etc, db-3
                                                                                app-shared-resources -> db-shared
root-app-of-apps-b  -> module app-of-apps-c -> app-7 -> pods, ssl, ingress etc etc, db-1
                                                                                app-8 -> pods, ssl, ingress etc etc, db-2
                                                                                app-9 -> pods, ssl, ingress etc etc, db-3
                                                                                app-shared-resources -> db-shared

@crenshaw-dev
Copy link
Member

@karlschriek can you check whether this might be a duplicate of either of these?

If so, can we consolidate on one of those threads? The existing thumbs-ups on those issues can help drive attention towards your use case. You could copy/paste your comments to one of those issues.

If they're sufficiently distinct, ignore me, and we'll keep this issue open. :-)

@karlschriek
Copy link
Author

@crenshaw-dev thanks, I have not come across those two issues before. The topic seems to be already quite splintered, so my apologies for opening up yet another one. #7437 seems like the right home for this one (altough #3517 actually hits the nail on the head by giving it right name: what we need is the ability to define a DAG).

Shall I close this issue in favour or #7437?

@karlschriek
Copy link
Author

@rouke-broersma that is an interesting approach. That means you cloud resources with controllers running on Kubernetes, correct?

@rouke-broersma
Copy link
Contributor

Yes exactly. I thought you did as well but I guess your modules are external from kubernetes so I see how that could be an issue then.

@crenshaw-dev
Copy link
Member

Yeah @karlschriek, let's continue to track this in #7437. Depending on where most engagement goes, we can decide whether to consolidate on #3517 or #7437.

@crenshaw-dev
Copy link
Member

"Not planned" is a terrible name for this "closed" state - just means duplicate in this context.

@crenshaw-dev crenshaw-dev closed this as not planned Won't fix, can't repro, duplicate, stale Aug 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants