Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move to community membership model closer to kubernetes one #15593

Merged
merged 1 commit into from
Apr 5, 2023

Conversation

serathius
Copy link
Member

Based on https://github.com/kubernetes/community/blob/master/community-membership.md

Changes:

  • Extracted contributor membership to separate file
  • Provide more detailed requirements for each role. Base maintainers on kubernetes subproject owners.
  • Introduction of member role

cc @ahrtr @ptabor @spzala @mitake

GOVERNANCE.md Show resolved Hide resolved

### Requirements

- Have made multiple contributions to the project or community. Contribution may include, but is not limited to:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kubernetes org puts a time box requirement on their definition of active member:

Inactive members are defined as members of one of the Kubernetes Organizations with no contributions across any organization within 18 months. This is measured by the CNCF DevStats project.

There is a whole section about it in their document but I am wondering if to keep things simple we could just say the time period as part of our requirements.

Suggested change
- Have made multiple contributions to the project or community. Contribution may include, but is not limited to:
- Have made multiple contributions to the project or community in the last 18 months. Contribution may include, but is not limited to:

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't want to touch removing members at this time. For maintainers we already have a process described below with 12 months deadline. For now we want to onboard new members, let's tackle removal as a follow up.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have also this thought. "Triage" permissions is a nothing that put's organisation on risk.
If we had auto-test running -> it's something that could be used for abuse... but I wouldn't solve not existing yet problem.


Contributor roles and responsibilities were written based on [Kubernetes community membership]

[MAINTAINERS]: /MAINTAINERS
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if at some point in the future we also want to adopt the OWNERS file approach that Kubernetes uses, and then use this to drive some more automation for Etcd. I think that should be done as a separate issue later on but curious what others think about going in this direction.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No strong opinion on this, but be good to discuss it separately.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that would be overkill for the project. The current structure seems good enough, but yes agree that it can be discussed separately in the future.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my opinion using OWNERS are great for a big project to scale reviews, however I don't think etcd project will or should grow further. We are also already splitting the codebase into separate repos which allows us to avoid per directory owners.

I think that adding OWNERS is not aligned with project direction which is to simplify and reduce size of codebase.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm seeing OWNERS as a way to give 'approvers' privileges to less sensitive parts of the code-base like tests, tools or docs that might be an additional engagement tool.
I agree with direction to limit scope of the code-base, but even if we cut 'proxy' out of etcd-io/etcd repo and put it as etcd-io/etcd-proxy, than it might make sense (if anyone is willing to) to have separate approvers/writers for this repo (this means we would have sub-project OWNERS, but not implemented using the OWNERs file yet).

Documentation/contributor-guide/contributor-membership.md Outdated Show resolved Hide resolved
Documentation/contributor-guide/contributor-membership.md Outdated Show resolved Hide resolved
- Assigned test bugs related to area of expertise
- Granted "triage access" to etcd project

## Maintainers
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just curious, how does this governance rules apply to etcd-io/bbolt?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As the maintainers discussed, we will add separate Maintainer file in bbolt, probably in raft as well. cc @tbg

I think the roles enumerated in this doc should be applied to all projects under etcd-io, at least including bbolt and raft. WDYT? @hexfusion @mitake @ptabor @serathius @spzala @tbg

Documentation/contributor-guide/contributor-membership.md Outdated Show resolved Hide resolved
Documentation/contributor-guide/contributor-membership.md Outdated Show resolved Hide resolved
- Assigned test bugs related to area of expertise
- Granted "triage access" to etcd project

## Maintainers
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As the maintainers discussed, we will add separate Maintainer file in bbolt, probably in raft as well. cc @tbg

I think the roles enumerated in this doc should be applied to all projects under etcd-io, at least including bbolt and raft. WDYT? @hexfusion @mitake @ptabor @serathius @spzala @tbg

- Ensure a healthy process for discussion and decision making is in place.
- Work with other maintainers to maintain the project's overall health and success holistically

