notifications-engine: oncePer condition logs expected error

In argocd notifications, I use the following trigger to filter notifications with the oncePer option.

    trigger.on-deployed: |
      - description: Application is synced and healthy. Triggered once per commit.
        when: app.status.health.status == 'Healthy' and app.status.operationState != nil and app.status.operationState.phase in ['Succeeded']
        oncePer: app.status.operationState.syncResult.revision
        send:
        - app-deployed

This returns the following error in the logs

{"level":"error","msg":"failed to execute oncePer condition: cannot fetch syncResult from \u003cnil\u003e (1:27)\n | app.status.operationState.syncResult.revision\n | ..........................^","time":"2022-01-28T15:41:46Z"}

Solution

I think the oncePer condition should not be evaluated when the when condition evaluate to false.

https://github.com/argoproj/notifications-engine/blob/b9d7d6c233cc0c52b9c731f2ce1e909794c4347c/pkg/triggers/service.go#L94

About this issue

Original URL
State: closed
Created 2 years ago
Reactions: 18
Comments: 16

Commits related to this issue

fix: oncePer condition only checked when `when` condition is true (#71) (#208) * fix: oncePer condition only checked when ` when` condition is true Signed-off-by: Anthony Mikhail <anthony.mikhail@... — committed to argoproj/notifications-engine by lol3909 8 months ago

Most upvoted comments

I suggest using the following condition:

oncePer: "[app.status?.operationState?.syncResult?.revision, app.status?.operationState?.syncResult?.revisions]"

This also directly supports multi source apps.

woehrl01 on Jun 23, 2023

Im experiencing something similar:

- description: Application is synced and healthy. Triggered once per commit.
              oncePer: app.status.operationState.syncResult.revision and app.metadata.name
              send:
              - app-deployed
              when: app.status.operationState.phase == 'Succeeded' and app.status.health.status == 'Healthy'

We are using a monorepo for multiple applications and environments. oncePer: app.status.operationState.syncResult.revision and app.metadata.name -> This prevents each application from spamming logs everytime any env or app changes. Its working, but the notification service is spamming logs:

time="2022-11-30T15:05:56Z" level=error msg="failed to execute oncePer condition: cannot fetch revision from <nil> (1:23)
 | app.status.syncResult.revision and app.metadata.name
 | ......................^"

DavidConnack on Nov 30, 2022

Hitting the same issue. Here is our on-sync-failed trigger definition (copied from the official Triggers and Templates catalog):

      trigger.on-sync-failed: |
        - description: Application syncing has failed
          send:
          - app-sync-failed
          when: app.status.operationState.phase in ['Error', 'Failed']

The notification controller send those logs:

failed to execute when condition: cannot fetch phase from <nil> (1:27)
 | app.status.operationState.phase in ['Error', 'Failed']
 | ..........................^

Version: v2.7.2

julien-aubert on May 26, 2023

Hitting the same issue

argocd-notifications-controller-7c8dcfd677-72wrh argocd-notifications-controller time="2023-03-10T16:08:06Z" level=error msg="failed to execute when condition: cannot fetch phase from <nil> (1:27)\n | app.status.operationState.phase in ['Error', 'Failed']\n | ..........................^"

  trigger.on-sync-failed: |
    - description: Application syncing has failed
      send:
      - app-sync-failed
      when: app.status.operationState.phase in ['Error', 'Failed']

version: v2.6.4+7be094f

tchellomello on Mar 10, 2023

Same error with defaults triggers from catalog: Version: 2.7.2

notifications-controller logs: cannot fetch phase from <nil> (1:27)\n | app.status.operationState.phase in ['Succeeded']\n | ..........................^"

luciano-buono on Jun 5, 2023