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

SIG-scalability charter. #1829

Closed
wants to merge 1 commit into from

Conversation

wasylkowski-a
Copy link

This is a copy of @porridge's PR #1607. @porridge will no longer be working on that PR, so I am taking it over with his consent.

SIG-scalability leads:
/cc @wojtek-t @countspongebob
Steering committee representatives:
/cc @thockin @smarterclayton @spiffxp

@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Feb 21, 2018
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: wasylkowski-a
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: countspongebob

Assign the PR to them by writing /assign @countspongebob in a comment when ready.

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:

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

@k8s-ci-robot k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Feb 21, 2018
@cblecker
Copy link
Member

/ok-to-test

@k8s-ci-robot k8s-ci-robot removed the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Feb 21, 2018
@k8s-github-robot k8s-github-robot added the sig/scalability Categorizes an issue or PR as relevant to SIG Scalability. label Feb 21, 2018
Copy link
Contributor

@countspongebob countspongebob left a comment

Choose a reason for hiding this comment

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

Content of the charter is good, but per some other comments from steering we need to address some basic governance issues like SIG leads. I'd like to see us approve this as it is the substantive content and do another PR to address remaining gaps.

@shyamjvs
Copy link
Member

Thanks @wasylkowski-a!

+1 to @countspongebob's comment on addressing gaps in a later PR.

@wojtek-t
Copy link
Member

Content of the charter is good, but per some other comments from steering we need to address some basic governance issues like SIG leads. I'd like to see us approve this as it is the substantive content and do another PR to address remaining gaps.

I'm fine with that, but let's add explicit TODOs to the charter to address the issues mentioned in the original PR.

@countspongebob
Copy link
Contributor

I will review the two SIG templates (there is a short one and a long one) and propose inclusion into the charter. Would you be ok with a single to-do that covers review of inclusion of the template with all of the relevant items?

@wojtek-t
Copy link
Member

Would you be ok with a single to-do that covers review of inclusion of the template with all of the relevant items?

I think that we need a TODO that will describe all the missing parts in the charter. If we mention all the missing things, I'm fine with that.

@wasylkowski-a
Copy link
Author

I will do that as soon as I get some uninterrupted time; hopefully this week. Stay tuned.


## What can we do/require from other SIGs

*By regression below we mean a regression identified by the set of release-blocking scalability/performance tests (as defined by [the sig-release-x.y-blocking dashboards](https://github.com/kubernetes/test-infra/blob/master/testgrid/config/config.yaml)).*
Copy link
Contributor

Choose a reason for hiding this comment

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

4. etc
5. once the mitigation is merged to master, unpause the merge queue

### Rationale
Copy link
Contributor

Choose a reason for hiding this comment

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

These are really good points.
One suggestion: put them in the first section to justify the proposal

1. this can be done by reading code changes, bisecting, running A/B tests, etc,
2. we say a PR is identified as the cause when we are reasonably confident that it indeed caused a regression, *even if the mechanism is not 100% understood* - this is because we want to minimize the time of merge queue pause
4. mitigate the regression; this can potentially be different things, e.g.:
1. reverting the PR,
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 this needs to be handled with care to ensure folks are informed of the tradeoff's associated with a revert. kubernetes/kubernetes#60891 (comment) is a good example that blindly reverting without conversing it through can be dangerous.

@spiffxp
Copy link
Member

spiffxp commented Apr 4, 2018

Same questions as in the previous PR

  • what are the responsibilities of your SIG leads?
  • what process do you use to change SIG leadership?
  • what code do you own?

See https://github.com/kubernetes/community/blob/master/committee-steering/governance/sig-governance-requirements.md for the checklist we use to evaluate charters

@k8s-ci-robot
Copy link
Contributor

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.


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. I understand the commands that are listed here.

@k8s-ci-robot k8s-ci-robot added cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. and removed cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Apr 4, 2018
@timothysc timothysc self-assigned this Apr 11, 2018
@wojtek-t
Copy link
Member

I'm going to rewrite it to more match the style SIG Architecture recommend for writing charters.

So I'm closing this one and will hopefully open a new one within the next 2 weeks.

@wojtek-t
Copy link
Member

@wasylkowski-a - I don't have persmissions to close this one, so please close :)

@cblecker
Copy link
Member

@wojtek-t you should be able to close with the /close command :)

@wojtek-t
Copy link
Member

/close

@wojtek-t
Copy link
Member

I've opened #2149 as a replacement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. sig/scalability Categorizes an issue or PR as relevant to SIG Scalability. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants