-
Notifications
You must be signed in to change notification settings - Fork 4.8k
Specify UI bind address in integration tests #1176
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
Conversation
|
[test] |
|
continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_openshift3/1203/) |
pkg/cmd/server/config.go
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like the default port for the assetPublicAddr should be assetBindAddr port. That would allow it to do match defaulting for masterPublicAddr (directly specified, masterAddr if specified, bindAddr).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if I was exposing the asset stuff via flags, I'd agree... maybe I'll just add that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if I was exposing the asset stuff via flags, I'd agree... maybe I'll just add that
may as well, it's easy to unit test :)
|
@deads2k I'd like to expose the UI config to the command line as a second step... mostly want to get this in to prevent test breakages at the moment |
|
lgtm [merge] |
|
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_requests_openshift3/1071/) (Image: devenv-fedora_929) |
|
Evaluated for origin up to 7c3bbef |
Merged by openshift-bot
No description provided.