Define and implement entry/exit criteria for kubernetes-maintainers GitHub team

[nikhita]

The legacy kubernetes-maintainers GitHub team has write access to the k/k repo. This issue is to discuss pruning the team.

There has been previous discussion about removing the team in kubernetes/kubernetes#57667. Given that we don't have good automated solutions around editing issue/PR bodies, completely removing write access and deleting the team is not feasible today.

Having said that, the kubernetes-maintainers team hasn't had any significant updates over the last ~3 years (!!!). There are members in that team who are no longer active in the Kubernetes project and should not have write access to k/k anymore.

My suggestion - we could limit this to milestone-maintainers + all approvers in k/k.

If there is general consensus on this GitHub issue, I'll start a thread on the k-dev and leads mailing lists.

[nikhita]

cc @kubernetes/owners
GitHub admin team

cc @dims @derekwaynecarr @johnbelamaric
sig-arch leads

cc @liggitt @sttts @deads2k @tpepper @justaugustus
who've had opinions about this team before (in kubernetes/kubernetes#57667 + #2330)

[mrbobbytables]

+1. It's been needed for some time.

[dims]

+1 to prune. isn't milestone-maintainers a larger group?

[idvoretskyi]

+1

[liggitt]

My suggestion - we could limit this to milestone-maintainers + all approvers in k/k.

Is this suggesting we remove current members of the kubernetes-maintainers list if either of the following are true:

• they are not in milestone-maintainers
• they are not an approver in k/k

If so, that seems very reasonable to me and moves in the desired direction of shrinking the list.

[nikhita]

Is this suggesting we remove current members of the kubernetes-maintainers list if either of the following are true:

• they are not in milestone-maintainers
• they are not an approver in k/k

Yes 👍

[nikhita]

To clarify - we won't bulk-add milestone-maintainers + k/k approvers if they aren't already in the kubernetes-maintainers GitHub team.

If someone in milestone-maintainers + k/k approvers wants to be on the team, they can create a PR to add themselves separately.

Want to keep the team as small as possible.

[dims]

@nikhita @liggitt sounds like a good plan. Let's do this!

[justaugustus]

One note

If someone in milestone-maintainers + k/k approvers wants to be on the team, they can create a PR to add themselves separately.

What would be the approval process here?
My one concern is that Release Team members (specifically Release Team shadows) are on the milestone-maintainers group and I'm not in favor of them getting write access to k/k without some process in place.

[nikhita]

Thinking more about this, how about:

• If an existing member of kubernetes-maintainers is not a k/k approver, we remove them from the team
• We don't add any new members to the team

[justaugustus]

* If an existing member of `kubernetes-maintainers` is not a k/k approver, we remove them from the team

I like this.

* We don't add any new members to the team

Hmmmmm, so I think there should be a route to write access.

@dims is not a root approver for k/k, but I do feel he's deserving of the access that was granted in #2330.

How can we cover those cases in future?

[fejta-bot]

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-contributor-experience at kubernetes/community.
/lifecycle stale

[justaugustus]

/remove-lifecycle stale

[BenTheElder]

NOTE: due to branch protection write isn't really write to the repo unless you are in the admin superset which is very small / restricted.

We should prune inactive members.

I disagree that we should refuse to add anyone new. Being able to manually correct PR state (PR body, labels) is quite useful, and we should be able to trust people with this (approvers or maybe SIG chairs / leads).

EDIT: write permission gates pretty much any mutable state of any sort (labels, titles, milestones, etc.), well beyond the repo contents. While branch protection turns around and restricts the part of write most people probably think about (pushing/merging to branches).

[justaugustus]

What requirements do triagers have that could not be covered with the Triage role?

Triage: Recommended for contributors who need to proactively manage issues and pull requests without write access

ref: https://docs.github.com/en/github/setting-up-and-managing-organizations-and-teams/repository-permission-levels-for-an-organization

[spiffxp]

I'm fine trying the triage role but would prefer we ensure peribolos has support for that. Unclear to me how actively it's used elsewhere

[spiffxp]

https://kubernetes.slack.com/archives/C1L57L91V/p1614017710014100 based on k/enhancements experience with the triage role, I don't think it meets our needs here (inability to edit issue/pr descriptions)

[spiffxp]

We need to decide on policy to prune, and policy to add. Then implement. Summarizing what I've read above from @nikhita @liggitt @BenTheElder @justaugustus

Possible reasons to prune:

• A: is not an approver somewhere in k/k
• B: is not a member of milestone-maintainers
• C: has not used superpowers in previous N months (superpowers: edit issue/title description, since this is something Triage cannot do)
• D: all of the above

Would like to understand what the reduction would be based on each of these. My preference would be C. If only I knew how to measure it at a glance...

Requirements to be added:

