Add PodContainerSource Extension Point
#1693
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,68 @@ | ||
| package org.csanchez.jenkins.plugins.kubernetes; | ||
|
|
||
| import edu.umd.cs.findbugs.annotations.NonNull; | ||
| import hudson.Extension; | ||
| import hudson.ExtensionList; | ||
| import hudson.ExtensionPoint; | ||
| import io.fabric8.kubernetes.api.model.Container; | ||
| import io.fabric8.kubernetes.api.model.Pod; | ||
| import io.fabric8.kubernetes.api.model.PodSpec; | ||
| import java.util.List; | ||
| import java.util.Objects; | ||
| import java.util.Optional; | ||
|
|
||
| /** | ||
| * Pod container sources are responsible to locating details about Pod containers. | ||
| */ | ||
| public abstract class PodContainerSource implements ExtensionPoint { | ||
|
|
||
| /** | ||
| * Lookup the working directory of the named container. | ||
| * @param pod pod reference to lookup container in | ||
| * @param containerName name of container to lookup | ||
| * @return working directory path if container found and working dir specified, otherwise empty | ||
| */ | ||
| public abstract Optional<String> getContainerWorkingDir(@NonNull Pod pod, @NonNull String containerName); | ||
|
|
||
| /** | ||
| * Lookup all {@link PodContainerSource} extensions. | ||
| * @return pod container source extension list | ||
| */ | ||
| @NonNull | ||
| public static List<PodContainerSource> all() { | ||
| return ExtensionList.lookup(PodContainerSource.class); | ||
| } | ||
|
|
||
| /** | ||
| * Lookup pod container working dir. Searches all {@link PodContainerSource} extensions and returns | ||
| * the first non-empty result. | ||
| * @param pod pod to inspect | ||
| * @param containerName container to search for | ||
| * @return optional working dir if container found and working dir, possibly empty | ||
| */ | ||
| public static Optional<String> lookupContainerWorkingDir(@NonNull Pod pod, @NonNull String containerName) { | ||
| return all().stream() | ||
| .map(cs -> cs.getContainerWorkingDir(pod, containerName)) | ||
| .filter(Optional::isPresent) | ||
| .map(Optional::get) | ||
| .findFirst(); | ||
| } | ||
|
|
||
| /** | ||
| * Default implementation of {@link PodContainerSource} that only searches the primary | ||
| * pod containers. Ephemeral or init containers are not included container lookups in | ||
| * this implementation. | ||
| * @see PodSpec#getContainers() | ||
| */ | ||
| @Extension | ||
| public static final class DefaultPodContainerSource extends PodContainerSource { | ||
|
|
||
| @Override | ||
| public Optional<String> getContainerWorkingDir(@NonNull Pod pod, @NonNull String containerName) { | ||
| return pod.getSpec().getContainers().stream() | ||
| .filter(c -> Objects.equals(c.getName(), containerName)) | ||
| .findAny() | ||
| .map(Container::getWorkingDir); | ||
| } | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,96 @@ | ||
| package org.csanchez.jenkins.plugins.kubernetes; | ||
|
|
||
| import static org.junit.Assert.*; | ||
|
|
||
| import edu.umd.cs.findbugs.annotations.NonNull; | ||
| import hudson.Extension; | ||
| import io.fabric8.kubernetes.api.model.EphemeralContainer; | ||
| import io.fabric8.kubernetes.api.model.Pod; | ||
| import io.fabric8.kubernetes.api.model.PodBuilder; | ||
| import java.util.Optional; | ||
| import org.apache.commons.lang3.StringUtils; | ||
| import org.junit.Rule; | ||
| import org.junit.Test; | ||
| import org.jvnet.hudson.test.JenkinsRule; | ||
| import org.jvnet.hudson.test.WithoutJenkins; | ||
|
|
||
| public class PodContainerSourceTest { | ||
|
|
||
| @Rule | ||
| public JenkinsRule j = new JenkinsRule(); | ||
|
|
||
| @Test | ||
| public void lookupContainerWorkingDir() { | ||
| Pod pod = new PodBuilder() | ||
| .withNewSpec() | ||
| .addNewContainer() | ||
| .withName("foo") | ||
| .withWorkingDir("/app/foo") | ||
| .endContainer() | ||
| .addNewEphemeralContainer() | ||
| .withName("bar") | ||
| .withWorkingDir("/app/bar") | ||
| .endEphemeralContainer() | ||
| .addNewEphemeralContainer() | ||
| .withName("foo") | ||
| .withWorkingDir("/app/ephemeral-foo") | ||
| .endEphemeralContainer() | ||
| .endSpec() | ||
| .build(); | ||
|
|
||
| Optional<String> wd = PodContainerSource.lookupContainerWorkingDir(pod, "foo"); | ||
| assertTrue(wd.isPresent()); | ||
| assertEquals("/app/foo", wd.get()); | ||
|
|
||
| // should use TestPodContainerSource to find ephemeral container | ||
| wd = PodContainerSource.lookupContainerWorkingDir(pod, "bar"); | ||
| assertTrue(wd.isPresent()); | ||
| assertEquals("/app/bar", wd.get()); | ||
|
|
||
| // no named container | ||
| wd = PodContainerSource.lookupContainerWorkingDir(pod, "fish"); | ||
| assertFalse(wd.isPresent()); | ||
| } | ||
|
|
||
| @WithoutJenkins | ||
| @Test | ||
| public void defaultPodContainerSourceGetContainerWorkingDir() { | ||
| Pod pod = new PodBuilder() | ||
| .withNewSpec() | ||
| .addNewContainer() | ||
| .withName("foo") | ||
| .withWorkingDir("/app/foo") | ||
| .endContainer() | ||
| .addNewEphemeralContainer() | ||
| .withName("bar") | ||
| .withWorkingDir("/app/bar") | ||
| .endEphemeralContainer() | ||
| .endSpec() | ||
| .build(); | ||
|
|
||
| PodContainerSource.DefaultPodContainerSource source = new PodContainerSource.DefaultPodContainerSource(); | ||
| Optional<String> wd = source.getContainerWorkingDir(pod, "foo"); | ||
| assertTrue(wd.isPresent()); | ||
| assertEquals("/app/foo", wd.get()); | ||
|
|
||
| // should not return ephemeral container | ||
| wd = source.getContainerWorkingDir(pod, "bar"); | ||
| assertFalse(wd.isPresent()); | ||
|
|
||
| // no named container | ||
| wd = source.getContainerWorkingDir(pod, "fish"); | ||
| assertFalse(wd.isPresent()); | ||
| } | ||
|
|
||
| @Extension | ||
| public static class TestPodContainerSource extends PodContainerSource { | ||
|
|
||
| @Override | ||
| public Optional<String> getContainerWorkingDir(@NonNull Pod pod, @NonNull String containerName) { | ||
| return pod.getSpec().getEphemeralContainers().stream() | ||
|
There was a problem hiding this comment. In a real pod, |
||
| .filter(c -> StringUtils.equals(c.getName(), containerName)) | ||
| .findAny() | ||
| .map(EphemeralContainer::getWorkingDir); | ||
| } | ||
| } | ||
| } | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wondering whether we have any guarantee that a given name can't be used by both containers and ephemeral containers, as it would create an ambiguity...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, would
public abstract Optional<Container> getContainerWorkingDir(Pod, String)be more flexible?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://kubernetes.io/docs/reference/kubernetes-api/workload-resources/pod-v1/#Container
This is confirmed if you attempt to apply a spec with duplicate names. It doesn't matter if they are defined in
containers,initContainers, orephemeralContainers.Yes, I will refactor to return the
Containerinstance.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I take back what I said 😄 . While it makes sense at first, fabric8 models
ContainerandEphemeralContaineras different types. From the previous PR it sounded like the whole purpose of this extension is to isolate ephemeral container references to extension plugins, so this extension is to abstract over those differences.In a future PR I will be adding 1 more method to
PodContainerSourceto get the container status. Statuses are not properties ofContainerorEphemeralContainer, they are accesses viapod.getStatus().getContainerStatuses().These 2 methods will allow my extension to reuse
ContainerExecDecorator, as shown in this PR, and display ephemeral container logs (future PR that updatesKubernetesComputer#goContainerLog).ContainerExecDecoratordoes A LOT of complicated stuff which would be a shame to replicate and maintain separately given this small abstraction makes it reusable. Hopefully we can figure out an acceptable solution.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😞
Indeed, I don't see anything better.