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

Switch to nip.io from xip.io for default cluster up wildcard DNS #13023

Merged
merged 1 commit into from Mar 3, 2017

Conversation

jimmidyson
Copy link
Contributor

Fixes #13020 if you're interested in doing that...

@mfojtik
Copy link
Member

mfojtik commented Feb 21, 2017

+1 I like nip.io more than xip.io

Copy link
Contributor

@csrwng csrwng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change LGTM, and as far as I can tell, it's working great.
Would like to get signoff from @smarterclayton and/or @bparees

@@ -1,6 +1,6 @@
package builds

// these tests are diabled because the xip.io dns hook was proving way too unreliable;
// these tests are diabled because the nip.io dns hook was proving way too unreliable;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/diabled/disabled/

also is it still unreliable?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(the latter is probably more a question for @csrwng )

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'd need to enable it and see. But I would create separate issue to look into that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@csrwng How about enabling it in this PR and running tests a few times to see if it solves the issue? That would give more confidence that this change actually switches to a more stable wildcard DNS service?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Corrected old diabled typo btw

@bparees
Copy link
Contributor

bparees commented Feb 21, 2017

one nit, otherwise lgtm.

@smarterclayton
Copy link
Contributor

Be sure to publicize this because people have scripted docs around it.

@e-minguez
Copy link

Wouldn't be nice to have our own cool domain like minishift.io? xip.io/nip.io are just dns servers with custom scripts... and as a bonus we can have some stats about who is using it :D

@openshift-bot openshift-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 27, 2017
@openshift-bot openshift-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 27, 2017
@jimmidyson
Copy link
Contributor Author

Any more thoughts on this?

@csrwng
Copy link
Contributor

csrwng commented Mar 1, 2017

@jimmidyson I haven't seen any strong objections from the mailing lists
LGTM

@bparees
Copy link
Contributor

bparees commented Mar 1, 2017

[merge]

@openshift-bot
Copy link
Contributor

Evaluated for origin merge up to d7d3b7c

@openshift-bot
Copy link
Contributor

[Test]ing while waiting on the merge queue

@openshift-bot
Copy link
Contributor

Evaluated for origin test up to d7d3b7c

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin_future/709/) (Base Commit: 2cac04a)

@openshift-bot
Copy link
Contributor

openshift-bot commented Mar 3, 2017

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin_future/778/) (Base Commit: e164e80) (Image: devenv-rhel7_6026)

@openshift-bot openshift-bot merged commit ddbe7ef into openshift:master Mar 3, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants