feat: ApplicationSet Progressive Rollouts (#9437)

[wmgroot]

Please feel free to comment on the implementation.

Checklist:

• Either (a) I've created an enhancement proposal and discussed it with the community, (b) this is a bug fix, or (c) this does not need to be in the release notes. docs: appset progressive rollout strategy proposal (#9437) #9979
• The title of the PR states what changed and the related issues number (used for the release note).
• I've included "Closes [ISSUE #]" or "Fixes [ISSUE #]" in the description to automatically close the associated issue.
• I've updated both the CLI and UI to expose my feature, or I plan to submit a second PR with them.
• Does this PR require documentation updates?
• I've updated documentation as required by this PR.
• Optional. My organization is added to USERS.md.
• I have signed off all my commits as required by DCO
• I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged.
• My build is green (troubleshooting builds).

Closes #9437.
Please see the linked enhancement issue for context and use cases for this capability.
#9437

Example ApplicationSet spec used for E2E testing.

---
apiVersion: argoproj.io/v1alpha1
kind: ApplicationSet
metadata:
  name: hello-world-nginx
spec:
  generators:
  - clusters:
  ...
  syncPolicy:
    preserveResourcesOnDeletion: true
  strategy:
    type: RollingUpdate
    rollingUpdate:
      steps:
        - matchExpressions:
            - key: rolloutWave
              operator: In
              values:
                - wave-1
          maxUpdate: 0 # if undefined or 0, all applications matched are updated together
        - matchExpressions:
            - key: rolloutWave
              operator: In
              values:
                - wave-2
          maxUpdate: 1 # maxUpdate supports both integer and percentage string values
  template:
    metadata:
      name: 'hello-world-nginx-{{name}}'
      annotations:
        indeed.com/argo-cd-instance: test-mgroot
        indeed.com/applicationset: hello-world-nginx
      labels:
        rolloutWave: "{{criticality}}"  # label can be provided explicitly from a list generator or from cluster secret labels, etc
    spec:
      project: hello-world-nginx
      source:
        repoURL: <internal repo with a simple dummy nginx Deployment>
        path: charts/nginx
        targetRevision: master
        helm:
          releaseName: hello-world-nginx
          values: |-
            clusterName: {{name}}
            deployment:
              template:
                annotations:
                  test: "30"   # this field is incremented to test the progressive rollout capability, nginx pods become ready after 2 mins
      destination:
        name: '{{name}}'
        namespace: hello-world-nginx--demo
      syncPolicy:
        automated:
          prune: true
          selfHeal: true
        syncOptions:
          - ApplyOutOfSyncOnly=true
          - CreateNamespace=true

Example ApplicationSet status progression.

# App #1 and App #2 stable on same AppSet version
status:
  applicationStatus:
  - application: hello-world-nginx-awscmhdev
    lastTransitionTime: "2022-07-19T22:17:29Z"
    message: Application resource becaame Healthy, updating status from Progressing
      to Healthy.
    observedGeneration: 3
    status: Healthy
  - application: hello-world-nginx-awscmhqa2
    lastTransitionTime: "2022-07-19T22:17:29Z"
    message: Application resource becaame Healthy, updating status from Progressing
      to Healthy.
    observedGeneration: 3
    status: Healthy


# App #1 Progressing, App #2 Waiting
status:
  applicationStatus:
  - application: hello-world-nginx-awscmhdev
    lastTransitionTime: "2022-07-19T22:56:58Z"
    message: Application resource became Progressing, updating status from Pending
      to Progressing.
    observedGeneration: 4
    status: Progressing
  - application: hello-world-nginx-awscmhqa2
    lastTransitionTime: "2022-07-19T22:56:58Z"
    message: Application is out of date with the current AppSet generation, setting
      status to Waiting.
    observedGeneration: 3
    status: Waiting

# App #1 Healthy, App #2 Progressing
status:
  applicationStatus:
  - application: hello-world-nginx-awscmhdev
    lastTransitionTime: "2022-07-19T22:59:04Z"
    message: Application resource becaame Healthy, updating status from Progressing
      to Healthy.
    observedGeneration: 4
    status: Healthy
  - application: hello-world-nginx-awscmhqa2
    lastTransitionTime: "2022-07-19T22:59:06Z"
    message: Application resource became Progressing, updating status from Pending
      to Progressing.
    observedGeneration: 4
    status: Progressing

# App #1 and App #2 on new AppSet version
status:
  applicationStatus:
  - application: hello-world-nginx-awscmhdev
    lastTransitionTime: "2022-07-19T22:59:04Z"
    message: Application resource becaame Healthy, updating status from Progressing
      to Healthy.
    observedGeneration: 4
    status: Healthy
  - application: hello-world-nginx-awscmhqa2
    lastTransitionTime: "2022-07-19T23:01:12Z"
    message: Application resource becaame Healthy, updating status from Progressing
      to Healthy.
    observedGeneration: 4
    status: Healthy

[leoluz]

@wmgroot Any reason why pointing this PR to the release-2.4 branch? I think it should point to master.

[wmgroot]

@wmgroot Any reason why pointing this PR to the release-2.4 branch? I think it should point to master.

Updated. We had this based on the 2.4 release branch to perform end to end testing in our test ArgoCD instance.

[codecov]

Codecov Report

Base: 47.19% // Head: 47.07% // Decreases project coverage by -0.11% ⚠️

Coverage data is based on head (b0fb2c2) compared to base (c6fa942).
Patch coverage: 57.91% of modified lines in pull request are covered.

❗ Current head b0fb2c2 differs from pull request most recent head 4da601d. Consider uploading reports for the commit 4da601d to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #10048      +/-   ##
==========================================
- Coverage   47.19%   47.07%   -0.12%     
==========================================
  Files         243      243              
  Lines       41088    40863     -225     
==========================================
- Hits        19393    19238     -155     
+ Misses      19734    19702      -32     
+ Partials     1961     1923      -38     

Impacted Files	Coverage Δ
.../apis/application/v1alpha1/applicationset_types.go	29.26% <0.00%> (-3.17%)	⬇️
...cationset/controllers/applicationset_controller.go	60.31% <58.98%> (-3.66%)	⬇️
util/io/paths.go	68.18% <0.00%> (-7.69%)	⬇️
util/git/creds.go	41.27% <0.00%> (-7.59%)	⬇️
util/webhook/webhook.go	65.56% <0.00%> (-2.73%)	⬇️
util/argo/argo.go	62.41% <0.00%> (-2.57%)	⬇️
cmd/argocd/commands/admin/app.go	29.96% <0.00%> (-1.16%)	⬇️
applicationset/generators/git.go	85.86% <0.00%> (-1.07%)	⬇️
pkg/apis/application/v1alpha1/types.go	58.22% <0.00%> (-0.42%)	⬇️
... and 22 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[Smirl]

Hey 👋

Looks great. I've not read through all of the code but this looks very similar to something that we have created in Skyscanner. (https://github.com/Skyscanner/applicationset-progressive-sync/).

If there is anything we can help with in bringing this concept into the main ArgoCD controller, i.e. help on this PR, don't hesitate to reach out to us.

[wmgroot]

Hey wave

Looks great. I've not read through all of the code but this looks very similar to something that we have created in Skyscanner. (https://github.com/Skyscanner/applicationset-progressive-sync/).

If there is anything we can help with in bringing this concept into the main ArgoCD controller, i.e. help on this PR, don't hesitate to reach out to us.

Hi @Smirl! The current implementation is based entirely on the work done in https://github.com/Skyscanner/applicationset-progressive-sync/. Thank you for the idea to trigger manual syncs in an automated way.

[crenshaw-dev]

I agree and would say that it would be even more interesting having a similar feature for Applications and not only for ApplicationSets.

@leoluz we already have this feature for Apps-of-Apps. :-)

