Test-infra: k8s-ci-robot shouldn't label "sig/api-machinery" if k8s-merge-robot comments "@kubernetes/sig-api-machinery-misc"
k8s-merge-robot comments on every issues which don't have a sig label. The comment includes e.g., @kubernetes/sig-api-machinery-misc for API Machinery(this example was added in #3102 as suggested in #3060). This comment leads to k8s-ci-robot labels "sig/api-machinery" for every issues. This is not expected. We should find a better way to address it.
/cc @@grodrigues3 @spxtr @crimsonfaith91
xiangpengzhao
All 26 comments
This has already been addressed by this PR: https://github.com/kubernetes/test-infra/pull/3174
crimsonfaith91
on 23 Jun 2017
@crimsonfaith91 thanks for pointing it out. It would be great to merge it and deploy ASAP.
xiangpengzhao
on 23 Jun 2017
got lgtm now.
crimsonfaith91
on 23 Jun 2017
merged. i will ask around whether now is the right time to deploy the change.
crimsonfaith91
on 23 Jun 2017
:+1:
xiangpengzhao
on 23 Jun 2017
The change is merged, but we still need permission for deployment. i will follow closely on this.
crimsonfaith91
on 24 Jun 2017
Seems like the issue still exists after the patch is deployed. See: https://github.com/kubernetes/kubernetes/issues/48020
@crimsonfaith91
xiangpengzhao
on 25 Jun 2017
The change hasn't deployed yet. The test-infra team moves prow to another submit queue recently. I am not able to access it.
crimsonfaith91
on 25 Jun 2017
Please follow this: https://github.com/kubernetes/kubernetes/issues/47941#issuecomment-310703871
I am not able to do anything until I obtain the access to the submit queue. I will inform you when it is deployed. Thanks!
crimsonfaith91
on 25 Jun 2017
@krzyzacy please redeploy the submit queue when you get a chance.
spxtr
on 25 Jun 2017
@spxtr Thanks a lot!
crimsonfaith91
on 25 Jun 2017
@spxtr I have no idea about submit-queue yet, I'll work with @cjwagner Monday. It probably should be documented somewhere so that we can all be able to do this.
krzyzacy
on 25 Jun 2017
@crimsonfaith91 But I see that there is already quote for @kubernetes/sig-api-machinery-misc in the newest comment of k8s-merge-robot.
see: https://github.com/kubernetes/kubernetes/issues/48020#issuecomment-310838866
xiangpengzhao
on 25 Jun 2017
The latest issue: https://github.com/kubernetes/kubernetes/issues/48025#issuecomment-310878746
xiangpengzhao
on 25 Jun 2017
@xiangpengzhao Quotation should not trigger any sig mentioning. It means that it is due to another issue. I am investigating it now.
@spxtr @krzyzacy @grodrigues3 It looks like the changes have been deployed. Thanks for the help! :)
crimsonfaith91
on 25 Jun 2017
@spxtr @krzyzacy @xiangpengzhao @grodrigues3
The behavior is not due to sig-mention munger, but the regex for label plugin: https://github.com/kubernetes/test-infra/blob/master/prow/plugins/label/label.go#L47
Based on how the regex is written, a notification will be triggered even it is quoted or tailed with other words behind. I will get the regex fixed tomorrow. Thanks!
crimsonfaith91
on 25 Jun 2017
/assign @crimsonfaith91
crimsonfaith91
on 25 Jun 2017
I created another issue to discuss about the label plugin regex. For now, IMO it may be best to have:
There are no sig labels on this issue. Please add a sig label by:
(1) mentioning a sig: @kubernetes/sig-<team-name>-misc
e.g., sig-api-machinery-misc for API Machinery
(2) specifying the label manually: /sig <label>
e.g., /sig scalability for sig/scalability
_Note: method (1) will trigger a notification to the team. You can find the team list here and label list here_
crimsonfaith91
on 26 Jun 2017
agreed. This is also clear enough.
xiangpengzhao
on 26 Jun 2017
yup! thanks!
crimsonfaith91
on 26 Jun 2017
I went over PRs to remove unnecessary api-machinery label manually. It is great that the affected PRs are in small amount of number (going over the PRs took only 10 minutes). I will keep an eye on new PRs rolling out before #3201 is merged.
crimsonfaith91
on 26 Jun 2017
Resolved! Final bot comment:
There are no sig labels on this issue. Please add a sig label by:
(1) mentioning a sig: @kubernetes/sig-<team-name>-misc
e.g., @kubernetes/sig-api-machinery-* for API Machinery
(2) specifying the label manually: /sig <label>
e.g., /sig scalability for sig/scalability
_Note: method (1) will trigger a notification to the team. You can find the team list here and label list here_
crimsonfaith91
on 27 Jun 2017
/close
crimsonfaith91
on 27 Jun 2017
Great! It would make sig-api-machinery folks life happy:)
But I still doubt whether contributors know what the * represents in @kubernetes/sig-api-machinery-*...
Can we add something more, as below.
e.g., @kubernetes/sig-api-machinery-* for API Machinery, * can be misc, bugs, test-failures
We may also want to make the docs of sigs more clear.
xiangpengzhao
on 27 Jun 2017
Good idea! There is another issue opened yesterday for this: https://github.com/kubernetes/test-infra/issues/3190
crimsonfaith91
on 27 Jun 2017
:+1:
xiangpengzhao
on 27 Jun 2017
Related issues
fen4o
路
4Comments
spzala
路
4Comments
stevekuznetsov
路
4Comments
chaosaffe
路
3Comments
cjwagner
路
3Comments
Most helpful comment
@spxtr @krzyzacy @xiangpengzhao @grodrigues3
The behavior is not due to sig-mention munger, but the regex for label plugin: https://github.com/kubernetes/test-infra/blob/master/prow/plugins/label/label.go#L47
Based on how the regex is written, a notification will be triggered even it is quoted or tailed with other words behind. I will get the regex fixed tomorrow. Thanks!