Re: Feature request: Revert-only OWNERS in kubernetes/kubernetes?

[Stephen Augustus]

Dims -- thanks for volunteering!

Maciej / Matt--

Appreciate the additional context, especially around the charter i.e., you have the mandate to have that power, but not the permissions to exercise it.

I've created an issue[1] to discuss updating the top-level reviewers/approvers in k/k and assigned the current OWNERS.

The suggestion:

• Add to OWNERS_ALIASES:
  • revert-approvers
  • sig-scalability-approvers
  • sig-scalability-reviewers
• Add aforementioned groups to top-level reviewers and approvers stanzas
• Use the opportunity to prune/update the current top-level reviewers/approvers

Adding SIG Arch and Steering as I couldn't find a policy on updating this file specifically (outside of --> [2]).

Happy to help ping people/make the OWNERS update, once we settle on something!

-- Stephen

[1] https://github.com/kubernetes/kubernetes/issues/78935

[2] https://github.com/kubernetes/community/blob/master/contributors/guide/owners.md#maintaining-owners-files

On Wed, Jun 12, 2019 at 6:04 AM Davanum Srinivas <dav...@gmail.com> wrote:

I am in US East TZ. I can volunteer for helping with approving reverts as well.

-- Dims

On Wed, Jun 12, 2019 at 4:39 AM 'Maciej Borsz' via kubernetes-sig-scale <kubernetes...@googlegroups.com> wrote:

https://github.com/kubernetes/kubernetes/pull/78931 is another example -- as part of debugging regression in 1.15 we found the faulty PR and now we need we are waiting for review.

If revert-only OWNERS is infeasible, I think that just another (next to Wojtek) Europe time zone approver will help a lot.

On Tue, Jun 11, 2019 at 10:31 AM Maciej Borsz <macie...@google.com> wrote:

On Mon, Jun 10, 2019 at 3:21 PM Stephen Augustus <Ste...@agst.us> wrote:

This goes back to the reviewers/approvers pools for SIG Scalability subprojects. The SIG needs to work on increasing it to more than just a few people for 1.16.

Sorry, I don't understand. The 'sig scalability subprojects' page you mentioned earlier shows OWNERS only for sig-scalability-owned code (like kubemark's code). This is not an issue I meant.

The problem is not about reverting changes to sig-scalability-owned code. The issue is that sometimes we need to revert changes to kubernetes/kubernetes repo as we find that they are breaking scalability tests (and more importantly the scalability of the kubernetes overall).

The reason why we (sig-scalability) need to manually revert PRs is that presubmits covers only basic 100 node cluster while some issues can be detected only in 5000-node scale. We are running a daily tests 5000-node tests and when we see that it's failing, when we find the culprit we simply want to revert this as fast as possible to make sure there is no other regression.

My ask is only to make this process smoother in case when only person in sig-scalability with approval permissions (wojtekt) is unavailable for some reason. Revert-only OWNERS seemed to be a good idea to me, but we can figure out some other solution here if this is infeasible.

 

On Mon, Jun 10, 2019, 09:05 Maciej Borsz <macie...@google.com> wrote:

On Mon, Jun 10, 2019 at 2:56 PM Jordan Liggitt <lig...@google.com> wrote:

Have there been examples of urgent reverts needed that did not find an approver in O(hours)? I tend to agree with Stephen that we wouldn't generally want to add approvers only for certain types of changes.

https://github.com/kubernetes/kubernetes/pull/76190 is an example revert that we wanted to merge quickly.

If we had to revert similar PR e.g. this week (when Wojtek is on vacation), we would have a problem finding an approver for it.

I think that preferably I would like to have more than one person in the team that is able to approve such PRs, but if it's infeasible could you suggest some other European time zone approver we can ask for help in such cases?

Thanks,

Maciej

 

On Mon, Jun 10, 2019 at 5:07 AM Stephen Augustus <Ste...@agst.us> wrote:

(take two; I didn't realize that kubernetes...@googlegroups.com was the current address for the SIG. Also, I don't believe external users have permissions to post to gke-kubernetes-scalability, so sending that list to bcc.)

On Mon, Jun 10, 2019 at 5:02 AM Stephen Augustus <Ste...@agst.us> wrote:

(+ SIG Scalability, SIG Testing)

AFAIK, that's not a thing we do today i.e., revert-only OWNERs. I've added SIG Testing to see if that's a thing they'd ever want to do. 

Committing and reverting code has similar potential impact, so I'd imagine we'd want to continue along the path of not weighting the two.

