No more hack/update* scripts, please

60 views
Skip to first unread message

Tim Hockin

unread,
May 30, 2017, 6:36:32 PM5/30/17
to kuberne...@googlegroups.com
The trend of `hack/update-something.sh` scripts is as old as the
project. It started as simple things that basically always worked and
ran quickly. Over time we have accumulated over 20 such scripts.
Some are still pretty fast. Some take literally 10s of minutes to
run. Some fail for absolutely inscrutable reasons. Even _I_ don't
know how to satisfy some of them.

And we expect our contributors to put up with this?

I'm proposing a full moratorium on new update scripts, and on new
behavior in existing scripts.

I'm proposing that we set a goal that every one of those scripts runs
to completion in 10 seconds or less, and that they have no external
requirements (GOPATH, etc). If they fail or are aborted, they must
leave the build functioning and leave no artifacts (as per `git
status`).

Every such script (and their corresponding verify script) needs a
documented owning SIG or person, and needs an explicit statement of
"this script has been sanitized".

I'm proposing that whatever else we do in the 1.8 release cycle, we
HAVE TO FIX THIS. If we don't, nobody will contribute, and I simply
would not blame them.

For those tests where this is hard, we need to find ways to do them
ourselves, without contributors needing to run them.

What think?

Michael Taufen

unread,
May 30, 2017, 7:23:26 PM5/30/17
to Tim Hockin, kuberne...@googlegroups.com
Yessssssssssssss. I'm down to throw my 20% at helping fix these for 1.8.


--
You received this message because you are subscribed to the Google Groups "Kubernetes developer/contributor discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to kubernetes-dev+unsubscribe@googlegroups.com.
To post to this group, send email to kubernetes-dev@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/kubernetes-dev/CAO_RewbsjjvEz%2Bpht0VjyoMrNowrW%2B%3Dxsf8sLXcjrhb6GLfa%3DQ%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.



--
Michael Taufen
MTV SWE

Christoph Blecker

unread,
May 30, 2017, 7:39:30 PM5/30/17
to Tim Hockin, kuberne...@googlegroups.com
Yeah, I think a moratorium for scripts that don't meet a certain set of guidelines is a worthwhile endeavour.

The external requirements one is a super pain point. Moving more of these into docker containers, and have them be more intelligible in their output will help too. The big sticking point I see on the 10 second goal would be for the scripts that deal with godeps..

Ultimately, many of these scripts wouldn't need to be run by contributors if we moved away from committing generated code. Then scripts like this could be run in CI and by the release process.

Matt Liggett

unread,
May 30, 2017, 7:39:35 PM5/30/17
to Michael Taufen, Tim Hockin, kuberne...@googlegroups.com
I think we should put the work in to run these ourselves as job 0.  How awesome would it be if we freed contributors from *all* of these, even the easy/fast ones like {verify,update}-boilerplate?  Then we could also start timing and profiling the runs (we run them all ourselves).

Brian Grant

unread,
May 30, 2017, 8:05:25 PM5/30/17
to Tim Hockin, kuberne...@googlegroups.com
I agree that it's long past time for these to go.

We should not check in generated code:

That goes for autogenerated BUILD files, also.

Docs don't belong in the kubernetes repo, anyway.

The rest need to be triggered only when touching relevant files.

I also agree about tool dependencies.

Wojciech Tyczynski

unread,
May 31, 2017, 2:26:33 AM5/31/17
to Brian Grant, Tim Hockin, kuberne...@googlegroups.com
 +100 to this effort.

There are also other aspects of it not mentioned explicitly (though Brian mentioned
one if it implicitly) which is conflicts between PRs.

We should try to go over all those scripts and prioritize how painful each of them is
(I guess noone really has problem with update-gofmt.sh). Once we have them
prioritized, we can come of with a solution for each of them (the solutions will most
probably be different for different scripts), including:

- not checking in generated code that Brian already mentioned #8830
- removing ugorji: #36120 (or replace it but our custom solution)
...

On Wed, May 31, 2017 at 2:05 AM, 'Brian Grant' via Kubernetes developer/contributor discussion <kuberne...@googlegroups.com> wrote:
I agree that it's long past time for these to go.

We should not check in generated code:

That goes for autogenerated BUILD files, also.

Docs don't belong in the kubernetes repo, anyway.

The rest need to be triggered only when touching relevant files.

I also agree about tool dependencies.
To post to this group, send email to kuberne...@googlegroups.com.

--
You received this message because you are subscribed to the Google Groups "Kubernetes developer/contributor discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to kubernetes-dev+unsubscribe@googlegroups.com.
To post to this group, send email to kubernetes-dev@googlegroups.com.

Tim Hockin

