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

Expose pod cfg via yaml to UI and merge tolerations #311

Merged
merged 2 commits into from Apr 23, 2018
Merged
Changes from 1 commit
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.
+63 −1
Diff settings

Always

Just for now

Copy path View file
@@ -175,7 +175,7 @@ spec:
}
```

You can use [`readFile` step](https://jenkins.io/doc/pipeline/steps/workflow-basic-steps/#code-readfile-code-read-file-from-workspace) to load the yaml from a file.
You can use [`readFile` step](https://jenkins.io/doc/pipeline/steps/workflow-basic-steps/#code-readfile-code-read-file-from-workspace) to load the yaml from a file. It is also accessible from this plugin's configuration panel in the Jenkins console.

#### Liveness Probe Usage
```groovy
@@ -45,6 +45,7 @@
import io.fabric8.kubernetes.api.model.PodFluent.SpecNested;
import io.fabric8.kubernetes.api.model.Quantity;
import io.fabric8.kubernetes.api.model.SecurityContext;
import io.fabric8.kubernetes.api.model.Toleration;
import io.fabric8.kubernetes.api.model.Volume;
import io.fabric8.kubernetes.api.model.VolumeMount;

@@ -201,6 +202,11 @@ public static Pod combine(Pod parent, Pod template) {
List<Volume> combinedVolumes = Lists.newLinkedList();
Optional.ofNullable(parent.getSpec().getVolumes()).ifPresent(combinedVolumes::addAll);
Optional.ofNullable(template.getSpec().getVolumes()).ifPresent(combinedVolumes::addAll);

// Tolerations
List<Toleration> combinedTolerations = Lists.newLinkedList();
Optional.ofNullable(parent.getSpec().getTolerations()).ifPresent(combinedTolerations::addAll);
Optional.ofNullable(template.getSpec().getTolerations()).ifPresent(combinedTolerations::addAll);

This comment has been minimized.

Copy link
@carlossg

carlossg Apr 17, 2018

This would never replace parent tolerations, always append. Is that the expected behavior ?
IIUC you can have multiple tolerations with the same key, but just checking

This comment has been minimized.

Copy link
@gabemontero

gabemontero Apr 18, 2018

Author

Yeah, per https://kubernetes.io/docs/concepts/configuration/taint-and-toleration/ you can have the same key with different contents ...several examples on that page that do that.

On whether to override or add ... for our uses cases at least, we can make some simplifying assumptions wrt the yaml field being the starting point, and most likely there won't be existing tolerations that we have to worry about overriding.

So I'm good with keeping it simple and not adding more levers around tuning the behaviour. But if you want those let me know.

This comment has been minimized.

Copy link
@gabemontero

gabemontero Apr 18, 2018

Author

Though upon re-reading the help text, it could be misleading wrt that ... I'll update it to clarify.


// WorkspaceVolume workspaceVolume = template.isCustomWorkspaceVolumeEnabled() && template.getWorkspaceVolume() != null ? template.getWorkspaceVolume() : parent.getWorkspaceVolume();

@@ -224,6 +230,7 @@ public static Pod combine(Pod parent, Pod template) {
.withServiceAccount(serviceAccount) //
.withContainers(Lists.newArrayList(combinedContainers.values())) //
.withVolumes(combinedVolumes) //
.withTolerations(combinedTolerations) //
.withImagePullSecrets(Lists.newArrayList(imagePullSecrets));

// podTemplate.setLabel(label);
@@ -94,4 +94,8 @@
</f:block>
</f:advanced>

<f:entry field="yaml" title="${%Raw yaml for the Pod}">
<f:textarea/>
</f:entry>

</j:jelly>
@@ -0,0 +1,26 @@
<!--
~ Copyright (C) 2015 Original Authors
~
~ Licensed under the Apache License, Version 2.0 (the "License");
~ you may not use this file except in compliance with the License.
~ You may obtain a copy of the License at
~
~ http://www.apache.org/licenses/LICENSE-2.0
~
~ Unless required by applicable law or agreed to in writing, software
~ distributed under the License is distributed on an "AS IS" BASIS,
~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
~ See the License for the specific language governing permissions and
~ limitations under the License.
-->

The raw yaml of a Pod API Object. Any pod fields set with non-empty values via the configuration fields above will take precedence
as they are merged with this yaml.

Fragments of a full Pod API Object yaml representation are allowed, but the yaml fragment must start with:

This comment has been minimized.

Copy link
@carlossg

This comment has been minimized.

Copy link
@gabemontero

gabemontero Apr 18, 2018

Author

Nope, you are right. Just need enough context ... i.e.

spec:
   tolerations:
      - key: CriticalAddonsOnly
        operator: Exists

works, but

   tolerations:
      - key: CriticalAddonsOnly
        operator: Exists

Does not since the code puts in a empty spec if one is not specified. I didn't try the middle ground before.

I'll update the help for this as well.

thanks


```
apiVersion: v1
kind: Pod
```

@@ -44,10 +44,13 @@

import io.fabric8.kubernetes.api.model.Container;
import io.fabric8.kubernetes.api.model.ContainerBuilder;
import io.fabric8.kubernetes.api.model.ObjectMeta;
import io.fabric8.kubernetes.api.model.Pod;
import io.fabric8.kubernetes.api.model.PodBuilder;
import io.fabric8.kubernetes.api.model.PodFluent.SpecNested;
import io.fabric8.kubernetes.api.model.PodSpec;
import io.fabric8.kubernetes.api.model.Quantity;
import io.fabric8.kubernetes.api.model.Toleration;
import io.fabric8.kubernetes.api.model.VolumeMount;

public class PodTemplateUtilsTest {
@@ -394,6 +397,28 @@ public void shouldCombineAllPodMounts() {
assertThat(containers.get(0).getVolumeMounts(), hasItems(vm2, vm3, vm4));
}

@Test
public void shouldCombineAllTolerations() {
PodSpec podSpec1 = new PodSpec();
Pod pod1 = new Pod();
Toleration toleration1 = new Toleration("effect1", "key1", "oper1", Long.parseLong("1"), "val1");
Toleration toleration2 = new Toleration("effect2", "key2", "oper2", Long.parseLong("2"), "val2");
podSpec1.setTolerations(asList(toleration1, toleration2));
pod1.setSpec(podSpec1);
pod1.setMetadata(new ObjectMeta());

PodSpec podSpec2 = new PodSpec();
Pod pod2 = new Pod();
Toleration toleration3 = new Toleration("effect3", "key3", "oper3", Long.parseLong("3"), "val3");
Toleration toleration4 = new Toleration("effect4", "key4", "oper4", Long.parseLong("4"), "val4");
podSpec2.setTolerations(asList(toleration3, toleration4));
pod2.setSpec(podSpec2);
pod2.setMetadata(new ObjectMeta());

Pod result = combine(pod1, pod2);
assertThat(result.getSpec().getTolerations(), hasItems(toleration1, toleration2, toleration3, toleration4));
}

@Test
public void shouldFilterOutNullOrEmptySecretEnvVars() {
ContainerTemplate template1 = new ContainerTemplate("name-1", "image-1");
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.