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.

Sign up for GitHub

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

Jump to bottom

Set default value of export CHE_INFRA_OPENSHIFT_MASTER__URL in ocp.sh #8236

Merged
merged 1 commit into from
Jan 15, 2018
Merged

Set default value of export CHE_INFRA_OPENSHIFT_MASTER__URL in ocp.sh #8236

merged 1 commit into from
Jan 15, 2018

Conversation

JamesDrummond
Copy link
Contributor

What does this PR do?

Set default value of export CHE_INFRA_OPENSHIFT_MASTER__URL to OPENSHIFT_ENDPOINT if CHE_INFRA_OPENSHIFT_MASTER__URL is not set.

What issues does this PR fix or reference?

#8147

Signed-off-by: James Drummond james@devcomb.com

@devcomb
Set default value of export CHE_INFRA_OPENSHIFT_MASTER__URL to OPENSH…
1e9e5df 
…IFT_ENDPOINT if

 CHE_INFRA_OPENSHIFT_MASTER__URL  is not set.

Signed-off-by: James Drummond <james@devcomb.com>
@JamesDrummond JamesDrummond added the kind/bug Outline of a bug - must adhere to the bug report template. label Jan 10, 2018
@JamesDrummond JamesDrummond added this to the 6.0.0-M5 milestone Jan 10, 2018
@JamesDrummond JamesDrummond self-assigned this Jan 10, 2018
@JamesDrummond JamesDrummond requested review from riuvshin and a user January 10, 2018 23:38
@JamesDrummond JamesDrummond mentioned this pull request Jan 10, 2018
Closed
@codenvy-ci
Copy link

Can one of the admins verify this patch?

1 similar comment
@codenvy-ci
Copy link

Can one of the admins verify this patch?

@benoitf benoitf added the status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. label Jan 10, 2018
@codenvy-ci
Copy link

Can one of the admins verify this patch?

@sleshchenko
Copy link
Member

@JamesDrummond You labeled this PR as a bug. Does it resolve any issues?
P.S. For me, it's OK to set master URL explicitly while we set credentials but I try to understand is it a bug or a small improvement?

@benoitf benoitf removed their request for review January 11, 2018 10:09
@JamesDrummond
Copy link
Contributor Author

JamesDrummond commented Jan 11, 2018 •
edited
Loading

@sleshchenko Think it's a bug as most people don't even know to set this and even OPENSHIFT_ENDPOINT has a default set but I will change to enhancement. I was on the fence when I pick this.

Please see the above reference issue what this resolves. If CHE_INFRA_OPENSHIFT_MASTER__URL is not explicitly set when using ocp the GET request defaults to "https://kubernetes.default.svc/apis/project.openshift.io/v1/projectrequests" when it should be "${OPENSHIFT_ENDPOINT}/apis/project.openshift.io/v1/projectrequests".

@JamesDrummond JamesDrummond added kind/enhancement A feature request - must adhere to the feature request template. and removed kind/bug Outline of a bug - must adhere to the bug report template. labels Jan 11, 2018
sleshchenko
sleshchenko approved these changes Jan 11, 2018
View reviewed changes
Copy link
Member

@sleshchenko sleshchenko left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I think setting master URL explicitly is good and can be useful.
Please double check that it works fine.

@JamesDrummond
Copy link
Contributor Author

@riuvshin or @eivantsov Please review/approve PR so I can merge.

@JamesDrummond
Copy link
Contributor Author

@sleshchenko I check this again. No problems.

@JamesDrummond JamesDrummond merged commit a0d0ade into master Jan 15, 2018
@JamesDrummond JamesDrummond deleted the openshift-default-infra-endpoint branch January 15, 2018 16:23
@benoitf benoitf removed the status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. label Jan 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@riuvshin riuvshin riuvshin approved these changes

@sleshchenko sleshchenko sleshchenko approved these changes

Assignees

@JamesDrummond JamesDrummond

Labels
kind/enhancement A feature request - must adhere to the feature request template.
Projects
None yet
Milestone
6.0.0-M5
Development

Successfully merging this pull request may close these issues.
6 participants
@JamesDrummond @codenvy-ci @sleshchenko @riuvshin @benoitf @devcomb