Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure all Buildbot builders listed for servo repo in Homu configuration are valid #443

Closed
aneeshusa opened this issue Jul 20, 2016 · 15 comments
Closed

Comments

@aneeshusa
Copy link
Member

@aneeshusa aneeshusa commented Jul 20, 2016

The Homu configuration (homu/cfg.toml) lists a number of Buildbot repos that are used for gating/try builds, in the builders and try_builders fields of repo.servo.buildbot. Recently, the windows builder got renamed to windows-dev in the Buildbot configuration without updating the Homu configuration, causing breakage. We should ensure that all the builders listed are valid builders.

Files:

  • Make a new test file tests/sls/homu/valid_builders.py.

The test should read in the templated, deployed Homu configuration using the toml library, and examine those two fields, checking them against the list of builders specified in the deployed Buildbot configuration to make sure they all exist.

Note: successfully reading the deployed Buildbot configuration is not that easy, because we need to import the master.cfg file, but it doesn't have a .py extension! If you look at how Buildbot 8 does this, it looks like this requires usage of exec. (I don't see anything in either importlib or imp that would help here.) I consider this a necessary evil for now.

Note 2: there may be problems due to the tests using Python 3 and the Buildbot configuration using Python 2; it is likely that a Python 2 subprocess will be needed to extract the list of builders from the configuration.

Tip: if you rename the master.cfg file to master.py, you can import the module from a Python REPL to look at the available bindings to get to the list of builders.

@loganmhb
Copy link

@loganmhb loganmhb commented Jul 21, 2016

I am interested in giving this a try.

@aneeshusa
Copy link
Member Author

@aneeshusa aneeshusa commented Jul 22, 2016

@loganmhb Great! Let us know if you have any questions.

@loganmhb
Copy link

@loganmhb loganmhb commented Jul 28, 2016

Just starting out with this, but it looks like the imp.load_source() function might enable us to load master.cfg without the use of exec, if that seems preferable. (See http://stackoverflow.com/questions/2601047/import-a-python-module-without-the-py-extension)

@aneeshusa
Copy link
Member Author

@aneeshusa aneeshusa commented Aug 4, 2016

@loganmhb If you can make it work without exec, I'd be overjoyed. Thanks for double-checking the docs!

@aneeshusa
Copy link
Member Author

@aneeshusa aneeshusa commented Aug 31, 2016

@loganmhb How's this going? Do you have any more questions?

@loganmhb
Copy link

@loganmhb loganmhb commented Sep 3, 2016

@aneeshusa apologies for slow progress here -- it's taken me a little while to get up to speed with the salt build process. I've got the local test environment with Vagrant running, but I can't figure out where to find the deployed Buildbot configuration -- can you point me in the right direction for that? Thanks!

@aneeshusa
Copy link
Member Author

@aneeshusa aneeshusa commented Sep 3, 2016

@loganmhb There's a bit of indirection in the Salt code that deploys the Buildbot master configuration, but you should be able to find it in /home/servo/buildbot/master/. I'll also update the issue description, sorry I didn't include that originally!

@loganmhb
Copy link

@loganmhb loganmhb commented Sep 6, 2016

Hmm, there must be something wrong with how I'm doing the setup then -- the /home/servo directory is empty. I'll try to figure out where that's going wrong.

@aneeshusa
Copy link
Member Author

@aneeshusa aneeshusa commented Sep 6, 2016

@loganmhb You need to be using the servo-master1 Vagrant VM - did you use another one perchance?

@jdm
Copy link
Member

@jdm jdm commented Oct 18, 2016

@loganmhb Did you sort out the issue? Are you still working on this?

@loganmhb
Copy link

@loganmhb loganmhb commented Oct 30, 2016

So sorry for disappearing! Thought I had already responded to this but I
didn't send it apparently.

I haven't been able to work out my virtualenv/environment issues yet - have
had limited time to work on it. I may be able to sometime soon, but I don't
want to block progress if someone else wants to handle it, so if you want
to reassign you should go ahead.

On Tue, Oct 18, 2016, 5:29 PM Josh Matthews notifications@github.com
wrote:

@loganmhb https://github.com/loganmhb Did you sort out the issue? Are
you still working on this?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#443 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AGz720Ik9gBBA8AZ8RZjLCLpaS6FfZPoks5q1TokgaJpZM4JQYvv
.

@aneeshusa aneeshusa removed the C-assigned label Nov 5, 2016
@aneeshusa
Copy link
Member Author

@aneeshusa aneeshusa commented Dec 22, 2016

#562 is another place this would have been helpful! (It always seems to be windows-related...)

@charlesvdv
Copy link
Contributor

@charlesvdv charlesvdv commented Dec 31, 2016

I'm interrested to work on this.
EDIT: fix my vagrant problem.

@aneeshusa
Copy link
Member Author

@aneeshusa aneeshusa commented Dec 31, 2016

@charlesvdv go for it!

bors-servo added a commit that referenced this issue Jan 11, 2017
Add test to verify if all homu builders are listed in the buildbot config

That should resolve the #443.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/saltfs/571)
<!-- Reviewable:end -->
@aneeshusa
Copy link
Member Author

@aneeshusa aneeshusa commented Jan 11, 2017

Fixed by #571.

@aneeshusa aneeshusa closed this Jan 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.