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

CoreDNS Governance proposition #2203

Closed
wants to merge 7 commits into from
Closed

Conversation

fturib
Copy link
Contributor

@fturib fturib commented Oct 16, 2018

Why is this pull request needed and what does it do?

In order to graduate on last level inside CNCF organization, we need to clarify our GOVERNANCE rules.

Description

After MAINTAINERS sync-up of today,

I merged the proposition of Miek,
replacing the chapter Benevolent Dictator for Life by a Decision making process moved toward the end of the document.

This Decision making process is based on consensus at first, and, in last resort, a vote where each maintainer has one vote whatever the organization. (Miek has 2 votes).

There is no mention of organization in this document, it is purely based on maintainer whatever their professional status.

fturib added 5 commits October 16, 2018 16:59
…or as un-affiliated

- integrate an "Expectation from Maintainers"
- adapt Maintainership with a loose removal event
- make OWNER the main source of list of Maintainers
- removed the automatic move to "owner" of Github project
- added a begin of list of maintainers - based on current list in the 'maintainers' team of Github (2 organization, 2 individuals)
… and change, project

- define the decision making process by consensus and vote if cannot.
@corbot corbot bot requested a review from stp-ip October 16, 2018 22:44
@corbot
Copy link

corbot bot commented Oct 16, 2018

Thank you for your contribution. I've just checked the OWNERS files to find a suitable reviewer. This search was successful and I've asked stp-ip (via /OWNERS) for a review.

If you have questions or suggestions for this bot, please file an issue against the miekg/dreck repository.

The bot understands the commands that are listed here.

Copy link
Collaborator

@dilyevsky dilyevsky left a comment

Choose a reason for hiding this comment

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

seems reasonable to me

GOVERNANCE.md Outdated
in a *reasonable* time frame, either providing insights, or assign the Pull Requests to other
maintainers.

