Merge pull request #500 from cliveseldon/hpa_fix

Fix HPA Nullpointer
SeldonIO · Apr 10, 2019 · d3c1fb1 · d3c1fb1
2 parents 20734ba + a4a8755
commit d3c1fb1
Show file tree

Hide file tree

Showing 4 changed files with 20 additions and 7 deletions.
diff --git a/cluster-manager/src/main/java/io/seldon/clustermanager/k8s/KubeCRDHandler.java b/cluster-manager/src/main/java/io/seldon/clustermanager/k8s/KubeCRDHandler.java
@@ -19,6 +19,7 @@
 import io.kubernetes.client.models.V1ServiceList;
 import io.kubernetes.client.models.V2beta1HorizontalPodAutoscalerList;
 import io.seldon.protos.DeploymentProtos.SeldonDeployment;
+import java.util.Optional;
 
 /**
  * Interactions to update and get details about Seldon deployments and associated resources
@@ -32,5 +33,5 @@ public interface KubeCRDHandler {
 	public SeldonDeployment getSeldonDeployment(String name, String version, String namespace);	
 	public ExtensionsV1beta1DeploymentList getOwnedDeployments(String seldonDeploymentName,String namespace);
 	public V1ServiceList getOwnedServices(String seldonDeploymentName,String namespace);
-	public V2beta1HorizontalPodAutoscalerList getOwnedHPAs(String seldonDeploymentName,String namespace);
+	public Optional<V2beta1HorizontalPodAutoscalerList> getOwnedHPAs(String seldonDeploymentName,String namespace);
 }
diff --git a/cluster-manager/src/main/java/io/seldon/clustermanager/k8s/KubeCRDHandlerImpl.java b/cluster-manager/src/main/java/io/seldon/clustermanager/k8s/KubeCRDHandlerImpl.java
@@ -17,6 +17,7 @@
 
 import java.io.IOException;
 
+import java.util.Optional;
 import org.apache.commons.lang3.StringUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -220,19 +221,19 @@ public V1ServiceList getOwnedServices(String seldonDeploymentName,String namespa
 	}
 
 	@Override
-	public V2beta1HorizontalPodAutoscalerList getOwnedHPAs(String seldonDeploymentName, String namespace) {
+	public Optional<V2beta1HorizontalPodAutoscalerList> getOwnedHPAs(String seldonDeploymentName, String namespace) {
 		try
 		{
 			ApiClient client = k8sClientProvider.getClient();
 			io.kubernetes.client.apis.AutoscalingV2beta1Api api = k8sApiProvider.getAutoScalingApi(client);
 			V2beta1HorizontalPodAutoscalerList l = api.listNamespacedHorizontalPodAutoscaler(namespace, null, null, null, false, Constants.LABEL_SELDON_ID+"="+seldonDeploymentName, null, null, null, null);
-			return l;
+			return Optional.of(l);
 		} catch (IOException e) {
             logger.error("Failed to get HPA list for "+seldonDeploymentName,e);
-            return null;
+            return Optional.empty();
         } catch (ApiException e) {
             logger.error("Failed to get HPA list for "+seldonDeploymentName,e);
-            return null;
+            return Optional.empty();
         }
 	}
 

diff --git a/cluster-manager/src/main/java/io/seldon/clustermanager/k8s/SeldonDeletionHandler.java b/cluster-manager/src/main/java/io/seldon/clustermanager/k8s/SeldonDeletionHandler.java
@@ -3,6 +3,7 @@
 import java.io.IOException;
 import java.util.HashSet;
 import java.util.List;
+import java.util.Optional;
 import java.util.Set;
 
 import org.slf4j.Logger;
@@ -140,8 +141,12 @@ public void removeServices(ApiClient client,String namespace,SeldonDeployment se
 	public void removeHPAs(ApiClient client,String namespace,SeldonDeployment seldonDeployment,List<HorizontalPodAutoscaler> hpas) throws ApiException, IOException, SeldonDeploymentException
 	{
 		Set<String> names = getHpaNames(hpas);
-		V2beta1HorizontalPodAutoscalerList hpaList = crdHandler.getOwnedHPAs(seldonNameCreator.getSeldonId(seldonDeployment),namespace);
-		for(V2beta1HorizontalPodAutoscaler hpa : hpaList.getItems())
+		Optional<V2beta1HorizontalPodAutoscalerList> hpaList = crdHandler.getOwnedHPAs(seldonNameCreator.getSeldonId(seldonDeployment),namespace);
+		if(!hpaList.isPresent()) {
+			logger.info("No HPAs found");
+			return;
+		}
+		for(V2beta1HorizontalPodAutoscaler hpa : hpaList.get().getItems())
 		{
 			if (!names.contains(hpa.getMetadata().getName()))
 			{	

diff --git a/cluster-manager/src/test/java/io/seldon/clustermanager/k8s/SeldonDeletionHandlerTest.java b/cluster-manager/src/test/java/io/seldon/clustermanager/k8s/SeldonDeletionHandlerTest.java
@@ -10,6 +10,7 @@
 import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Optional;
 
 import org.junit.Test;
 
@@ -23,6 +24,7 @@
 import io.kubernetes.client.models.ExtensionsV1beta1DeploymentList;
 import io.kubernetes.client.models.V1ObjectMeta;
 import io.kubernetes.client.proto.Meta.DeleteOptions;
+import io.kubernetes.client.proto.V2beta1Autoscaling.HorizontalPodAutoscaler;
 import io.seldon.clustermanager.AppTest;
 import io.seldon.clustermanager.ClusterManagerProperites;
 import io.seldon.clustermanager.k8s.SeldonDeploymentOperatorImpl.DeploymentResources;
@@ -101,12 +103,16 @@ public void testDeploymentsNoDelete() throws IOException, ApiException, SeldonDe
 		when(client.getApiClient()).thenReturn(apiClient);
 		when(client.delete(any(Message.Builder.class), any(String.class), any(DeleteOptions.class))).thenReturn(new ObjectOrStatus<>(mlDep,null));
 
+		when(crdHandler.getOwnedHPAs(any(String.class), any(String.class))).thenReturn(Optional.empty());
 
         SeldonDeletionHandler delHandler = new SeldonDeletionHandler(crdHandler);
 
         delHandler.removeDeployments(client, namespace, mlDep, resources.deployments, false);
         verify(client, never()).delete(any(Message.Builder.class), any(String.class), any(DeleteOptions.class));
 
+        List<HorizontalPodAutoscaler> hpas = new ArrayList<>();
+        delHandler.removeHPAs(apiClient, namespace, mlDep, hpas);
+
 	}
 
 }