Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade kubernetes-client-api to 6.8.1 #1342

Merged
merged 21 commits into from
Aug 29, 2023
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 15 additions & 6 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -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-220.v1f66736f9a_42</version>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
Expand All @@ -79,7 +81,8 @@
<dependency>
<groupId>org.jenkinsci.plugins</groupId>
<artifactId>kubernetes-credentials</artifactId>
<version>0.10.0</version>
<!-- TODO https://github.com/jenkinsci/kubernetes-credentials-plugin/pull/39 @ f62dc9ee13617d84ec899274ed897668a4703242 -->
<version>0.10.1-rc150.f62dc9ee1361</version>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
Expand Down Expand Up @@ -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>
Expand All @@ -254,8 +257,8 @@
<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>
Expand All @@ -264,6 +267,12 @@
<artifactId>mockwebserver</artifactId>
<version>4.9.3</version><!-- should be aligned with the version provided by okhttp-api-plugin -->
</dependency>
<dependency>
<groupId>io.jenkins.plugins</groupId>
<artifactId>snakeyaml-api</artifactId>
<!-- TODO https://github.com/jenkinsci/snakeyaml-api-plugin/pull/94 @ 4960f40749788fcfe6ddc7def8e0bbc0ce937046-->
<version>2.1-110.v4960f4074978</version>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

</dependency>
</dependencies>
</dependencyManagement>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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());
}
Expand All @@ -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()));
Expand Down Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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());
Expand All @@ -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) //
Expand Down Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,7 @@ public void eventReceived(Action action, Pod pod) {
*/
void stop() {
if (watch != null) {
LOGGER.info("Stopping watch for kubernetes cloud " + cloudName);
this.watch.close();
}
}
Expand Down
10 changes: 10 additions & 0 deletions src/spotbugs/excludesFilter.xml
Original file line number Diff line number Diff line change
@@ -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>
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand All @@ -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();
Expand Down Expand Up @@ -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()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
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;
Expand Down Expand Up @@ -513,6 +514,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();
Expand All @@ -521,13 +531,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();
Expand Down Expand Up @@ -628,6 +633,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()
Expand All @@ -637,7 +647,7 @@ private Pod withContainerStatusTerminated(Pod pod) {
.endTerminated()
.endState()
.build();
pod.getStatus().getContainerStatuses().add(status);
podStatus.getContainerStatuses().add(status);
return pod;
}

Expand Down