Every Maintainer is listed in the top-level [OWNERS](https://github.com/coredns/coredns/OWNERS)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Contributor Author

@fturib fturib Oct 17, 2018

Choose a reason for hiding this comment

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

Thanks. I think I can simplify to OWNERS

GOVERNANCE.md Outdated

## Expectations from Maintainers

"Every one carries water."
Copy link
Member

Choose a reason for hiding this comment

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

Is this a quote? If so, it should be attributed. If not, it should not have quotations around it.

Also as a general note, if any sections of this document are copied/borrowed, we should acknowledge the source.

Copy link
Contributor Author

@fturib fturib Oct 17, 2018

Choose a reason for hiding this comment

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

about Everyone carries water that is more an idiom no ?

about sections copied .. yes some are, at least some sentences that I copy/paste from other cncf governance doc, which themselves seems copied. Are you sure we need to add the source here ? (I do not remind where I took it).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

for now, will remove the quotes.

GOVERNANCE.md Outdated
maintainers.

Every Maintainer is listed in the top-level [OWNERS](https://github.com/coredns/coredns/OWNERS)
file, with their Github handle and an (possible obfuscated) email address. Every one in the
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
file, with their Github handle and an (possible obfuscated) email address. Every one in the
file, with their Github handle and a possibly obfuscated email address. Everyone in the

GOVERNANCE.md Outdated

## Expectations from Maintainers

"Every one carries water."
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"Every one carries water."
"Everyone carries water."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hum. I do not think it is a quote.
I cannot find another match in google.

GOVERNANCE.md Outdated
A Maintainer is also listed in a plugin specific OWNER file.

A Maintainer should be a member of `maintainer@coredns.io`, although this is not a hard requirement.
A Maintainer that hasn't been active in the CoreDNS repository for 12 months is considered inactive.
Copy link
Member

Choose a reason for hiding this comment

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

What does it mean to be "inactive"? What are the implications?
If there are none, we should remove this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree. removed.

GOVERNANCE.md Outdated

## Becoming a Maintainers

On successful completion (it was merged) of a (large) pull request, any current maintainer can reach
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
On successful completion (it was merged) of a (large) pull request, any current maintainer can reach
On successful merge of a significant pull request, any current maintainer can reach

Do we need to define "significant" here?

Copy link
Contributor Author

@fturib fturib Oct 17, 2018

Choose a reason for hiding this comment

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

I do not think so. Anyway the idea is to welcome anyone that is interested to contribute.

GOVERNANCE.md Outdated

The CoreDNS community adheres to the following principles:

- Open: CoreDNS is open source. See repository guidelines, below.
Copy link
Member

Choose a reason for hiding this comment

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

It's unclear to me what section below this is referring to.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

right. will rather add a link to coredns.io/community that advertize it.

GOVERNANCE.md Outdated
A Maintainer should be a member of `maintainer@coredns.io`, although this is not a hard requirement.
A Maintainer that hasn't been active in the CoreDNS repository for 12 months is considered inactive.

## Becoming a Maintainers
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
## Becoming a Maintainers
## Becoming a Maintainer

GOVERNANCE.md Outdated

"Every one carries water."

Making a community work requires input/effort from every one. Maintainers should actively
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Making a community work requires input/effort from every one. Maintainers should actively
Making a community work requires input/effort from everyone. Maintainers should actively

GOVERNANCE.md Outdated

## Code of Conduct

CoreDNS follows the [CNCF Code of Conduct](https://github.com/coredns/coredns/CODE-OF-CONDUCT.md).
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
CoreDNS follows the [CNCF Code of Conduct](https://github.com/coredns/coredns/CODE-OF-CONDUCT.md).
The [CoreDNS Code of Conduct](https://github.com/coredns/coredns/blob/master/CODE-OF-CONDUCT.md) is aligned with the CNCF Code of Conduct.

GOVERNANCE.md Outdated
A 2/3 majority vote is needed for the statement to be approved.

Each maintainer weighs one vote.<br>
Miek Gieben (@miekg), as the historical owner of CoreDNS, weighs two votes.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Miek Gieben (@miekg), as the historical owner of CoreDNS, weighs two votes.
Miek Gieben (@miekg), as the owner of CoreDNS, weighs two votes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not sure how to define the owner of coreDNS - and if there is one, since we moved to cncf and it is opensource project.
From point of view of Github there are several owners.

That is why I prefer the "historical owner" that show, whatever owners are defined today, that the project were started and grown up by Miek.

I propose to keep "historical owner" for now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

unless there is a more appropriate way to say the same thing ?

@chrisohaver
Copy link
Member

What happened to the "corporate takeover prevention clause"? Where maintainers working on behalf of an organization only get one vote collectively for their whole organization.

Are we not doing that anymore? Or is it in there and i'm just not seeing it?

GOVERNANCE.md Outdated

Every Maintainer is listed in the top-level [OWNERS](https://github.com/coredns/coredns/OWNERS)
file, with their Github handle and an (possible obfuscated) email address. Every one in the
`reviewers` list is a Maintainer.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
`reviewers` list is a Maintainer.
`approvers` list is a Maintainer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good catch.

@fturib
Copy link
Contributor Author

fturib commented Oct 17, 2018

What happened to the "corporate takeover prevention clause"? Where maintainers working on behalf of an organization only get one vote collectively for their whole organization.

@miekg : I removed the initial limitation of a single vote per organization (which is the "Corporate takeover prevention clause" that @chrisohaver is referring above).

I understood here that you prefer to have no reference at all to organization.

But I may have misunderstood and be wrong here.

what is your position ?

NOTE: Initial "corporate takeover prevention clause" is:

The CoreDNS project employs "organization voting" to ensure no single organization can dominate the project.

Individuals declare if they contribute as un-affiliated or as associated with or employee of an organization or a company.

Individuals that contribute as un-affiliated are allowed one organization vote. Each company or organization (regardless of the number of maintainers associated with or employed by that company/organization) receives one organization vote.

In other words, if two maintainers are employed by Company X, two by Company Y, two by Company Z, and one maintainer is an un-affiliated individual, a total of four "organization votes" are possible; one for X, one for Y, one for Z, and one for the un-affiliated individual.

Any maintainer from an organization may cast the vote for that organization.

@fturib
Copy link
Contributor Author

fturib commented Oct 17, 2018

@chrisohaver, @rajansandeep : I think I either integrated your proposition of change or reply on your comment in this PR. Thanks to verify.

@codecov-io
Copy link

codecov-io commented Oct 17, 2018

Codecov Report

Merging #2203 into master will decrease coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2203      +/-   ##
==========================================
- Coverage   55.86%   55.84%   -0.03%     
==========================================
  Files         203      203              
  Lines        9991     9991              
==========================================
- Hits         5581     5579       -2     
- Misses       3987     3988       +1     
- Partials      423      424       +1
Impacted Files Coverage Δ
plugin/proxy/lookup.go 71.66% <0%> (-3.34%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d9efa96...e147e0e. Read the comment docs.

@chrisohaver
Copy link
Member

@chrisohaver, @rajansandeep : I think I either integrated your proposition of change or reply on your comment in this PR. Thanks to verify.

@fturib, It appears that most of my suggestions were not accepted ... Some are grammatical/spelling corrections that I would urge us to address before merging.

@fturib
Copy link
Contributor Author

fturib commented Oct 18, 2018

Some are grammatical/spelling corrections that I would urge us to address before merging.

I though I addressed all those. Let me double check...
My bad ! ... I was looking into "conversation" that was showing only a part of your comments.
=> I will work integrating the propositions I did not see.

@fturib
Copy link
Contributor Author

fturib commented Oct 18, 2018

@chrisohaver : can you have a second look ? I think this time I addressed all of your comment (hopefully).

@chrisohaver
Copy link
Member

chrisohaver commented Oct 19, 2018

Thanks @fturib,

About the "CNCF umbrella" thing, I think we should be prepared to be able to answer what that means if we are asked. It would be even better to describe what it means in this document or link to a place that explains it.

@fturib
Copy link
Contributor Author

fturib commented Oct 20, 2018

@miekg : can you review and provide your feedback or approval ?


For formal votes, the following should be added to the relevant github issue or PR:
* A specific statement of what is being voted on.
* The voting period - a suitable amount of time during which voting will occur.
Copy link
Member

@yongtang yongtang Oct 20, 2018

Choose a reason for hiding this comment

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

I noticed that organization vote restriction has been removed. It is fine as long as the organization stays with the principal of open and transparent transparent and accessible. I do think some clarifications should be made to make sure no single organization is able to dominate the project. I will share some of my thinking about this area.

Copy link
Contributor Author

@fturib fturib Oct 20, 2018

Choose a reason for hiding this comment

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

I will share some of my thinking about this area.

Let me know what you would like to change here to address "no single organization is able to dominate the project".

There is one in this comment, that is coming from former version of the proposal, and that I copied from other project that had the same concern.

I would like also the opinion of @miekg on this, as he was reluctant to add "organization" reference in this document.

@yongtang
Copy link
Member

yongtang commented Oct 20, 2018

I think transparent and accessible is very important to prevent single organization from being able to dominate. Some clarifications might be needed here.

One important thing is that, make sure decision making process stays public, either through maintainers@coredns.io (open to every maintainers), or through pull requests on GitHub (open to every community members).

@yongtang
Copy link
Member

I believe every coredns community members have the intentions to help the project better, so as long as everything remains public, different opinions will always be sorted out in a good way.

@yongtang
Copy link
Member

yongtang commented Oct 20, 2018

We do need to make sure there is no small circle decisions by a carefully selected group.

There are several ways to get around the proposed decision making process and play the systems. For example:

  • One maintainer or one organization could host a maintainer's meeting, invite only a small set of maintainers it likes (or invites many non-maintainers to water down the votes), and reaches a self-claimed consensus.
  • Even if a meeting is open and accessible to every maintainers, the host of the meeting, could still run the table and sway the decisions by predefining the agenda.

How to make sure the above scenarios are not going to happen? I think

  • we have to make sure maintainers@coredns.io (open to all maintainers) or GitHub pull requests (open to everyone) are the only proper channels for any decision making process.
  • any proposal should leave enough reasonable time for every maintainer to think, digest, and express his or her opinions.

@yongtang
Copy link
Member

yongtang commented Oct 20, 2018

Now coredns is under the umbrella of cncf. There will be communications between coredns project and cncf.

A question is, who should be the person of contact for coredns to cncf? Should the person be selected elected by maintainers, or should maintainers@coredns.io be used for conversations between coredns and cncf?

I think maintainers@coredns.io could be a better way as it allows every maintainer to be in the loop, though I am happy to hear any suggestions.

@yongtang
Copy link
Member

Another question is, there are two proposals #2152 and #2203. Which one should be used as the basis for update and modification? and why?

@fturib
Copy link
Contributor Author

fturib commented Oct 20, 2018

How to make sure the above scenarios are not going to happen?

I am not sure you can avoid that any side-conversation between maintainers or non-maintainers can happen.
But from the 'making decision process', there is always a way for any maintainer to verify that decision is common: it is by calling the vote. And this mechanism seems clear and public.

@fturib
Copy link
Contributor Author

fturib commented Oct 20, 2018

who should be the person of contact for coredns to cncf?

Here I am not sure as there is no person dedicated for that.

CNCF is part of this project, so let's ask @caniszczyk to tell us how it is defined for other projects under umbrella of cncf.

@fturib
Copy link
Contributor Author

fturib commented Oct 20, 2018

Another question is, there are two proposals #2152 and #2203. Which one should be used as the basis for update and modification? and why?

Hum ... initially I created one proposal : #2112, then Miek made another proposal #2152.
After sync-up I closed the initial #2112, and created this one, based on #2152, as I explained in the description of this PR.

Looks like the whole point is too agree on the chapter Decision making process initially Benevolent Dictator for Life

We can work on any of these PR that have the same root.
If you prefer to update #2152, up to you, that is fine with me.
(In that case, please ensure to take care about all grammatical/spelling change called by @chrisohaver that I have already addressed in this PR).

@yongtang
Copy link
Member

How to make sure the above scenarios are not going to happen?
I am not sure you can avoid that any side-conversation between maintainers or non-maintainers can happen.
But from the 'making decision process', there is always a way for any maintainer to verify that decision is common: it is by calling the vote. And this mechanism seems clear and public.

Any maintainer can talk to any one, any maintainer can also have sync up meetings with any one, or any group of his or her choice.

The clarification is make sure any "decision" is coming from maintainers@coredns.io or GitHub pull requests, not from "oh, two or three of us met, and here are the decision". Keep in mind the proposed Decision making process have the following clause:

Proposals and ideas can either be submitted for agreement via an \
github issue or by sending an email to `maintainer@coredns.io`

If decisions from other means are ok, you can certainly modify the proposed Decision making process.

@fturib
Copy link
Contributor Author

fturib commented Oct 20, 2018

Keep in mind the proposed Decision making process have the following clause:
Proposals and ideas can either be submitted for agreement via an \ github issue or by sending an email to maintainer@coredns.io`
If decisions from other means are ok, you can certainly modify the proposed Decision making process.

I am not sure how to interpret ...

I understand that you agree that the current clause in Decision making process is good enough and ensure decision will be public. So we do not need to change.

Am I correct ?

@yongtang
Copy link
Member

The clause is certainly good enough, expect what are the implications if it is not followed?

Similarly, some other comments from the review:

What does it mean to be "inactive"? What are the implications?
If there are none, we should remove this.

@yongtang
Copy link
Member

I would prefer the discussion to be in the other PR, but just in case, here is the copied context #2152 (comment):

I think the issue with coredns is that, coredns is not unique compared with many other open source projects. It is however, might be special inside cncf.

Many other open source projects in cncf have a (or more than one) sponsoring company and the company behind is also the major technical contributor. Companies certainly have more non-technical resources but that typically aligns with technical contributions collectively.

So cncf projects still largely follows merit-based open source model and works well.

For coredns, there is one independent contributor (@miekg) that has more than 50% of the technical contributions day-to-day. There is also a company that may have more non-technical resources, but, simply just could not have enough technical contributions.

Open source software should be merit-based in the area of technical contributions.

For me, it will be a bigger joke if THE major contributor of the project is not involved or does not have a final say on anything in this project.

Note things may change in the future.

@yongtang
Copy link
Member

Continue #2152 (comment):

A merit-based project lead can help incubating, set vision, and reduce noice early on. Things may change when contributions of a project diversify.

But at the current stage, I see more benefits for a project lead that drives the direction of the coredns.

@fturib
Copy link
Contributor Author

fturib commented Oct 20, 2018

A merit-based project lead can help incubating, set vision, and reduce noice early on. Things may change when contributions of a project diversify.

But at the current stage, I see more benefits for a project lead that drives the direction of the coredns.

I think that can be fine to have a project lead, as long as we define its role, how that project lead is elected, for how long, and what is the process of replacement/continuation.
I mean a periodic revision of who can take this role - based on merit -, with the agreement of the CoreDNS community.

That would made an "open governance" - because of the periodic validation by the community - and keep all the positive point to have a quick decision process.

what do you think ?

@fturib
Copy link
Contributor Author

fturib commented Nov 12, 2018

Closing this proposition.
The active proposition for Governance is here : #2220

@fturib fturib closed this Nov 12, 2018
@fturib fturib deleted the governance-vote branch November 12, 2018 18:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants