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

Run dashboard as separate deployment #684

Merged
merged 5 commits into from
Apr 21, 2021

Conversation

sleshchenko
Copy link
Member

@sleshchenko sleshchenko commented Feb 19, 2021

What does this PR do?

It makes the Che operator run the dashboard as a separate Deployment. The same was previously done for helm charts.
The main reason why these changes were made, it's

STEP1 - running the HTTP server that serves the dashboard in its own pod: customising the dashboard is just a matter of rebuilding the dashboard container image.
from eclipse-che/che#15778 (comment)

Now things changed and we have a plan to introduce a backend for the dashboard. So, it becomes a need.

It depends on #760

Screenshot/screencast of this PR

What issues does this PR fix or reference?

It's the first step for eclipse-che/che#15778
It's the second step for eclipse-che/che#17802
And a prerequisite for Dashboard backend (not sure if an issue exists at this point)

How to test this PR?

Test that dashboard is run as dedicated deployment and is available on Che host.

I tested(and prepared a lot of fixes):

Minikube:

  • multihost
  • gateway singlehost
  • native singlehost

OpenShift (RHPDS):

  • multihost
  • gateway singlehost

PR Checklist

As the author of this Pull Request I made sure that:

Reviewers

Reviewers, please comment how you tested the PR when approving it.

@sleshchenko sleshchenko self-assigned this Feb 19, 2021
@sleshchenko sleshchenko changed the title Run dashboard as separate deployment WIP Run dashboard as separate deployment Feb 19, 2021
@openshift-merge-robot

This comment has been minimized.

@nickboldt
Copy link
Contributor

I made this draft to expose the made work but not sure when we should finalize and get it merged since it influences CRW productization, CRW should onboard +1 container.

Hard -10 from me. Why do we need Yet Another Tiny Container to have to productize, maintain, keep updated, stickhandle CVEs? What's the value for the customer here? Or for development/dogfooding? Or for dev sandbox users?

Today CRW is 25 images, wtih 2 more coming in 2.8. If we add async support in 2.9, that's 2 more. And then intellij support is another one. So that's 30 containers, and of those 30, only 1/3rd are sidecars/lang support. That'll be 20 containers to JUST START CRW, before even loading a workspace.

Could we make this process simpler, not more complex?

So... without a visible customer need or loud applause from @sympatheticmoose I don't immediately see the value in adding more technical debt for this.

@sleshchenko
Copy link
Member Author

@nickboldt I understand your pain but

Another Tiny Container

It's not going to be tiny in the future.
Dashboard Apache server will be extended soon with node backend, which would allow us to do Che + DevWorkspace Integration we want in a proper way.

Potentially, we can merge different processes (based on different technologies) into one container, but it's not the way K8s suggests us to go =)

Now we need to get on timeframe when CRW will pull the new image for dashboard, sorry about that.

@nickboldt
Copy link
Contributor

nickboldt commented Apr 6, 2021

merge different processes

Could we merge some of these?

  • machine exec
  • theia endpoint
  • 2 brokers
  • jwt proxy
  • configbump
  • traefik
  • server
  • operator

Maybe we can implement a "one in, one out" policy so that overall deployment complexity doesn't spiral out of control? Eg., you want dashboard to be a separate container, then configbump and traefik have to be combined.

Now we need to get on timeframe when CRW will pull the new image for dashboard, sorry about that.

When will the new dashboard image be available in upstream? Or is it the existing image at https://quay.io/repository/eclipse/che-dashboard?tag=latest&tab=tags that you want to see productized as crw/dashboard-rhel8?

@sleshchenko
Copy link
Member Author

2 brokers

we can merge 2 brokers, others are out of my team power, sorry )

@sleshchenko sleshchenko changed the title WIP Run dashboard as separate deployment 🚧 Run dashboard as separate deployment Apr 6, 2021
@tolusha tolusha mentioned this pull request Apr 7, 2021
38 tasks
@sleshchenko sleshchenko force-pushed the separateDashboardNextUpdated branch 3 times, most recently from 2746c20 to 736efef Compare April 7, 2021 14:41
@sleshchenko
Copy link
Member Author

Rebased against main branch.
The only made change: fix to run on OpenShift https://github.com/eclipse-che/che-operator/compare/888544117b5dddad5c4415c90f5ce8dfd32f7e35..56e66d862e6dc2795b100ff194203352843723fa#diff-f26d61ce8d3fc7ba6d48229eb77344a2be0d209ac7b4eeb9f850e4d05873adacR129

-   if util.IsOpenShift {
+   if !util.IsOpenShift {

@sleshchenko
Copy link
Member Author

For some reason, events exposed by chectl server:logs are empty https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/origin-ci-test/pr-logs/pull/eclipse-che_che-operator/684/pull-ci-eclipse-che-che-operator-main-v7-single-host-nightly-deployment/1384132729083269120/artifacts/single-host-nightly-deployment/single-host-nightly-deployment/artifacts/eclipse-che/events.txt
Which make impossible to figure out the tests failure reason.
For some unclear reasons it worked if I deploy with chectl server:deploy but did not work with opm (CatalogSource).

@sleshchenko
Copy link
Member Author

/retest

@tolusha
Copy link
Contributor

tolusha commented Apr 21, 2021

MInishift tests passed locally.

@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: AndrienkoAleksandr, sleshchenko, tolusha

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

@tolusha
Copy link
Contributor

tolusha commented Apr 21, 2021

/test v7-devworkspace-happy-path

@sleshchenko
Copy link
Member Author

Rebased against master to fix happy path tests.
Added fix into release scripts https://github.com/eclipse-che/che-operator/pull/684/files#diff-80b084421ff03b0d4874193917f225f185012ca25a072ea9aa94d38ef8a4ffefR88 Thank @tolusha for catching it.

Signed-off-by: Anatolii Bazko <abazko@redhat.com>
@tolusha
Copy link
Contributor

tolusha commented Apr 21, 2021

/test v7-olm-nightly-deployment

@tolusha tolusha merged commit 0353a44 into eclipse-che:main Apr 21, 2021
@sleshchenko sleshchenko deleted the separateDashboardNextUpdated branch April 22, 2021 04:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants