rhtap: fix update-deployment condition #1452
Conversation
The update-deployment task is supposed to run on push, but has been reported to run for MR comments as well. To fix this, rather than excluding the task for "pull_request" and "Merge Request" event types, run the task only for "push" and "Push" event types. --- Based on the pipelines-as-code codebase [1], mainly pkg/provider/*/parse_payload.go, the possible events and their corresponding event_type values are: | | anything except gitlab | gitlab |----------------|-----------------------------------|--------------- | PR | pull_request | Merge Request | PR comment | pull_request* | pull_request** | push | push | Push | tag push | push | Tag Push | commit comment | push (only github) | N/A | rerun button | pull_request / push (only github) | N/A * Depending on the Pipelines as Code version, PR comments may get categorized into many different event_types [2]. In version v0.27.2 (latest as of the date of this commit), they result in a pull_request event. See https://issues.redhat.com/browse/SRVKP-5775. ** In version v0.27.2, gitlab MR comments get the pull_request event_type due to the workaround for SRVKP-5775. Prior to version v0.25.0, they used to get the Note event_type. Between these versions, the would get one of the comment-specific event types [2]. --- TLDR: the user who noticed the task running for MR comments must have been using Pipelines as Code < v0.27.2, where the event_type would be {something}-comment (or Note, if < v0.25.0). Fix the condition to work for all PaC version. [1]: https://github.com/openshift-pipelines/pipelines-as-code [2]: https://pipelinesascode.com/docs/guide/gitops_commands/#event-type-annotation-and-dynamic-variables Signed-off-by: Adam Cmiel <[email protected]>
chmeliik
force-pushed
the
fix-rhtap-update-deployment-condition
branch
from
bed985c to
67c7d44
Compare
| operator: notin | ||
| values: ["pull_request", "Merge Request"] | ||
| operator: in | ||
| values: |
There was a problem hiding this comment.
Do we need a value in the list for incoming request (pipeline rerun?)
There was a problem hiding this comment.
I didn't find a specific event type for that, let me check again
There was a problem hiding this comment.
From the PaC docs here:
kind: PipelineRun
metadata:
annotations:
pipelinesascode.tekton.dev/on-event: "[incoming]"
There was a problem hiding this comment.
Hmm, I don't really understand how the incoming requests work. Since the RHTAP PipelineRun are configured with on-event: ["push"], would they even trigger for an incoming request? Can we assume that an incoming request is for a push pipeline, not a PR pipeline? And does RHTAP ever send any incoming requests?
TLDR, I would rather not add it by default
There was a problem hiding this comment.
I'm not sure if RHTAP uses it. With exclude PR events it was theoretically working.
There was a problem hiding this comment.
True... well, let's see what happens
RHTAPBUGS-1307
The update-deployment task is supposed to run on push, but has been
reported to run for MR comments as well. To fix this, rather than
excluding the task for "pull_request" and "Merge Request" event types,
run the task only for "push" and "Push" event types.
Based on the pipelines-as-code codebase 1, mainly
pkg/provider/*/parse_payload.go, the possible events and their
corresponding event_type values are:
* Depending on the Pipelines as Code version, PR comments may get
categorized into many different event_types 2. In version v0.27.2
(latest as of the date of this commit), they result in a pull_request
event. See https://issues.redhat.com/browse/SRVKP-5775.
** In version v0.27.2, gitlab MR comments get the pull_request
event_type due to the workaround for SRVKP-5775. Prior to version
v0.25.0, they used to get the Note event_type. Between these
versions, the would get one of the comment-specific event types 2.
TLDR: the user who noticed the task running for MR comments must have
been using Pipelines as Code < v0.27.2, where the event_type would be
{something}-comment (or Note, if < v0.25.0). Fix the condition to work
for all PaC version.