Move buildbot logic to scripts in `/etc/ci`

[edunham]

Possibly of interest to @shinglyu.

Right now a lot of build logic is hard-coded in https://github.com/servo/saltfs/blob/master/buildbot/master/files/config/master.cfg . Ideally, Buildbot would only invoke scripts from https://github.com/servo/servo/tree/master/etc/ci.

It looks like the only templating in master.cfg is to insert the value of common.servo_home, which is set in https://github.com/servo/saltfs/blob/master/common/map.jinja because it differs between Mac and Linux.

to deploy:

• PR the build scripts to servo/servo; land them. servo/servo#10849
• PR servo/saltfs changing buildbot config to invoke the build scripts
• @larsbergstrom and I will test the buildbot changes on the buildmaster to make sure they work
• land saltfs PR & highstate servo-master1

[Ms2ger]

How about the "steps"?

[shinglyu]

Cool, I'll take a look next monday

Ms2ger notifications@github.com 於 2016年4月16日 週六 下午3:37寫道：

How about the "steps"?

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#316 (comment)

[aneeshusa]

Because we're executing arbitrary code from the git checkout, I don't feel comfortable moving the existing build scripts into the servo repo for two reasons: they depend on the buildbot python package which is not installed on the workers and has no need to be installed, and I'd rather not execute arbitrary Python code which will get injected back into our Buildbot process.

@Ms2ger I saw your comment about keeping the "steps" in and I started looking into it then. It can be done, but it's not quite as simple as @edunham's checklist makes out. Buildbot coordinates everything on the master and runs 'remote' shell commands on the workers, so we need a step that dynamically creates new steps from the git checkout; just running a script on the worker will make everything look like one step.

I've prototyped a better solution for this, but it's a little hacky to achieve the dynamic steps behavior. (Buildbot Nine has explicit support for this though, so I feel comfortable using it as it appears to be sanctioned.)
@shinglyu I started working on this before this issue got opened but I don't want to step on your toes; I can push what I have already to a branch so you can take a look and finish it up.

[shinglyu]

@Ms2ger @aneeshusa Excuse my ignorance, but what is the main benefit we want from steps? Is it for reusing the step? or more fine-grained selection of steps to run? or for error recovery or pin-pointing the failed step? I don't have much experience with buildbot so I'm curious.

@aneeshusa : Thank you! Perhaps you can create a commit for your work so far so I can keep you as the author of that commit?

[aneeshusa]

@shinglyu See #325 for what I have so far (sorry it took me a while to get it together).

I'd like to see #325 landed, then we can follow @edunham's checklist. We'll need to change the configuration so instead of having DynamicServoFactory be responsible for creating a bunch of steps, we have a step that reads the yaml file from the servo repo and dynamically adds more steps. This can be accomplished by modifying self.build.steps from inside a Step instance; see https://github.com/isotoma/buildbot_travis/blob/master/buildbot_travis/steps/create_steps.py for an example.

Sorry for being a bit terse; I'm going to bed now (it's 4:30 AM here...) but I can explain things in more detail tomorrow.

[Ms2ger]

I'm particularly interested in having separate logs for stdout of different steps

[larsbergstrom]

@aneeshusa Thanks for putting together #325!

A big part of the reason I'd like to see this separated out (i.e., the main benefit) is that improvements to the specific steps, processing of the per-step output (as noted by @Ms2ger), etc. can all be made by contributors without direct access to our CI infrastructure. Today, rolling out CI changes is and has to be a manual process, sometimes involving complicated coordination with landing of PRs into servo/servo.

[shinglyu]

@aneeshusa So what you mean is that we have a step that reads the yaml, and modifies its self.build.steps in runtime to add more dynamic steps?

When #325 landed, we can merge the yaml into servo/servo's /etc/ci. And then we prepare a PR for servo/saltfs that switches to the read-yaml-and-add-step thing?

[aneeshusa]

@shinglyu yes, that's right.

The current DynamicServoFactory reads the steps.yml file when the master starts up, and adds all the steps during start-up. After PRing the YAML file to the main servo repo, we need to change DynamicServoFactory to only add one new step to each build; that step will be responsible for interrogating the steps.yml file and adding more steps dynamically via self.build.steps (the make_step method should likely be moved from DynamicServoFactory to the new step class). The new step should also have the behavior of BadConfigurationStep - that is, fail the build - in case there are any problems reading the YAML file or setting up the new dynamic steps.

[shinglyu]

@aneeshusa Can I PR the YAML file into main servo repo? Or do you want to do it yourself?

[aneeshusa]

@shinglyu Feel free to make a PR to copy it over! I'll probably wait for #325 to get deployed and confirm there are no problems before approving it, but you can work on the PR to update servo/saltfs in the meantime.

[shinglyu]

@aneeshusa Sounds good. I'll do as you said.

[shinglyu]

@aneeshusa I'm struggling to understand how salt and buildbot works together. I tried to setup the Vagrant VMs using the hack in #344, but I'm not sure what to do next. What is the minimal setup + step I can test the code you changed? And I'd be very grateful if you can explain how things works together, e.g. how the VMs work together and what triggers what.

