search

kubernetes-sigs / prow

Prow is a Kubernetes based CI/CD system developed to serve the Kubernetes community. This repository contains Prow source code and Hugo sources for Prow documentation site.
https://docs.prow.k8s.io
Apache License 2.0
127 stars 98 forks source link

[BUG] CC multiple people does not work anymore #204

Closed gregpavl closed 3 months ago

gregpavl commented 4 months ago

What happened:

We upgraded the prow components from v20231227 to v20240515.

example: 

-        image: gcr.io/k8s-prow/prow-controller-manager:v20231227-1f5d684b52
+        image: gcr.io/k8s-prow/prow-controller-manager:v20240515-dd5d0eeaa

Before the upgrade, the command /cc @person_1 @person_2 @person_3 @person_4 was CC the GH usernames correctly (all at once) in the PRs.

After the upgrade, the CC functionality seems broken. If you try to CC multiple people, the bot will catch only the first CCed user.

What you expected to happen:

to continue CC multiple people with the same command inline at once.

How to reproduce it (as minimally and precisely as possible):

try to CC people on the same line. /cc @person_1 @person_2 @person_3 @person_4 you can even try to /cc @person_X multiple people in the same comment. It has the same behavior.

Right now, you need to do CC people in separate comments.

Eg. Comment 1: /cc @person_1 Comment 2: /cc @person_2 Comment 3: /cc @person_3 Comment 4: /cc @person_4

Please provide links to example occurrences, if any:

n/a

Anything else we need to know?:

original ref: https://github.com/kubernetes/test-infra/issues/32905

droslean commented 4 months ago

My first thought is that maybe https://github.com/kubernetes-sigs/prow/pull/164/ is related. @Okabe-Junya Any ideas?

Okabe-Junya commented 4 months ago

Upon checking the git history, it seems that the only change to the assign plugin after the migration to k-sigs/prow was my PR(#164).

I apologize if this caused any inconvenience. I have created a PR to revert #164, which might serve as a useful workaround for now.

Okabe-Junya commented 4 months ago

/kind bug

Okabe-Junya commented 4 months ago

What I don't understand is that while my PR was merged on May 28, the tag where the bug happened is v20240515. (Is this tag based on the date the image was built?)

Moreover, my PR only changed the behavior for teams, so it shouldn't have affected the /cc Alice Bob ... format command at all. All elements should result in the isTeamLogin function returning false, meaning that simply users[login] = add would be executed.

https://github.com/kubernetes-sigs/prow/blob/79d27b6e3be35974fbe103d3f574d70dfea6f03c/pkg/plugins/assign/assign.go#L137-L148

Okabe-Junya commented 4 months ago

I am not sure if this will directly resolve the bug, but I did find one bug. I misunderstood the signature of the ListTeamMembersBySlug function and made an incorrect call. I have created a PR to fix this - #206

Okabe-Junya commented 4 months ago

/assign

Okabe-Junya commented 4 months ago

@droslean

I want to check the image tag. It looks like the tag is from before my PR was merged. Do you have any idea what could be the cause?

ref. https://github.com/kubernetes-sigs/prow/issues/204#issuecomment-2209361096

What I don't understand is that while my PR was merged on May 28, the tag where the bug happened is v20240515. (Is this tag based on the date the image was built?)

Okabe-Junya commented 4 months ago

@gregpavl

I created and merged the revert PR - this issue might be resolved in the latest Prow image.

If possible, please let me know if this bug is resolved, thanks!

Okabe-Junya commented 4 months ago

ref. https://github.com/kubernetes-sigs/prow/issues/145#issuecomment-2239909009

I see that this feature was attempted and then reverted, but I am skeptical that a change here is necessary at all. I have always been able to use cc with a github team. For example, I use /cc @openshift/test-platform often in our installation, and it ccs the team itself rather than the individual members. This had stopped working for awhile, and I see now it was due to the bug described in #204.

I think the reason why issue #145 was created is that the command in the format cc @org-name/team-name could not be executed. Even after checking the source code, it appears that mentions in the format containing "/" are not handled.

https://github.com/kubernetes-sigs/prow/blob/e49eac6829a97ec9c4a6e9509e5588b87aaeada0/pkg/plugins/assign/assign.go#L122-L131

Meaning, isn't it just using GitHub's feature to mention, rather than a command from prow?

smg247 commented 3 months ago

I see that this feature was attempted and then reverted, but I am skeptical that a change here is necessary at all. I have always been able to use cc with a github team. For example, I use /cc @openshift/test-platform often in our installation, and it ccs the team itself rather than the individual members.

If this feature is attempted again, it should at least be behind a configuration so that installations that would like to retain the existing behavior are able to do so.

droslean commented 3 months ago

/close

k8s-ci-robot commented 3 months ago

@droslean: Closing this issue.

In response to [this](https://github.com/kubernetes-sigs/prow/issues/204#issuecomment-2244633033): >/close Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.
Okabe-Junya commented 3 months ago

I don't think this discussion has been resolved yet, though...

https://github.com/kubernetes-sigs/prow/issues/204#issuecomment-2241171624