• A: !(A or B from above) aka "is an approver somewhere in k/k AND is a member of milestone-maintainers"
• B: +1 from a root approver
• C: all of the above
• D: none of the above, nobody new gets added

My preference would be C

[spiffxp]

I don't think Triage as a role is really helpful for us:

• Apply/dismiss labels - it can do this, so it can bypass OWNERS by directly adding the approved label
• Edit and delete anyone's comments on commits, pull requests, and issues - it can't do this, so it wouldn't help correct release-note foibles

[spiffxp]

/retitle Define and implement entry/exit criteria for kubernetes-maintainers GitHub team

[spiffxp]

/help
We could really use someone's help in measuring the impact of reasons to prune: #2332 (comment)

A: parse all OWNERS files in kubernetes/kubernetes

• I have done this in the past with find and yq in a pinch, or python with glob and ruamel.yaml when I needed to parse OWNERS_ALIASES too
• Another maybe easier approach is write golang that uses k8s.io/test-infra/prow/repoowners

B: diff the two teams' membership

• can use files in kubernetes/org, or github's api

C: this is the one I'm not sure about

• does a github search query for involves: spiffxp pick up issues I have edited (but not commented on, reviewed, or been mentioned)? -> can implement script based on "what this member has done on issues/PRs updated in last N months"
• is there a GitHub API way of identifying edits to an issue/PR?

D: intersection of the above

• if all done in the same script/golang, whatever boolean logic suits you best
• silly bash way of doing intersection: dump "who gets pruned" into a file for each of the above, then cat pruned-by-[ABC].txt | sort | uniq -c | grep '^[ ]*3

[k8s-ci-robot]

@spiffxp:
This request has been marked as needing help from a contributor.

Please ensure the request meets the requirements listed here.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-help command.

In response to this:

/help
We could really use someone's help in measuring the impact of reasons to prune: #2332 (comment)

A: parse all OWNERS files in kubernetes/kubernetes

• I have done this in the past with find and yq in a pinch, or python with glob and ruamel.yaml when I needed to parse OWNERS_ALIASES too
• Another maybe easier approach is write golang that uses k8s.io/test-infra/prow/repoowners

B: diff the two teams' membership

• can use files in kubernetes/org, or github's api

C: this is the one I'm not sure about

• does a github search query for involves: spiffxp pick up issues I have edited (but not commented on, reviewed, or been mentioned)? -> can implement script based on "what this member has done on issues/PRs updated in last N months"
• is there a GitHub API way of identifying edits to an issue/PR?

D: intersection of the above

• if all done in the same script/golang, whatever boolean logic suits you best
• silly bash way of doing intersection: dump "who gets pruned" into a file for each of the above, then cat pruned-by-[ABC].txt | sort | uniq -c | grep '^[ ]*3

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.

[justaugustus]

/help
We could really use someone's help in measuring the impact of reasons to prune: #2332 (comment)

@kubernetes/release-engineering -- Anyone interested in helping out here?

[fejta-bot]

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-contributor-experience at kubernetes/community.
/lifecycle stale

[nikhita]

/remove-lifecycle stale

[BenTheElder]

I have also recently found it more productive to fix PR bodies that do not meet our template criteria after one attempt at telling the author to please do this than lengthy back and forth (especially with time zone latency this can become very drawn out). Things like the release-note "language" code block trip people up and are trivial for a maintainer with write to fix.

We have a lot of things built around expecting PR metadata to be just so and everything is built around having the source of truth be in PRs themselves. Otherwise I'd suggest redesigning the mechanisms. E.g. cherry picks rely on parent PR's note so it would be somewhat prohibitive to move it to say scanning all comments on the PR.

In addition to brach protection we also monitor pushes and alert in other repos, we should be doing this here.

[spiffxp]

/milestone v1.23

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue or PR as fresh with /remove-lifecycle stale
• Mark this issue or PR as rotten with /lifecycle rotten
• Close this issue or PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[k8s-triage-robot]

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue or PR as fresh with /remove-lifecycle rotten
• Close this issue or PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

[justaugustus]

/remove-lifecycle rotten
/lifecycle frozen

[BenTheElder]

I have also recently found it more productive to fix PR bodies that do not meet our template criteria after one attempt at telling the author to please do this than lengthy back and forth (especially with time zone latency this can become very drawn out). Things like the release-note "language" code block trip people up and are trivial for a maintainer with write to fix.

TIL https://prow.k8s.io/command-help /release-note-edit (which ... can't be linked to ... that's broken ...)

It looks ... awkward to use (multi-line??) but ... one possible option for that aspect ... if we make people aware of it. I'm also not sure if this works in the face of mangling the "language" as mentioned previously, I think it just substitutes the contents of an already correctly formatted block.

Still, a somewhat promising thought.

In addition to brach protection we also monitor pushes and alert in other repos, we should be doing this here.

we should still really have this