Add Container Lifecycle support #16391

vparfonov · 2020-03-18T16:12:45Z

Signed-off-by: Vitalii Parfonov [email protected]

What does this PR do?

Add Container Lifecycle support.
This should allow postStart and preStop actions for plugin containers.
More information here: https://kubernetes.io/docs/tasks/configure-pod-container/attach-handler-lifecycle-event/
depends on: eclipse-che/che-plugin-broker#98

What issues does this PR fix or reference?

#16464

Release Notes

Docs PR

che-bot · 2020-03-18T16:33:20Z

✅ E2E Happy path tests succeed 🎉

See Details

Jenkins job
logs and configs
Happy path tests DevFile
image: maxura/che-server:16391

Tested with Eclipse Che Multiuser User on K8S (minikube v1.1.1)

che-bot · 2020-04-01T13:57:11Z

❌ E2E Happy path tests failed ❗

See Details

Jenkins job
test report
logs and configs
Happy path tests DevFile
image: maxura/che-server:16391

Tested with Eclipse Che Multiuser User on K8S (minikube v1.1.1)

⚠️ https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

ℹ️ Use comment "crw-ci-test" to rerun happy path E2E test.

dmytro-ndp · 2020-04-01T16:03:13Z

crw-ci-test

che-bot · 2020-04-01T16:26:49Z

✅ E2E Happy path tests succeed 🎉

See Details

Jenkins job
logs and configs
Happy path tests DevFile
image: maxura/che-server:16391

Tested with Eclipse Che Multiuser User on K8S (minikube v1.1.1)

che-bot · 2020-04-02T10:39:37Z

❌ E2E Happy path tests failed ❗

See Details

Jenkins job
test report
logs and configs
Happy path tests DevFile
image: maxura/che-server:16391

Tested with Eclipse Che Multiuser User on K8S (minikube v1.1.1)

⚠️ https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

ℹ️ Use comment "crw-ci-test" to rerun happy path E2E test.

che-bot · 2020-04-02T12:25:31Z

✅ E2E Happy path tests succeed 🎉

See Details

Jenkins job
logs and configs
Happy path tests DevFile
image: maxura/che-server:16391

Tested with Eclipse Che Multiuser User on K8S (minikube v1.1.1)

Signed-off-by: Vitalii Parfonov <[email protected]>

che-bot · 2020-04-02T15:46:15Z

✅ E2E Happy path tests succeed 🎉

See Details

Jenkins job
logs and configs
Happy path tests DevFile
image: maxura/che-server:16391

Tested with Eclipse Che Multiuser User on K8S (minikube v1.1.1)

metlos

Generally LGTM apart from 2 minor objections.

...e-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/wsplugins/model/Exec.java

metlos · 2020-04-02T16:50:51Z

...pi-workspace/src/main/java/org/eclipse/che/api/workspace/server/wsplugins/model/Handler.java

