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

[regression] Fix provisioning volume for ephemeral-mode workspaces. #14768

Merged
merged 2 commits into from
Oct 7, 2019

Conversation

amisevsk
Copy link
Contributor

@amisevsk amisevsk commented Oct 3, 2019

What does this PR do?

Fixes provisioning of volumes for ephemeral workspaces (#14749).

It also brings the EphemeralWorkspaceAdapter more in line with UniqueWorkspacePVCProvisioner, which I think is an overall good change.

Background

PR #14539 modified the way that certain volumes are stored in the environment: it removed adding plugin volumes to the plugin's corresponding machine and instead placed them in the pod's container directly in order to support ephemeral mode volumes for workspace plugins. This means that the plugins volume added for every sidecar is no longer in the machine and instead attached to each container.

The EphemeralWorkspaceAdapter was working off a previous assumption -- that all workspace volumes could be found in the list of machines in the environment -- and used that fact to reconcile workspace volumes (for each volume in machine, update the corresponding pod to use emptyDir). This meant that the plugins volume was not being provisioned at all (and thus had no subpath assigned).

This PR changes EphemeralWorkspaceAdapter to be more in line with UniqueWorkspacePVCProvisioner, basically doing the same thing and then converting all PVCs to EmptyDir volumes.

What issues does this PR fix or reference?

#14749

@amisevsk amisevsk added the severity/blocker Causes system to crash and be non-recoverable or prevents Che developers from working on Che code. label Oct 3, 2019
@che-bot che-bot added status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. kind/bug Outline of a bug - must adhere to the bug report template. labels Oct 3, 2019
@che-bot
Copy link
Contributor

che-bot commented Oct 3, 2019

E2E Happy path tests of Eclipse Che Single User on K8S (minikube v1.1.1) has failed:

@che-bot
Copy link
Contributor

che-bot commented Oct 3, 2019

E2E tests of Eclipse Che Multiuser on OCP has been successful:

@Katka92
Copy link
Contributor

Katka92 commented Oct 4, 2019

crw-ci-test

@che-bot
Copy link
Contributor

che-bot commented Oct 4, 2019

E2E Happy path tests of Eclipse Che Single User on K8S (minikube v1.1.1) has failed:

Copy link
Member

@sleshchenko sleshchenko left a comment

Choose a reason for hiding this comment

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

LGTM

@dmytro-ndp
Copy link
Contributor

crw-ci-test

@che-bot
Copy link
Contributor

che-bot commented Oct 4, 2019

E2E Happy path tests of Eclipse Che Single User on K8S (minikube v1.1.1) has failed:

@dmytro-ndp
Copy link
Contributor

crw-ci-test

@che-bot
Copy link
Contributor

che-bot commented Oct 4, 2019

E2E Happy path tests of Eclipse Che Single User on K8S (minikube v1.1.1) has failed:

@dmytro-ndp
Copy link
Contributor

@amisevsk: E2E tests hasn't shown regression.

@dmytro-ndp
Copy link
Contributor

@amisevsk: there is fixup of Happy path tests in master branch. You can take that changes an rerun the tests.

Commit dd47af6 changes the way that certain volumes are handled;
rather than storing the volume information in the relevant container's
machine, it is stored in the environment's pods directly. To accomodate
this, the EphemeralWorkspaceAdapter needs to pull the list of PVCs it
provisions from the environment rather than machines.

Signed-off-by: Angel Misevski <amisevsk@redhat.com>
Signed-off-by: Angel Misevski <amisevsk@redhat.com>
@amisevsk
Copy link
Contributor Author

amisevsk commented Oct 7, 2019

Thanks @dmytro-ndp

@che-bot
Copy link
Contributor

che-bot commented Oct 7, 2019

E2E Happy path tests of Eclipse Che Single User on K8S (minikube v1.1.1) has been successful:

@che-bot
Copy link
Contributor

che-bot commented Oct 7, 2019

E2E tests of Eclipse Che Multiuser on OCP has been successful:

@amisevsk amisevsk merged commit 5be73ff into eclipse-che:master Oct 7, 2019
@che-bot che-bot removed the status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. label Oct 7, 2019
@che-bot che-bot added this to the 7.3.0 milestone Oct 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Outline of a bug - must adhere to the bug report template. severity/blocker Causes system to crash and be non-recoverable or prevents Che developers from working on Che code.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants