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

Target servo-master\d+ with PCRE in the top file #292

Merged
merged 1 commit into from Apr 4, 2016

Conversation

@aneeshusa
Copy link
Member

aneeshusa commented Apr 3, 2016

This fixes the targeting for servo-master\d+ so that the extra states
are actually applied.

I noticed this while reviewing the travis builds for #291.
This was missed in both #284 and #285!


This change is Reviewable

We now number the servo-master IDs like other machines,
so use PCRE targeting to match the `servo-master\d+` pattern
for both states and the Travis test pillars.

This ensures the relevant pillars are available to master machines
and the master-specific states are applied.
@aneeshusa aneeshusa force-pushed the aneeshusa:fix-servo-master-top-match branch from 77b1a6c to 00f2802 Apr 3, 2016
@larsbergstrom
Copy link
Contributor

larsbergstrom commented Apr 4, 2016

r? @edunham

I think there were a couple of these edits you were also looking at after the moveover?

@larsbergstrom
Copy link
Contributor

larsbergstrom commented Apr 4, 2016

Namely, does this also need:

root@servo-master1:/srv/salt# git diff
diff --git a/common/map.jinja b/common/map.jinja
index 52f99f4..c7d6239 100644
--- a/common/map.jinja
+++ b/common/map.jinja
@@ -3,7 +3,7 @@
       'defaults': {
         'servo_home': '/home/servo',
         'hosts': {
-          'servo-master': '96.126.125.232',
+          'servo-master1': '52.37.76.55',
           'servo-linux1': '66.228.48.56',
           'servo-linux2': '173.255.201.95',
           'servo-linux3': '45.79.167.177',
@aneeshusa
Copy link
Member Author

aneeshusa commented Apr 4, 2016

I asked about it before, but I think now we're actually trying to get rid of that entire list (see #287). If anything is using those hosts entries for lookups instead of DNS, it's a bug.

@larsbergstrom
Copy link
Contributor

larsbergstrom commented Apr 4, 2016

@bors-servo
Copy link
Contributor

bors-servo commented Apr 4, 2016

📌 Commit 00f2802 has been approved by larsbergstrom

@bors-servo
Copy link
Contributor

bors-servo commented Apr 4, 2016

Testing commit 00f2802 with merge fe36d77...

bors-servo added a commit that referenced this pull request Apr 4, 2016
…strom

Target servo-master\d+ with PCRE in the top file

This fixes the targeting for servo-master\d+ so that the extra states
are actually applied.

I noticed this while reviewing the travis builds for #291.
This was missed in both #284 and #285!

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

bors-servo commented Apr 4, 2016

☀️ Test successful - travis

@bors-servo bors-servo merged commit 00f2802 into servo:master Apr 4, 2016
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.