Skip to content

Commit ac0cf1c

Browse files
cronikcronik
authored
Fetch Pod reference if unassigned (#1696)
1 parent 4ec3349 commit ac0cf1c

File tree

3 files changed

+165
-9
lines changed

3 files changed

+165
-9
lines changed

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

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import com.cloudbees.plugins.credentials.domains.URIRequirementBuilder;
1111
import edu.umd.cs.findbugs.annotations.CheckForNull;
1212
import edu.umd.cs.findbugs.annotations.NonNull;
13+
import edu.umd.cs.findbugs.annotations.Nullable;
1314
import hudson.Extension;
1415
import hudson.Main;
1516
import hudson.TcpSlaveAgentListener;
@@ -34,6 +35,7 @@
3435
import io.fabric8.kubernetes.client.KubernetesClient;
3536
import io.fabric8.kubernetes.client.KubernetesClientException;
3637
import io.fabric8.kubernetes.client.VersionInfo;
38+
import io.fabric8.kubernetes.client.dsl.PodResource;
3739
import io.fabric8.kubernetes.client.informers.SharedIndexInformer;
3840
import jakarta.servlet.ServletException;
3941
import java.io.IOException;
@@ -600,6 +602,42 @@ public KubernetesClient connect() throws KubernetesAuthException, IOException {
600602
return client;
601603
}
602604

605+
/**
606+
* Get {@link PodResource} from {@link KubernetesClient}.
607+
* @param namespace namespace pod is located in, possibly null
608+
* @param name pod name, not null
609+
* @return pod resource, never null
610+
* @throws KubernetesAuthException if cluster authentication failed
611+
* @throws IOException if connection failed
612+
* @see #connect()
613+
* @see #getPodResource(KubernetesClient, String, String)
614+
*/
615+
@NonNull
616+
public PodResource getPodResource(@Nullable String namespace, @NonNull String name)
617+
throws KubernetesAuthException, IOException {
618+
return getPodResource(connect(), namespace, name);
619+
}
620+
621+
/**
622+
* Get {@link PodResource} from {@link KubernetesClient}.
623+
* @param client kubernetes client, not null
624+
* @param namespace namespace pod is located in, possibly null
625+
* @param name pod name, not null
626+
* @return pod resource, never null
627+
* @throws KubernetesAuthException if cluster authentication failed
628+
* @throws IOException if connection failed
629+
* @see #connect()
630+
*/
631+
@NonNull
632+
static PodResource getPodResource(
633+
@NonNull KubernetesClient client, @Nullable String namespace, @NonNull String name) {
634+
if (namespace == null) {
635+
return client.pods().withName(name);
636+
} else {
637+
return client.pods().inNamespace(namespace).withName(name);
638+
}
639+
}
640+
603641
@Override
604642
public Collection<NodeProvisioner.PlannedNode> provision(
605643
@NonNull final Cloud.CloudState state, final int excessWorkload) {

‎src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesSlave.java‎

Lines changed: 34 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -272,8 +272,37 @@ public Cloud getCloud() {
272272
return Jenkins.getInstance().getCloud(getCloudName());
273273
}
274274

275+
/**
276+
* Get {@link Pod} reference for this agent. If it hasn't been assigned yet
277+
* or the reference was lost due to restart, it will be looked up.
278+
* @return assigned pod, or empty if not assigned or no longer exists
279+
*/
275280
public Optional<Pod> getPod() {
276-
return Optional.ofNullable(pod);
281+
Pod p = pod;
282+
if (p == null) {
283+
// if jenkins restarts the transient pod reference may not be available
284+
try {
285+
p = getKubernetesCloud()
286+
.getPodResource(getNamespace(), getPodName())
287+
.get();
288+
if (p != null) {
289+
assignPod(p);
290+
return Optional.of(p);
291+
} else {
292+
return Optional.empty();
293+
}
294+
} catch (KubernetesAuthException | IOException | IllegalStateException e) {
295+
LOGGER.log(
296+
Level.WARNING,
297+
e,
298+
() -> String.format(
299+
"Failed to connect to cloud %s to get pod %s/%s",
300+
getCloudName(), getNamespace(), getPodName()));
301+
return Optional.empty();
302+
}
303+
} else {
304+
return Optional.of(p);
305+
}
277306
}
278307

279308
/**
@@ -372,10 +401,9 @@ protected void _terminate(TaskListener listener) throws IOException, Interrupted
372401
// Prior to termination, determine if we should delete the slave pod based on
373402
// the slave pod's current state and the pod retention policy.
374403
// Healthy slave pods should still have a JNLP agent running at this point.
375-
boolean deletePod = getPodRetention(cloud).shouldDeletePod(cloud, () -> client.pods()
376-
.inNamespace(getNamespace())
377-
.withName(name)
378-
.get());
404+
boolean deletePod = getPodRetention(cloud)
405+
.shouldDeletePod(cloud, () -> KubernetesCloud.getPodResource(client, getNamespace(), name)
406+
.get());
379407

380408
Computer computer = toComputer();
381409
if (computer == null) {
@@ -542,10 +570,7 @@ public void annotateTtl(TaskListener listener) {
542570
var l = Instant.now();
543571
try {
544572
kubernetesCloud
545-
.connect()
546-
.pods()
547-
.inNamespace(ns)
548-
.withName(name)
573+
.getPodResource(ns, name)
549574
.patch("{\"metadata\":{\"annotations\":{\"" + GarbageCollection.ANNOTATION_LAST_REFRESH
550575
+ "\":\"" + l.toEpochMilli() + "\"}}}");
551576
} catch (KubernetesAuthException e) {

‎src/test/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesSlaveTest.java‎

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,24 +24,32 @@
2424

2525
package org.csanchez.jenkins.plugins.kubernetes;
2626

27+
import static java.util.Map.entry;
2728
import static org.csanchez.jenkins.plugins.kubernetes.KubernetesTestUtil.assertRegex;
2829
import static org.junit.Assert.*;
30+
import static org.mockito.Mockito.*;
2931

3032
import hudson.model.Descriptor;
33+
import io.fabric8.kubernetes.api.model.Pod;
34+
import io.fabric8.kubernetes.client.dsl.PodResource;
3135
import java.io.IOException;
3236
import java.util.Arrays;
3337
import java.util.Collections;
3438
import java.util.List;
39+
import java.util.Map;
40+
import java.util.Optional;
3541
import org.csanchez.jenkins.plugins.kubernetes.pod.retention.Always;
3642
import org.csanchez.jenkins.plugins.kubernetes.pod.retention.Default;
3743
import org.csanchez.jenkins.plugins.kubernetes.pod.retention.Never;
3844
import org.csanchez.jenkins.plugins.kubernetes.pod.retention.OnFailure;
3945
import org.csanchez.jenkins.plugins.kubernetes.pod.retention.PodRetention;
4046
import org.csanchez.jenkins.plugins.kubernetes.volumes.PodVolume;
47+
import org.jenkinsci.plugins.kubernetes.auth.KubernetesAuthException;
4148
import org.junit.Rule;
4249
import org.junit.Test;
4350
import org.jvnet.hudson.test.JenkinsRule;
4451
import org.jvnet.hudson.test.WithoutJenkins;
52+
import org.mockito.Mockito;
4553

4654
/**
4755
* @author Carlos Sanchez
@@ -70,6 +78,91 @@ public void testGetSlaveName() {
7078
("jenkins-agent-[0-9a-z]{5}"));
7179
}
7280

81+
@Test
82+
public void testGetPod() throws Exception {
83+
Map<String, GetPodTestCase> testCases = Map.ofEntries(
84+
entry("assigned", (KubernetesCloud cloud, KubernetesSlave slave, PodResource podResource) -> {
85+
Pod p = new Pod();
86+
slave.assignPod(p);
87+
Optional<Pod> pod = slave.getPod();
88+
assertTrue(pod.isPresent());
89+
assertSame(p, pod.get());
90+
verify(cloud, never()).getPodResource(anyString(), anyString());
91+
}),
92+
entry("unassigned", (KubernetesCloud cloud, KubernetesSlave slave, PodResource podResource) -> {
93+
Pod p = new Pod();
94+
doReturn(podResource).when(cloud).getPodResource(eq("bar"), startsWith("foo-"));
95+
when(podResource.get()).thenReturn(p);
96+
97+
// test
98+
Optional<Pod> pod = slave.getPod();
99+
100+
// verify
101+
assertTrue(pod.isPresent());
102+
assertSame(p, pod.get());
103+
verify(cloud).getPodResource(eq("bar"), startsWith("foo-"));
104+
}),
105+
entry("not found", (KubernetesCloud cloud, KubernetesSlave slave, PodResource podResource) -> {
106+
doReturn(podResource).when(cloud).getPodResource(eq("bar"), startsWith("foo-"));
107+
when(podResource.get()).thenReturn(null);
108+
109+
// test
110+
Optional<Pod> pod = slave.getPod();
111+
112+
// verify
113+
assertTrue(pod.isEmpty());
114+
verify(cloud).getPodResource(eq("bar"), startsWith("foo-"));
115+
}),
116+
entry("auth failure", (KubernetesCloud cloud, KubernetesSlave slave, PodResource podResource) -> {
117+
doThrow(KubernetesAuthException.class).when(cloud).getPodResource(eq("bar"), startsWith("foo-"));
118+
119+
// test
120+
Optional<Pod> pod = slave.getPod();
121+
122+
// verify
123+
assertTrue(pod.isEmpty());
124+
verify(cloud).getPodResource(eq("bar"), startsWith("foo-"));
125+
}),
126+
entry("connect error", (KubernetesCloud cloud, KubernetesSlave slave, PodResource podResource) -> {
127+
doThrow(IOException.class).when(cloud).getPodResource(eq("bar"), startsWith("foo-"));
128+
129+
// test
130+
Optional<Pod> pod = slave.getPod();
131+
132+
// verify
133+
assertTrue(pod.isEmpty());
134+
verify(cloud).getPodResource(eq("bar"), startsWith("foo-"));
135+
}),
136+
entry("cloud not found", (KubernetesCloud cloud, KubernetesSlave slave, PodResource podResource) -> {
137+
r.jenkins.clouds.clear();
138+
139+
// test
140+
Optional<Pod> pod = slave.getPod();
141+
142+
// verify
143+
assertTrue(pod.isEmpty());
144+
verify(cloud, never()).getPodResource(eq("bar"), startsWith("foo-"));
145+
}));
146+
147+
for (Map.Entry<String, GetPodTestCase> testCase : testCases.entrySet()) {
148+
KubernetesCloud cloud = Mockito.spy(new KubernetesCloud("kube"));
149+
PodResource podResource = Mockito.mock(PodResource.class);
150+
PodTemplate podTemplate = new PodTemplate("foo", Collections.emptyList(), Collections.emptyList());
151+
KubernetesSlave slave = new KubernetesSlave.Builder()
152+
.podTemplate(podTemplate)
153+
.cloud(cloud)
154+
.build();
155+
slave.setNamespace("bar");
156+
r.jenkins.clouds.clear();
157+
r.jenkins.clouds.add(cloud);
158+
testCase.getValue().test(cloud, slave, podResource);
159+
}
160+
}
161+
162+
private interface GetPodTestCase {
163+
void test(KubernetesCloud cloud, KubernetesSlave slave, PodResource podResource) throws Exception;
164+
}
165+
73166
@Test
74167
public void testGetPodRetention() {
75168
try {

0 commit comments

Comments
 (0)