1342257 - tc-worker should schedule a self-reboot at least every 96 hours

Status: RESOLVED FIXED

(Taskcluster :: Workers, defect)

Description

[Dustin J. Mitchell [:dustin] (he/him)]

Puppet / OCC will run on reboot only, and we need hosts to keep their config up to date.  Per bug 1336050, we'll also need hosts to renew their credentials.

So, taskcluster-worker will need a feature to shutdown before 96 hours have expired.  Maybe this can be a "soft stop" when idle after, say, 86 hours; or a hard stop at 96 hours.

Comment 1

[Wander Lairson Costa]

This can be easily implemented inside the reboot plugin. I can implement this after bug 1341743

Comment 2

[Dustin J. Mitchell [:dustin] (he/him)]

<whine>but I wanna help!</whine>

Comment 3

[Dustin J. Mitchell [:dustin] (he/him)]

https://github.com/taskcluster/taskcluster-worker/pull/181

Comment 4

[[github robot]]

Commits pushed to master at https://github.com/taskcluster/taskcluster-worker

https://github.com/taskcluster/taskcluster-worker/commit/b6b87cf71e2aa647eb1093d080ca35a507f7e21d
Bug 1342257: support rebooting after a configurable number of hours

* use a RWLock to avoid race conditions in shutting down the worker
* add required plugin config
  * reboot delay (in hours)
  * reboot command (to allow use of sudo where necessary)

* update POSIX reboot technique:
  * Wait for the reboot command to complete and panic if it fails

Note that in my testing, this terminates the worker before it can
execute the reboot command.

https://github.com/taskcluster/taskcluster-worker/commit/e88eb851442fd99c0b5a20b4dbfd6ce46954b5a5
Merge pull request #181 from taskcluster/bug1342257

support rebooting after a configurable number of hours

Comment 5

[Dustin J. Mitchell [:dustin] (he/him)]

waiting to deploy this until we're ready with the secrets service

Comment 6

[Wander Lairson Costa]

Can we close this bug?

Comment 7

[Dustin J. Mitchell [:dustin] (he/him)]

Actually, I was mixing this up with the hostsecrets config plugin.  This one could get deployed now, right?  How do I go about making a release of tc-worker and deploying it on the macs?

Comment 8

[Wander Lairson Costa]

I don't think we need to deploy to close the bug, at least this hasn't been the way we were doing, but nothing stops us from doing that either :)

Comment 9

[Dustin J. Mitchell [:dustin] (he/him)]

I'd like to see hosts rebooting so I can be confident this worked :)

Comment 10

[Dustin J. Mitchell [:dustin] (he/him)]

Wander, can you point me to instructions for deploying a new worker?  If I practice with this one, then I will know how to deploy when the worker-secrets service is ready :)

Comment 11

[Wander Lairson Costa]

(In reply to Dustin J. Mitchell [:dustin] from comment #10)
> Wander, can you point me to instructions for deploying a new worker?  If I
> practice with this one, then I will know how to deploy when the
> worker-secrets service is ready :)

