Test-infra: k8s-ci-robot closes PR in different repo without right to reopen it
This has happened in https://github.com/kubernetes/heapster/pull/1844. Author has referenced in one of his comments PR from the test-infra. Once PR in test-infra was merged original PR was closed by the k8s-ci-robot, and looks like no one has a permission to reopen it.
loburm
All 31 comments
/cc @cjwagner
BenTheElder
on 18 Oct 2017
is the original comment text something like fixes kubernetes/heapster#1844? This is a github thing afaik
krzyzacy
on 18 Oct 2017
regardless of why it was closed, they cannot reopen the PR, which is itself a bug
BenTheElder
on 18 Oct 2017
I just checked the logs and it doesn't look like Prow closed the PR, I think its because the issue was linked in the PR body.
As to why the /reopen command isn't working:
"status code 422 not one of [200], body: {"message":"Validation Failed","errors":[{"resource":"PullRequest","code":"custom","field":"state","message":"state cannot be changed. The bump_kubernetes_to_1.8 branch was force-pushed or recreated."}],"documentation_url":"https://developer.github.com/v3/pulls/#update-a-pull-request"}"
The relevant bit:
state cannot be changed. The bump_kubernetes_to_1.8 branch was force-pushed or recreated.
cjwagner
on 18 Oct 2017
This nailed us as well ... :(
It is reading all of the linked issues and closing them
chrislovecnm
on 18 Oct 2017
This is just an annoying GitHub feature. If a PR body or commit message says "fixes someorg/somerepo#12345" then when that PR is merged GitHub will close the thing. This is particularly annoying in commit messages, because sometimes when I sync my k/k fork I'll close random issues in other repos.
spxtr
on 18 Oct 2017
@cjwagner ah, I think I have an open issue about catching that error and reporting it to the user.
BenTheElder
on 18 Oct 2017
BenTheElder
on 18 Oct 2017
I did not do that, don't blame me :disappointed: How about I tell you a joke
/joke
k8s-ci-robot
on 18 Oct 2017
@k8s-ci-robot: I bought shoes from a drug dealer once. I don't know what he laced them with, but I was tripping all day.
In response to this:
I did not do that, don't blame me :disappointed: How about I tell you a joke
/joke
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.
k8s-ci-robot
on 18 Oct 2017
This is just an annoying GitHub feature. If a PR body or commit message says "fixes someorg/somerepo#12345" then when that PR is merged GitHub will close the thing. This is particularly annoying in commit messages, because sometimes when I sync my k/k fork I'll close random issues in other repos.
Yah this may be different. The issue was just mentioned, and the merge bot closed the issue NOT GitHub.
chrislovecnm
on 18 Oct 2017
the merge bot closed the issue NOT GitHub.
GitHub closes it as whoever merged the PR or pushed the commit. See this example where I supposedly closed an issue. Actually all I did was sync my fork of kubernetes.
The issue was just mentioned
That's strange. Maybe the author edited the PR body?
spxtr
on 18 Oct 2017
I'm ok with closing issues by PR, but why PR A is closed when PR B is merged, considering that PR B was just mentioned in PR A. By mentioned I mean no one used keyword as fixes, closes, etc.
loburm
on 19 Oct 2017
This is a GitHub feature. "k8s-ci-robot closed this in..." is misleading.
The robot merged the PR which caused GitHub to close the mentioned PR in
response because to GitHub PRs are also issues. We cannot change this
behavior unfortunately since it is internal to GitHub.
On Oct 19, 2017 00:29, "Marian Lobur" notifications@github.com wrote:
I'm ok with closing issues by PR, but why PR A is closed when PR B is
merged, considering that PR B was just mentioned in PR A. By mentioned I
mean no one used keyword as fixes, closes, etc.—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/kubernetes/test-infra/issues/5032#issuecomment-337823321,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AA4Bq71CE026nh2hAMBcYI3DRnTjuPdlks5stvpogaJpZM4P7uOu
.
BenTheElder
on 19 Oct 2017
If the robot had just closed it on its own you would see something like "closed by k8s-ci-robot" whereas "@user closed this in
BenTheElder
on 19 Oct 2017
Wow, that's really terrible feature. I'm surprised that I can't actually mention other PR's from the comments of different PR.
@BenTheElder Thanks for the explanation, I think that we can close it now.
loburm
on 19 Oct 2017
@loburm I think the problem is mention fix|fixes #some-pr, mention PR only is definitely fine. That original PR comment from https://github.com/kubernetes/test-infra/pull/5009#issue-265649379 might contain fix before author edited it?
krzyzacy
on 19 Oct 2017
@krzyzacy @loburm I'm pretty sure if you mention a PR in a comment it will be fine if you don't say fix|fixes, but I think you also might need to not link to it in the PR's opening comment.
BenTheElder
on 19 Oct 2017
We should disallow commit messages with fixes #foo
fejta
on 19 Oct 2017
We should disallow commit messages with fixes #foo
Heh, I wish.
spxtr
on 20 Oct 2017
We could block PRs with commits that contain messages like that. It could just manage a new do-not-merge/ label.
cjwagner
on 20 Oct 2017
The "annoying feature" https://help.github.com/articles/closing-issues-using-keywords/
When the pull request is merged into your repository's default branch, the corresponding issue is automatically closed.
spiffxp
on 20 Oct 2017
The coresponding issue? This is not corresponding. This smells like a github bug
chrislovecnm
on 23 Oct 2017
https://github.com/kubernetes/features/issues/383 :(
Can we re-raise this with GitHub? Does anyone have an existing point of contact with them?
BenTheElder
on 13 Dec 2017
Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
If this issue is safe to close now please do so with /close.
Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale
fejta-bot
on 13 Mar 2018
Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
If this issue is safe to close now please do so with /close.
Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten
/remove-lifecycle stale
fejta-bot
on 12 Apr 2018
/lifecycle frozen
/remove-lifecycle rotten
chrislovecnm
on 12 Apr 2018
Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
If this issue is safe to close now please do so with /close.
Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale
fejta-bot
on 19 Sep 2018
Recently we've had people looking at blocking merges that would trigger this behavior cc @nikhita
/remove-lifecycle stale
BenTheElder
on 19 Sep 2018
/close
Closing in favor of https://github.com/kubernetes/test-infra/issues/9360 which aims to prevent this from happening
spiffxp
on 19 Sep 2018
@spiffxp: Closing this issue.
In response to this:
/close
Closing in favor of https://github.com/kubernetes/test-infra/issues/9360 which aims to prevent this from happening
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.
k8s-ci-robot
on 19 Sep 2018
Related issues
fejta
·
4Comments
zacharysarah
·
3Comments
sjenning
·
4Comments
stevekuznetsov
·
4Comments
xiangpengzhao
·
3Comments
Most helpful comment
@k8s-ci-robot: I bought shoes from a drug dealer once. I don't know what he laced them with, but I was tripping all day.
In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.