Jenkins-46519: Support for new affinities abstraction for slave templates #243

alorlea · 2017-10-31T16:32:24Z

With this feature we should be able to run the slaves on nodes based on different affinity options for a pod.

One use case, for us for example is we want to run the slaves in different kubernetes slaves without getting colocated on the kube slave where jenkins master is running.

So far I tried it out with the new NodeAffinity in a jenkins slave pod in our staging setup and it was able to attach the affinity onto the slave upon loading the template.

carlossg

Added some comments after a quick look.
Tests are currently failing and this would need some new tests

carlossg · 2017-11-10T10:45:41Z

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

@@ -584,6 +588,22 @@ protected Object readResolve() {
        return this;
    }

+    @Nonnull
+    public List<Affinity> getAffinities() {
+        if(affinities ==  null) {


can you fix the formatting generally to match the rest of the code?
default eclipse java format

carlossg · 2017-11-10T10:49:47Z

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

+                    kubernetesPodAffinity.setNodeAffinity(nodeAffinity.buildAffinity());
+                    LOGGER.log(Level.INFO, "Loading node affinity for slave!");
+                } catch (IOException e) {
+                    e.printStackTrace();


needs to do something or log to logger

carlossg · 2017-11-10T10:50:28Z

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

@@ -293,6 +312,7 @@ private Pod getPodTemplate(KubernetesSlave slave, PodTemplate template) {
                .withContainers(containers.values().toArray(new Container[containers.size()]))
                .withNodeSelector(getNodeSelectorMap(template.getNodeSelector()))
                .withRestartPolicy("Never")
+                .withAffinity(kubernetesPodAffinity)


maybe this can't be added if affinity is not set ?

I went over the code, and initially, if there is no node affinity, pod affinity or pod antiaffinity, we give an Affinity without setting anything. This is how the underying object behaviour of the Affinity object in the kubernetes fabric8 model.

alorlea · 2017-11-29T20:55:53Z

I have been struggling this last couple of weeks to get a grasp on bringing tests in place. I have already some groovy templates in place for the test cases but I am getting issues to figure out the best way to fix the test. Any recommendations on how to debug this locally? I am using minikube & have access to a k8s cluster @carlossg

alorlea · 2017-12-05T15:08:13Z

@carlossg I added the corresponding tests for this feature.

Unfortunately, it is failing one of the other tests, ContainerExecDecoratorTest.testCommandExecution with a timeout.

I am not sure why this test is failing, as the other tests in that test suite are passing. I hope it is not an incompatibility issue with me updating the kubernetesclient dependency.

adding possibility to select an affinity from the dropdownlist defining structure for affinities adding directory structure for the UI first working sample for node affinity with a JSON blob! starting to add pod affinity and anti affinity in the kubernetes launcher class Pod affinity and anti affinity support for JSON deserialization minor fixes and updating UI code

* adding warning log traces when failing to add affinities * fix code formatting Reverting build volume upgrade based on the Kubernetes client. It was missing the medium string.

one test at time Node Affinity test loads correct template fixed node & pod affinities tests fix race condition on tests fix broken asserts for mixed affinities test

mumoshu · 2017-12-26T10:18:52Z

src/test/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesTestUtil.java

+                    testingNamespace = namespaceWithBranch;
+                }
+            } catch (KubernetesClientException e) {
+                // nothing to do


@alorlea Hi. Just curious but shouldn't you just fail-fast in this case, rather than suppressing a KubernetseClientException like this?

mumoshu · 2017-12-26T10:24:57Z

@alorlea Hi, thanks for your efforts on this! Really hope this gets merged someday.

Btw, does the failure persist across runs? Just confirming but I could find nothing suspicious other than https://github.com/jenkinsci/kubernetes-plugin/pull/243/files#diff-6fac64c3dbf99f73936f276f23b4386cR81, which doesn't seem to related with the failure.

Also, is the updated kubernetes-client necessary? Hopefully you can extract the kubernetes-client bump to an another PR to isolate the cause.

carlossg · 2017-12-27T09:26:05Z

src/test/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/KubernetesAffinityTest.java

+        WorkflowRun b = p.scheduleBuild2(0).waitForStart();
+        assertNotNull(b);
+
+        Thread.sleep(1000);


why the sleeps? what is the condition that must be met ?

@carlossg I had to add the sleep, because it seemed that the template seems to take time to load. This made the test fail as it would not find it on the list of templates.

alorlea · 2018-01-03T22:46:11Z

@mumoshu Yes, it is consistent the failure with the ContainerExecDecoratorTest.testCommandExecution tests. The bump of the library as I understand is needed, due to the newer versions of Kubernetes, they added the new affinities API as part of their model. https://github.com/fabric8io/kubernetes-client#compatibility

carlossg · 2018-01-09T20:25:29Z

The update of kubernetes-client is breaking the test, see #271

carlossg · 2018-01-21T08:53:55Z

I have created #275 that will supersede this one, allowing any Pod configuration to be passed on. Otherwise adding all possible options (and options as complicated as this one) will be a maintenance nightmare.

Probably #271 will need to be fixed too

alorlea · 2018-03-25T13:16:18Z

Hi @carlossg what is the current status with the migration to use YAML kubernetes file? Is there anything we can help with #275.

alorlea · 2018-05-21T12:41:03Z

I will close this PR this corresponding functionality is now available with the new YAML abstraction

alorlea force-pushed the JENKINS-46519 branch 3 times, most recently from 9c9c7ac to 99d82cd Compare November 3, 2017 09:12

carlossg requested changes Nov 10, 2017

View reviewed changes

alorlea force-pushed the JENKINS-46519 branch from c6a53f9 to 3c4a218 Compare November 19, 2017 19:28

alorlea changed the title ~~[WIP] Jenkins-46519: Support for new affinities abstraction for slave templates~~ Jenkins-46519: Support for new affinities abstraction for slave templates Nov 28, 2017

alorlea force-pushed the JENKINS-46519 branch from 2bcff37 to 80ee0ae Compare November 28, 2017 16:01

alorlea force-pushed the JENKINS-46519 branch 4 times, most recently from 75a5e14 to 3e89d44 Compare December 5, 2017 14:34

Martin Sander and others added 7 commits December 13, 2017 10:39

archive artifact and findbugs findings

45ca3c5

[maven-release-plugin] prepare for next development iteration

bd8a7a6

Fix pipeline findbugs

b9949f0

Do not fail if missing permission to get other namespaces

b053d97

adding support for pod template step

8ecc725

* adding warning log traces when failing to add affinities * fix code formatting Reverting build volume upgrade based on the Kubernetes client. It was missing the medium string.

Adding pipeline groovy tests for node & pod affinities

f6273f8

one test at time Node Affinity test loads correct template fixed node & pod affinities tests fix race condition on tests fix broken asserts for mixed affinities test

alorlea force-pushed the JENKINS-46519 branch from 3e89d44 to f6273f8 Compare December 13, 2017 09:40

mumoshu reviewed Dec 26, 2017

View reviewed changes

carlossg added 2 commits December 27, 2017 10:18

Merge branch 'master' into alorlea-JENKINS-46519

ed660d4

Split affinity tests into their own class

19427db

carlossg reviewed Dec 27, 2017

View reviewed changes

alorlea force-pushed the JENKINS-46519 branch 2 times, most recently from 3e6c8b6 to cbfa3e4 Compare January 2, 2018 11:20

bumping client to 3.0.3

9c49b5b

alorlea force-pushed the JENKINS-46519 branch from cbfa3e4 to 9c49b5b Compare January 2, 2018 11:49

alorlea closed this May 21, 2018

alorlea deleted the JENKINS-46519 branch May 21, 2018 12:41

alorlea restored the JENKINS-46519 branch May 21, 2018 12:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Jenkins-46519: Support for new affinities abstraction for slave templates #243

Jenkins-46519: Support for new affinities abstraction for slave templates #243

alorlea commented Oct 31, 2017

carlossg left a comment

carlossg Nov 10, 2017

carlossg Nov 10, 2017

carlossg Nov 10, 2017

alorlea Nov 19, 2017

alorlea commented Nov 29, 2017

alorlea commented Dec 5, 2017

mumoshu Dec 26, 2017

mumoshu commented Dec 26, 2017

carlossg Dec 27, 2017

alorlea Jan 2, 2018 •

edited

Loading

alorlea commented Jan 3, 2018 •

edited

Loading

carlossg commented Jan 9, 2018

carlossg commented Jan 21, 2018

alorlea commented Mar 25, 2018

alorlea commented May 21, 2018

Jenkins-46519: Support for new affinities abstraction for slave templates #243

Jenkins-46519: Support for new affinities abstraction for slave templates #243

Conversation

alorlea commented Oct 31, 2017

carlossg left a comment

Choose a reason for hiding this comment

carlossg Nov 10, 2017

Choose a reason for hiding this comment

carlossg Nov 10, 2017

Choose a reason for hiding this comment

carlossg Nov 10, 2017

Choose a reason for hiding this comment

alorlea Nov 19, 2017

Choose a reason for hiding this comment

alorlea commented Nov 29, 2017

alorlea commented Dec 5, 2017

mumoshu Dec 26, 2017

Choose a reason for hiding this comment

mumoshu commented Dec 26, 2017

carlossg Dec 27, 2017

Choose a reason for hiding this comment

alorlea Jan 2, 2018 • edited Loading

Choose a reason for hiding this comment

alorlea commented Jan 3, 2018 • edited Loading

carlossg commented Jan 9, 2018

carlossg commented Jan 21, 2018

alorlea commented Mar 25, 2018

alorlea commented May 21, 2018

alorlea Jan 2, 2018 •

edited

Loading

alorlea commented Jan 3, 2018 •

edited

Loading