Bug 1839233: send activity tick to keep the cloud shell terminal alive #5546
Conversation
openshift-ci-robot
added
the
component/core
openshift-ci-robot
added
the
approved
christianvogt
changed the title
openshift-ci-robot
added
the
bugzilla/severity-high
|
@christianvogt: This pull request references Bugzilla bug 1839233, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker. 3 validation(s) were run on this bug
In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
openshift-ci-robot
added
the
bugzilla/valid-bug
|
@christianvogt: This pull request references Bugzilla bug 1839233, which is valid. 3 validation(s) were run on this bug
In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
christianvogt
force-pushed
the
terminal-tick
branch
from
388a599
to
b58540d
Compare
| } | ||
| }); | ||
| } else { | ||
| setTerminalAvailable(false); |
There was a problem hiding this comment.
Need a check for mounted here as well.
There was a problem hiding this comment.
@rohitkrai03 this doesn't require a check because it's not executed asynchronously.
|
/retest |
|
@christianvogt: This pull request references Bugzilla bug 1839233, which is valid. 3 validation(s) were run on this bug
In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
christianvogt
force-pushed
the
terminal-tick
branch
from
e7230fe
to
5a0010e
Compare
|
/retest |
|
/assign |
|
@christianvogt Once the workspace pod scales down to zero, the workspace phase changes to
|
There was a problem hiding this comment.
/lgtm
Verified locally. Works fine except for the comment about stopped workspaces above. Seems like that fix is coming in #5554. So approving this one.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: christianvogt, rohitkrai03 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@christianvogt: All pull requests linked via external trackers have merged: openshift/console#5546. Bugzilla bug 1839233 has been moved to the MODIFIED state. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Fixes:
https://issues.redhat.com/browse/ODC-3926
Also removed
dockerimagefrom the workspace resource as per devfile/devworkspace-operator#82Analysis / Root cause:
Cloudshell pod never scales to zero. Therefore indefinitely consuming resources per user per terminal.
Solution Description:
A new endpoint is created in the backend at
/api/$namespace/$workspaceName/terminal/activity/tickWe notify this endpoint every minute while there is activity in the console.
The controller will keep the pod alive so long as there has been a tick within a certain time frame (TBD and set on the controller).
Also using the new endpoint to check if the terminal is available before enabling the masthead button.
Unit test coverage report:
Test setup:
Requires #5332
When using local bridge. Need to do the following: https://gist.github.com/amisevsk/de48e5e2664857fcd53f0f9550a92082
https://gist.github.com/amisevsk/788ade83b172259f1130691a8c4a1a25
Browser conformance: