Enforce ConfusingTernary and PrematureDeclaration rules

config/pmd/ruleset.xml

@@ -28,7 +28,6 @@
   <rule ref="category/java/codestyle.xml">
     <exclude name="AtLeastOneConstructor" />
     <exclude name="CommentDefaultAccessModifier" />
-    <exclude name="ConfusingTernary" />
     <exclude name="FieldDeclarationsShouldBeAtStartOfClass" />
     <exclude name="FieldNamingConventions" />
     <exclude name="GenericsNaming" />
@@ -37,7 +36,6 @@
     <exclude name="LongVariable" />
     <exclude name="MethodArgumentCouldBeFinal" />
     <exclude name="OnlyOneReturn" />
-    <exclude name="PrematureDeclaration" />
     <exclude name="ShortVariable" />
     <exclude name="TooManyStaticImports" />
     <exclude name="UseUnderscoresInNumericLiterals" />

spark-operator/src/main/java/org/apache/spark/k8s/operator/metrics/source/OperatorJosdkMetrics.java

@@ -271,23 +271,23 @@ private long toSeconds(long startTimeInMilliseconds) {
   private Histogram getHistogram(Class<?> kclass, String... names) {
     String name = MetricRegistry.name(kclass.getSimpleName(), names).toLowerCase();
     Histogram histogram;
-    if (!histograms.containsKey(name)) {
+    if (histograms.containsKey(name)) {
+      histogram = histograms.get(name);
+    } else {
       histogram = metricRegistry.histogram(name);
       histograms.put(name, histogram);
-    } else {
-      histogram = histograms.get(name);
     }
     return histogram;
   }
 
   private Counter getCounter(Class<?> klass, String... names) {
     String name = MetricRegistry.name(klass.getSimpleName(), names).toLowerCase();
     Counter counter;
-    if (!counters.containsKey(name)) {
+    if (counters.containsKey(name)) {
+      counter = counters.get(name);
+    } else {
       counter = metricRegistry.counter(name);
       counters.put(name, counter);
-    } else {
-      counter = counters.get(name);
     }
     return counter;
   }

spark-operator/src/main/java/org/apache/spark/k8s/operator/probe/ProbeService.java

@@ -41,14 +41,13 @@ public class ProbeService {
 
   public ProbeService(
       List<Operator> operators, List<SentinelManager<?>> sentinelManagers, Executor executor) {
-    HealthProbe healthProbe = new HealthProbe(operators, sentinelManagers);
     try {
       this.server = HttpServer.create(new InetSocketAddress(OPERATOR_PROBE_PORT.getValue()), 0);
     } catch (IOException e) {
       throw new RuntimeException("Failed to create Probe Service Server", e);
     }
     server.createContext(READYZ, new ReadinessProbe(operators));
-    server.createContext(HEALTHZ, healthProbe);
+    server.createContext(HEALTHZ, new HealthProbe(operators, sentinelManagers));
     server.setExecutor(executor);
   }
 

spark-operator/src/main/java/org/apache/spark/k8s/operator/reconciler/SparkAppReconciler.java

@@ -204,7 +204,6 @@ protected List<AppReconcileStep> getReconcileSteps(final SparkApplication app) {
   public DeleteControl cleanup(
       SparkApplication sparkApplication, Context<SparkApplication> context) {
     LoggingUtils.TrackedMDC trackedMDC = new LoggingUtils.TrackedMDC();
-    DeleteControl deleteControl = DeleteControl.defaultDelete();
     try {
       trackedMDC.set(sparkApplication);
       log.info("Cleaning up resources for SparkApp.");
@@ -229,6 +228,6 @@ public DeleteControl cleanup(
       trackedMDC.reset();
     }
     sparkAppStatusRecorder.removeCachedStatus(sparkApplication);
-    return deleteControl;
+    return DeleteControl.defaultDelete();
   }
 }

spark-operator/src/main/java/org/apache/spark/k8s/operator/reconciler/observers/AppDriverTimeoutObserver.java

@@ -58,8 +58,6 @@ public class AppDriverTimeoutObserver extends BaseAppDriverObserver {
   @Override
   public Optional<ApplicationState> observe(
       Pod driver, ApplicationSpec spec, ApplicationStatus currentStatus) {
-    Instant lastTransitionTime =
-        Instant.parse(currentStatus.getCurrentState().getLastTransitionTime());
     long timeoutThreshold;
     Supplier<ApplicationState> supplier;
     ApplicationTimeoutConfig timeoutConfig =
@@ -82,6 +80,8 @@ public Optional<ApplicationState> observe(
         // No timeout check needed for other states
         return Optional.empty();
     }
+    Instant lastTransitionTime =
+        Instant.parse(currentStatus.getCurrentState().getLastTransitionTime());
     if (timeoutThreshold > 0L
         && lastTransitionTime.plusMillis(timeoutThreshold).isBefore(Instant.now())) {
       ApplicationState state = supplier.get();

spark-operator/src/main/java/org/apache/spark/k8s/operator/reconciler/reconcilesteps/AppRunningStep.java

@@ -76,17 +76,17 @@ public ReconcileProgress reconcile(
         }
       }
     }
-    if (!proposedStateSummary.equals(prevStateSummary)) {
+    if (proposedStateSummary.equals(prevStateSummary)) {
+      return observeDriver(
+          context, statusRecorder, Collections.singletonList(new AppDriverRunningObserver()));
+    } else {
       statusRecorder.persistStatus(
           context,
           context
               .getResource()
               .getStatus()
               .appendNewState(new ApplicationState(proposedStateSummary, stateMessage)));
       return completeAndDefaultRequeue();
-    } else {
-      return observeDriver(
-          context, statusRecorder, Collections.singletonList(new AppDriverRunningObserver()));
     }
   }
 }

spark-operator/src/main/java/org/apache/spark/k8s/operator/utils/ReconcilerUtils.java

@@ -87,9 +87,7 @@ public static <T extends HasMetadata> Optional<T> getOrCreateSecondaryResource(
                 maxAttempts);
           }
           // retry only on 409 Conflict
-          if (e.getCode() != 409) {
-            throw e;
-          } else {
+          if (e.getCode() == 409) {
             if (isConflictForExistingResource(e)) {
               current = getResource(client, resource);
               if (current.isPresent()) {
@@ -100,6 +98,8 @@ public static <T extends HasMetadata> Optional<T> getOrCreateSecondaryResource(
               log.error("Max Retries exceeded while trying to create resource");
               throw e;
             }
+          } else {
+            throw e;
           }
         }
       }
@@ -147,10 +147,10 @@ public static <T extends HasMetadata> void deleteResourceIfExists(
             .delete();
       }
     } catch (KubernetesClientException e) {
-      if (e.getCode() != 404) {
-        throw e;
-      } else {
+      if (e.getCode() == 404) {
         log.info("Pod to delete does not exist, proceeding...");
+      } else {
+        throw e;
       }
     }
   }

spark-operator/src/main/java/org/apache/spark/k8s/operator/utils/StatusRecorder.java

@@ -88,8 +88,6 @@ private void patchAndStatusWithVersionLocked(CR resource, KubernetesClient clien
       return;
     }
 
-    STATUS prevStatus = objectMapper.convertValue(previousStatusNode, statusClass);
-
     Exception err = null;
     long maxRetry = API_STATUS_PATCH_MAX_ATTEMPTS.getValue();
     for (long i = 0; i < maxRetry; i++) {
@@ -110,6 +108,7 @@ private void patchAndStatusWithVersionLocked(CR resource, KubernetesClient clien
     }
 
     statusCache.put(resourceId, newStatusNode);
+    STATUS prevStatus = objectMapper.convertValue(previousStatusNode, statusClass);
     statusListeners.forEach(
         listener -> {
           listener.listenStatus(resource, prevStatus, resource.getStatus());