Merge pull request #1342 from Vlatombe/kubernetes-client-api-6.5.1

jenkinsci · Aug 29, 2023 · 5712230 · 5712230
2 parents 6332799 + a3a6df8
commit 5712230
Show file tree

Hide file tree

Showing 7 changed files with 114 additions and 36 deletions.
diff --git a/pom.xml b/pom.xml
@@ -46,7 +46,9 @@
     <jenkins.host.address />
     <slaveAgentPort />
     <!-- TODO fix KubernetesCloudTest todos once past 2.414 -->
-    <jenkins.version>2.387.3</jenkins.version>
+    <jenkins.version>2.401.1</jenkins.version>
+    <bom>2.401.x</bom>
+    <bom.version>2357.v1043f8578392</bom.version>
     <no-test-jar>false</no-test-jar>
     <useBeta>true</useBeta>
     <gitHubRepo>jenkinsci/${project.artifactId}-plugin</gitHubRepo>
@@ -58,7 +60,7 @@
     <dependency>
       <groupId>org.jenkins-ci.plugins</groupId>
       <artifactId>kubernetes-client-api</artifactId>
-      <version>6.4.1-215.v2ed17097a_8e9</version>
+      <version>6.8.1-224.vd388fca_4db_3b_</version>
     </dependency>
     <dependency>
       <groupId>org.jenkins-ci.plugins</groupId>
@@ -79,12 +81,12 @@
     <dependency>
       <groupId>org.jenkinsci.plugins</groupId>
       <artifactId>kubernetes-credentials</artifactId>
-      <version>0.10.0</version>
+      <version>0.11</version>
     </dependency>
     <dependency>
       <groupId>org.jenkins-ci.plugins</groupId>
       <artifactId>authentication-tokens</artifactId>
-      <version>1.4</version>
+      <version>1.53.v1c90fd9191a_b_</version>
     </dependency>
 
     <dependency> <!-- Requires Permission -->
@@ -107,6 +109,7 @@
     <dependency>
       <groupId>org.jenkins-ci.plugins.workflow</groupId>
       <artifactId>workflow-api</artifactId>
+      <version>1267.vd9b_a_ddd9eb_47</version>
     </dependency>
     <dependency> <!-- DeclarativeAgent -->
       <groupId>org.jenkinsci.plugins</groupId>
@@ -238,7 +241,7 @@
     <dependency>
       <groupId>io.fabric8</groupId>
       <artifactId>kubernetes-server-mock</artifactId>
-      <version>6.4.1</version>
+      <version>6.8.1</version>
       <scope>test</scope>
       <exclusions>
         <exclusion>
@@ -247,15 +250,27 @@
         </exclusion>
       </exclusions>
     </dependency>
+    <dependency>
+      <groupId>org.awaitility</groupId>
+      <artifactId>awaitility</artifactId>
+      <version>4.2.0</version>
+      <scope>test</scope>
+      <exclusions>
+        <exclusion>
+          <groupId>org.hamcrest</groupId>
+          <artifactId>hamcrest</artifactId>
+        </exclusion>
+      </exclusions>
+    </dependency>
   </dependencies>
 
 
   <dependencyManagement>
     <dependencies>
       <dependency>
         <groupId>io.jenkins.tools.bom</groupId>
-        <artifactId>bom-2.387.x</artifactId>
-        <version>2163.v2d916d90c305</version>
+        <artifactId>bom-${bom}</artifactId>
+        <version>${bom.version}</version>
         <scope>import</scope>
         <type>pom</type>
       </dependency>

diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateBuilder.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateBuilder.java
@@ -42,13 +42,11 @@
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 import java.util.stream.Collectors;
-import java.util.stream.Stream;
 
 import edu.umd.cs.findbugs.annotations.CheckForNull;
 import edu.umd.cs.findbugs.annotations.NonNull;
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import hudson.Util;
-import io.fabric8.kubernetes.api.model.PodSpecFluent;
 import org.apache.commons.lang.StringUtils;
 import org.csanchez.jenkins.plugins.kubernetes.model.TemplateEnvVar;
 import org.csanchez.jenkins.plugins.kubernetes.pipeline.PodTemplateStepExecution;
@@ -69,8 +67,6 @@
 import io.fabric8.kubernetes.api.model.LocalObjectReference;
 import io.fabric8.kubernetes.api.model.Pod;
 import io.fabric8.kubernetes.api.model.PodBuilder;
-import io.fabric8.kubernetes.api.model.PodFluent.MetadataNested;
-import io.fabric8.kubernetes.api.model.PodFluent.SpecNested;
 import io.fabric8.kubernetes.api.model.Probe;
 import io.fabric8.kubernetes.api.model.ProbeBuilder;
 import io.fabric8.kubernetes.api.model.Quantity;
@@ -221,7 +217,7 @@ public Pod build() {
                     createContainer(containerTemplate, template.getEnvVars(), volumeMounts.values()));
         }
 
-        MetadataNested<PodBuilder> metadataBuilder = new PodBuilder().withNewMetadata();
+        var metadataBuilder = new PodBuilder().withNewMetadata();
         if (agent != null) {
             metadataBuilder.withName(agent.getPodName());
         }
@@ -240,7 +236,7 @@ public Pod build() {
             metadataBuilder.withAnnotations(annotations);
         }
 
-        SpecNested<PodBuilder> builder = metadataBuilder.endMetadata().withNewSpec();
+        var builder = metadataBuilder.endMetadata().withNewSpec();
 
         if (template.getActiveDeadlineSeconds() > 0) {
             builder = builder.withActiveDeadlineSeconds(Long.valueOf(template.getActiveDeadlineSeconds()));
@@ -276,7 +272,7 @@ public Pod build() {
         Long runAsGroup = template.getRunAsGroupAsLong();
         String supplementalGroups = template.getSupplementalGroups();
         if (runAsUser != null || runAsGroup != null || supplementalGroups != null) {
-            PodSpecFluent.SecurityContextNested<SpecNested<PodBuilder>> securityContext = builder.editOrNewSecurityContext();
+            var securityContext = builder.editOrNewSecurityContext();
             if (runAsUser != null) {
                 securityContext.withRunAsUser(runAsUser);
             }

diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateUtils.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateUtils.java
@@ -1,5 +1,8 @@
 package org.csanchez.jenkins.plugins.kubernetes;
 
+import io.fabric8.kubernetes.api.model.ConfigMapProjection;
+import io.fabric8.kubernetes.api.model.KeyToPath;
+import io.fabric8.kubernetes.api.model.SecretProjection;
 import java.io.ByteArrayInputStream;
 import java.io.IOException;
 import java.io.InputStream;
@@ -292,7 +295,7 @@ public static Pod combine(Pod parent, Pod template) {
 //        toolLocationNodeProperties.addAll(parent.getNodeProperties());
 //        toolLocationNodeProperties.addAll(template.getNodeProperties());
 
-        MetadataNested<PodBuilder> metadataBuilder = new PodBuilder(parent).withNewMetadataLike(parent.getMetadata()) //
+        var metadataBuilder = new PodBuilder(parent).withNewMetadataLike(parent.getMetadata()) //
                 .withAnnotations(podAnnotations).withLabels(podLabels);
         if (!isNullOrEmpty(template.getMetadata().getName())) {
             metadataBuilder.withName(template.getMetadata().getName());
@@ -301,7 +304,7 @@ public static Pod combine(Pod parent, Pod template) {
             metadataBuilder.withNamespace(template.getMetadata().getNamespace());
         }
 
-        SpecNested<PodBuilder> specBuilder = metadataBuilder.endMetadata() //
+        var specBuilder = metadataBuilder.endMetadata() //
                 .withNewSpecLike(parent.getSpec()) //
                 .withNodeSelector(nodeSelector) //
                 .withServiceAccount(serviceAccount) //
@@ -619,10 +622,51 @@ public static Pod parseFromYaml(String yaml) {
             if (podFromYaml.getSpec() == null) {
                 podFromYaml.setSpec(new PodSpec());
             }
+            fixOctal(podFromYaml);
             return podFromYaml;
         }
     }
 
+    private static void fixOctal(@NonNull Pod podFromYaml) {
+        podFromYaml.getSpec().getVolumes().stream()
+                .map(Volume::getProjected)
+                .forEach(projected -> {
+                    if (projected != null) {
+                        Integer defaultMode = projected.getDefaultMode();
+                        if (defaultMode != null) {
+                            projected.setDefaultMode(convertToOctal(defaultMode));
+                        }
+                        projected.getSources()
+                                .stream()
+                                .forEach(source -> {
+                                    ConfigMapProjection configMap = source.getConfigMap();
+                                    if (configMap != null) {
+                                        convertDecimalIntegersToOctal(configMap.getItems());
+                                    }
+                                    SecretProjection secret = source.getSecret();
+                                    if (secret != null) {
+                                        convertDecimalIntegersToOctal(secret.getItems());
+                                    }
+                                });
+                    }
+                });
+    }
+
+    private static void convertDecimalIntegersToOctal(List<KeyToPath> items) {
+        items
+                .stream()
+                .forEach(i -> {
+            Integer mode = i.getMode();
+            if (mode != null) {
+                i.setMode(convertToOctal(mode));
+            }
+        });
+    }
+
+    private static int convertToOctal(Integer defaultMode) {
+        return Integer.parseInt(Integer.toString(defaultMode, 10), 8);
+    }
+
     public static Collection<String> validateYamlContainerNames(List<String> yamls) {
         Collection<String> errors = new ArrayList<>();
         for (String yaml : yamls) {

diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/retention/Reaper.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/retention/Reaper.java
@@ -240,6 +240,10 @@ boolean isWatchingCloud(String name) {
         return watchers.get(name) != null;
     }
 
+    public Map<String, ?> getWatchers() {
+        return watchers;
+    }
+
     /**
      * Check if the given cloud pod watcher exists and is still valid. Watchers may become invalid
      * of the kubernetes client configuration changes.
@@ -330,6 +334,7 @@ public void eventReceived(Action action, Pod pod) {
          */
         void stop() {
             if (watch != null) {
+                LOGGER.info("Stopping watch for kubernetes cloud " + cloudName);
                 this.watch.close();
             }
         }
@@ -529,4 +534,4 @@ public void onChange(Saveable o, XmlFile file) {
             }
         }
     }
-}
+}
diff --git a/src/spotbugs/excludesFilter.xml b/src/spotbugs/excludesFilter.xml
@@ -0,0 +1,10 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<FindBugsFilter
+        xmlns="https://github.com/spotbugs/filter/3.0.0"
+        xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+        xsi:schemaLocation="https://github.com/spotbugs/filter/3.0.0 https://raw.githubusercontent.com/spotbugs/spotbugs/4.7.3/spotbugs/etc/findbugsfilter.xsd">
+<Match>
+  <!-- False positive, https://github.com/spotbugs/spotbugs/issues/1219 -->
+  <Bug pattern="BC_UNCONFIRMED_CAST_OF_RETURN_VALUE" />
+</Match>
+</FindBugsFilter>
diff --git a/src/test/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateUtilsTest.java b/src/test/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateUtilsTest.java
@@ -540,10 +540,6 @@ public void shouldCombineAllMounts() {
         assertThat(result.getVolumes(), containsInAnyOrder(hostPathVolume1, hostPathVolume2, hostPathVolume3, hostPathVolume4));
     }
 
-    private SpecNested<PodBuilder> podBuilder() {
-        return new PodBuilder().withNewMetadata().endMetadata().withNewSpec();
-    }
-
     private ContainerBuilder containerBuilder() {
         Map<String, Quantity> limitMap = new HashMap<>();
         limitMap.put("cpu", new Quantity());
@@ -567,9 +563,9 @@ public void shouldCombineAllPodMounts() {
         VolumeMount vm4 = new VolumeMountBuilder().withMountPath("/host/mnt1").withName("volume-4").withReadOnly(false)
                 .build();
         Container container1 = containerBuilder().withName("jnlp").withVolumeMounts(vm1, vm2).build();
-        Pod pod1 = podBuilder().withContainers(container1).endSpec().build();
+        Pod pod1 = new PodBuilder().withNewMetadata().endMetadata().withNewSpec().withContainers(container1).endSpec().build();
         Container container2 = containerBuilder().withName("jnlp").withVolumeMounts(vm3, vm4).build();
-        Pod pod2 = podBuilder().withContainers(container2).endSpec().build();
+        Pod pod2 = new PodBuilder().withNewMetadata().endMetadata().withNewSpec().withContainers(container2).endSpec().build();
 
         Pod result = combine(pod1, pod2);
         List<Container> containers = result.getSpec().getContainers();
@@ -648,12 +644,12 @@ public void shouldCombineAllResources() {
     public void shouldCombineContainersInOrder() {
         Container container1 = containerBuilder().withName("mysql").build();
         Container container2 = containerBuilder().withName("jnlp").build();
-        Pod pod1 = podBuilder().withContainers(container1, container2).endSpec().build();
+        Pod pod1 = new PodBuilder().withNewMetadata().endMetadata().withNewSpec().withContainers(container1, container2).endSpec().build();
 
         Container container3 = containerBuilder().withName("alpine").build();
         Container container4 = containerBuilder().withName("node").build();
         Container container5 = containerBuilder().withName("mvn").build();
-        Pod pod2 = podBuilder().withContainers(container3, container4, container5).endSpec().build();
+        Pod pod2 = new PodBuilder().withNewMetadata().endMetadata().withNewSpec().withContainers(container3, container4, container5).endSpec().build();
 
         Pod result = combine(pod1, pod2);
         assertEquals(Arrays.asList("mysql", "jnlp", "alpine", "node", "mvn"), result.getSpec().getContainers().stream().map(Container::getName).collect(Collectors.toList()));

diff --git a/src/test/java/org/csanchez/jenkins/plugins/kubernetes/pod/retention/ReaperTest.java b/src/test/java/org/csanchez/jenkins/plugins/kubernetes/pod/retention/ReaperTest.java
@@ -24,10 +24,13 @@
 package org.csanchez.jenkins.plugins.kubernetes.pod.retention;
 
 
+import static org.awaitility.Awaitility.await;
 import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.*;
 import static org.junit.Assert.*;
 import static org.mockito.Mockito.*;
 
+import io.fabric8.kubernetes.client.utils.Utils;
 import java.io.IOException;
 import java.net.HttpURLConnection;
 import java.util.LinkedList;
@@ -451,9 +454,9 @@ public void testCloseWatchersOnShutdown() throws InterruptedException {
         new Reaper.ReaperShutdownListener().onBeforeShutdown();
 
         // watchers removed
-        assertFalse(r.isWatchingCloud(cloud.name));
-        assertFalse(r.isWatchingCloud(cloud2.name));
-        assertFalse(r.isWatchingCloud(cloud3.name));
+        await().until(() -> r.isWatchingCloud(cloud.name), is(false));
+        await().until(() -> r.isWatchingCloud(cloud2.name), is(false));
+        await().until(() -> r.isWatchingCloud(cloud3.name), is(false));
     }
 
     @Test(timeout = 10_000)
@@ -513,6 +516,15 @@ public void testTerminateAgentOnContainerTerminated() throws IOException, Interr
         .always();
         // don't remove pod on activate
         server.expect().withPath("/api/v1/namespaces/foo/pods/node-123").andReturn(200, node123).once();
+        // Get logs
+        server.expect()
+                .withPath("/api/v1/namespaces/foo/pods?fieldSelector=" + Utils.toUrlEncoded("metadata.name=node-123"))
+                .andReturn(200, new PodListBuilder().withNewMetadata().endMetadata().withItems(node123).build())
+                .always();
+        server.expect()
+                .withPath("/api/v1/namespaces/foo/pods/node-123/log?pretty=false&tailLines=30")
+                .andReturn(200, "some log")
+                .always();
 
         // activate reaper
         Reaper r = Reaper.getInstance();
@@ -521,13 +533,8 @@ public void testTerminateAgentOnContainerTerminated() throws IOException, Interr
         // verify node is still registered
         assertEquals("jenkins nodes", j.jenkins.getNodes().size(), 1);
 
-        // wait for the delete event to be processed
-        waitForKubeClientRequests(6)
-                .assertRequestCountAtLeast(watchPodsPath, 3);
-
         // verify listener got notified
-        listener.waitForEvents()
-                .expectEvent(Watcher.Action.MODIFIED, node);
+        listener.waitForEvents().expectEvent(Watcher.Action.MODIFIED, node);
 
         // expect node to be terminated
         verify(node, atLeastOnce()).terminate();
@@ -628,6 +635,11 @@ private Pod withContainerImagePullBackoff(Pod pod) {
     }
 
     private Pod withContainerStatusTerminated(Pod pod) {
+        PodStatus podStatus = pod.getStatus();
+        podStatus.getConditions().add(new PodConditionBuilder()
+                .withType("Ready")
+                .withStatus("True")
+                .build());
         ContainerStatus status = new ContainerStatusBuilder()
                 .withNewState()
                 .withNewTerminated()
@@ -637,7 +649,7 @@ private Pod withContainerStatusTerminated(Pod pod) {
                 .endTerminated()
                 .endState()
                 .build();
-        pod.getStatus().getContainerStatuses().add(status);
+        podStatus.getContainerStatuses().add(status);
         return pod;
     }
 
@@ -745,7 +757,7 @@ CapturedRequests assertRequestCount(String path, long count) {
         }
 
         CapturedRequests assertRequestCountAtLeast(String path, long count) {
-            assertThat(path + " count at least", countByPath.getOrDefault(path, 0L), Matchers.greaterThanOrEqualTo(count));
+            assertThat(path + " count at least", countByPath.getOrDefault(path, 0L), greaterThanOrEqualTo(count));
             return this;
         }
     }