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

Allow to customize cheHost #380

Merged
merged 12 commits into from
Aug 12, 2020
Merged

Allow to customize cheHost #380

merged 12 commits into from
Aug 12, 2020

Conversation

tolusha
Copy link
Contributor

@tolusha tolusha commented Aug 3, 2020 •
edited
Loading

Signed-off-by: Anatolii Bazko [email protected]

What does this PR do

  • Allows to customize cheHost + secure cheHost ingress/route with a certificate

Reference issue

https://issues.redhat.com/browse/CRW-1022

Documentation

eclipse-che/che-docs#1442

@tolusha tolusha requested a review from mmorhun August 3, 2020 11:13
@tolusha tolusha changed the title Allow to customize cheHost [WIP] Allow to customize cheHost Aug 3, 2020
mmorhun
mmorhun approved these changes Aug 4, 2020
View reviewed changes
Copy link
Contributor

@mmorhun mmorhun left a comment

Choose a reason for hiding this comment

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

LGTM

@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: mmorhun, tolusha
To complete the pull request process, please assign
You can assign the PR to them by writing /assign in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@mmorhun mmorhun assigned tolusha and unassigned mmorhun Aug 4, 2020
@tolusha tolusha mentioned this pull request Aug 4, 2020
Closed
42 tasks
@tolusha
Copy link
Contributor Author

tolusha commented Aug 4, 2020

[test]

@openshift-ci-robot
Copy link

New changes are detected. LGTM label has been removed.

@tolusha tolusha changed the title [WIP] Allow to customize cheHost Allow to customize cheHost Aug 5, 2020
@tolusha tolusha requested review from davidfestal and removed request for davidfestal August 5, 2020 09:12
davidfestal
davidfestal reviewed Aug 6, 2020
View reviewed changes
Copy link
Contributor

@davidfestal davidfestal left a comment

Choose a reason for hiding this comment

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

Are we sure that all the types of OpenShift 4 platforms allow setting custom hosts ?
Afair on Openshift Online at some point it was not possible.
And now I'm especially thinking of OSD for example. Maybe this ability is driven by some cluster-wide setting ?

I'm asking that because the first time you detect that no Che host is specified (empty string), then it is set back to the calculated value in the CR spec.
so the next time the controller sees the CR, it would behave as if is was set explicitly by the user in the CR, and set the host explicitly in the configured route.

So finally we'd end up with the controller always needing the custom-host permission, which might be quite a hard and restrictive requirement.

Or do you check that the value in the CR is the same as the expected value (that OS would set itself), and then just remove the host from the created route to let OpenShift set it on its own ?

@tolusha
Copy link
Contributor Author

tolusha commented Aug 7, 2020

@davidfestal
PR is updated.
First we detect the default cheHost value and if it is different from a value in the CR then we recreate a route.

@tolusha
Copy link
Contributor Author

tolusha commented Aug 7, 2020

Retested on minikube and openshift 4.4

  • without cheHost in CR
  • with default cheHost in CR
  • with custom cheHost in CR (requires extra roles)

@tolusha
Copy link
Contributor Author

tolusha commented Aug 7, 2020

[test]

tolusha added 5 commits August 10, 2020 14:11
@tolusha
Allow to customize cheHost
a874712 
Signed-off-by: Anatolii Bazko <[email protected]>
@tolusha
Add required roles
ecfe7dd 
Signed-off-by: Anatolii Bazko <[email protected]>
@tolusha
Don't creat che route with a default hostname
915d5ef 
Signed-off-by: Anatolii Bazko <[email protected]>
@tolusha
Fix
56494dd 
Signed-off-by: Anatolii Bazko <[email protected]>
@tolusha
Fixes
a8992b2 
Signed-off-by: Anatolii Bazko <[email protected]>
@che-bot
Copy link
Contributor

che-bot commented Aug 10, 2020

Latest version of Eclipse Che failed installation in minishift.

@che-bot
Copy link
Contributor

che-bot commented Aug 10, 2020