* Tag a new release and push it to github.com. It (should) generates the binary automatically
* Download the macosx64 binary to puppet master /repo/data/EXEs/ (I can't recall the exact path)
* Update the version https://github.com/mozilla/build-puppet/blob/master/modules/packages/manifests/mozilla/taskcluster_worker.pp#L11
* Reboot the machines 0040-0049 (or ask :aselagea to re-image them if you want to play safe).

Comment 12

[Dustin J. Mitchell [:dustin] (he/him)]

> * Tag a new release and push it to github.com. It (should) generates the binary automatically

This doesn't seem to work:
  https://travis-ci.org/taskcluster/taskcluster-worker/jobs/214687349
mv: rename /Users/travis/gopath/src/github.com/taskcluster/taskcluster-worker/taskcluster-worker to /Users/travis/gopath/src/github.com/taskcluster/taskcluster-worker/taskcluster-worker-0.0.11-darwin-amd64: No such file or directory

I also see the `.travis_rename_releases.sh` script setting an env var just before exiting the shell (taking the var out of scope), but that env var is referenced in .travis.yml.  I'm not sure how that would work.  Pete, can you help?

Comment 13

[Pete Moore [:pmoore][:pete]]

(In reply to Dustin J. Mitchell [:dustin] from comment #12)
> > * Tag a new release and push it to github.com. It (should) generates the binary automatically
> 
> This doesn't seem to work:
>   https://travis-ci.org/taskcluster/taskcluster-worker/jobs/214687349
> mv: rename
> /Users/travis/gopath/src/github.com/taskcluster/taskcluster-worker/
> taskcluster-worker to
> /Users/travis/gopath/src/github.com/taskcluster/taskcluster-worker/
> taskcluster-worker-0.0.11-darwin-amd64: No such file or directory

It looks like when we changed from using `go build` to `go install` (which I think we did because some linters require/inspect pkg files) we forgot to update .travis_rename_releases.sh to use the install path, rather than the source directory. In other words, 

mv "${TRAVIS_BUILD_DIR}/taskcluster-worker${FILE_EXT}" "${RELEASE_FILE}"

should be instead:

mv "${GOPATH}/bin/${GOOS}_${GOARCH}/taskcluster-worker${FILE_EXT}" "${RELEASE_FILE}"


> I also see the `.travis_rename_releases.sh` script setting an env var just
> before exiting the shell (taking the var out of scope), but that env var is
> referenced in .travis.yml.  I'm not sure how that would work.  Pete, can you
> help?


This file is sourced, not executed in .travis.yml:

before_deploy:
  - "source .travis_rename_releases.sh"


Also note, release instructions are in https://github.com/taskcluster/taskcluster-worker#releasing

Comment 14

[Dustin J. Mitchell [:dustin] (he/him)]

Thanks!  I missed the sourcing, that's cool :)

Comment 15

[Dustin J. Mitchell [:dustin] (he/him)]

That worked for Windows but not Darwin or Linux. I'll see if I can figure it out.

Comment 16

[Dustin J. Mitchell [:dustin] (he/him)]

Great success!  Pete, please feel free to re-import this fix into generic-worker.

Now, to deploy.

Comment 17

[Greg Arndt [:garndt]]

With generic-worker, do we need to reboot every 96 hours if it already reboots after every task?  I would hope that the machine health API would find machines that haven't run a task in XX hours, otherwise, machines should be rebooting fairly frequently.

This also opens up the question...should we be rebooting after every task?

Comment 18

[Dustin J. Mitchell [:dustin] (he/him)]

If machines aren't busy enough to be running tasks every 96 hours, then we would need something like this.  And, rebooting after every task takes a little bit of time and also loses the buffer cache, so not rebooting would be more efficient.  That said, macs reboot in less than 1 minute, so it's not a huge hit.

Comment 19

[Greg Arndt [:garndt]]

re: minute overhead
This might be ok, as long as it doesn't cause a nasty backlog.  Macs reboot quick, hopefully the Windows and Linux machines are not bad.

re: rebooting after ever job
Currently the macs can have a perfect storm scenario and cause strain on the puppet masters.  If we are increasing the amount of reboots these machines experience, we'll need to coordinate with relops so they can beef up these instances.

re: max uptime of a machine
Good point that it's not guaranteed that a machine will claim a task causing it to reboot every 96 hours.  There is an expectation that configuration changes can be deployed and machines running with that in a certain amount of time.  I believe that this is 24 hours now, so we might even need to lower the 96 hours, which is quite a bit of time to elapse.

Comment 20

[Dustin J. Mitchell [:dustin] (he/him)]

Attachment: Bug 1342257: upgrade taskcluster-worker;

Review commit: https://reviewboard.mozilla.org/r/123940/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/123940/

Comment 21

[Jake Watkins [:dividehex]]

Comment on attachment 8851668 [details]
Bug 1342257: upgrade taskcluster-worker;

https://reviewboard.mozilla.org/r/123940/#review126544

r+ shipit

Comment 22

[Pete Moore [:pmoore][:pete]]

In generic worker, it is possible to configure the worker to reboot after executing a configurable number of tasks. In AWS, on win7, we do this every task - on the other platforms, we do not use this feature, but instead, we poll a url every X minutes (default = 30) between tasks, never when a task is running, to see if the worker should terminate or not. We can also do this on hardware workers. We'd have to hit a different url, but that is quite doable.

In the meantime, a good start would be that we just set the workers to reboot after running e.g. 20 tasks - this is certainly better than not rebooting at all, and means on average we only get the cost of 1/20th of the reboot time per task, since it only happens 1 in 20 times. Typically, at our high utilisation, I would expect this to result in most workers rebooting daily during the working week. We can monitor, and tweak accordingly, until we have a more tuned solution.

Comment 23

[Dustin J. Mitchell [:dustin] (he/him)]

Bug 1342263 covers rebooting in generic-worker.  The needinfo was just to import the release script fix :)

Comment 24

[Dustin J. Mitchell [:dustin] (he/him)]

Attachment: reboot-config.patch

Comment 25

[Dustin J. Mitchell [:dustin] (he/him)]

Attachment: reboot-slower.patch

On the plus side, the macs I rebooted are now rebooting every 96 seconds.

Comment 26

[Dustin J. Mitchell [:dustin] (he/him)]

Comment on attachment 8852597 [details] [diff] [review]
reboot-slower.patch

actually, never mind - I'll just land this as a bustage patch

Comment 27

[Dustin J. Mitchell [:dustin] (he/him)]

I rebooted minis 0040-0048 just now, so they get this change.  After this point we should never see more than 4 days of uptime (most were at 34 days!)