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

Add possibility to disable DockerInstanceStopDetector #6415

Merged
merged 2 commits into from
Sep 25, 2017

Conversation

garagatyi
Copy link

What does this PR do?

It is needed for OpenShift infra where getting events
is not supported due to security limits.
To disable containers stop detector variable
CHE_DOCKER_ENABLE__CONTAINER__STOP__DETECTOR=false should be
defined.

What issues does this PR fix or reference?

Changelog

Add possibility to disable DockerInstanceStopDetector

Release Notes

Docs PR

It is needed for OpenShift infra where getting events
is not supported due to security limits.
To disable containers stop detector variable
CHE_DOCKER_ENABLE__CONTAINER__STOP__DETECTOR=false should be
defined.
Signed-off-by: Oleksandr Garagatyi <ogaragat@redhat.com>
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.

OK for me

@@ -89,7 +97,9 @@ public DockerInstanceStopDetector(
* @param workspaceId id of a workspace that owns machine
*/
public void startDetection(String containerId, String machineId, String workspaceId) {
instances.put(containerId, Pair.of(machineId, workspaceId));
if (enableDetector) {
Copy link
Member

Choose a reason for hiding this comment

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

I'd name this variable as isEnabled

Copy link
Author

Choose a reason for hiding this comment

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

ok

@codenvy-ci
Copy link

@benoitf
Copy link
Contributor

benoitf commented Sep 25, 2017

there is a missing test

@garagatyi
Copy link
Author

yes

@garagatyi garagatyi merged commit 7c7e596 into eclipse-che:master Sep 25, 2017
@garagatyi garagatyi deleted the disableStopDetector branch September 25, 2017 12:47
@benoitf
Copy link
Contributor

benoitf commented Sep 25, 2017

@garagatyi I don't see the unit test ?

@benoitf
Copy link
Contributor

benoitf commented Sep 25, 2017

also I don't see any labels set and milestone

@garagatyi garagatyi added the kind/task Internal things, technical debt, and to-do tasks to be performed. label Sep 25, 2017
@garagatyi
Copy link
Author

I didn't add tests.
I added a label. Do I have to add other labels?
As far as I can remember I haven't heard anything about the addition of milestones to PRs.

@slemeur
Copy link
Contributor

slemeur commented Sep 25, 2017

labels and milestone must be defined when merging a PR

@garagatyi garagatyi added this to the 5.19.0 milestone Sep 25, 2017
@garagatyi
Copy link
Author

@slemeur which labels?
@slemeur @benoitf I would rather not expose this variable. Basically, it is a workaround to be able to run OpenShiftConnector on OSIO. It should be removed after moving to spi

@codenvy-ci
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/task Internal things, technical debt, and to-do tasks to be performed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants