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

set up SIG-etcd #7372

Merged
merged 11 commits into from
Sep 12, 2023
Merged

set up SIG-etcd #7372

merged 11 commits into from
Sep 12, 2023

Conversation

logicalhan
Copy link
Member

Setting up SIG-etcd.

@k8s-ci-robot
Copy link
Contributor

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jun 23, 2023
@k8s-ci-robot k8s-ci-robot requested a review from jdumars June 23, 2023 16:29
@k8s-ci-robot k8s-ci-robot added the committee/steering Denotes an issue or PR intended to be handled by the steering committee. label Jun 23, 2023
@k8s-ci-robot k8s-ci-robot added the do-not-merge/invalid-owners-file Indicates that a PR should not merge because it has an invalid OWNERS file in it. label Jun 23, 2023
@tpepper
Copy link
Member

tpepper commented Jul 10, 2023

Conceptually this a +1 from me. We all need to see etcd through to a more stable ground. The Kubernetes project can try to foster that. But we really need to see the general shape of a charter, how the project will remain usable for existing non-Kubernetes, and get into the details of how that varies from our normal K8s SIG related processes and tools.

@ritazh
Copy link
Member

ritazh commented Jul 10, 2023

+1
This is great to see especially from a security perspective. Wearing my SRC hat.

@mrbobbytables
Copy link
Member

+1'ing @tpepper 's statements, we discussed it a bit during the steering meeting today. Overall we are good with the concept of bringing on etcd as a SIG.

For explicit next steps -

  • What from our regular sig governance would not work for etcd? (if anything)
  • Similar thing for our org and GitHub management - what of those current practices & policies would not work for etcd? (One note: the org could still remain as etcd, but would require our management)

@serathius
Copy link
Contributor

serathius commented Jul 11, 2023

@tpepper @mrbobbytables Please check out first section of SIG-etcd Charter & Vision.

What from our regular sig governance would not work for etcd? (if anything)
Similar thing for our org and GitHub management - what of those current practices & policies would not work for etcd? (One note: the org could still remain as etcd, but would require our management)

Could we create a dedicated document with a checklist? I wanted Do's and don'ts of etcd SIG-ifycation to address that, but the list might long enough to motivate a separate document.

@BenTheElder
Copy link
Member

To elaborate a little ...

What from our regular sig governance would not work for etcd? (if anything)

Most SIG Charters are pretty close to the generic boilerplate SIG charter with some details filled in as to what the SIG owns, some of them the only thing to approve is that they will exist and what they will own (since steering delegates all ownership/technical decisions to the SIGs we want to make sure there's reasonably boundaries between them).

Some SIGs are e.g. trying deviations in further expanding leadership positions or other things which are edited in and called out in their charters.

We expect etcd will have some similar exceptions, like keeping it's own github organization instead of "donating" repos into github.com/kubernetes-sigs (most new projects), which raises some questions about ownership and management (github.com/kubernetes, github.com/kubernetes-sigs and other orgs are managed by a subproject of SIG Contributor Experience, enabling things like required 2FA and enforcing the CLA bot on all repos).

We'll want to understand what those exceptions are.

Otherwise spelling out the scope of SIG-etcd + the boilerplate should be sufficient.


Also to further underline: We had all members in attendance at the mentioned public steering committee meeting and again everyone is supportive, we're just looking to etcd for what exceptions are required vs the standard SIG charter so we can make sure those are understood and agreed by everyone.

Besides the boilerplate text we have 30+ existing groups to reference and I'm happy to help answer any clarifying questions, just send me a note.

@BenTheElder
Copy link
Member

we should probably have @ahrtr apply for org membership :-)

https://github.com/kubernetes/org/issues/new?assignees=&labels=area%2Fgithub-membership&projects=&template=membership.yml&title=REQUEST%3A+New+membership+for+%3Cyour-GH-handle%3E

@logicalhan
Copy link
Member Author

we should probably have @ahrtr apply for org membership :-)

https://github.com/kubernetes/org/issues/new?assignees=&labels=area%2Fgithub-membership&projects=&template=membership.yml&title=REQUEST%3A+New+membership+for+%3Cyour-GH-handle%3E

Yeah, I was reaching out to @serathius to ask him to join. He also should probably sign up for slack.

sig-etcd/charter.md Outdated Show resolved Hide resolved
### Deviations from [kubernetes-repositories]

- SIG etcd repositories live in github.com/etcd-io
- SIG etcd repositories should (but not must) adopt merge bot, Kubernetes PR commands/bot, OWNERS file.
Copy link
Contributor

Choose a reason for hiding this comment

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

This will require our contributors to apply for Kubernetes Org membership. That's probably a good thing overall, but will involve some bootstrapping.

Copy link
Contributor

Choose a reason for hiding this comment

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

That's not true, I expect similar to Kubernetes Org vs Kubernetes-SIGs Org, the membership will be separated from Kubernetes.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's not, even kubernetes sigs have to apply for membership through org.

Copy link
Member

Choose a reason for hiding this comment

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

yup - our orgs all use the same process of applying. Essentially you apply and if approved you can join any of the primary k8s orgs.

I mentioned out of band (#7372 (comment)) but to directly comment here on some of the hard reqs:

  • etcd org is owned and managed by k8s OR repos are transferred to K8s org (NOTE: people can still be granted repo admin rights, its just management of the org)
  • membership will be managed by our our automation and applying for org membership will use our process (current etcd org members wouldn't have to apply, we'd directly add)
  • EasyCLA will be enabled on the etcd org or any repos transferred to K8s org
  • github actions or the CI currently in play can still be used, but we'd still have to run our global stuff (e.g. cncf-cla, lgtm/approve etc.)

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for bringing this up, Josh. I have no problem with that.

Copy link
Member

Choose a reason for hiding this comment

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

One comment on the owners files, I think those will be a "must" insofar as we can link to the people responsible for the management of the subproject.

Copy link
Contributor

Choose a reason for hiding this comment

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

I am ok with that, do we just need a k8s style OWNERS file in the top level of each subproject under https://github.com/etcd-io?

cc @serathius @ahrtr @spzala @jmhbnz

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good to me too!

Copy link
Member

Choose a reason for hiding this comment

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

+1 sounds good to me.

Copy link
Contributor

Choose a reason for hiding this comment

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

Removed the "OWNERS file" from deviation, and added an issue in etcd repo to track the effort of adding top level OWNERS file in each subproject repo: etcd-io/etcd#16367

@mrbobbytables
Copy link
Member

mrbobbytables commented Jul 12, 2023

To be clear - there are a few hard requirements if etcd wants to be a k8s sig and use our tooling & automation + SRC/CoCC etc:

  • etcd org is owned and managed by k8s OR repos are transferred to K8s org (NOTE: people can still be granted repo admin rights, its just management of the org)
  • membership will be managed by our our automation and applying for org membership will use our process (current etcd org members wouldn't have to apply, we'd directly add)
  • EasyCLA will be enabled on the etcd org or any repos transferred to K8s org

EDIT: github actions or the CI currently in play can still be used, but we'd still have to run our global stuff (e.g. cncf-cla check)

@serathius
Copy link
Contributor

Thanks @mrbobbytables for providing the list. I don't think this will be a problem. cc @ahrtr @ptabor @spzala @mitake

@serathius serathius force-pushed the sig-etcd branch 2 times, most recently from bb9fbac to 09e5fcc Compare July 13, 2023 12:03
@logicalhan logicalhan changed the title [WIP] set up SIG-etcd set up SIG-etcd Jul 13, 2023
@logicalhan logicalhan marked this pull request as ready for review July 13, 2023 14:22
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 13, 2023
@logicalhan
Copy link
Member Author

@mrbobbytables we should default add all existing etcd maintainers to kubernetes org, imo.

@mrbobbytables
Copy link
Member

@logicalhan I agree. I think just before we start doing that on a large scale we should sort out the charter etc.

@palnabarun
Copy link
Member

palnabarun commented Sep 7, 2023

Steering Votes:

@wenjiaswe
Copy link
Contributor

Thanks @palnabarun .

Friendly ping @cblecker @justaugustus @tpepper , please let us know if there is any other concerns. Thanks!

@BenTheElder
Copy link
Member

/approve


## Scope

Owns the etcd project and how it is used by Kubernetes.
Copy link
Member

@neolit123 neolit123 Sep 8, 2023

Choose a reason for hiding this comment

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

note that you gain ownership of the current, very cumbersome and undeterministic process of updating etcd server/client in k8s.

this is currently a best effort from community members and issues and PRs just run stale:
kubernetes/kubernetes#117648
https://github.com/kubernetes/kubernetes/issues?q=is%3Aopen+label%3Aarea%2Fkubeadm+etcd

various product tooling allows some form of etcd version customization, custom images, fips builds, etc. thus the public updates are not a p0-1.

Copy link
Contributor

Choose a reason for hiding this comment

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

Historically it were etcd maintainers who bumped the etcd image in Kubernetes. Reason was that Kubernetes scalability tests are the major signal in etcd qualification, so minor bumps etcd version were done immediately.

What's more cumbersome is security patching, which is a problem because there are the etcd k8s image is also used by kubeadm. With the SIG in place, I think we can discuss changing/improving the release process.

Copy link
Member

@neolit123 neolit123 Sep 8, 2023

Choose a reason for hiding this comment

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

Historically it were etcd maintainers who bumped the etcd image in Kubernetes. Reason was that Kubernetes scalability tests are the major signal in etcd qualification, so minor bumps etcd version were done immediately.

i think i vaguelly recall this period.

What's more cumbersome is security patching, which is a problem because there are the etcd k8s image is also used by kubeadm. With the SIG in place,

the biggest current pain points for etcd update in k8s are:

  • undocumented, manual image promotion process.
  • no dedicated top level approver to own etcd k/k updates as it touches a lot of things.
  • too many steps, different repos, client vs server updates

kubeadm updates are simple - changing a small version map / constant. by updating kubeadm k8s+etcd gains upgrade signal from etcd version at k8s N-1 to etcd version at k8s N. the kube-up upgrade suite is still not working, IIRC.

I think we can discuss changing/improving the release process.

happy to participate.

@serathius
Copy link
Contributor

Steering Votes:

Do we need all votes or just quorum is enough? FYI etcd community does things by quorum :P

Copy link
Member

@cblecker cblecker 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 one non-blocking call out

sig-etcd/charter.md Show resolved Hide resolved
@cblecker
Copy link
Member

cblecker commented Sep 8, 2023

@serathius The Kubernetes Steering Committee normally gives opportunities for all seven of us to weigh in on a subject wherever possible as ideas/changes/etc are improved by the diversity of perspectives and inputs. We will sometimes push through small changes with 1 person or a simple majority signing off, but establishment of a new SIG is one of those things where we will try to let everyone chime in.

@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 8, 2023
@cblecker
Copy link
Member

cblecker commented Sep 8, 2023

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 8, 2023
@serathius
Copy link
Contributor

can unhold?

@cblecker
Copy link
Member

cblecker commented Sep 9, 2023

Not yet. I was reapplying an LGTM after a minor change (updating zoom link).

We will likely remove the hold on Monday when the SC meets.

@wenjiaswe
Copy link
Contributor

@cblecker Thanks Christoph, do we need someone from etcd to attend the SC meeting to answer questions if there is any?

@cblecker
Copy link
Member

cblecker commented Sep 9, 2023

@wenjiaswe It is a public meeting so you're welcome to attend, however I don't think it'll be required. I, at least, don't have any outstanding questions.

@cpanato
Copy link
Member

cpanato commented Sep 11, 2023

/lgtm
(+1, steering)

@tpepper
Copy link
Member

tpepper commented Sep 11, 2023

/lgtm
(Steering)

We can set the liaison subsequently.

Copy link
Member

@justaugustus justaugustus left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve (Steering)

As we have unanimous support from @kubernetes/steering-committee, I'm lifting the hold on this.

Welcome SIG etcd!
/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 12, 2023
@k8s-ci-robot k8s-ci-robot merged commit 3edd1c6 into kubernetes:master Sep 12, 2023
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: BenTheElder, cblecker, justaugustus, logicalhan, palnabarun

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [BenTheElder,cblecker,justaugustus,palnabarun]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

approvers:
- sig-etcd-leads
labels:
- sig/etcd
Copy link
Member

@pacoxu pacoxu Sep 12, 2023

Choose a reason for hiding this comment

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

Is this label ready?

@pacoxu: The label(s) sig/etcd cannot be applied, because the repository doesn't have them.

kubernetes/kubernetes#118077 (comment)

Currently, we have area/etcd label.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for reminding, this is done now: kubernetes/test-infra#30948

@neolit123
Copy link
Member

neolit123 commented Oct 5, 2023

Is this label ready?

FTR, @cici37 added the /sig etcd label, which can now be used in k/k and other repositories with the appropriate prow config.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. committee/steering Denotes an issue or PR intended to be handled by the steering committee. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges.
Projects
None yet
Development

Successfully merging this pull request may close these issues.