argoproj / argo-cd

Declarative Continuous Deployment for Kubernetes
https://argo-cd.readthedocs.io
Apache License 2.0
16.73k stars 5.07k forks source link

applicationset controller crashes when rollingSync steps missing #18817

Open ashutosh16 opened 1 week ago

ashutosh16 commented 1 week ago

In applicationset, when using the rollingSync strategy, the user accidentally removed the steps that caused the controller start crashing.

To Reproduce https://argo-cd.readthedocs.io/en/stable/operator-manual/applicationset/Progressive-Syncs/#rollingsync

set the strategy: rollingSync: {}

Expected behavior The controller errored out the applicationset when incorrect configuration issue but the controller shouldn't never crash due to misconfiguration.

Logs

application-set step list:","time":"2024-06-25T14:34:24Z"}
{"applicationset":{"Namespace":"dev-","Name":"*-application-set"},"level":"info","msg":"Application allowed to sync before maxUpdate?: map[]","time":"2024-06-25T14:34:24Z"}
panic: runtime error: index out of range [0] with length 0 [recovered]
panic: runtime error: index out of range [0] with length 0
goroutine 191 [running]:
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Reconcile.func1()
/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.14.7/pkg/internal/controller/controller.go:119 +0x1e5
panic({0x3ab57e0?, 0xc001fa1740?})
/usr/local/go/src/runtime/panic.go:914 +0x21f

 msg: Application allowed to sync before maxUpdate?: map[]
christianh814 commented 1 week ago

I agree that it shouldn't crash, but I do wonder what the behavior should be. I think the lack of configuration (i.e. rollingSync: {}) should make the ApplicationSet controller "fall back" as if progressive syncs weren't enabled.

agaudreault commented 1 week ago

The strategy is configured as

  strategy:
    type: RollingSync
    rollingSync: {}

Probably the behavior should be the same as one step that does not match any applications.

  strategy:
    type: RollingSync
    rollingSync:
      steps:
        - matchExpressions:
            - key: foo
              operator: In
              values:
                - a-value-that-does-not-match-anything

Not sure what happens when applications are not selected by any matchExpression, are they updated last? If so, it ends up to the same behavior @christianh814 described, while following the specified type: RollingSync codepath.

Unless there is an existing validation that all applications must be part of at least one step, then I think this behavior makes sense.

The doc should be updated to explain the behavior when an app is not selected in any steps.