Don't stop workspace recovery when error is encountered by amisevsk · Pull Request #11446 · eclipse-che/che

amisevsk · 2018-10-02T20:49:43Z

Fix issue where encountering an exception while recovering workspaces on startup prevents recovery of further workspaces.

What does this PR do?

Fix issue where encountering an exception while recovering workspaces on startup prevents recovery of further workspaces.

What issues does this PR fix or reference?

#11445

Testing

Start Che multiuser on minishift
Start a few workspaces
Scale Che server down to 0
modify postgres database to set owner_id to 0 for some workspace in table che_k8s_runtime
restart Che server
logs contain something like:

2018-10-02 20:41:56,358[ost-startStop-1]  [ERROR] [o.e.c.a.w.s.WorkspaceRuntimes 544]   - An error occurred while attempted to recover runtimes using infrastructure 'openshift'. Reason: 'Couldn't recover runtime 'workspace3ob15dnpujw9eamy:default'. Error: The user `0` doesn't have the required `use` permission for workspace `workspace3ob15dnpujw9eamy`'
2018-10-02 20:41:57,647[ost-startStop-1]  [INFO ] [o.e.c.a.w.s.WorkspaceRuntimes 590]   - Successfully recovered workspace runtime 'workspacekvgznl4cdef4u8r7'
2018-10-02 20:41:57,739[ost-startStop-1]  [INFO ] [o.e.c.a.w.s.WorkspaceRuntimes 590]   - Successfully recovered workspace runtime 'workspacec7tt9hc681m1uvr7'

and workspaces that can be recovered are. Workspace that had owner_id modified is listed as "not running" in Che, and deployment is not cleaned up, but there's not much we can do there.

Additionally, this will allow logging of all recovery errors, so this could be used to better track recovery bugs.

Fix issue where encountering an exception while recovering workspaces on startup prevents recovery of further workspaces. Signed-off-by: Angel Misevski <amisevsk@redhat.com>

sleshchenko

Good fix!

You can make this PR even better by adding a unit test that second workspace will be recovered if the first one failed.

sleshchenko · 2018-10-03T06:05:30Z

...core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/WorkspaceRuntimes.java

+      return;
+    } catch (InfrastructureException e) {
      LOG.error(
          "An error occurred while attempted to recover runtimes using infrastructure '{}'. Reason: '{}'",


Le'ts rephrase this message to make it clear that error occurred during identities fetching.

garagatyi

Looks good! Please, add tests.

sleshchenko · 2018-10-03T09:43:34Z

ci-test

riuvshin · 2018-10-03T12:26:33Z

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

garagatyi · 2018-10-03T12:35:37Z

@riuvshin I cannot express how I happy with just 3-hours long tests! It is so good to receive the feedback on the same day. THANK YOU!

riuvshin · 2018-10-03T12:46:39Z

@garagatyi this particular run Took 2 hr 42 min :P I've enabled mvn parallel build which decreased execution time for 20mins

Signed-off-by: Angel Misevski <amisevsk@redhat.com>

amisevsk · 2018-10-03T14:19:32Z

I've added a test case to check that recovery continues past error.

amisevsk · 2018-10-03T14:19:40Z

ci-test

garagatyi · 2018-10-03T14:26:44Z

@amisevsk adding unit tests and changing the message usually doesn't require a new run of citest

garagatyi · 2018-10-03T14:26:54Z

ci-build

riuvshin · 2018-10-03T17:22:21Z

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

Don't stop workspace recovery when error is encountered

276faae

Fix issue where encountering an exception while recovering workspaces on startup prevents recovery of further workspaces. Signed-off-by: Angel Misevski <amisevsk@redhat.com>

amisevsk added the kind/bug Outline of a bug - must adhere to the bug report template. label Oct 2, 2018

amisevsk requested review from akorneta, skabashnyuk and sleshchenko October 2, 2018 20:49

amisevsk requested a review from garagatyi as a code owner October 2, 2018 20:49

amisevsk mentioned this pull request Oct 2, 2018

Ghost deployment preventing starting of workspaces openshiftio/openshift.io#4379

Closed

sleshchenko approved these changes Oct 3, 2018

View reviewed changes

benoitf added the status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. label Oct 3, 2018

skabashnyuk approved these changes Oct 3, 2018

View reviewed changes

garagatyi reviewed Oct 3, 2018

View reviewed changes

akorneta approved these changes Oct 3, 2018

View reviewed changes

Add test for workspace recovery and improve error log messages

7775bcb

Signed-off-by: Angel Misevski <amisevsk@redhat.com>

garagatyi approved these changes Oct 3, 2018

View reviewed changes

amisevsk merged commit 220f10c into eclipse-che:master Oct 3, 2018

benoitf 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 3, 2018

benoitf added this to the 6.12.0 milestone Oct 3, 2018

Conversation

amisevsk commented Oct 2, 2018

What does this PR do?

What issues does this PR fix or reference?

Testing

Uh oh!

sleshchenko left a comment

Choose a reason for hiding this comment

Uh oh!

sleshchenko Oct 3, 2018

Choose a reason for hiding this comment

Uh oh!

amisevsk Oct 3, 2018

Choose a reason for hiding this comment

Uh oh!

garagatyi left a comment

Choose a reason for hiding this comment

Uh oh!

sleshchenko commented Oct 3, 2018

Uh oh!

riuvshin commented Oct 3, 2018

Uh oh!

garagatyi commented Oct 3, 2018

Uh oh!

riuvshin commented Oct 3, 2018

Uh oh!

amisevsk commented Oct 3, 2018

Uh oh!

amisevsk commented Oct 3, 2018

Uh oh!

garagatyi commented Oct 3, 2018

Uh oh!

garagatyi commented Oct 3, 2018

Uh oh!

riuvshin commented Oct 3, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants