restart homu when configuring

[edunham]

I just had to do this manually after a salt servo-master1 state.apply homu to see changes in the Homu configs show up in the web interface, so it should probably be in the state.

r? @aneeshusa

---

This change is

[aneeshusa]

Hmm, that's strange. I've never used state.apply - can you tell me why you didn't run a state.highstate? A quick glance at the docs doesn't reveal any information about why the service didn't pick up on the watch and restart.

In any case, reload: True isn't what we want here. reload: True changes the behavior of the service.running state to reload a service instead of restart it when it's triggered by another state that it's watching.

What was the config change/which states were changed? Do you still have the output from the state.apply command?

[larsbergstrom]

@aneeshusa If I'm guessing correctly, it's because if you do a highstate and there is a buildbot modification that needs to happen, that can only safely be done with the current configuration when there isn't a currently-executing job. Otherwise, buildbot gets restarted and all of the jobs have to be manually retried (homu will also be stuck waiting for results that will never come).

Homu, on the other hand, has its state separate and can be started and stopped at will while everything is up. The only real risk there is of missing a GH event, but the downtime is pretty minimal and that can always be fixed with a synchronize on the repo in question.

Maybe if there were a way to deploy buildbot changes and just do the "request a reload the next time buildbot is idle" thing, that would let us do a highstate?

[aneeshusa]

@edunham I'm assuming you were applying the changes in #297 (Homu only)?

@larsbergstrom If you update the Buildbot config (and restart Buildbot), does Homu need to be restarted as well? If not, let's open another issue for handling Buildbot restarting - it sounds like that's separate from this one about Homu not restarting.

Also, just yesterday I read a post about fronting a webhook accepting service with a durable queue :) But I think that's overkill for us.

[edunham]

@aneeshusa Correct; I'd made changes to Homu only and didn't want to kick Buildbot as part of applying them.

I think a more robust solution long-term would be for a highstate to tell Buildbot to reload its configs after the current test run finishes. It might be possible to simply trigger a build which runs a MasterShellCommand to kill buildbot, then have buildbot configured as a service that systemd automatically restarts when it goes away.

For now, though, I'd like applying the Homu state to reload the Homu configs. If the Homu service gets restarted on highstate but not on when we apply the homu state independently, it's probably a bug in our state factoring.

[aneeshusa]

Let's move the Buildbot discussion to #304.

I added watch statements in #148 explicitly to restart the Homu service if there have been Homu configuration changes in the same run, so our states should be OK. I'm more inclined to think something about using state.apply instead of state.highstate is causing this. I'll see if I can repro this in Vagrant.

[aneeshusa]

Aha, it looks like state.apply fails because of missing requisites. The homu/init.sls file requires the pip: virtualenv state, which is in another file. state.apply homu only runs the homu/init.sls file, so it's able to complete the config change since those don't have external dependencies, but dies when it gets to restarting the service since it can't find the pip: virtualenv state. The wonky outputter that state.apply uses makes it a bit hard to read.

For now, the solution is to not use state.apply and instead always use state.highstate, so that all the appropriate requisites are found. Long-term, we could add a bunch of include statements to make state.apply work, but I feel this might add tighter cross-sls coupling than it's worth and am not sure about doing.

[aneeshusa]

With #307 merged, I think this can be closed as I'm not aware of any other reasons we need to use state.apply at the moment. We can open a separate issue about doing the work to make state.apply work.

[edunham]

ok, I'll edit the wiki to stop saying state.apply works, and close this :)