Latest version of Eclipse Che installed and tested successfully in minikube.

@tolusha
Fix deploying on minishift
7391259 
Signed-off-by: Anatolii Bazko <[email protected]>
@che-bot
Copy link
Contributor

che-bot commented Aug 10, 2020

Latest version of Eclipse Che installed and tested successfully in minikube.

@che-bot
Copy link
Contributor

che-bot commented Aug 10, 2020

Latest version of Eclipse Che installed and tested successfully in minishift.

@tolusha
Copy link
Contributor Author

tolusha commented Aug 10, 2020

[test]

@che-bot
Copy link
Contributor

che-bot commented Aug 10, 2020

Latest version of Eclipse Che installed and tested successfully in minikube.

@che-bot
Copy link
Contributor

che-bot commented Aug 10, 2020

Latest version of Eclipse Che installed and tested successfully in minishift.

@tolusha
Fixes
74167fb 
Signed-off-by: Anatolii Bazko <[email protected]>
@che-bot
Copy link
Contributor

che-bot commented Aug 11, 2020

Latest version of Eclipse Che installed and tested successfully in minikube.

@che-bot
Copy link
Contributor

che-bot commented Aug 11, 2020

Latest version of Eclipse Che installed and tested successfully in minishift.

@tolusha
Fixes
6d17eb8 
Signed-off-by: Anatolii Bazko <[email protected]>
@che-bot
Copy link
Contributor

che-bot commented Aug 11, 2020

Latest version of Eclipse Che failed installation in minishift.

@che-bot
Copy link
Contributor

che-bot commented Aug 11, 2020

Latest version of Eclipse Che installed and tested successfully in minikube.

@tolusha
Fixes
08a0680 
Signed-off-by: Anatolii Bazko <[email protected]>
@che-bot
Copy link
Contributor

che-bot commented Aug 11, 2020

Latest version of Eclipse Che installed and tested successfully in minikube.

@che-bot
Copy link
Contributor

che-bot commented Aug 11, 2020

Latest version of Eclipse Che failed installation in minishift.

@tolusha
Fixes
85372d2 
Signed-off-by: Anatolii Bazko <[email protected]>
@che-bot
Copy link
Contributor

che-bot commented Aug 11, 2020

Latest version of Eclipse Che installed and tested successfully in minikube.

@che-bot
Copy link
Contributor

che-bot commented Aug 11, 2020

Latest version of Eclipse Che failed installation in minishift.

@tolusha
Copy link
Contributor Author

tolusha commented Aug 12, 2020

[test]

@che-bot
Copy link
Contributor

che-bot commented Aug 12, 2020

Latest version of Eclipse Che failed installation in minishift.

@tolusha
Copy link
Contributor Author

tolusha commented Aug 12, 2020

[test]

@che-bot
Copy link
Contributor

che-bot commented Aug 12, 2020

Latest version of Eclipse Che installed and tested successfully in minikube.

@che-bot
Copy link
Contributor

che-bot commented Aug 12, 2020

Latest version of Eclipse Che installed and tested successfully in minishift.

@tolusha tolusha merged commit bcf77d4 into master Aug 12, 2020
@tolusha tolusha deleted the customizechehost branch August 12, 2020 08:21
@tolusha tolusha mentioned this pull request Aug 12, 2020
Merged
@che-bot che-bot added this to the 7.18 milestone Aug 12, 2020
@che-bot
Copy link
Contributor

che-bot commented Aug 12, 2020

Latest version of Eclipse Che failed installation in minikube.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davidfestal davidfestal davidfestal left review comments

@mmorhun mmorhun mmorhun approved these changes

@l0rd l0rd Awaiting requested review from l0rd

@nickboldt nickboldt Awaiting requested review from nickboldt

@vparfonov vparfonov Awaiting requested review from vparfonov

Assignees

@tolusha tolusha

Labels
None yet
Projects
None yet
Milestone
7.18
Development

Successfully merging this pull request may close these issues.
5 participants
@tolusha @openshift-ci-robot @che-bot @davidfestal @mmorhun