I'm a big +1 to adding more reviewers/approvers to scalability OWNERs, but that's a question/task for SIG Scalability and their subprojects[1], so I've added them here.

-- Stephen

[1] https://github.com/kubernetes/community/tree/master/sig-scalability#subprojects

On Mon, Jun 10, 2019 at 2:52 AM 'Maciej Borsz' via kubernetes-sig-release <kubernetes-...@googlegroups.com> wrote:

Hi all,

In sig-scalability we relatively often have to manually revert PRs affecting gce scalability.

So far, we used wojtekt's superpowers to approve such reverts, but he is a single person in our team that has such permissions (and e.g. this week he is on vacation).

So I wanted to ask to add more folks from our team to OWNERS in kubernetes/kubernetes repo. Given limited knowledge of the codebase, I was thinking that maybe giving us approval power only for reverts makes sense. It would be sufficient for our work and limit the risk.

Few questions:

1. What do you think?

2. (I'm not that familiar with github) Is it technically possible to distinguish revert PRs from other PRs?

Thanks,

Maciej

--
You received this message because you are subscribed to the Google Groups "kubernetes-sig-release" group.
To unsubscribe from this group and stop receiving emails from it, send an email to kubernetes-sig-re...@googlegroups.com.
To post to this group, send email to kubernetes-...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/kubernetes-sig-release/CA%2BbcoB5T9nZqezFZ1vJc5y8BaeC%2BxV77yZbumtSM4L000cnqfQ%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

--
You received this message because you are subscribed to the Google Groups "kubernetes-sig-release" group.
To unsubscribe from this group and stop receiving emails from it, send an email to kubernetes-sig-re...@googlegroups.com.
To post to this group, send email to kubernetes-...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/kubernetes-sig-release/CAFQm5yRCuX5a1HMwvZ9XqWKZBf%2BKMaQV6DsFGJTWD9RJpNV_9Q%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

--
You received this message because you are subscribed to the Google Groups "kubernetes-sig-scale" group.
To unsubscribe from this group and stop receiving emails from it, send an email to kubernetes-sig-s...@googlegroups.com.
To post to this group, send email to kubernetes...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/kubernetes-sig-scale/CA%2BbcoB5i8TiDK%3D9n%2BM7kH3RK6r7KQWOAWZRbgwpqBOxgtt-4%2BA%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

--

Davanum Srinivas :: https://twitter.com/dims

[Tim Hockin]

Apparently my reply only went to sig-scale.

Hey all.  I read through the thread.  Here are my thoughts (as if you asked :)

SIG-Scalability is chartered with this authority, so they should be able to act on it.  I do not believe they have or would act irresponsibly, but like so many other things in our community, we'll be watching and adjust course if we need to.

I don't quite buy that revert PRs are exactly the same as other PRs.  In no way am I  advocating for carelessness or ignorance of context -- PRs should be reverted if and when it is "safe" to do so, and I trust sig-scalability to make that call, if they have to, without another top-level approver.  Now, if a PR fixes a release blocking bug but introduces a release blocking scalability issue - which state is better?  Both are release-blocking.  Context matters.

IMO a bot that approves sig-scale revert PRs is a good compromise that provides visibility and delegation.  It does not answer the "safety" question -- only a human can do that.

Until such time as a bot can be installed, I support granting top-level approval to a SMALL set of sig-scale people, with the caveat that it is for reverts and if they abuse it, there will be trouble.  Ideally it would be a couple people from different companies from different time-zones, and they would seek each other's approvals in a pinch.

Concretely, assume a PR that they want to roll back mid-day CET.  That's middle of night in US.  Person X from Google could make a PR and get approval from person Y at Red Hat (just as examples).  If they can not reach agreement that it is important, it has to wait until others can weigh in.

Let's give people the benefit of the doubt here - it's not like sig-scale is made up of a bunch of newcomers who don't understand the system.  They are tackling some of the hardest problems we have and deserve a modicum of trust.

> we wouldn't generally want to add approvers only for certain types of changes.

It's not without precedent.  We have at least one commented case in k/k/OWNERS.

> We could possibly provide a mechanism to hold on 5k jobs

Yes, but this is a mitigation, not a solution.  A Big Red Button on scale tests seems like a nice idea, especially as the bill shifts to CNCF.

Tim

You received this message because you are subscribed to the Google Groups "steering" group.
To unsubscribe from this group and stop receiving emails from it, send an email to steering+u...@kubernetes.io.
To post to this group, send email to stee...@kubernetes.io.
Visit this group at https://groups.google.com/a/kubernetes.io/group/steering/.
To view this discussion on the web visit https://groups.google.com/a/kubernetes.io/d/msgid/steering/CAFQm5yQ9UX-_ZsE1Q6bjnzwJ6vCoSCdc333CQ%2BXDG9DVaQP%2BOg%40mail.gmail.com.

[Brendan Burns]

I'm a little worried about the flow here. If we give sig-scalability the right to revert PRs we're going to make it very difficult for the original PR author to get the PR re-submitted.

Note, I'm not talking about obvious, smoking gun that PR needs performance improvements kind of PRs, but rather the subtle "we think it might be this PR" kind of PRs.

I know that in the past we have reverted PRs that we thought were the cause, only to find out that it wasn't the root cause. I worry that if we allow sig-scalability to be aggressive here, it will be challenging for the original author to "fix" their PR since they won't have the resources/experience to run the scale tests.

Ultimately, I trust the sig-scalability folks to be able to make the decisions here, but I want to make certain that they hold reverts to a _really_ high bar for proof before they enact a revert. Part of the value in having a neutral third-party observer for such reverts is that they can hear arguments from both sides before the revert is enacted.

Again, this is in the case of ambiguous PRs, not obvious smoking guns.

--brendan

---

From: 'Tim Hockin' via steering <stee...@kubernetes.io>
Sent: Friday, June 14, 2019 3:58 PM
To: Stephen Augustus
Cc: Maciej Borsz; Davanum Srinivas; steering; kubernetes-sig-architecture; Jordan Liggitt; kubernetes...@googlegroups.com; kubernetes-sig-testing; kubernetes-sig-release; Wojciech Tyczynski
Subject: Re: [k8s-steering] Re: Feature request: Revert-only OWNERS in kubernetes/kubernetes?

 

To view this discussion on the web visit https://groups.google.com/a/kubernetes.io/d/msgid/steering/CAO_RewbpXhvNZ%3DP1EEnKVMrnZC-HMTpyGj0hWN1d%3D2bWtVuGLg%40mail.gmail.com.

[Tim Hockin]

I totally agree, of course.  I assume that is the intention and I have no reason to think otherwise.  Great power, yadda yadda ...

[Aaron Crickenberger]

I have no binding say in this since I'm not in kubernetes/kubernetes/OWNERS, but I just want to put it out there, since I tried to state this before, and feel like it was echoed by others.

Why is it our responsibility to make a shortcut for this one SIG, and not their responsibility to have trained someone up through the same reviewer and approver path that everyone else has to follow to accomplish what they need?  If we're making exceptions for this SIG, what else are we going to make exceptions for?

A thing sig-scalability can do, if they need to change what their jobs run off of in a timely manner so urgent that they cannot wait for an existing approver, is to run their jobs off of forks or branches under their control.  Changes to what their jobs run off of are completely under their approval (https://github.com/kubernetes/test-infra/blob/master/config/jobs/kubernetes/sig-scalability/OWNERS).

I don't think we're very good at the "trust but verify" part that compliments "great responsibility".  To use the prior art Tim mentions as an example, https://github.com/kubernetes/community/pull/598 is closed, pointing to https://github.com/kubernetes/kubectl/pull/179, which is also closed and points nowhere.  Do we still need someone in root OWNERS for this?

- aaron

You received this message because you are subscribed to the Google Groups "kubernetes-sig-architecture" group.
To unsubscribe from this group and stop receiving emails from it, send an email to kubernetes-sig-arch...@googlegroups.com.
To post to this group, send email to kubernetes-si...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/kubernetes-sig-architecture/CAO_Rewa2OvDBFMpuMqozny0jYhW5HeWoFiZOhqm243hn6dQCKg%40mail.gmail.com.

[Tim Hockin]

On Tue, Jun 18, 2019 at 9:31 AM Aaron Crickenberger <spi...@google.com> wrote:
>
> I have no binding say in this since I'm not in kubernetes/kubernetes/OWNERS, but I just want to put it out there, since I tried to state this before, and feel like it was echoed by others.
>
> Why is it our responsibility to make a shortcut for this one SIG, and not their responsibility to have trained someone up through the same reviewer and approver path that everyone else has to follow to accomplish what they need? If we're making exceptions for this SIG, what else are we going to make exceptions for?

The exception is that this is what they are chartered for. Drawing on
experience, the ability to allow relatively "fresh" changes to be
reverted by less-authorized users has been very useful inside Google
(where, admittedly, the monorepo helps).

> A thing sig-scalability can do, if they need to change what their jobs run off of in a timely manner so urgent that they cannot wait for an existing approver, is to run their jobs off of forks or branches under their control. Changes to what their jobs run off of are completely under their approval (https://github.com/kubernetes/test-infra/blob/master/config/jobs/kubernetes/sig-scalability/OWNERS).

This is not a bad idea.

[Brendan Burns]

I think Aaron captures my concerns quite succinctly.

I think that the "prove it in your branch, then argue for revert" approach seems like a really good way to get the right options for people...

--brendan

---

From: Tim Hockin <tho...@google.com>
Sent: Tuesday, June 18, 2019 11:24 AM
To: Aaron Crickenberger
Cc: Brendan Burns; Stephen Augustus; Maciej Borsz; Davanum Srinivas; steering; kubernetes-sig-architecture; Jordan Liggitt; kubernetes...@googlegroups.com; kubernetes-sig-testing; kubernetes-sig-release; Wojciech Tyczynski

Subject: Re: [k8s-steering] Re: Feature request: Revert-only OWNERS in kubernetes/kubernetes?

On Tue, Jun 18, 2019 at 9:31 AM Aaron Crickenberger <spi...@google.com> wrote:
>
> I have no binding say in this since I'm not in kubernetes/kubernetes/OWNERS, but I just want to put it out there, since I tried to state this before, and feel like it was echoed by others.
>
> Why is it our responsibility to make a shortcut for this one SIG, and not their responsibility to have trained someone up through the same reviewer and approver path that everyone else has to follow to accomplish what they need?  If we're making exceptions for this SIG, what else are we going to make exceptions for?

The exception is that this is what they are chartered for.  Drawing on
experience, the ability to allow relatively "fresh" changes to be
reverted by less-authorized users has been very useful inside Google
(where, admittedly, the monorepo helps).

> A thing sig-scalability can do, if they need to change what their jobs run off of in a timely manner so urgent that they cannot wait for an existing approver, is to run their jobs off of forks or branches under their control.  Changes to what their jobs run off of are completely under their approval (https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fkubernetes%2Ftest-infra%2Fblob%2Fmaster%2Fconfig%2Fjobs%2Fkubernetes%2Fsig-scalability%2FOWNERS&amp;data=02%7C01%7Cbburns%40microsoft.com%7Cd138316ee10f43a39f6d08d6f41a46af%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636964791038440737&amp;sdata=Pn2Xvq9BNZV%2BWjxnJzYkG%2BpFuNZKYgKll0kW8w3rlGg%3D&amp;reserved=0).

This is not a bad idea.

> I don't think we're very good at the "trust but verify" part that compliments "great responsibility".  To use the prior art Tim mentions as an example, https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fkubernetes%2Fcommunity%2Fpull%2F598&amp;data=02%7C01%7Cbburns%40microsoft.com%7Cd138316ee10f43a39f6d08d6f41a46af%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636964791038440737&amp;sdata=hIZNnsnCojhtVOiDwtg3KBrU13aYopvRAdkZhQt3jBc%3D&amp;reserved=0 is closed, pointing to https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fkubernetes%2Fkubectl%2Fpull%2F179&amp;data=02%7C01%7Cbburns%40microsoft.com%7Cd138316ee10f43a39f6d08d6f41a46af%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636964791038440737&amp;sdata=cQNQL0DpUuuek04iN6%2BZSGBDQJKU4%2FM5BIX2DQiJoAo%3D&amp;reserved=0, which is also closed and points nowhere.  Do we still need someone in root OWNERS for this?

>>> [1] https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fkubernetes%2Fkubernetes%2Fissues%2F78935&amp;data=02%7C01%7Cbburns%40microsoft.com%7Cd138316ee10f43a39f6d08d6f41a46af%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636964791038440737&amp;sdata=J0%2BpQpvsWbbymv0lkl2h5RucOuc8uPnECpgSRlDrmBY%3D&amp;reserved=0
>>> [2] https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fkubernetes%2Fcommunity%2Fblob%2Fmaster%2Fcontributors%2Fguide%2Fowners.md%23maintaining-owners-files&amp;data=02%7C01%7Cbburns%40microsoft.com%7Cd138316ee10f43a39f6d08d6f41a46af%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636964791038440737&amp;sdata=BTzIx%2B20V%2BL%2FBZjYTSOohQgQf%2B%2FBHeFmuqKBtZukNGk%3D&amp;reserved=0

>>>
>>> On Wed, Jun 12, 2019 at 6:04 AM Davanum Srinivas <dav...@gmail.com> wrote:
>>>
>>> I am in US East TZ. I can volunteer for helping with approving reverts as well.
>>>
>>> -- Dims
>>>
>>> On Wed, Jun 12, 2019 at 4:39 AM 'Maciej Borsz' via kubernetes-sig-scale <kubernetes...@googlegroups.com> wrote:
>>>

>>> https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fkubernetes%2Fkubernetes%2Fpull%2F78931&amp;data=02%7C01%7Cbburns%40microsoft.com%7Cd138316ee10f43a39f6d08d6f41a46af%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636964791038440737&amp;sdata=pMPNA8sycnUmRpBYIXieHkhUqSTpmAIQTi9EE0m41vE%3D&amp;reserved=0 is another example -- as part of debugging regression in 1.15 we found the faulty PR and now we need we are waiting for review.

>>>
>>> If revert-only OWNERS is infeasible, I think that just another (next to Wojtek) Europe time zone approver will help a lot.
>>>
>>>
>>>
>>>
>>>
>>>
>>> On Tue, Jun 11, 2019 at 10:31 AM Maciej Borsz <macie...@google.com> wrote:
>>>
>>>
>>>
>>> On Mon, Jun 10, 2019 at 3:21 PM Stephen Augustus <Ste...@agst.us> wrote:
>>>
>>> This goes back to the reviewers/approvers pools for SIG Scalability subprojects. The SIG needs to work on increasing it to more than just a few people for 1.16.
>>>
>>> Sorry, I don't understand. The 'sig scalability subprojects' page you mentioned earlier shows OWNERS only for sig-scalability-owned code (like kubemark's code). This is not an issue I meant.
>>>
>>> The problem is not about reverting changes to sig-scalability-owned code. The issue is that sometimes we need to revert changes to kubernetes/kubernetes repo as we find that they are breaking scalability tests (and more importantly the scalability of the kubernetes overall).
>>>
>>> The reason why we (sig-scalability) need to manually revert PRs is that presubmits covers only basic 100 node cluster while some issues can be detected only in 5000-node scale. We are running a daily tests 5000-node tests and when we see that it's failing, when we find the culprit we simply want to revert this as fast as possible to make sure there is no other regression.
>>>
>>> My ask is only to make this process smoother in case when only person in sig-scalability with approval permissions (wojtekt) is unavailable for some reason. Revert-only OWNERS seemed to be a good idea to me, but we can figure out some other solution here if this is infeasible.
>>>
>>>
>>> On Mon, Jun 10, 2019, 09:05 Maciej Borsz <macie...@google.com> wrote:
>>>
>>>
>>>
>>> On Mon, Jun 10, 2019 at 2:56 PM Jordan Liggitt <lig...@google.com> wrote:
>>>
>>> Have there been examples of urgent reverts needed that did not find an approver in O(hours)? I tend to agree with Stephen that we wouldn't generally want to add approvers only for certain types of changes.
>>>

>>> https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fkubernetes%2Fkubernetes%2Fpull%2F76190&amp;data=02%7C01%7Cbburns%40microsoft.com%7Cd138316ee10f43a39f6d08d6f41a46af%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636964791038440737&amp;sdata=vdcdrR515f4XqVelZ6MbBZ19tL21MlaWZjYxdgttLq0%3D&amp;reserved=0 is an example revert that we wanted to merge quickly.

>>> If we had to revert similar PR e.g. this week (when Wojtek is on vacation), we would have a problem finding an approver for it.
>>>
>>> I think that preferably I would like to have more than one person in the team that is able to approve such PRs, but if it's infeasible could you suggest some other European time zone approver we can ask for help in such cases?
>>>
>>> Thanks,
>>> Maciej
>>>
>>>
>>>
>>> On Mon, Jun 10, 2019 at 5:07 AM Stephen Augustus <Ste...@agst.us> wrote:
>>>
>>> (take two; I didn't realize that kubernetes...@googlegroups.com was the current address for the SIG. Also, I don't believe external users have permissions to post to gke-kubernetes-scalability, so sending that list to bcc.)
>>>
>>> On Mon, Jun 10, 2019 at 5:02 AM Stephen Augustus <Ste...@agst.us> wrote:
>>>
>>> (+ SIG Scalability, SIG Testing)
>>>
>>> AFAIK, that's not a thing we do today i.e., revert-only OWNERs. I've added SIG Testing to see if that's a thing they'd ever want to do.
>>> Committing and reverting code has similar potential impact, so I'd imagine we'd want to continue along the path of not weighting the two.
>>>
>>> I'm a big +1 to adding more reviewers/approvers to scalability OWNERs, but that's a question/task for SIG Scalability and their subprojects[1], so I've added them here.
>>>
>>> -- Stephen
>>>

>>> [1] https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fkubernetes%2Fcommunity%2Ftree%2Fmaster%2Fsig-scalability%23subprojects&amp;data=02%7C01%7Cbburns%40microsoft.com%7Cd138316ee10f43a39f6d08d6f41a46af%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636964791038440737&amp;sdata=FYC%2BRgVfLOHTUSP%2Fqj2A1s5Nv3aOCyGwx66cbq0AxOA%3D&amp;reserved=0

>>>
>>> On Mon, Jun 10, 2019 at 2:52 AM 'Maciej Borsz' via kubernetes-sig-release <kubernetes-...@googlegroups.com> wrote:
>>>
>>> Hi all,
>>>
>>> In sig-scalability we relatively often have to manually revert PRs affecting gce scalability.
>>> So far, we used wojtekt's superpowers to approve such reverts, but he is a single person in our team that has such permissions (and e.g. this week he is on vacation).
>>>
>>> So I wanted to ask to add more folks from our team to OWNERS in kubernetes/kubernetes repo. Given limited knowledge of the codebase, I was thinking that maybe giving us approval power only for reverts makes sense. It would be sufficient for our work and limit the risk.
>>>
>>> Few questions:
>>> 1. What do you think?
>>> 2. (I'm not that familiar with github) Is it technically possible to distinguish revert PRs from other PRs?
>>>
>>> Thanks,
>>> Maciej
>>>
>>>
>>> --
>>> You received this message because you are subscribed to the Google Groups "kubernetes-sig-release" group.
>>> To unsubscribe from this group and stop receiving emails from it, send an email to kubernetes-sig-re...@googlegroups.com.
>>> To post to this group, send email to kubernetes-...@googlegroups.com.

>>> To view this discussion on the web visit https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgroups.google.com%2Fd%2Fmsgid%2Fkubernetes-sig-release%2FCA%252BbcoB5T9nZqezFZ1vJc5y8BaeC%252BxV77yZbumtSM4L000cnqfQ%2540mail.gmail.com&amp;data=02%7C01%7Cbburns%40microsoft.com%7Cd138316ee10f43a39f6d08d6f41a46af%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636964791038450732&amp;sdata=%2F%2BcCb6Lq6DH2rizVBezu0yn2Q74g6cILur%2FjCS5qngY%3D&amp;reserved=0.
>>> For more options, visit https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgroups.google.com%2Fd%2Foptout&amp;data=02%7C01%7Cbburns%40microsoft.com%7Cd138316ee10f43a39f6d08d6f41a46af%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636964791038450732&amp;sdata=%2Bp4GYIzt5viISKVxdqzXGFO2tIjVGkw%2FlaQlO4Y3XNo%3D&amp;reserved=0.

>>>
>>> --
>>> You received this message because you are subscribed to the Google Groups "kubernetes-sig-release" group.
>>> To unsubscribe from this group and stop receiving emails from it, send an email to kubernetes-sig-re...@googlegroups.com.
>>> To post to this group, send email to kubernetes-...@googlegroups.com.

>>> To view this discussion on the web visit https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgroups.google.com%2Fd%2Fmsgid%2Fkubernetes-sig-release%2FCAFQm5yRCuX5a1HMwvZ9XqWKZBf%252BKMaQV6DsFGJTWD9RJpNV_9Q%2540mail.gmail.com&amp;data=02%7C01%7Cbburns%40microsoft.com%7Cd138316ee10f43a39f6d08d6f41a46af%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636964791038450732&amp;sdata=o3JmcYIBqQD2vZz4Y7PNOAUlmLaZNfAUcBUMhl0Ci2s%3D&amp;reserved=0.
>>> For more options, visit https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgroups.google.com%2Fd%2Foptout&amp;data=02%7C01%7Cbburns%40microsoft.com%7Cd138316ee10f43a39f6d08d6f41a46af%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636964791038450732&amp;sdata=%2Bp4GYIzt5viISKVxdqzXGFO2tIjVGkw%2FlaQlO4Y3XNo%3D&amp;reserved=0.

>>>
>>> --
>>> You received this message because you are subscribed to the Google Groups "kubernetes-sig-scale" group.
>>> To unsubscribe from this group and stop receiving emails from it, send an email to kubernetes-sig-s...@googlegroups.com.
>>> To post to this group, send email to kubernetes...@googlegroups.com.

>>> To view this discussion on the web visit https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgroups.google.com%2Fd%2Fmsgid%2Fkubernetes-sig-scale%2FCA%252BbcoB5i8TiDK%253D9n%252BM7kH3RK6r7KQWOAWZRbgwpqBOxgtt-4%252BA%2540mail.gmail.com&amp;data=02%7C01%7Cbburns%40microsoft.com%7Cd138316ee10f43a39f6d08d6f41a46af%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636964791038450732&amp;sdata=pKj8yMDsmho8UK57%2Frtb6%2FR33avw2yLBPmfAJeHehLA%3D&amp;reserved=0.
>>> For more options, visit https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgroups.google.com%2Fd%2Foptout&amp;data=02%7C01%7Cbburns%40microsoft.com%7Cd138316ee10f43a39f6d08d6f41a46af%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636964791038450732&amp;sdata=%2Bp4GYIzt5viISKVxdqzXGFO2tIjVGkw%2FlaQlO4Y3XNo%3D&amp;reserved=0.
>>>
>>>
>>>
>>> --
>>> Davanum Srinivas :: https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Ftwitter.com%2Fdims&amp;data=02%7C01%7Cbburns%40microsoft.com%7Cd138316ee10f43a39f6d08d6f41a46af%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636964791038450732&amp;sdata=43QFzP8qeiDX%2FSLHf5%2B%2FQUerlP5ozZuwzhjIMgEL0Dc%3D&amp;reserved=0

>>>
>>> --
>>> You received this message because you are subscribed to the Google Groups "steering" group.
>>> To unsubscribe from this group and stop receiving emails from it, send an email to steering+u...@kubernetes.io.
>>> To post to this group, send email to stee...@kubernetes.io.

>>> Visit this group at https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgroups.google.com%2Fa%2Fkubernetes.io%2Fgroup%2Fsteering%2F&amp;data=02%7C01%7Cbburns%40microsoft.com%7Cd138316ee10f43a39f6d08d6f41a46af%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636964791038450732&amp;sdata=v8Tqktvv%2Fn%2Bjb%2BEZyqNJ4bFyCpHa2F2vdwAyt3otlt0%3D&amp;reserved=0.
>>> To view this discussion on the web visit https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgroups.google.com%2Fa%2Fkubernetes.io%2Fd%2Fmsgid%2Fsteering%2FCAFQm5yQ9UX-_ZsE1Q6bjnzwJ6vCoSCdc333CQ%252BXDG9DVaQP%252BOg%2540mail.gmail.com&amp;data=02%7C01%7Cbburns%40microsoft.com%7Cd138316ee10f43a39f6d08d6f41a46af%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636964791038450732&amp;sdata=rXU8s0hfA3Hvk7eBpHzylgOTiCRdjtiSI5%2BevsOFH%2Fc%3D&amp;reserved=0.

>>>
>>> --
>>> You received this message because you are subscribed to the Google Groups "steering" group.
>>> To unsubscribe from this group and stop receiving emails from it, send an email to steering+u...@kubernetes.io.

>>> To view this discussion on the web visit https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgroups.google.com%2Fa%2Fkubernetes.io%2Fd%2Fmsgid%2Fsteering%2FCAO_RewbpXhvNZ%253DP1EEnKVMrnZC-HMTpyGj0hWN1d%253D2bWtVuGLg%2540mail.gmail.com&amp;data=02%7C01%7Cbburns%40microsoft.com%7Cd138316ee10f43a39f6d08d6f41a46af%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636964791038450732&amp;sdata=zTW9NhDNsaKjFQmHE4%2FEyo2KF9tCMxRaq7CY8D0VgxI%3D&amp;reserved=0.

>>
>> --
>> You received this message because you are subscribed to the Google Groups "kubernetes-sig-architecture" group.
>> To unsubscribe from this group and stop receiving emails from it, send an email to kubernetes-sig-arch...@googlegroups.com.
>> To post to this group, send email to kubernetes-si...@googlegroups.com.

>> To view this discussion on the web visit https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgroups.google.com%2Fd%2Fmsgid%2Fkubernetes-sig-architecture%2FCAO_Rewa2OvDBFMpuMqozny0jYhW5HeWoFiZOhqm243hn6dQCKg%2540mail.gmail.com&amp;data=02%7C01%7Cbburns%40microsoft.com%7Cd138316ee10f43a39f6d08d6f41a46af%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636964791038450732&amp;sdata=95WRe0m%2FanSyDC5Ih%2FcKlhLIn6YP%2FWIlpGebIq3Jiww%3D&amp;reserved=0.
>> For more options, visit https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgroups.google.com%2Fd%2Foptout&amp;data=02%7C01%7Cbburns%40microsoft.com%7Cd138316ee10f43a39f6d08d6f41a46af%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636964791038450732&amp;sdata=%2Bp4GYIzt5viISKVxdqzXGFO2tIjVGkw%2FlaQlO4Y3XNo%3D&amp;reserved=0.

[Wojciech Tyczynski]

  Thank you all for the discussion and sorry for jumping so late (3 weeks of vacation are always good thing).

  I went through the whole conversation and I think that there were couple orthogonal things mentioned in this thread,

which I'm going to respond below.

1. The power to revert for SIG Scalability.

 This is part of the charter that was community-approved. If we want to have the discussion about it again, let's have it,

but until that's resolved we still should have this right.

 One aspect of it is what and when can we revert and whether there are any follow-ups needs for it. Those are all valid

points and I agree we should probably clarify it in more details in charter. We should look into clarifying it in Q3.

2. Scalability reviewers/approvers

 This is not something that solves the problem. The PRs that require reverting are not the ones for things owned by our SIG

but for things that are owned by other SIGs. Note that scalability can be broken by any single change in any part of the system.

So I don't think this discussion is relevant to this topic (and it's not relevant to people outside of SIG scalability).

3. Adding "revert approvers".

 While I personally don't share the concerns here that the powers may be misused (technically when I'm around we could

already revert anything except from api changes, with me being top-level approver), I can probably understand that they

may appear. I want to emphasize Tim`s point that if bug fix is introducing scalability regression, release is blocked

with the PR fixing it and without it. So it doesn't really matter for the release if we revert it or not.

Brendan - to your point about reverts, do you have any example on your mind when we reverted a PR to check if it helps?

The only examples that I'm aware of were actually consulted with authors of those PRs so those weren't really like that.

All the PRs approved by SIG Scalability should have proved impact - if there are other example, I would like to be aware of them.

4. Other mitigations of the problem.

 There were couple of ideas, but i want to point to one that we weren't thinking about before. Which is dedicated branch

that Aaron suggested. I think this is a really great idea that I think we should try to explore in a bit more details. And

potentially experiment with.

 And there is one more that I think noone has been thinking before, which I actually think we should seriously consider.

Which is slight modifications of our test schedule. Currently our tests start at 10am CET. So we often want to revert

something before then to not loose the run if we find something by then. That requires someone with approvals right

around that time. What if we would move that by couple hours so that tests would start somewhere around noon "US East Time"?

That would still mean that tests would finish before European morning (where scalability-related folks are based)

but gives us possibility for approver from at least US East Time (thank you Dims!) to approve before next pass start?

5. Other issues

 You may expect one more email in the context of scalability-related retrospective and AIs in the next day or so from me.

 So just to summarize, based on the above discussion, what I would suggest proceeding with is:

1. Adjusting schedule to mitigate the requirement for quick approval (see point 4 above)

2. Explore a bit deeper the idea of dedicated branch and figure out how exactly it would work 

 (I actually think it can really be done to mitigate all the issues, but it requires a bit more thinking)

3. Clarify couple details in the charter about reverting.

 Does that sounds reasonable?

 thanks

wojtek

[Brendan Burns]

That sounds great to me. I will try to find examples, my concerns were based on my recollection (which could be old/dim)

And to be clear, none of this should be taken as an indication that we think people will do the wrong thing, just a question of baking different priorities.

---

From: Wojciech Tyczynski <woj...@google.com>
Sent: Tuesday, June 25, 2019 12:56:56 AM
To: Brendan Burns
Cc: Tim Hockin; Aaron Crickenberger; Stephen Augustus; Maciej Borsz; Davanum Srinivas; steering; kubernetes-sig-architecture; Jordan Liggitt; kubernetes...@googlegroups.com; kubernetes-sig-testing; kubernetes-sig-release

[Davanum Srinivas]

I'd love to see them as well as i have been closely watching the scalability reverts and bisects especially during release crunch times.

thanks,

Dims