+
+  @Override
+  public String toString() {
+    StringBuilder sb = new StringBuilder();


toString() formatting.

...-workspace/src/main/java/org/eclipse/che/api/workspace/server/wsplugins/model/Lifecycle.java

...kspace/src/test/java/org/eclipse/che/api/workspace/server/wsplugins/PluginFQNParserTest.java

sleshchenko

I wonder if preStop handlers are run even when we forcibly remove pods
https://github.com/eclipse/che/blob/master/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties#L413

vparfonov · 2020-04-03T11:17:35Z

I wonder if preStop handlers are run even when we forcibly remove pods
https://github.com/eclipse/che/blob/master/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties#L413

For normal work preStop handlers we need to increase terminationGracePeriodSeconds but value depends on handler itself so this open discussion, and we can increase it in separate PR.
This is note from official k8s documentations:

If one of the Pod’s containers has defined a preStop hook, it is invoked inside of the container. If the preStop hook is still running after the grace period expires, step 2 is then invoked with a small (2 second) one-time extended grace period. You must modify terminationGracePeriodSeconds if the preStop hook needs longer to complete.

Signed-off-by: Vitalii Parfonov <[email protected]>

che-bot · 2020-04-03T15:51:57Z

✅ E2E Happy path tests succeed 🎉

See Details

Jenkins job
logs and configs
Happy path tests DevFile
image: maxura/che-server:16391

Tested with Eclipse Che Multiuser User on K8S (minikube v1.1.1)

amisevsk · 2020-04-03T16:36:44Z

If one of the Pod’s containers has defined a preStop hook, it is invoked inside of the container. If the preStop hook is still running after the grace period expires, step 2 is then invoked with a small (2 second) one-time extended grace period. You must modify terminationGracePeriodSeconds if the preStop hook needs longer to complete.

The reason we initially set the termination grace period to 0 is that workspaces would be in a terminating state until it expired, so maybe some process in the workspace isn't properly respecting signals?

...kspace/src/test/java/org/eclipse/che/api/workspace/server/wsplugins/PluginFQNParserTest.java

Signed-off-by: Vitalii Parfonov <[email protected]>

vparfonov · 2020-04-06T08:44:29Z

So @davidfestal @ibuziuk @amisevsk @sleshchenko @l0rd what do you think about terminationGracePeriodSeconds? Should we increase it in this PR?

...kspace/src/test/java/org/eclipse/che/api/workspace/server/wsplugins/PluginFQNParserTest.java

ibuziuk · 2020-04-06T08:50:16Z

what do you think about terminationGracePeriodSeconds? Should we increase it in this PR?

IMHO, we should make this property configurable via toggle per-workspace smth. similar to what is proposed in #15960

Signed-off-by: Vitalii Parfonov <[email protected]>

vparfonov · 2020-04-06T10:06:51Z

what do you think about terminationGracePeriodSeconds? Should we increase it in this PR?

IMHO, we should make this property configurable via toggle per-workspace smth. similar to what is proposed in #15960

So it this case it will be done via separate PR, right?

ibuziuk · 2020-04-06T10:15:20Z

So it this case it will be done via separate PR, right?

sure

che-bot · 2020-04-06T10:18:01Z

❌ E2E Happy path tests failed ❗

See Details

Jenkins job
test report
logs and configs
Happy path tests DevFile
image: maxura/che-server:16391

Tested with Eclipse Che Multiuser User on K8S (minikube v1.1.1)

⚠️ https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

ℹ️ Use comment "crw-ci-test" to rerun happy path E2E test.

vparfonov · 2020-04-06T12:51:30Z

crw-ci-test

che-bot · 2020-04-06T16:21:05Z

❌ E2E Happy path tests failed ❗

See Details

Jenkins job
test report
logs and configs
Happy path tests DevFile
image: maxura/che-server:16391

Tested with Eclipse Che Multiuser User on K8S (minikube v1.1.1)

⚠️ https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

ℹ️ Use comment "crw-ci-test" to rerun happy path E2E test.

ibuziuk

LGTM, having some tests would be really nice to have though ;)

vparfonov · 2020-04-07T07:44:57Z

crw-ci-test

che-bot · 2020-04-07T07:55:48Z

❌ E2E Happy path tests failed ❗

See Details

Jenkins job
test report
logs and configs
Happy path tests DevFile
image: maxura/che-server:16391

Tested with Eclipse Che Multiuser User on K8S (minikube v1.1.1)

⚠️ https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

ℹ️ Use comment "crw-ci-test" to rerun happy path E2E test.

che-bot · 2020-04-08T07:17:02Z

✅ E2E Happy path tests succeed 🎉

See Details

Jenkins job
logs and configs
Happy path tests DevFile
image: maxura/che-server:16391

Tested with Eclipse Che Multiuser User on K8S (minikube v1.1.1)

vparfonov · 2020-04-08T09:29:13Z

@metlos @sleshchenko @amisevsk can i merge it?

ibuziuk · 2020-04-08T15:53:22Z

@metlos @sleshchenko folks, are you ok to merge this PR?

amisevsk

LGTM, sorry about the delay.

I agree with Ilya that TerminationGracePeriod should be configurable (separate PR/issue), but it might open the door to some subtle configuration issues that are hard to notice. I also think there's some work to be done in implementing a nonzero termination grace period, as in my experience workspaces will take the entire termination grace period (even if it's a minute) -- that may have changed somewhere along the line though.

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/enhancement A feature request - must adhere to the feature request template. labels Mar 18, 2020

vparfonov mentioned this pull request Mar 18, 2020

Async PV for workspace: PoC with data sync pod #16042

Closed

Add Container Lifecycle support

680fd3a

Signed-off-by: Vitalii Parfonov <[email protected]>

vparfonov force-pushed the lifecycle branch from a5de2ed to 680fd3a Compare April 2, 2020 15:23

vparfonov marked this pull request as ready for review April 2, 2020 15:25

vparfonov requested review from amisevsk, l0rd, metlos, mshaposhnik, nickboldt, rhopp and sleshchenko as code owners April 2, 2020 15:25

metlos reviewed Apr 2, 2020

View reviewed changes

sleshchenko reviewed Apr 3, 2020

View reviewed changes

vparfonov added 2 commits April 3, 2020 18:30

Rewrite toString() methods according coomon rules in codebase

cdfb568

Signed-off-by: Vitalii Parfonov <[email protected]>

Fix formating

ae2a4d9

Signed-off-by: Vitalii Parfonov <[email protected]>

ibuziuk reviewed Apr 6, 2020

View reviewed changes

...kspace/src/test/java/org/eclipse/che/api/workspace/server/wsplugins/PluginFQNParserTest.java Outdated Show resolved Hide resolved

Remove test output

ab4d6e8

Signed-off-by: Vitalii Parfonov <[email protected]>

ibuziuk reviewed Apr 6, 2020

View reviewed changes

...kspace/src/test/java/org/eclipse/che/api/workspace/server/wsplugins/PluginFQNParserTest.java Outdated Show resolved Hide resolved

Remove unused import

6174563

Signed-off-by: Vitalii Parfonov <[email protected]>

ibuziuk approved these changes Apr 7, 2020

View reviewed changes

Merge branch 'master' into lifecycle

b3194ab

amisevsk approved these changes Apr 8, 2020

View reviewed changes

sleshchenko approved these changes Apr 9, 2020

View reviewed changes

vparfonov merged commit 71de584 into master Apr 9, 2020

vparfonov deleted the lifecycle branch April 9, 2020 10:38

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 Apr 9, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Container Lifecycle support #16391

Add Container Lifecycle support #16391

vparfonov commented Mar 18, 2020 •

edited

Loading

che-bot commented Mar 18, 2020

che-bot commented Apr 1, 2020

dmytro-ndp commented Apr 1, 2020

che-bot commented Apr 1, 2020

che-bot commented Apr 2, 2020

che-bot commented Apr 2, 2020

che-bot commented Apr 2, 2020

metlos left a comment

metlos Apr 2, 2020

sleshchenko left a comment

vparfonov commented Apr 3, 2020

che-bot commented Apr 3, 2020

amisevsk commented Apr 3, 2020

vparfonov commented Apr 6, 2020

ibuziuk commented Apr 6, 2020

vparfonov commented Apr 6, 2020

ibuziuk commented Apr 6, 2020

che-bot commented Apr 6, 2020

vparfonov commented Apr 6, 2020

che-bot commented Apr 6, 2020

ibuziuk left a comment

vparfonov commented Apr 7, 2020

che-bot commented Apr 7, 2020

che-bot commented Apr 8, 2020

vparfonov commented Apr 8, 2020

ibuziuk commented Apr 8, 2020

amisevsk left a comment

Add Container Lifecycle support #16391

Add Container Lifecycle support #16391

Conversation

vparfonov commented Mar 18, 2020 • edited Loading

What does this PR do?

What issues does this PR fix or reference?

Release Notes

Docs PR

che-bot commented Mar 18, 2020

che-bot commented Apr 1, 2020

dmytro-ndp commented Apr 1, 2020

che-bot commented Apr 1, 2020

che-bot commented Apr 2, 2020

che-bot commented Apr 2, 2020

che-bot commented Apr 2, 2020

metlos left a comment

Choose a reason for hiding this comment

metlos Apr 2, 2020

Choose a reason for hiding this comment

sleshchenko left a comment

Choose a reason for hiding this comment

vparfonov commented Apr 3, 2020

che-bot commented Apr 3, 2020

amisevsk commented Apr 3, 2020

vparfonov commented Apr 6, 2020

ibuziuk commented Apr 6, 2020

vparfonov commented Apr 6, 2020

ibuziuk commented Apr 6, 2020

che-bot commented Apr 6, 2020

vparfonov commented Apr 6, 2020

che-bot commented Apr 6, 2020

ibuziuk left a comment

Choose a reason for hiding this comment

vparfonov commented Apr 7, 2020

che-bot commented Apr 7, 2020

che-bot commented Apr 8, 2020

vparfonov commented Apr 8, 2020

ibuziuk commented Apr 8, 2020

amisevsk left a comment

Choose a reason for hiding this comment

vparfonov commented Mar 18, 2020 •

edited

Loading