Skip to content

Refactored external servers exposition in K8s/OS infrastructures#10266

Merged
sleshchenko merged 5 commits intoeclipse-che:masterfrom
sleshchenko:k8sServerExposingRefactor
Jul 5, 2018
Merged

Refactored external servers exposition in K8s/OS infrastructures#10266
sleshchenko merged 5 commits intoeclipse-che:masterfrom
sleshchenko:k8sServerExposingRefactor

Conversation

@sleshchenko
Copy link
Copy Markdown
Member

@sleshchenko sleshchenko commented Jul 4, 2018

What does this PR do?

Refactors external servers exposition in K8s/OS infrastructures:

  • moves external servers related classes to a separated package;

  • fixes java doc in OpenShiftExternalServerExposer;

  • makes ServerServiceBuilder non-internal class to make it possible to reuse it;

  • refactors external server exposer strategy to expose service port one by one;
    Previously expose server strategy had to match servers with services port, but sometimes it is impossible like in case of JWT proxy. Because in this case, server config contains port that is different to service port. Like wsagent server is exposed on 4401 original service port, but it will be publicly accessible via 4911 (random one) JWTProxy service port.

  • fixes and improve tests

What issues does this PR fix or reference?

It is changes needed for #10081.
Separated changes from #10252 to make review easier.

Release Notes

N/A

Docs PR

N/A

@sleshchenko sleshchenko added kind/enhancement A feature request - must adhere to the feature request template. status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. labels Jul 4, 2018
@sleshchenko sleshchenko self-assigned this Jul 4, 2018
@sleshchenko sleshchenko requested a review from garagatyi July 4, 2018 13:19
Copy link
Copy Markdown

@garagatyi garagatyi left a comment

Choose a reason for hiding this comment

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

Looks good!
BTW each significant change we do in this area I realize that had to do the code simpler and cleaner in the first place.
But thank you for moving the code in the direction of simplicity!

@sleshchenko
Copy link
Copy Markdown
Member Author

ci-test

@codenvy-ci
Copy link
Copy Markdown

ci-test build report:
Build details
Test report
selenium tests report data
docker image: eclipseche/che-server:10266
https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

@sleshchenko sleshchenko force-pushed the k8sServerExposingRefactor branch from 423de2d to 59b4301 Compare July 5, 2018 08:51
@vkuznyetsov vkuznyetsov mentioned this pull request Jul 5, 2018
20 tasks
@musienko-maxim
Copy link
Copy Markdown
Contributor

QE team did not find new regressions after selenium session.

…by one

Previously expose server strategy had to match servers with services port,
but sometimes it is impossible like in case of JWT proxy. Because in this case,
server config contains port that is different to service port. Like `wsagent`
server is exposed on 4401 original service port, but it will be publicly
accessible via 4911 (random one) JWTProxy service port.
@sleshchenko sleshchenko force-pushed the k8sServerExposingRefactor branch from 59b4301 to 68424c9 Compare July 5, 2018 14:42
@sleshchenko sleshchenko merged commit 11c5e3f into eclipse-che:master Jul 5, 2018
@sleshchenko sleshchenko deleted the k8sServerExposingRefactor branch July 5, 2018 14:46
@benoitf benoitf added this to the 6.8.0 milestone Jul 5, 2018
@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 Jul 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/enhancement A feature request - must adhere to the feature request template.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants