Fix exception when reconciling object deletion (#147)

Author: jogrogan

hoptimator-k8s/src/main/java/com/linkedin/hoptimator/k8s/K8sUtils.java

@@ -103,9 +103,9 @@ static void checkResponse(Supplier<String> msgSupplier, KubernetesApiResponse<?>
       case 409: // conflict
       case 410: // gone
       case 412: // precondition failed
-        throw new SQLTransientException(msgSupplier.get(), e);
+        throw new SQLTransientException(msgSupplier.get(), null, resp.getHttpStatusCode(), e);
       default:
-        throw new SQLNonTransientException(msgSupplier.get(), e);
+        throw new SQLNonTransientException(msgSupplier.get(), null, resp.getHttpStatusCode(), e);
       }
     }
   }

hoptimator-k8s/src/testFixtures/java/com/linkedin/hoptimator/k8s/FakeK8sApi.java

@@ -26,7 +26,7 @@ public Collection<T> list() throws SQLException {
   @Override
   public T get(String name) throws SQLException {
     return objects.stream().filter(x -> x.getMetadata().getName().equals(name)).findFirst()
-        .orElseThrow(() -> new SQLException("No object named " + name));
+        .orElseThrow(() -> new SQLException("No object named " + name, null, 404));
   }
 
   @Override

hoptimator-operator/src/main/java/com/linkedin/hoptimator/operator/pipeline/PipelineReconciler.java

@@ -46,11 +46,15 @@ public Result reconcile(Request request) {
 
     Result result = new Result(true, pendingRetryDuration());
     try {
-      V1alpha1Pipeline object = pipelineApi.get(namespace, name);
-
-      if (object == null) {
-        log.info("Object {} deleted. Skipping.", name);
-        return new Result(false);
+      V1alpha1Pipeline object;
+      try {
+        object = pipelineApi.get(namespace, name);
+      } catch (SQLException e) {
+        if (e.getErrorCode() == 404) {
+          log.info("Object {} deleted. Skipping.", name);
+          return new Result(false);
+        }
+        throw e;
       }
 
       V1alpha1PipelineStatus status = object.getStatus();
@@ -79,13 +83,6 @@ public Result reconcile(Request request) {
 
       pipelineApi.updateStatus(object, status);
     } catch (Exception e) {
-      if (e instanceof SQLException) {
-        SQLException sqlException = (SQLException) e;
-        if (sqlException.getErrorCode() == 404) {
-          log.info("Object {} deleted. Skipping.", name);
-          return new Result(false);
-        }
-      }
       log.error("Encountered exception while reconciling Pipeline {}.", name, e);
       return new Result(true, failureRetryDuration());
     }

hoptimator-operator/src/main/java/com/linkedin/hoptimator/operator/trigger/TableTriggerReconciler.java

@@ -82,7 +82,7 @@ private TableTriggerReconciler(K8sContext context) {
     this.tableTriggerApi = tableTriggerApi;
     this.jobApi = jobApi;
     this.yamlApi = yamlApi;
-  } 
+  }
 
   @Override
   public Result reconcile(Request request) {
@@ -91,11 +91,15 @@ public Result reconcile(Request request) {
     String namespace = request.getNamespace();
 
     try {
-      V1alpha1TableTrigger object = tableTriggerApi.get(namespace, name);
-
-      if (object == null) {
-        log.info("Object {} deleted. Skipping.", name);
-        return new Result(false);
+      V1alpha1TableTrigger object;
+      try {
+        object = tableTriggerApi.get(namespace, name);
+      } catch (SQLException e) {
+        if (e.getErrorCode() == 404) {
+          log.info("Object {} deleted. Skipping.", name);
+          return new Result(false);
+        }
+        throw e;
       }
 
       V1alpha1TableTriggerStatus status = object.getStatus();

hoptimator-operator/src/test/java/com/linkedin/hoptimator/operator/trigger/TestTableTriggerReconciler.java

@@ -1,5 +1,6 @@
 package com.linkedin.hoptimator.operator.trigger;
 
+import io.kubernetes.client.extended.controller.reconciler.Result;
 import java.time.OffsetDateTime;
 import java.util.ArrayList;
 import java.util.HashMap;
@@ -43,6 +44,13 @@ void beforeEach() {
     yamls.clear();
   }
 
+  @Test
+  void deletedJob() {
+    Result result = reconciler.reconcile(new Request("namespace", "table-trigger"));
+    Assertions.assertFalse(result.isRequeue());
+    Assertions.assertTrue(yamls.isEmpty(), "Job should not exist");
+  }
+
   @Test
   void createsNewJob() {
     V1Job job = new V1Job().apiVersion("v1/batch").kind("Job")
@@ -51,7 +59,8 @@ void createsNewJob() {
         .metadata(new V1ObjectMeta().name("table-trigger"))
         .spec(new V1alpha1TableTriggerSpec().yaml(Yaml.dump(job)))
         .status(new V1alpha1TableTriggerStatus().timestamp(OffsetDateTime.now())));
-    reconciler.reconcile(new Request("namespace", "table-trigger"));
+    Result result = reconciler.reconcile(new Request("namespace", "table-trigger"));
+    Assertions.assertTrue(result.isRequeue());
     Assertions.assertFalse(yamls.isEmpty(), "Job was not created");
   }
 
@@ -66,7 +75,8 @@ void deletesCompletedJob() {
         .status(new V1alpha1TableTriggerStatus().timestamp(OffsetDateTime.now()));
     triggers.add(trigger);
     jobs.add(job);
-    reconciler.reconcile(new Request("namespace", "table-trigger"));
+    Result result = reconciler.reconcile(new Request("namespace", "table-trigger"));
+    Assertions.assertTrue(result.isRequeue());
     Assertions.assertTrue(jobs.isEmpty(), "Job was not deleted");
   }
 
@@ -83,7 +93,8 @@ void updatesWatermark() {
         .status(new V1alpha1TableTriggerStatus().timestamp(OffsetDateTime.now()));
     triggers.add(trigger);
     jobs.add(job);
-    reconciler.reconcile(new Request("namespace", "table-trigger"));
+    Result result =  reconciler.reconcile(new Request("namespace", "table-trigger"));
+    Assertions.assertTrue(result.isRequeue());
     Assertions.assertNotNull(trigger.getStatus().getWatermark(), "Watermark was not set");
   }
 }