unread,
May 31, 2017, 2:32:08 AM5/31/17
to Wojciech Tyczynski, Brian Grant, kuberne...@googlegroups.com
I've been working on an alternative JSON codec, but the truth is that
when I benchmark against Go's standard JSON, I'm having a hard time
beating it. I think we should take ugorji out and just measure.
>>> email to kubernetes-de...@googlegroups.com.
>>> To post to this group, send email to kuberne...@googlegroups.com.
>>> To view this discussion on the web visit
>>> https://groups.google.com/d/msgid/kubernetes-dev/CAO_RewbsjjvEz%2Bpht0VjyoMrNowrW%2B%3Dxsf8sLXcjrhb6GLfa%3DQ%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 developer/contributor discussion" group.
>> To unsubscribe from this group and stop receiving emails from it, send an
>> email to kubernetes-de...@googlegroups.com.
>> To post to this group, send email to kuberne...@googlegroups.com.
>> To view this discussion on the web visit

Klaus Ma

unread,
May 31, 2017, 2:33:39 AM5/31/17
to Kubernetes developer/contributor discussion, brian...@google.com, tho...@google.com
I love "scripts runs to completion in 10 seconds or less" :). 


On Wednesday, May 31, 2017 at 2:26:33 PM UTC+8, Wojciech Tyczynski wrote:
 +100 to this effort.

There are also other aspects of it not mentioned explicitly (though Brian mentioned
one if it implicitly) which is conflicts between PRs.

We should try to go over all those scripts and prioritize how painful each of them is
(I guess noone really has problem with update-gofmt.sh). Once we have them
prioritized, we can come of with a solution for each of them (the solutions will most
probably be different for different scripts), including:

- not checking in generated code that Brian already mentioned #8830
- removing ugorji: #36120 (or replace it but our custom solution)
...
On Wed, May 31, 2017 at 2:05 AM, 'Brian Grant' via Kubernetes developer/contributor discussion <kuberne...@googlegroups.com> wrote:
I agree that it's long past time for these to go.

We should not check in generated code:

That goes for autogenerated BUILD files, also.

Docs don't belong in the kubernetes repo, anyway.

The rest need to be triggered only when touching relevant files.

I also agree about tool dependencies.
To unsubscribe from this group and stop receiving emails from it, send an email to kubernetes-de...@googlegroups.com.

--
You received this message because you are subscribed to the Google Groups "Kubernetes developer/contributor discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to kubernetes-de...@googlegroups.com.

To post to this group, send email to kuberne...@googlegroups.com.

Chao Xu

unread,
May 31, 2017, 1:11:39 PM5/31/17
to Klaus Ma, Kubernetes developer/contributor discussion, Brian Grant, Tim Hockin
hack/update-staging-client-go.sh will be removed as part of #44784.

To unsubscribe from this group and stop receiving emails from it, send an email to kubernetes-dev+unsubscribe@googlegroups.com.
To post to this group, send email to kubernetes-dev@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/kubernetes-dev/c174a692-13e6-4a75-ae8b-489d4305f3b2%40googlegroups.com.

Rodrigo Campos

unread,
May 31, 2017, 1:59:29 PM5/31/17
to Tim Hockin, kuberne...@googlegroups.com
On Tue, May 30, 2017 at 03:36:09PM -0700, 'Tim Hockin' via Kubernetes developer/contributor discussion wrote:
> The trend of `hack/update-something.sh` scripts is as old as the
> project. It started as simple things that basically always worked and
> ran quickly. Over time we have accumulated over 20 such scripts.
> Some are still pretty fast. Some take literally 10s of minutes to
> run. Some fail for absolutely inscrutable reasons. Even _I_ don't
> know how to satisfy some of them.
>
> And we expect our contributors to put up with this?
>
> I'm proposing a full moratorium on new update scripts, and on new
> behavior in existing scripts.
>
> I'm proposing that we set a goal that every one of those scripts runs
> to completion in 10 seconds or less, and that they have no external
> requirements (GOPATH, etc). If they fail or are aborted, they must
> leave the build functioning and leave no artifacts (as per `git
> status`).
>
> Every such script (and their corresponding verify script) needs a
> documented owning SIG or person, and needs an explicit statement of
> "this script has been sanitized".
>
> I'm proposing that whatever else we do in the 1.8 release cycle, we
> HAVE TO FIX THIS. If we don't, nobody will contribute, and I simply
> would not blame them.

I agree with all of this, but would like to share some experience when I first
contributed to kubernetes, that was very nice and because of scripts in `hack/`.

When I started hacking on kubernetes, it was REALLY useful for me to find a
script there that runs a kubernetes cluster so I could easily play with my
changes in a cluster locally. I used the `hack/local-up-cluster.sh` script for
that, at the time.

Maybe now there is a better way, but for my first experience (now I know it has
tons of other things) that was really good. And I'd really would like to keep an
easy way to do it for new contributors :)

But just that, keep in mind some are really useful (or were, at least :)).



Thanks a lot,
Rodrigo
Reply all
Reply to author
Forward
0 new messages