[aneeshusa]

@shinglyu
Salt is our configuration management system; we use it to make sure all the right packages are installed, all the configuration files have the right contents in the right place with the right permissions, etc. In relation to Buildbot, Salt is responsible for installing and configuring our Buildbot setup. You shouldn't need to make any Salt changes, just use it.

The Buildbot master is on the servo-master1 machine, so use vagrant up servo-master1 to bring up the machine, and vagrant ssh servo-master1 to log into it and poke around. Inside the VM, if you rename master.cfg to master.py, you should be to able to open up a python2 REPL and import the module to check out the configuration and see if it does what you want.

We also have a test suite that checks a few things about the Buildbot configuration; running it in Vagrant requires some extra steps but you can use sudo buildbot checkconfig /home/servo/buildbot/master to check your config (rename the master.py back to master.cfg for this). Catch me on IRC if you want to hear more about running the actual test suite.

We don't yet have a good story for testing Buildbot's functionality to this level (i.e. having the master running with some workers, sending builds and making sure they work properly). The VMs are all run individually right now, and don't connect to each other yet; we have mostly been using Vagrant to check files and permissions, not services yet. Since our testing story is weak, I would ask that in your PR you simply add a new builder that uses the updated logic in your PR (and keep the existing ones), so that we can test that one out without breaking our existing deployment. Improvements to testing are always welcome!

The workflow I'd recommend is:

• Make changes to the buildbot configuration (in /vagrant or outside the VM)
• Run vagrant provision servo-master1 outside the VM to update the config via Salt
• Use the checkconfig command and a python2 REPL to test your changes

This could definitely be documented better, so feel free to update the wiki Buildbot and SaltStack pages with anything you found helpful.

Also, I'm in the opposite time zone from you (EDT), but you should be able to catch me on IRC late my time/early your time.

[shinglyu]

@aneeshusa Thank you!

So I only have to work with servo-master1, and I'll create a new builder instead of just override the old one, is the correct?

I'll give it a try and get back to you, thank you :)

[aneeshusa]

@shinglyu Yup, that's right. Once we've deployed the new builder and made sure it works properly, we can switch over the other builders in another PR.

[shinglyu]

@aneeshusa How do I start buildbot in the servo-master1 VM? should I use the user servo? I used the default user vagrant but failed. Here is what I did:

vagrant ssh servo-master1
# In VM now
cd /home/servo/buildbot
buildbot start master
# Failed! In `./master/twisted.log I found an instruction that ask me to run buildbot upgrade-master master
buildbot upgrade-master
buildbot start master
# Failed, the ./master/twisted.log did not change
sudo rm -rf master/twisted.log # Can't delete with normal user
buildbot start master
# IOError: [Errno 13] Permission denied: '/home/servo/buildbot/master/twistd.log'
# Seems like the logger can't access the file.

[shinglyu]

@larsbergstrom Suggested me not to setup everything for testing, since it's too complicated and doesn't worth the effort.

@aneeshusa I wonder how we can get the workdir inside the build step? https://github.com/servo/saltfs/compare/master...shinglyu:yaml?expand=1#diff-ad2d9737c89c906fb7307c5661a5415cR61

[metajack]

Are we still moving forward with this?

[aneeshusa]

I just got around to reviewing @shinglyu's PR a few days ago, so waiting for him to address the comments. It looks pretty close though already.

[aneeshusa]

I don't think this needs an absolute path - the docs use an relative path - so I'd like to look at the logs again (e.g. did we run the git step first?). However, since you rolled back, Buildbot no longer remembers anything about linux-dev-yaml and won't serve the logs anymore. Can you roll this back out? The PR is architected to run safely - it simply adds the non-gated linux-dev-yaml without touching the existing builders, and buildbot handles exceptions safely as well.

[larsbergstrom]

This has been rolled back out, and you can see the non-gated linux-dev-yaml build failing on every PR :-)

[aneeshusa]

#424 should fix the "no such file or directory" error - failing to even exec is what tippped me off.

[Ms2ger]

Are we going anywhere with this?

[aneeshusa]

@Ms2ger TL; DR; Yes.

At this point, we're successfully reading the steps from the servo repo but our new step creation is missing a few things. The latest development was #424 to fix a small bug in step lookup. It needs to be deployed but based on some previous testing in prod and looking at the log files, we need to do a little more tweaking when we create steps to avoid breaking Buildbot's internal invariants. (Buildbot 0.8.12 which we run doesn't explicitly support this behavior, but Buildbot 9 will.) I'm fairly sure I can figure it out based on the log files and this related project, which has a Buildbot contributor as one of the main authors. I'm hoping to look at this when I have some time later this week.

[aneeshusa]

The saltfs Buildbot configuration is now able to successfully run builds from the steps.yml in the main servo tree! See http://build.servo.org/builders/linux-dev-yaml/ (also http://build.servo.org/builders/linux-dev-yaml/builds/1632).