argoproj / argo-cd

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

github commit status notifications not working with multi source application #15617

Open CarlosLanderas opened 11 months ago

CarlosLanderas commented 11 months ago

Checklist:

argocd: v2.8.2+dbdfc71.dirty
  BuildDate: 2023-08-25T03:59:19Z
  GitCommit: dbdfc712702ce2f781910a795d2e5385a4f5a0f9
  GitTreeState: dirty
  GoVersion: go1.21.0
  Compiler: gc
  Platform: darwin/arm64
argocd-server: v2.8.2+dbdfc71.dirty
  BuildDate: 2023-08-25T03:59:19Z
  GitCommit: dbdfc712702ce2f781910a795d2e5385a4f5a0f9
  GitTreeState: dirty
  GoVersion: go1.21.0
  Compiler: gc
  Platform: darwin/arm64
  Kustomize Version: could not get kustomize version: exec: "kustomize": executable file not found in $PATH
  Helm Version: v3.11.2+g912ebc1
  Kubectl Version: v0.24.2
  Jsonnet Version: v0.20.0

Describe the bug

When using an application with multiple sources (helm chart and manifests), github notifications do not work. And the notification controller throws the following error:

Index out of range [1] with length 1\ngoroutine 189 [running]:\nruntime/debug.Stack()\n\t/usr/local/go/src/runtime/debug/stack.go:24 +0x65\ngithub.com/argoproj/notifications-engine/pkg/controller.(*notificationController).processQueueItem.func1()\n\t/go/pkg/mod/github.com/argoproj/notifications-engine@v0.4.1-0.20230620204159-3446d4ae8520/pkg/controller/controller.go:258 +0x65\npanic({0x3788a00, 0xc000c6cc48})\n\t/usr/local/go/src/runtime/panic.go:890 +0x263\ngithub.com/argoproj/notifications-engine/pkg/services.gitHubService.Send({{{0x3211380, 0xc0019cbdf8}, {0x3211380, 0xc0019cbe08}, {0xc00075e000, 0x68f}, {0x0, 0x0}}, 0xc0004ce600}, {{0xc00037f3a0, ...}, ...}, ...)\n\t/go/pkg/mod/github.com/argoproj/notifications-engine@v0.4.1-0.20230620204159-3446d4ae8520/pkg/services/github.go:280 +0x6c9\ngithub.com/argoproj/notifications-engine/pkg/api.(*api).Send(0xc0000daf00, 0x4?, {0xc000f7bf40, 0x1, 0x4}, {{0xc00062a4f3?, 0x4?}, {0x0?, 0x0?}})\n\t/go/pkg/mod/github.com/argoproj/notifications-engine@v0.4.1-0.20230620204159-3446d4ae8520/pkg/api/api.go:71 +0x45e\ngithub.com/argoproj/notifications-engine/pkg/controller.(*notificationController).processResource(0xc0003da0e0, {0x4c37188, 0xc0002e60d0}, 0x326c980?, 0xc00060fdf8)\n\t/go/pkg/mod/github.com/argoproj/notifications-engine@v0.4.1-0.20230620204159-3446d4ae8520/pkg/controller/controller.go:217 +0x8d8\ngithub.com/argoproj/notifications-engine/pkg/controller.(*notificationController).processQueueItem(0xc0003da0e0)\n\t/go/pkg/mod/github.com/argoproj/notifications-engine@v0.4.1-0.20230620204159-3446d4ae8520/pkg/controller/controller.go:297 +0x575\ngithub.com/argoproj/notifications-engine/pkg/controller.(*notificationController).Run.func1()\n\t/go/pkg/mod/github.com/argoproj/notifications-engine@v0.4.1-0.20230620204159-3446d4ae8520/pkg/controller/controller.go:164 +0x29\nk8s.io/apimachinery/pkg/util/wait.BackoffUntil.func1(0x30?)\n\t/go/pkg/mod/k8s.io/apimachinery@v0.24.2/pkg/util/wait/wait.go:155 +0x3e\nk8s.io/apimachinery/pkg/util/wait.BackoffUntil(0x0?, {0x4be70a0, 0xc000e085d0}, 0x1, 0x0)\n\t/go/pkg/mod/k8s.io/apimachinery@v0.24.2/pkg/util/wait/wait.go:156 +0xb6\nk8s.io/apimachinery/pkg/util/wait.JitterUntil(0xc0003da0e0?, 0x3b9aca00, 0x0, 0x1?, 0xc000e0efd0?)\n\t/go/pkg/mod/k8s.io/apimachinery@v0.24.2/pkg/util/wait/wait.go:133 +0x89\nk8s.io/apimachinery/pkg/util/wait.Until(0x4c0eae0?, 0xc00005c030?, 0xc000e34400?)\n\t/go/pkg/mod/k8s.io/apimachinery@v0.24.2/pkg/util/wait/wait.go:90 +0x25\ncreated by github.com/argoproj/notifications-engine/pkg/controller.(*notificationController).Run\n\t/go/pkg/mod/github.com/argoproj/notifications-engine@v0.4.1-0.20230620204159-3446d4ae8520/pkg/controller/controller.go:163 +0xe5\n"

I've found this error described in this bug

This is a sample of that application current revisions, having a chart version and a commit id:

revisions:
   - 2.2.3
   - 7cbf03c6b6a7eb6f24c6f96dda1c047d6686035d
   - 7cbf03c6b6a7eb6f24c6f96dda1c047d6686035d

If I switch the application back to single source, github commit statuses start working again.

Notifications configuration:

  template.github-commit-status-success: |
    message: |
      Application status is {{.app.status.health.status}}.
    github:
      status:
        state: success
        label: "cd/{{.app.metadata.name}}"
        targetURL: "{{.context.argocdUrl}}/applications/{{.app.metadata.name}}?operation=true"
  trigger.on-sync-status: |
    - when: app.status.health.status == 'Healthy' and app.status.operationState != nil and app.status.operationState.phase in ['Succeeded']
      oncePer: "[app.status?.operationState?.syncResult?.revision, app.status?.operationState?.syncResult?.revisions]"
      send: [github-commit-status-success]

The trigger oncePer with "revisions" configuration is something I was trying after reading this comment but only applications with single source Repo as working with these settings.

gespi1 commented 11 months ago

oncePer: "[app.status?.sync?.revision or app.status?.sync?.revisions]" is working for me on multi-source apps. But i am not using the github integration and instead using webhooks

now i want to add more item to measure for once per. Something like oncePer: [app.status?.sync?.revision or app.status?.sync?.revisions] and app.status.operationState.finishedAt but i dont know how to implement it properly. It doesnt like the brackets

CarlosLanderas commented 10 months ago

oncePer: "[app.status?.sync?.revision or app.status?.sync?.revisions]" is working for me on multi-source apps. But i am not using the github integration and instead using webhooks

now i want to add more item to measure for once per. Something like oncePer: [app.status?.sync?.revision or app.status?.sync?.revisions] and app.status.operationState.finishedAt but i dont know how to implement it properly. It doesnt like the brackets

I've tried with your proposed oncePer definition and I receive the same out of range error.

I've seen github service in the notification engine has not added support to multi source repoUrls

https://github.com/argoproj/notifications-engine/blob/master/pkg/services/github.go#L64

jsoref commented 10 months ago

I'd expect this not to crash this way once ArgoCD updates to the next release of notifications-controller.