Merge pull request #1506 from Vlatombe/provisioning-limits-exception-handling

Properly unregister slots that were reserved during provisioning if the provisioning ended up failing.

Author: Vlatombe

src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud.java

@@ -570,6 +570,7 @@ public KubernetesClient connect() throws KubernetesAuthException, IOException {
     @Override
     public Collection<NodeProvisioner.PlannedNode> provision(
             @NonNull final Cloud.CloudState state, final int excessWorkload) {
+        var limitRegistrationResults = new LimitRegistrationResults(this);
         try {
             Metrics.metricRegistry().meter(metricNameForLabel(state.getLabel())).mark(excessWorkload);
             Label label = state.getLabel();
@@ -588,8 +589,7 @@ public Collection<NodeProvisioner.PlannedNode> provision(
                 // check overall concurrency limit using the default label(s) on all templates
                 int numExecutors = 1;
                 PodTemplate unwrappedTemplate = getUnwrappedTemplate(podTemplate);
-                while (toBeProvisioned > 0
-                        && KubernetesProvisioningLimits.get().register(this, podTemplate, numExecutors)) {
+                while (toBeProvisioned > 0 && limitRegistrationResults.register(podTemplate, numExecutors)) {
                     plannedNodes.add(PlannedNodeBuilderFactory.createInstance()
                             .cloud(this)
                             .template(unwrappedTemplate)
@@ -626,8 +626,11 @@ public Collection<NodeProvisioner.PlannedNode> provision(
                         "Failed to count the # of live instances on Kubernetes",
                         cause != null ? cause : e);
             }
+            limitRegistrationResults.unregister();
         } catch (Exception e) {
+            Metrics.metricRegistry().counter(MetricNames.PROVISION_FAILED).inc();
             LOGGER.log(Level.WARNING, "Failed to count the # of live instances on Kubernetes", e);
+            limitRegistrationResults.unregister();
         }
         return Collections.emptyList();
     }

src/main/java/org/csanchez/jenkins/plugins/kubernetes/LimitRegistrationResults.java

@@ -0,0 +1,72 @@
+package org.csanchez.jenkins.plugins.kubernetes;
+
+import edu.umd.cs.findbugs.annotations.NonNull;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.logging.Level;
+import java.util.logging.Logger;
+
+/**
+ * Registers result for slots registration and provides an easy way to unregister.
+ */
+class LimitRegistrationResults {
+    private static final Logger LOGGER = Logger.getLogger(LimitRegistrationResults.class.getName());
+
+    private final KubernetesCloud cloud;
+
+    private final List<Result> results = new ArrayList<>();
+
+    public LimitRegistrationResults(@NonNull KubernetesCloud cloud) {
+        this.cloud = cloud;
+    }
+
+    /**
+     * Register a slot for the given pod template and number of executors.
+     * @return true is the registration was successful
+     */
+    public boolean register(@NonNull PodTemplate podTemplate, int numExecutors) {
+        var result = new Result(
+                KubernetesProvisioningLimits.get().register(cloud, podTemplate, numExecutors),
+                podTemplate,
+                numExecutors);
+        results.add(result);
+        return result.success;
+    }
+
+    /**
+     * Unregister all slots that were registered through this object previously.
+     */
+    public void unregister() {
+        results.forEach(result -> result.unregister(cloud));
+    }
+
+    private static class Result {
+        final boolean success;
+        final int numExecutors;
+
+        @NonNull
+        PodTemplate podTemplate;
+
+        Result(boolean success, @NonNull PodTemplate podTemplate, int numExecutors) {
+            this.success = success;
+            this.podTemplate = podTemplate;
+            this.numExecutors = numExecutors;
+        }
+
+        void unregister(KubernetesCloud cloud) {
+            if (success) {
+                LOGGER.log(
+                        Level.FINEST,
+                        () -> "Registration was successful, unregistering slot for podTemplate " + podTemplate.getName()
+                                + " from cloud " + cloud.name + " with " + numExecutors + " executors");
+                KubernetesProvisioningLimits.get().unregister(cloud, podTemplate, numExecutors);
+            } else {
+                LOGGER.log(
+                        Level.FINEST,
+                        () -> "Registration previously failed, no need to unregister slot for podTemplate "
+                                + podTemplate.getName() + " from cloud " + cloud.name + " with " + numExecutors
+                                + " executors");
+            }
+        }
+    }
+}