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

watch PVC WaitForFirstConsumer event to avoid deadlock #14239

Merged
merged 4 commits into from
Aug 16, 2019
Merged

watch PVC WaitForFirstConsumer event to avoid deadlock #14239

merged 4 commits into from
Aug 16, 2019

Conversation

sparkoo
Copy link
Member

@sparkoo sparkoo commented Aug 14, 2019

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

What does this PR do?

watches for PVC WaitForFirstConsumer event when creating the workspace to avoid deadlock sitation in case that used StorageClass has VolumeBindingMode: WaitForFirstConsumer

What issues does this PR fix or reference?

#13437
ref: #12889 (comment)

@che-bot che-bot added kind/bug Outline of a bug - must adhere to the bug report template. status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. labels Aug 14, 2019
@che-bot
Copy link
Contributor

che-bot commented Aug 14, 2019

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

sleshchenko
sleshchenko reviewed Aug 15, 2019
View reviewed changes
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.

In general LGTM.

Please take a look my inline comments

...ipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesPersistentVolumeClaims.java Outdated Show resolved Hide resolved
...ipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesPersistentVolumeClaims.java Show resolved Hide resolved
@che-bot
Copy link
Contributor

che-bot commented Aug 15, 2019

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

@sparkoo sparkoo marked this pull request as ready for review August 15, 2019 11:48
@sparkoo
Copy link
Member Author

sparkoo commented Aug 15, 2019

ci-test

@che-bot
Copy link
Contributor

che-bot commented Aug 15, 2019

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

sleshchenko
sleshchenko approved these changes Aug 15, 2019
View reviewed changes
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

Please take a look a few more comments

...ipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesPersistentVolumeClaims.java Outdated Show resolved Hide resolved
...ipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesPersistentVolumeClaims.java Outdated Show resolved Hide resolved
...ipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesPersistentVolumeClaims.java Outdated Show resolved Hide resolved
...ipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesPersistentVolumeClaims.java Outdated Show resolved Hide resolved
...ipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesPersistentVolumeClaims.java Show resolved Hide resolved
@che-bot
Copy link
Contributor

che-bot commented Aug 15, 2019

Results of automated E2E tests of Eclipse Che Multiuser on OCP:
Build details
Test report
docker image: eclipseche/che-server:14239
https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

@che-bot
Copy link
Contributor

che-bot commented Aug 15, 2019

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

@sparkoo
Copy link
Member Author

sparkoo commented Aug 15, 2019

ci-test

@che-bot
Copy link
Contributor

che-bot commented Aug 15, 2019

Results of automated E2E tests of Eclipse Che Multiuser on OCP:
Build details
Test report
docker image: eclipseche/che-server:14239
https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

@che-bot
Copy link
Contributor

che-bot commented Aug 15, 2019

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

amisevsk
amisevsk approved these changes Aug 15, 2019
View reviewed changes
Copy link
Contributor

@amisevsk amisevsk left a comment

Choose a reason for hiding this comment

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

LGTM

...ipse/che/workspace/infrastructure/kubernetes/namespace/KubernetesPersistentVolumeClaims.java Show resolved Hide resolved
@sparkoo
Copy link
Member Author

sparkoo commented Aug 16, 2019

ci-test

@che-bot
Copy link
Contributor

che-bot commented Aug 16, 2019

Results of automated E2E tests of Eclipse Che Multiuser on OCP:
Build details
Test report
docker image: eclipseche/che-server:14239
https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

@che-bot
Copy link
Contributor

che-bot commented Aug 16, 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 Aug 16, 2019

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

@sparkoo
watch PVC if it is waiting for consumer action to avoid deadlock
b27de28 
Signed-off-by: Michal Vala <mvala@redhat.com>
@sparkoo
PVC bound wait - fixed exception handling, code and javadoc cleanup
b22382a 
Signed-off-by: Michal Vala <mvala@redhat.com>
@sparkoo
PVC bound wait - code cleanup, fixed possible future completion issue
e2cbb8c 
Signed-off-by: Michal Vala <mvala@redhat.com>
@sparkoo
pvc wait bound - properly close future
27b16c0 
Signed-off-by: Michal Vala <mvala@redhat.com>
@che-bot
Copy link
Contributor

che-bot commented Aug 16, 2019

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

  • build details
  • "che-server" docker image: maxura/che-server:14239

sleshchenko
sleshchenko approved these changes Aug 16, 2019
View reviewed changes
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.

Still LGTM )

@sleshchenko sleshchenko merged commit f8b7779 into eclipse-che:master Aug 16, 2019
@sleshchenko sleshchenko mentioned this pull request Aug 16, 2019
Closed
@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 Aug 16, 2019
@che-bot che-bot added this to the 7.1.0 milestone Aug 16, 2019
@sparkoo sparkoo deleted the pvcBoundDeadlock branch August 16, 2019 10:58
@nickboldt
Copy link
Contributor

Please backport to 6.19.6 so we can pick this up in CRW 1.2.x. cc: @slemeur

@sparkoo sparkoo mentioned this pull request Aug 17, 2019
Merged
sleshchenko added a commit that referenced this pull request Aug 20, 2019
@sleshchenko
Merge pull request #14269 from sparkoo/pvcBoundDeadlock6
892804c 
6 backport - Watch PVC `WaitForFirstConsumer` event to avoid deadlock (#14239)
@sleshchenko sleshchenko mentioned this pull request Aug 22, 2019
Closed
@amisevsk amisevsk mentioned this pull request Aug 22, 2019
Closed
@nickboldt nickboldt mentioned this pull request Aug 23, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sleshchenko sleshchenko sleshchenko approved these changes

@amisevsk amisevsk amisevsk approved these changes

@l0rd l0rd Awaiting requested review from l0rd

@metlos metlos Awaiting requested review from metlos

@nickboldt nickboldt Awaiting requested review from nickboldt

@rhopp rhopp Awaiting requested review from rhopp

Assignees
No one assigned
Labels
kind/bug Outline of a bug - must adhere to the bug report template.
Projects
None yet
Milestone
7.1.0
Development

Successfully merging this pull request may close these issues.
5 participants
@sparkoo @che-bot @nickboldt @sleshchenko @amisevsk