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

CHE-220: Improving 'removeContainer' API. Refactoring #5766

Merged
merged 1 commit into from
Jul 24, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@
import io.fabric8.kubernetes.api.model.VolumeMountBuilder;
import io.fabric8.kubernetes.api.model.extensions.Deployment;
import io.fabric8.kubernetes.api.model.extensions.DeploymentBuilder;
import io.fabric8.kubernetes.api.model.extensions.ReplicaSet;
import io.fabric8.kubernetes.client.KubernetesClientException;
import io.fabric8.kubernetes.client.Watcher;
import io.fabric8.kubernetes.client.dsl.ExecWatch;
Expand All @@ -52,8 +51,6 @@
import io.fabric8.openshift.api.model.Image;
import io.fabric8.openshift.api.model.ImageStream;
import io.fabric8.openshift.api.model.ImageStreamTag;
import io.fabric8.openshift.api.model.Route;
import io.fabric8.openshift.api.model.RouteList;
import io.fabric8.openshift.client.DefaultOpenShiftClient;
import io.fabric8.openshift.client.OpenShiftClient;
import io.fabric8.openshift.client.dsl.DeployableScalableResource;
Expand Down Expand Up @@ -173,8 +170,8 @@ public class OpenShiftConnector extends DockerConnector {
private static final int CHE_WORKSPACE_AGENT_PORT = 4401;
private static final int CHE_TERMINAL_AGENT_PORT = 4411;
private static final String DOCKER_PROTOCOL_PORT_DELIMITER = "/";
private static final int OPENSHIFT_WAIT_POD_DELAY = 1000;
private static final int OPENSHIFT_WAIT_POD_TIMEOUT = 240;
private static final int OPENSHIFT_WAIT_POD_DELAY = 1000;
private static final int OPENSHIFT_IMAGESTREAM_WAIT_DELAY = 2000;
private static final int OPENSHIFT_IMAGESTREAM_MAX_WAIT_COUNT = 30;
private static final String OPENSHIFT_POD_STATUS_RUNNING = "Running";
Expand All @@ -191,26 +188,30 @@ public class OpenShiftConnector extends DockerConnector {

private Map<String, KubernetesExecHolder> execMap = new HashMap<>();

private final String openShiftCheProjectName;
private final int openShiftLivenessProbeDelay;
private final int openShiftLivenessProbeTimeout;
private final String workspacesPersistentVolumeClaim;
private final String workspacesPvcQuantity;
private final String cheWorkspaceStorage;
private final String cheWorkspaceProjectsStorage;
private final String cheServerExternalAddress;
private final String cheWorkspaceMemoryLimit;
private final String cheWorkspaceMemoryRequest;
private final boolean secureRoutes;
private final boolean createWorkspaceDirs;
private final OpenShiftPvcHelper openShiftPvcHelper;
private final String openShiftCheProjectName;
private final int openShiftLivenessProbeDelay;
private final int openShiftLivenessProbeTimeout;
private final String workspacesPersistentVolumeClaim;
private final String workspacesPvcQuantity;
private final String cheWorkspaceStorage;
private final String cheWorkspaceProjectsStorage;
private final String cheServerExternalAddress;
private final String cheWorkspaceMemoryLimit;
private final String cheWorkspaceMemoryRequest;
private final boolean secureRoutes;
private final boolean createWorkspaceDirs;
private final OpenShiftPvcHelper openShiftPvcHelper;
private final OpenShiftRouteCreator openShiftRouteCreator;
private final OpenShiftDeploymentCleaner openShiftDeploymentCleaner;

@Inject
public OpenShiftConnector(DockerConnectorConfiguration connectorConfiguration,
DockerConnectionFactory connectionFactory,
DockerRegistryAuthResolver authResolver,
DockerApiVersionPathPrefixProvider dockerApiVersionPathPrefixProvider,
OpenShiftPvcHelper openShiftPvcHelper,
OpenShiftRouteCreator openShiftRouteCreator,
OpenShiftDeploymentCleaner openShiftDeploymentCleaner,
EventService eventService,
@Nullable @Named("che.docker.ip.external") String cheServerExternalAddress,
@Named("che.openshift.project") String openShiftCheProjectName,
Expand Down Expand Up @@ -239,6 +240,8 @@ public OpenShiftConnector(DockerConnectorConfiguration connectorConfiguration,
this.secureRoutes = secureRoutes;
this.createWorkspaceDirs = createWorkspaceDirs;
this.openShiftPvcHelper = openShiftPvcHelper;
this.openShiftRouteCreator = openShiftRouteCreator;
this.openShiftDeploymentCleaner = openShiftDeploymentCleaner;
eventService.subscribe(new EventSubscriber<ServerIdleEvent>() {

@Override
Expand Down Expand Up @@ -407,7 +410,7 @@ public ContainerCreated createContainer(CreateContainerParams createContainerPar
// in an inconsistent state.
LOG.info("Error while creating Pod, removing deployment");
LOG.info(e.getMessage());
cleanUpWorkspaceResources(deploymentName);
openShiftDeploymentCleaner.cleanDeploymentResources(deploymentName, openShiftCheProjectName);
openShiftClient.resource(imageStreamTag).delete();
throw e;
} finally {
Expand Down Expand Up @@ -493,10 +496,8 @@ public ContainerInfo inspectContainer(String containerId) throws IOException {

@Override
public void removeContainer(final RemoveContainerParams params) throws IOException {
String containerId = params.getContainer();
Pod pod = getChePodByContainerId(containerId);
String deploymentName = pod.getMetadata().getLabels().get(OPENSHIFT_DEPLOYMENT_LABEL);
cleanUpWorkspaceResources(deploymentName);
String deploymentName = getDeploymentName(params);
openShiftDeploymentCleaner.cleanDeploymentResources(deploymentName, openShiftCheProjectName);
}

@Override
Expand Down Expand Up @@ -1005,50 +1006,6 @@ private Service getCheServiceBySelector(String selectorKey, String selectorValue
}
}

private Deployment getDeploymentByName(String deploymentName) throws IOException {
try (OpenShiftClient openShiftClient = new DefaultOpenShiftClient()) {
Deployment deployment = openShiftClient
.extensions().deployments()
.inNamespace(this.openShiftCheProjectName)
.withName(deploymentName)
.get();
if (deployment == null) {
LOG.warn("No Deployment with name {} could be found", deploymentName);
}
return deployment;
}
}

private List<Route> getRoutesByLabel(String labelKey, String labelValue) throws IOException {
try (OpenShiftClient openShiftClient = new DefaultOpenShiftClient()) {
RouteList routeList = openShiftClient
.routes()
.inNamespace(this.openShiftCheProjectName)
.withLabel(labelKey, labelValue)
.list();

List<Route> items = routeList.getItems();

if (items.isEmpty()) {
LOG.warn("No Route with label {}={} could be found", labelKey, labelValue);
throw new IOException("No Route with label " + labelKey + "=" + labelValue + " could be found");
}

return items;
}
}

private List<ReplicaSet> getReplicaSetByLabel(String key, String value) {
try (OpenShiftClient openShiftClient = new DefaultOpenShiftClient()) {
List<ReplicaSet> replicaSets = openShiftClient.extensions()
.replicaSets()
.inNamespace(openShiftCheProjectName)
.withLabel(key, value)
.list().getItems();
return replicaSets;
}
}

private Pod getChePodByContainerId(String containerId) throws IOException {
try (OpenShiftClient openShiftClient = new DefaultOpenShiftClient()) {
PodList pods = openShiftClient.pods()
Expand Down Expand Up @@ -1155,7 +1112,7 @@ private void createOpenShiftRoute(String serviceName,
String deploymentName,
String serverRef) {
String routeId = serviceName.replaceFirst(CHE_OPENSHIFT_RESOURCES_PREFIX, "");
OpenShiftRouteCreator.createRoute(openShiftCheProjectName,
openShiftRouteCreator.createRoute(openShiftCheProjectName,
cheServerExternalAddress,
serverRef,
serviceName,
Expand Down Expand Up @@ -1383,56 +1340,6 @@ private List<ContainerState> getContainerStates(final Pod pod) throws OpenShiftE
return containerStates;
}

private void cleanUpWorkspaceResources(String deploymentName) throws IOException {

Deployment deployment = getDeploymentByName(deploymentName);
Service service = getCheServiceBySelector(OPENSHIFT_DEPLOYMENT_LABEL, deploymentName);
List<Route> routes = getRoutesByLabel(OPENSHIFT_DEPLOYMENT_LABEL, deploymentName);
List<ReplicaSet> replicaSets = getReplicaSetByLabel(OPENSHIFT_DEPLOYMENT_LABEL, deploymentName);

try (OpenShiftClient openShiftClient = new DefaultOpenShiftClient()) {
if (routes != null) {
for (Route route: routes) {
LOG.info("Removing OpenShift Route {}", route.getMetadata().getName());
openShiftClient.resource(route).delete();
}
}

if (service != null) {
LOG.info("Removing OpenShift Service {}", service.getMetadata().getName());
openShiftClient.resource(service).delete();
}

if (deployment != null) {
LOG.info("Removing OpenShift Deployment {}", deployment.getMetadata().getName());
openShiftClient.resource(deployment).delete();
}

if (replicaSets != null && replicaSets.size() > 0) {
LOG.info("Removing OpenShift ReplicaSets for deployment {}", deploymentName);
replicaSets.forEach(rs -> openShiftClient.resource(rs).delete());
}

// Wait for all pods to terminate before returning.
for (int waitCount = 0; waitCount < OPENSHIFT_WAIT_POD_TIMEOUT; waitCount++) {
List<Pod> pods = openShiftClient.pods()
.inNamespace(openShiftCheProjectName)
.withLabel(OPENSHIFT_DEPLOYMENT_LABEL, deploymentName)
.list()
.getItems();
if (pods.size() == 0) {
return;
}
Thread.sleep(OPENSHIFT_WAIT_POD_DELAY);
}
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
LOG.info("Thread interrupted while cleaning up workspace");
}

throw new OpenShiftException("Timeout while waiting for pods to terminate");
}

private void createWorkspaceDir(String[] volumes) throws OpenShiftException {
PersistentVolumeClaim pvc = getClaimCheWorkspace();
String workspaceSubpath = getWorkspaceSubpath(volumes);
Expand Down Expand Up @@ -1681,4 +1588,12 @@ private boolean isTerminalAgentInjected(final Set<String> exposedPorts) {
private boolean isDevMachine(final Set<String> exposedPorts) {
return exposedPorts.contains(CHE_WORKSPACE_AGENT_PORT + "/tcp");
}

private String getDeploymentName(final RemoveContainerParams params) throws IOException {
String containerId = params.getContainer();
Pod pod = getChePodByContainerId(containerId);
String deploymentName = pod.getMetadata().getLabels().get(OPENSHIFT_DEPLOYMENT_LABEL);
return deploymentName;
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
/*******************************************************************************
* Copyright (c) 2012-2017 Red Hat, Inc.
* All rights reserved. This program and the accompanying materials
* are made available under the terms of the Eclipse Public License v1.0
* which accompanies this distribution, and is available at
* http://www.eclipse.org/legal/epl-v10.html
*
* Contributors:
* Red Hat, Inc. - initial API and implementation
*******************************************************************************/
package org.eclipse.che.plugin.openshift.client;

import java.io.IOException;
import java.util.List;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;

import javax.inject.Singleton;

import org.eclipse.che.plugin.openshift.client.exception.OpenShiftException;
import org.eclipse.che.plugin.openshift.client.kubernetes.KubernetesResourceUtil;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import io.fabric8.kubernetes.api.model.Pod;
import io.fabric8.kubernetes.api.model.PodList;
import io.fabric8.kubernetes.api.model.Service;
import io.fabric8.kubernetes.api.model.extensions.Deployment;
import io.fabric8.kubernetes.api.model.extensions.DoneableDeployment;
import io.fabric8.kubernetes.api.model.extensions.ReplicaSet;
import io.fabric8.kubernetes.client.KubernetesClientException;
import io.fabric8.kubernetes.client.Watch;
import io.fabric8.kubernetes.client.Watcher;
import io.fabric8.kubernetes.client.dsl.FilterWatchListDeletable;
import io.fabric8.kubernetes.client.dsl.ScalableResource;
import io.fabric8.openshift.api.model.Route;
import io.fabric8.openshift.client.DefaultOpenShiftClient;
import io.fabric8.openshift.client.OpenShiftClient;

@Singleton
public class OpenShiftDeploymentCleaner {
private static final Logger LOG = LoggerFactory.getLogger(OpenShiftDeploymentCleaner.class);
private static final int OPENSHIFT_POD_DELETION_TIMEOUT = 120;

public void cleanDeploymentResources(final String deploymentName, final String namespace) throws IOException {
scaleDownDeployment(deploymentName, namespace);
cleanUpWorkspaceResources(deploymentName, namespace);
waitUntilWorkspacePodIsDeleted(deploymentName, namespace);
}

private void scaleDownDeployment(String deploymentName, final String namespace) throws OpenShiftException {
try (OpenShiftClient openShiftClient = new DefaultOpenShiftClient()) {
ScalableResource<Deployment, DoneableDeployment> deployment = openShiftClient.extensions()
.deployments()
.inNamespace(namespace)
.withName(deploymentName);

if (deployment != null) {
deployment.scale(0, true);
Copy link
Contributor

Choose a reason for hiding this comment

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

Does waiting for the scale down take significantly longer than the previous way, or does the scale down take grace termination period into account? I ask because we had the issue on OpenShift where stopping and restarting a pod quickly could case PV issues.

Copy link
Member Author

Choose a reason for hiding this comment

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

there should not be PV issues since this change was applied in openshift-connector rebased, but grace termination period is taken into account

Copy link
Member Author

Choose a reason for hiding this comment

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

@amisevsk btw in master I do not see that we use "terminationGracePeriodSeconds", So while testing it on minishift scale down does not happen fast and require some time - this might be the main reason of quota limit issue that was reported today

Copy link
Member Author

@ibuziuk ibuziuk Jul 20, 2017

Choose a reason for hiding this comment

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

I think this PR is not applied to master #5007
But this is a separate conversation, I will provide another PR for that

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good - thanks @ibuziuk !

}
}
}

private void cleanUpWorkspaceResources(final String deploymentName, final String namespace) throws IOException {
Deployment deployment = KubernetesResourceUtil.getDeploymentByName(deploymentName, namespace);
Service service = KubernetesResourceUtil.getServiceBySelector(OpenShiftConnector.OPENSHIFT_DEPLOYMENT_LABEL, deploymentName, namespace);
List<Route> routes = KubernetesResourceUtil.getRoutesByLabel(OpenShiftConnector.OPENSHIFT_DEPLOYMENT_LABEL, deploymentName, namespace);
List<ReplicaSet> replicaSets = KubernetesResourceUtil.getReplicaSetByLabel(OpenShiftConnector.OPENSHIFT_DEPLOYMENT_LABEL, deploymentName, namespace);

try (OpenShiftClient openShiftClient = new DefaultOpenShiftClient()) {

if (deployment != null) {
LOG.info("Removing OpenShift Deployment {}", deployment.getMetadata().getName());
openShiftClient.resource(deployment).delete();
}

if (replicaSets != null && replicaSets.size() > 0) {
LOG.info("Removing OpenShift ReplicaSets for deployment {}", deploymentName);
replicaSets.forEach(rs -> openShiftClient.resource(rs).delete());
}

if (routes != null) {
for (Route route: routes) {
LOG.info("Removing OpenShift Route {}", route.getMetadata().getName());
openShiftClient.resource(route).delete();
}
}

if (service != null) {
LOG.info("Removing OpenShift Service {}", service.getMetadata().getName());
openShiftClient.resource(service).delete();
}
}
}

private void waitUntilWorkspacePodIsDeleted(final String deploymentName, final String namespace) throws OpenShiftException {
try (OpenShiftClient client = new DefaultOpenShiftClient()) {
FilterWatchListDeletable<Pod, PodList, Boolean, Watch, Watcher<Pod>> pods = client.pods()
.inNamespace(namespace)
.withLabel(OpenShiftConnector.OPENSHIFT_DEPLOYMENT_LABEL, deploymentName);

int numberOfPodsToStop = pods.list().getItems().size();
LOG.info("Number of workspace pods to stop {}", numberOfPodsToStop);
if (numberOfPodsToStop > 0) {
final CountDownLatch podCount = new CountDownLatch(numberOfPodsToStop);
pods.watch(new Watcher<Pod>() {
@Override
public void eventReceived(Action action, Pod pod) {
try {
switch (action) {
case ADDED:
case MODIFIED:
case ERROR:
break;
case DELETED:
LOG.info("Pod {} deleted", pod.getMetadata().getName());
podCount.countDown();
break;
}
} catch (Exception e) {
LOG.error("Failed to process {} on Pod {}. Error: ", action, pod, e);
}
}

@Override
public void onClose(KubernetesClientException ex) {
}
});

try {
LOG.info("Waiting for all pods to be deleted for deployment '{}'", deploymentName);
podCount.await(OPENSHIFT_POD_DELETION_TIMEOUT, TimeUnit.SECONDS);
} catch (InterruptedException e) {
LOG.error("Exception while waiting for pods to be deleted", e);
throw new OpenShiftException("Timeout while waiting for pods to terminate", e);
}
}
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@

import javax.inject.Inject;
import javax.inject.Named;
import javax.inject.Singleton;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
Expand All @@ -54,6 +56,7 @@
*
* @author amisevsk
*/
@Singleton
public class OpenShiftPvcHelper {

private static final Logger LOG = LoggerFactory.getLogger(OpenShiftPvcHelper.class);
Expand Down
Loading