[wmgroot]

I'm still getting endless syncs without prune on the latest commit.

I think this is working is intended if there's a resource to be deleted, minus the need for some kind of backoff logic to prevent the appset controller from flooding the application controller too hard. If you have a resource to be deleted, and prune disabled, the Application will remain OutOfSync and thus be valid for AppSet Rollouts to attempt to sync.

Would an Application with autosync enabled and prune disabled not produce the same behavior? Do we think that a backoff is enough to handle this problem?

[crenshaw-dev]

Would an Application with autosync enabled and prune disabled not produce the same behavior?

I don't think it would. I think it would sync on a revision change and then quit. If another revision came through, it would attempt another sync and remain out-of-sync.

If a retry is set up, it might retry. But I think retry only applies for errors, not when the sync simply doesn't bring the app completely in-sync.

I'll test what happens with automated+auto-heal.

Do we think that a backoff is enough to handle this problem?

I think a backoff is probably sufficient. Will think about it some more.

[crenshaw-dev]

@wmgroot apologies for the ignorant question, because we've definitely talked about this at some point, but... is there enough information to know that we've already requested a sync for the current rollout? If so, we could rely on the Application controller using the configured retry policy.

[wmgroot]

It's not immediately available with the current implementation. But, I would have to implement some kind of state tracking to support backoffs anyway. I think it's a good idea to leverage the existing retries from the Application controller, I'll see what I can do.

[romuduck]

@wmgroot I cant remember what s the behavior with "self-heal"
With auto-sync disabled i bet this feature is dropped am I right?

[crenshaw-dev]

@wmgroot with 2.6-RC1 being Monday, how are you feeling about merging what we have so far? I think gracefully handling sync failures is important, but this is an alpha-level feature.

[wmgroot]

@crenshaw-dev Conflicts have been addressed, I'm happy merging this now and will continue poking at some sync failure improvements.

[wmgroot]

I'm re-running codegen again.

[crenshaw-dev]

@wmgroot can I ask for one last rebase? :-)

[wmgroot]

Done.

[diversario]

I agree and would say that it would be even more interesting having a similar feature for Applications and not only for ApplicationSets.

@leoluz we already have this feature for Apps-of-Apps. :-)

Could you elaborate? Do you mean that it's possible to set argocd.argoproj.io/sync-wave on an Application resource and ArgoCD will order sync accordingly?

[jukie]

@diversario you have to add back health statuses but yes sync waves work for Applications in an apps-of-apps pattern.

[ron1]

I suspect the relevant question here is does this new appset progressive rollouts feature support sync waves for apps deployed within an app-of-appsets pattern?