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

fix: devfile endpoint with single-host exposure #157

Merged
merged 3 commits into from
Oct 6, 2021
Merged

fix: devfile endpoint with single-host exposure #157

merged 3 commits into from
Oct 6, 2021

Conversation

sparkoo
Copy link
Member

@sparkoo sparkoo commented Oct 6, 2021 •
edited
Loading

…devfile endpoint exposure is enabled

Signed-off-by: Michal Vala mvala@redhat.com

What does this PR do?

See analysis of eclipse-che/che#20593

This PR basically checks the CHE_INFRA_KUBERNETES_SINGLEHOST_WORKSPACE_DEVFILE__ENDPOINT__EXPOSURE and set requireSubdomain only for cases when it is not single-host.

Screenshot/screencast of this PR

What issues does this PR fix or reference?

eclipse-che/che#20593

How to test this PR?

latest image quay.io/mvala/che-server:gh20593-singleHostDevfileEndpoint

  1. deploy with chectl server:deploy --platform openshift --cheimage=quay.io/mvala/che-server:gh20593-singleHostDevfileEndpoint --che-operator-cr-patch-yaml=patch.yml where patch.yml is:
spec:
  server:
    customCheProperties:
      CHE_INFRA_KUBERNETES_SINGLEHOST_WORKSPACE_DEVFILE__ENDPOINT__EXPOSURE: single-host
    serverExposureStrategy: single-host
  1. Start any workspace that has endpoint defined in the devfile component
  2. Workspace must start and be accessible from the browser (prone to fail is .net core sample devfile, but it's not failing 100%)
  3. Run workspace task to use devfile endpoint and open it from theia, it must open on correct url and must work.
  4. You can also observe browser network traffic for GET /workspace request, it must have server for the devfile endpoint with proper path suffix, like /

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.

@sparkoo
does not set require subdomain to devfile endpoints when single-host …
71e95f5 
…devfile endpoint exposure is enabled

Signed-off-by: Michal Vala <mvala@redhat.com>
@che-bot
Copy link
Contributor

che-bot commented Oct 6, 2021

✅ E2E Happy path tests succeed 🎉

See Details

Test product:

  • Use comment "[crw-ci-test]" to rerun happy path E2E test.
  • Use comment "[crw-ci-test --rebuild]" to re-build the images and rerun happy path E2E test.

Eclipse Che QE channel: https://mattermost.eclipse.org/eclipse/channels/eclipse-che-qe

@sparkoo
fix openshift component applier
a96ffa2 
Signed-off-by: Michal Vala <mvala@redhat.com>
@che-bot
Copy link
Contributor

che-bot commented Oct 6, 2021

❌ E2E Happy path tests failed ❗

See Details

Test product:

  • Use comment "[crw-ci-test]" to rerun happy path E2E test.
  • Use comment "[crw-ci-test --rebuild]" to re-build the images and rerun happy path E2E test.

Eclipse Che QE channel: https://mattermost.eclipse.org/eclipse/channels/eclipse-che-qe

metlos
metlos approved these changes Oct 6, 2021
View reviewed changes
Copy link
Contributor

@metlos metlos left a comment

Choose a reason for hiding this comment

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

A fine piece of detective work 👍🏼 Just a small readability nitpick otherwise LGTM.

@dmytro-ndp
Copy link
Contributor

[crw-ci-test]

@che-bot
Copy link
Contributor

che-bot commented Oct 6, 2021

❌ E2E Happy path tests failed ❗

See Details

Test product:

  • Use comment "[crw-ci-test]" to rerun happy path E2E test.
  • Use comment "[crw-ci-test --rebuild]" to re-build the images and rerun happy path E2E test.

Eclipse Che QE channel: https://mattermost.eclipse.org/eclipse/channels/eclipse-che-qe

@sparkoo
tests
cd4f9f1 
Signed-off-by: Michal Vala <mvala@redhat.com>
@sparkoo sparkoo changed the title does not set require subdomain to devfile endpoints when single-host … fix: properly configure devfile endpoint with single-host exposure Oct 6, 2021
@sparkoo sparkoo changed the title fix: properly configure devfile endpoint with single-host exposure fix: devfile endpoint with single-host exposure Oct 6, 2021
skabashnyuk
skabashnyuk approved these changes Oct 6, 2021
View reviewed changes
Copy link
Contributor

@skabashnyuk skabashnyuk left a comment

Choose a reason for hiding this comment

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

Before
Знімок екрана 2021-10-06 о 16 46 50
after
Знімок екрана 2021-10-06 о 16 57 55

@che-bot
Copy link
Contributor

che-bot commented Oct 6, 2021

✅ E2E Happy path tests succeed 🎉

See Details

Test product:

  • Use comment "[crw-ci-test]" to rerun happy path E2E test.
  • Use comment "[crw-ci-test --rebuild]" to re-build the images and rerun happy path E2E test.

Eclipse Che QE channel: https://mattermost.eclipse.org/eclipse/channels/eclipse-che-qe

@sparkoo sparkoo merged commit 3a99240 into eclipse-che:main Oct 6, 2021
@sparkoo sparkoo deleted the gh20593-singleHostDevfileEndpoint branch October 6, 2021 16:35
@sparkoo sparkoo mentioned this pull request Oct 6, 2021
Closed
@che-bot che-bot added this to the 7.38 milestone Oct 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@metlos metlos metlos approved these changes

@skabashnyuk skabashnyuk skabashnyuk approved these changes

@amisevsk amisevsk Awaiting requested review from amisevsk

@mshaposhnik mshaposhnik Awaiting requested review from mshaposhnik

@nickboldt nickboldt Awaiting requested review from nickboldt

@sleshchenko sleshchenko Awaiting requested review from sleshchenko

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
7.38
Development

Successfully merging this pull request may close these issues.
5 participants
@sparkoo @che-bot @dmytro-ndp @metlos @skabashnyuk