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

Add additional warning for oc cluster up not being able to access port 8443 #11597

Merged
merged 1 commit into from
Oct 28, 2016
Merged

Add additional warning for oc cluster up not being able to access port 8443 #11597

merged 1 commit into from
Oct 28, 2016

Conversation

coreydaley
Copy link
Member

Adding warning about port 8443 potentially being blocked by firewall rules on oc cluster up

Fixes issue #10807

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.

Just one change requested, otherwise looks good

@@ -372,7 +372,7 @@ func (h *Helper) Start(opt *StartOptions, out io.Writer) (string, error) {
fmt.Fprintf(out, "Waiting for API server to start listening\n")
masterHost := fmt.Sprintf("%s:8443", opt.ServerIP)
if err = cmdutil.WaitForSuccessfulDial(true, "tcp", masterHost, 200*time.Millisecond, 1*time.Second, serverUpTimeout); err != nil {
return "", ErrTimedOutWaitingForStart(h.containerName).WithDetails(h.OriginLog())
return "", errors.NewError("timed out waiting for OpenShift container %q \nWARNING: %s:8443 may be blocked by firewall rules", h.containerName, opt.ServerIP).WithDetails(h.OriginLog())
Copy link
Contributor

Choose a reason for hiding this comment

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

I would add something to the effect of
.WithSolution("Ensure that you can access %s from your machine", masterHost)

@coreydaley
Copy link
Member Author

test failure was due to #11560

@coreydaley
Copy link
Member Author

[test]
fixed s/WithResolution/WithSolution/

@openshift-bot
Copy link
Contributor

Evaluated for origin test up to 8c99bd5

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/10733/) (Base Commit: 7366167)

@csrwng
Copy link
Contributor

csrwng commented Oct 27, 2016

LGTM, @bparees ptal

@bparees
Copy link
Contributor

bparees commented Oct 27, 2016

[merge]

@openshift-bot
Copy link
Contributor

openshift-bot commented Oct 27, 2016

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/10733/) (Image: devenv-rhel7_5263)

@openshift-bot
Copy link
Contributor

Evaluated for origin merge up to 8c99bd5

@openshift-bot openshift-bot merged commit a3623ca into openshift:master Oct 28, 2016
@coreydaley coreydaley deleted the github_10807_better_warning_8443_not_accessible branch October 28, 2016 14:53
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.

4 participants