SyncWindow should allow to have multiple sync windows which has same kind,schedule,duration

[keyolk]

Checklist:

• [x] I've searched in the docs and FAQ for my answer: https://bit.ly/argocd-faq.
• [x] I've included steps to reproduce the bug.
• [x] I've pasted the output of argocd version.

Describe the bug

Currently, it does not allow having multiple sync windows which have same kind+schedule+duration combination Let's say I want to have sync windows like the below

[
  {
    "kind": "allow",
    "schedule": "0 0 * * *",
    "duration": "6h",
    "applications": [
      "two*"
    ],
    "timeZone": "Asia/Seoul"
  },
  {
    "kind": "allow",
    "schedule": "0 0 * * *",
    "duration": "6h",
    "applications": [
      "one*"
    ],
    "timeZone": "Asia/Seoul"
  },
...

It would stuck at project validation step at https://github.com/argoproj/argo-cd/blob/master/pkg/apis/application/v1alpha1/app_project_types.go#L261

We can mitigate this by just splits it by minutes, but hope it been resolved to make it simple to manage.

To Reproduce

Just create multiple sync windows having the same kind+schedule+duration

Expected behavior

Allow to have multiple time duplicated sync windows

Version

argocd: v2.11.1+9f40df0
  BuildDate: 2024-05-21T22:43:41Z
  GitCommit: 9f40df0c29eca7e45a73f802f033dfd1ed0068e3
  GitTreeState: clean
  GoVersion: go1.22.3
  Compiler: gc
  Platform: darwin/amd64
argocd-server: v2.11.5+c4b283c
  BuildDate: 2024-07-15T17:39:54Z
  GitCommit: c4b283ce0c092aeda00c78ae7b3b2d3b28e7feec
  GitTreeState: clean
  GoVersion: go1.21.10
  Compiler: gc
  Platform: linux/arm64
  Kustomize Version: v5.2.1 2023-10-19T20:13:51Z
  Helm Version: v3.14.4+g81c902a
  Kubectl Version: v0.26.11
  Jsonnet Version: v0.20.0

[andrii-korotkov-verkada]

Actually the example above should work due to using different durations. If kind + schedule + duration is the same, why not just merge applications lists and have one rule? We might want to add timeZone to the key tho.

[keyolk]

@andrii-korotkov-verkada just updated the example, we holds per app sync windows cause it is much easy to maintain sync window for the apps. if a sync window is shared from multiple apps, it would need much work to push and pull the apps from the sync windows.

[andrii-korotkov-verkada]

I mean, what's the difference between the updated example and this?

  {
    "kind": "allow",
    "schedule": "0 0 * * *",
    "duration": "6h",
    "applications": [
      "one*",
      "two*"
    ],
    "timeZone": "Asia/Seoul"
  }

There are wildcards used anyways.

[keyolk]

in the case above, when the sync window of one* should be updated, we can't just update its schedule. It is just about thinking it by application or schedule.

[andrii-korotkov-verkada]

If you need a partial update, then you can split the configuration into multiple. But until then I think it's not necessary.

[keyolk]

Currently, I split it with minutes. Just hope someday I can remove the minute parts later. If the # of applications goes thousands and more it would make more contention. Yes, we can split it again with updating duration as well, but would better make people think in simple way.

[andrii-korotkov-verkada]

We can add an additional optional field like name and use it as a part of a key, but that'd increase the complexity and I'm not sure it's worth it - you'd have to specify a lot of extra fields.

[keyolk]

I see, but hope to have the optional field. I think it is not that huge change.