### Retiring
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the retiring process should be applied to all roles, not just Maintainers.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may also want to clearly request all roles (Members, Reviewers and Maintainers) to notify the community when they are going to be inactive for more than 1 week, e.g. on vacation without computer.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't require this from Members definitely. I'm not sure about reviewers but probably not.
That make sense for maintainers, as it's likely blocking for others.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree with @ptabor, I'm also not super pressed to clarify process of removing members/reviewers, as we first need to nominate them :P. Let's do it in followup.


Contributor roles and responsibilities were written based on [Kubernetes community membership]

[MAINTAINERS]: /MAINTAINERS
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No strong opinion on this, but be good to discuss it separately.

Copy link
Contributor

@ptabor ptabor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for working on this.
Few details, but overall LGTM.

Documentation/contributor-guide/contributor-membership.md Outdated Show resolved Hide resolved
- Sponsored by two active maintainer.
- Sponsors must be from multiple member companies to demonstrate integration across community.
- With no objections from other maintainers
- Reviewers can be removed by a supermajority of the maintainers or can resign by notifying
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we consider to force member to enable 2FA?
I remember that the kubernetes requires this one for security reason.

And BTW github will force user to enable 2FA end of this year. https://docs.github.com/en/authentication/securing-your-account-with-two-factor-authentication-2fa

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a great followup. I would not want to enforce this without previous discussion and preparation. Don't want to cut access to people without proper notification. There are also etcd org members for other subprojects that would be affected.

Copy link
Member

@spzala spzala left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @serathius
LGTM with couple of small points:

  • address @ptabor comment on clarifying Member requirements mismatch "2 reviewers in the table vs one maintainer in the details"
  • we should provide a link to this doc in the CONTRIBUTING.doc to encourage new contributors to check membership ladder


Contributor roles and responsibilities were written based on [Kubernetes community membership]

[MAINTAINERS]: /MAINTAINERS
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that would be overkill for the project. The current structure seems good enough, but yes agree that it can be discussed separately in the future.

Documentation/contributor-guide/contributor-membership.md Outdated Show resolved Hide resolved
Copy link
Member

@ahrtr ahrtr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with minor comments as summarized by @spzala + a couple of typos

@serathius serathius force-pushed the community-membership branch from adec4aa to 908c441 Compare April 3, 2023 14:39
Based on https://github.com/kubernetes/community/blob/master/community-membership.md

Changes:
* Extracted contributor membership to separate file
* Provide more detailed requirements for each role. Base maintainers on
  kubernetes subproject owners.
* Introduction of member role

Signed-off-by: Marek Siarkowicz <siarkowicz@google.com>
@serathius serathius force-pushed the community-membership branch from 908c441 to b4c1fb1 Compare April 3, 2023 14:42
@serathius
Copy link
Member Author

Addressed all comments. With supermajority of maintainers (4/6) approving the PR we are ready to merge, still I will leave the PR open for couple of days to ensure everyone has time to read it again. Please take a look and feel free to file issues with follow up ideas.

Copy link
Contributor

@ptabor ptabor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. LGTM.

I hate to say that, but the only concern I have is that with the requirement of 2 active maintainers, we might risk getting 'deadlock' at some moment that might require
"breaking" the governance rules in such situation.

We might:

  • postpone solving the problem when it (hopefully not) happens...
  • declare lazy consensus, i.e. lack of disagreement of any maintainer for 2 weeks is like an agreement.
  • delegate solving deadlocks to e.g. k8s-leads (if they happen).

@serathius
Copy link
Member Author

serathius commented Apr 4, 2023

I think we should follow https://github.com/etcd-io/etcd/blob/main/GOVERNANCE.md#conflict-resolution in any case of deadlock including community membership.

@serathius
Copy link
Member Author

Everyone involved left a thumbs up on #15593 (comment) so I assume they have familiarize with the latest version. Merging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

7 participants