feat(#4958): Supporting node selectors for the builder pod #4968

lsergio · 2023-12-05T16:43:10Z

This is PR is to support specifying node selectors for the builder pods by adding a nodeSelector parameter to the Builder Trait and Build custom resources.

This my first attempt to submit a PR to the camel-k code base, so there's a high chance I'm missing something.

Release Note

feature: support for nodeSelector on Integration Builder Traits

squakez · 2023-12-05T17:15:48Z

Cool stuff. Thanks for contributing. I'll give a deeper look on Thursday. In the while you need to run make generate to regen CRDs and trait documentation and it could be good to add some extra documentation to explain how the trait has to be specified with a simple example. You can add some code after this line

camel-k/docs/modules/traits/pages/builder.adoc

Line 103 in 5aed911

// End of autogenerated code - DO NOT EDIT! (configuration)

and follow the example we provide for instance in other traits, ie,

camel-k/docs/modules/traits/pages/pod.adoc

Line 49 in 5aed911

== Sidecar containers

Feel free to have a look to unit test as well. It would be good to cover a basic scenario, here a reference as well

camel-k/pkg/trait/builder_test.go

Line 290 in 5aed911

func TestInvalidMavenProfilesBuilderTrait(t *testing.T) {

I just saw the build failed because we have many pending PRs running simultaneously. @christophd or @claudio4j please, re-trigger the checks tomorrow when there is no high PR traffic.

github-actions · 2023-12-05T17:26:00Z

🐫 Thank you for contributing! 🐫

Unable to create Coverage Report ⚠️.
Merge conflicts found.

lsergio · 2023-12-05T18:04:13Z

I added the generated code from make generate. I'll try and work on unit tests and doc tomorrow.

squakez

Excellent work indeed. Thanks for taking care of it. Just one remark in order to maintain the actual design. Builder tasks configuration is defined at [1]. Ideally the NodeSelector configuration should be added here instead of the Environment. In the builder trait, you will be able to get the value passed from the user into the specific builder task configuration, see [2]. Finally, when you're setting up the Pod, you should be able to use those values for the correct NodeSelector, similar to what we're doing with container resources, see [3].

[1]

camel-k/pkg/apis/camel/v1/common_types.go

Line 39 in 70bd9e4

type BuildConfiguration struct {

[2]

camel-k/pkg/trait/builder.go

Line 172 in 70bd9e4

builderTask, err := t.builderTask(e, taskConfOrDefault(tasksConf, "builder"))

[3]

camel-k/pkg/controller/build/build_pod.go

Line 194 in 70bd9e4

    
           func configureResources(taskName string, build *v1.Build, container *corev1.Container) {

squakez · 2023-12-07T07:58:16Z

docs/modules/traits/pages/builder.adoc

+
+== Node Selectors
+
+With this trait you will also be able to define node selectors for the `builder`` pod when using the `pod`` build strategy.


Suggested change

With this trait you will also be able to define node selectors for the `builder`` pod when using the `pod`` build strategy.

With this trait you will also be able to define node selectors for the `builder` pod when using the `pod` build strategy.

squakez · 2023-12-07T08:02:21Z

docs/modules/traits/pages/builder.adoc

+        size: large
+----
+
+The `builder` pod will be created with a node selector that allows it to run only on nodes where the `size`` label is equal to `large`.`


Suggested change

The `builder` pod will be created with a node selector that allows it to run only on nodes where the `size`` label is equal to `large`.`

The `builder` pod will be created with a node selector that allows it to run only on nodes where the `size` label is equal to `large`.

docs/modules/traits/pages/builder.adoc

claudio4j · 2023-12-07T14:00:12Z

pkg/apis/camel/v1/trait/builder.go

@@ -66,4 +66,6 @@ type BuilderTrait struct {
 	TasksLimitCPU []string `property:"tasks-limit-cpu" json:"tasksLimitCPU,omitempty"`
 	// A list of limit memory configuration for the specific task with format `<task-name>:<limit-memory-conf>`.
 	TasksLimitMemory []string `property:"tasks-limit-memory" json:"tasksLimitMemory,omitempty"`
+	// Defines a set of nodes the builder pod is eligible to be scheduled on, based on labels on the node.
+	NodeSelector map[string]string `json:"nodeSelector,omitempty"`


It's missing the property:"node-selector"

…ation

lsergio · 2023-12-07T15:27:24Z

After my latest changes, I tested the code applying this CR to my cluster:

apiVersion: camel.apache.org/v1
kind: Integration
metadata:
  name: test
spec:
  sources:
  - name: main.groovy
    content: |-
      rest("/test-mode")
          .post()
          .to("direct:start")

      from("direct:start").to("log:info")
  traits:
    quarkus:
      buildMode:
      - "native"
    builder:
      nodeSelector:
        size: large

and this is the builder pod:

apiVersion: v1
kind: Pod
metadata:
  creationTimestamp: "2023-12-07T15:23:06Z"
  labels:
    camel.apache.org/build: kit-clou6miqjnf7ksc5tj6g
    camel.apache.org/component: builder
    camel.apache.org/created.by.kind: Integration
    camel.apache.org/created.by.name: test
    camel.apache.org/created.by.namespace: sensedia
    camel.apache.org/created.by.version: "1028014"
  name: camel-k-kit-clou6miqjnf7ksc5tj6g-builder
  namespace: sensedia
  ownerReferences:
  - apiVersion: camel.apache.org/v1
    blockOwnerDeletion: true
    controller: true
    kind: Build
    name: kit-clou6miqjnf7ksc5tj6g
    uid: 1f7dfb6c-48c4-492b-ab9e-2dbb1408d44e
  resourceVersion: "1028038"
  uid: 45b52b58-671f-4247-9f92-89d553e7a5ff
spec:
  containers:
  - command:
    - kamel
    - builder
    - --namespace
    - sensedia
    - --build-name
    - kit-clou6miqjnf7ksc5tj6g
    - --task-name
    - spectrum
    env:
    - name: HOME
      value: /builder/kit-clou6miqjnf7ksc5tj6g
    image: docker.io/apache/camel-k:2.2.0-SNAPSHOT
    imagePullPolicy: IfNotPresent
    name: spectrum
    resources: {}
    terminationMessagePath: /dev/termination-log
    terminationMessagePolicy: File
    volumeMounts:
    - mountPath: /builder/kit-clou6miqjnf7ksc5tj6g
      name: camel-k-builder
    - mountPath: /var/run/secrets/kubernetes.io/serviceaccount
      name: kube-api-access-dv2xz
      readOnly: true
    workingDir: /builder/kit-clou6miqjnf7ksc5tj6g
  dnsPolicy: ClusterFirst
  enableServiceLinks: true
  initContainers:
  - command:
    - kamel
    - builder
    - --namespace
    - sensedia
    - --build-name
    - kit-clou6miqjnf7ksc5tj6g
    - --task-name
    - builder
    env:
    - name: HOME
      value: /builder/kit-clou6miqjnf7ksc5tj6g
    image: docker.io/apache/camel-k:2.2.0-SNAPSHOT
    imagePullPolicy: IfNotPresent
    name: builder
    resources: {}
    terminationMessagePath: /dev/termination-log
    terminationMessagePolicy: File
    volumeMounts:
    - mountPath: /builder/kit-clou6miqjnf7ksc5tj6g
      name: camel-k-builder
    - mountPath: /var/run/secrets/kubernetes.io/serviceaccount
      name: kube-api-access-dv2xz
      readOnly: true
    workingDir: /builder/kit-clou6miqjnf7ksc5tj6g
  - command:
    - /bin/bash
    - -c
    - cd maven && ./mvnw $(cat MAVEN_CONTEXT) package -Dquarkus.package.type=native
      -Dmaven.repo.local=./repo
    image: quay.io/quarkus/ubi-quarkus-mandrel-builder-image:23.0-jdk-17
    imagePullPolicy: IfNotPresent
    name: quarkus-native
    resources:
      requests:
        cpu: "1"
        memory: 4Gi
    terminationMessagePath: /dev/termination-log
    terminationMessagePolicy: File
    volumeMounts:
    - mountPath: /builder/kit-clou6miqjnf7ksc5tj6g
      name: camel-k-builder
    - mountPath: /var/run/secrets/kubernetes.io/serviceaccount
      name: kube-api-access-dv2xz
      readOnly: true
    workingDir: /builder/kit-clou6miqjnf7ksc5tj6g
  - command:
    - kamel
    - builder
    - --namespace
    - sensedia
    - --build-name
    - kit-clou6miqjnf7ksc5tj6g
    - --task-name
    - package
    env:
    - name: HOME
      value: /builder/kit-clou6miqjnf7ksc5tj6g
    image: docker.io/apache/camel-k:2.2.0-SNAPSHOT
    imagePullPolicy: IfNotPresent
    name: package
    resources: {}
    terminationMessagePath: /dev/termination-log
    terminationMessagePolicy: File
    volumeMounts:
    - mountPath: /builder/kit-clou6miqjnf7ksc5tj6g
      name: camel-k-builder
    - mountPath: /var/run/secrets/kubernetes.io/serviceaccount
      name: kube-api-access-dv2xz
      readOnly: true
    workingDir: /builder/kit-clou6miqjnf7ksc5tj6g
  nodeSelector:
    size: large
  preemptionPolicy: PreemptLowerPriority
  priority: 0
  restartPolicy: Never
  schedulerName: default-scheduler
  securityContext:
    fsGroup: 1001
    runAsGroup: 1001
    runAsUser: 1001
  serviceAccount: camel-k-builder
  serviceAccountName: camel-k-builder
  terminationGracePeriodSeconds: 30
  tolerations:
  - effect: NoExecute
    key: node.kubernetes.io/not-ready
    operator: Exists
    tolerationSeconds: 300
  - effect: NoExecute
    key: node.kubernetes.io/unreachable
    operator: Exists
    tolerationSeconds: 300
  volumes:
  - emptyDir: {}
    name: camel-k-builder
  - name: kube-api-access-dv2xz
    projected:
      defaultMode: 420
      sources:
      - serviceAccountToken:
          expirationSeconds: 3607
          path: token
      - configMap:
          items:
          - key: ca.crt
            path: ca.crt
          name: kube-root-ca.crt
      - downwardAPI:
          items:
          - fieldRef:
              apiVersion: v1
              fieldPath: metadata.namespace
            path: namespace
status:
  conditions:
  - lastProbeTime: null
    lastTransitionTime: "2023-12-07T15:23:06Z"
    message: '0/3 nodes are available: 3 node(s) didn''t match Pod''s node affinity/selector.
      preemption: 0/3 nodes are available: 3 Preemption is not helpful for scheduling..'
    reason: Unschedulable
    status: "False"
    type: PodScheduled
  phase: Pending
  qosClass: Burstable

The nodeSelector was generated as expected

claudio4j · 2023-12-07T15:40:00Z

I tried to run with an integration with label that didn't exist on any node -t builder.node-selector.test=null and the build run in the camel-k-operator pod as is in strategy: routine, not in a camel-k-kit pod.
In name of simplicity I am happy to have this feature as it is, but it would be good to add to the builder trait doc that if the nodeSelector label it not set in any node, then the build will run in the camel-k-operator pod.

lsergio · 2023-12-07T15:45:52Z

@claudio4j In my test none of the nodes has the size=large label. As my Integration requests a native build, it spawned a new pod.
With a regular jvm build, I changed my Integration platform buildConfiguration strategy to pod and it spawned a builder pod as well.
With routine strategy and jvm build, the nodeSelector configuration has no effect.

The builder pod status also shows that there are no available nodes:

    message: '0/3 nodes are available: 3 node(s) didn''t match Pod''s node affinity/selector.
      preemption: 0/3 nodes are available: 3 Preemption is not helpful for scheduling..'

claudio4j · 2023-12-07T16:19:34Z

I am using minikube with kvm2 driver, there is the standard single node, named "minikube". The build strategy is pod and this is the cli: kamel run teste2.yaml -t builder.node-selector.teste=nul
I will have a look later why there is not a builder pod.

squakez

Excellent work, thanks!

squakez · 2023-12-07T16:38:41Z

docs/modules/traits/pages/builder.adoc

+
+[source,console]
+----
+$ kamel run --trait builder.node-selector.size=large integration.groovy


I think this is something like builder.node-selector.'size'=large, is it?

I tested it exactly like this and it worked. I think quotes are not required as there are no spaces in the label.

yeah, possibly in this case it works because "size" is a simple string. However, it may fail when using more complex annotations, so, better to be safe and keep the notation we're using for a map in the rest of the project.

squakez · 2023-12-07T16:42:47Z

I am using minikube with kvm2 driver, there is the standard single node, named "minikube". The build strategy is pod and this is the cli: kamel run teste2.yaml -t builder.node-selector.teste=nul I will have a look later why there is not a builder pod.

Maybe it's because the notation from CLI has to be builder.node-selector.'size'=large - please, give it a try.

squakez · 2023-12-07T16:55:14Z

Many thanks for the contribution @lsergio !!

lsergio · 2023-12-07T16:57:51Z

Thanks for the quick feedback, @squakez . This feature will be very helpful in our project.
We'll test the 2.2 release as soon as it's available.

squakez added the kind/feature New feature or request label Dec 5, 2023

squakez linked an issue Dec 5, 2023 that may be closed by this pull request

Ability to apply node selectors/affinity configuration #4958

Closed

squakez added this to the 2.2.0 milestone Dec 5, 2023

lsergio marked this pull request as ready for review December 6, 2023 12:53

lsergio added 5 commits December 6, 2023 10:11

feat(#4958): Supporting node selectors for the builder pod

d8b2593

feat(#4958): adding nodeSelector example to the builder trait

ea119a1

feat(#4958): adding unit tests

77df16e

feat(#4958): minor fix in docs

a3c4a9f

feat(#4598): minor fix in docs

3b9fb1b

squakez reviewed Dec 7, 2023

View reviewed changes

feat(#4598): fix doc formatting

826c9a0

claudio4j reviewed Dec 7, 2023

View reviewed changes

lsergio added 5 commits December 7, 2023 11:03

feat(#4598): using BuildConfiguration to handle nodeSelector configur…

912a970

…ation

feat(#4598): added cli example for the builder trait node selector

4f57281

feat(#4598): added property tag to BuilderTrait

ece93f8

feat(#4598): updating generated code

5c5089c

feat(#4598): updating generated code

cc56472

lsergio marked this pull request as draft December 7, 2023 15:10

lsergio marked this pull request as ready for review December 7, 2023 15:24

squakez approved these changes Dec 7, 2023

View reviewed changes

chore(doc): minor cli notation

29b5d2a

squakez merged commit ed869f7 into apache:main Dec 7, 2023
14 of 16 checks passed

BrewTestBot mentioned this pull request Jan 8, 2024

kamel 2.2.0 Homebrew/homebrew-core#159288

Merged

squakez mentioned this pull request Apr 12, 2024

Ability to define K8s toleration for Builder pod #5354

Open

vkrejcirik mentioned this pull request Jun 25, 2024

feat(#5354): Allow adding toleration into builder pod #5667

Closed

lsergio mentioned this pull request Sep 10, 2024

BuilderTrait nodeSelector configuration not working for jvm builds #5838

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(#4958): Supporting node selectors for the builder pod #4968

feat(#4958): Supporting node selectors for the builder pod #4968

lsergio commented Dec 5, 2023

squakez commented Dec 5, 2023

github-actions bot commented Dec 5, 2023

lsergio commented Dec 5, 2023

squakez left a comment

squakez Dec 7, 2023

squakez Dec 7, 2023

claudio4j Dec 7, 2023

lsergio commented Dec 7, 2023

claudio4j commented Dec 7, 2023

lsergio commented Dec 7, 2023 •

edited

Loading

claudio4j commented Dec 7, 2023

squakez left a comment

squakez Dec 7, 2023

lsergio Dec 7, 2023 •

edited

Loading

squakez Dec 7, 2023

squakez commented Dec 7, 2023

squakez commented Dec 7, 2023

lsergio commented Dec 7, 2023 •

edited

Loading


		== Node Selectors

		With this trait you will also be able to define node selectors for the `builder`` pod when using the `pod`` build strategy.

	The `builder` pod will be created with a node selector that allows it to run only on nodes where the `size`` label is equal to `large`.`
	The `builder` pod will be created with a node selector that allows it to run only on nodes where the `size` label is equal to `large`.

feat(#4958): Supporting node selectors for the builder pod #4968

feat(#4958): Supporting node selectors for the builder pod #4968

Conversation

lsergio commented Dec 5, 2023

squakez commented Dec 5, 2023

github-actions bot commented Dec 5, 2023

lsergio commented Dec 5, 2023

squakez left a comment

Choose a reason for hiding this comment

squakez Dec 7, 2023

Choose a reason for hiding this comment

squakez Dec 7, 2023

Choose a reason for hiding this comment

claudio4j Dec 7, 2023

Choose a reason for hiding this comment

lsergio commented Dec 7, 2023

claudio4j commented Dec 7, 2023

lsergio commented Dec 7, 2023 • edited Loading

claudio4j commented Dec 7, 2023

squakez left a comment

Choose a reason for hiding this comment

squakez Dec 7, 2023

Choose a reason for hiding this comment

lsergio Dec 7, 2023 • edited Loading

Choose a reason for hiding this comment

squakez Dec 7, 2023

Choose a reason for hiding this comment

squakez commented Dec 7, 2023

squakez commented Dec 7, 2023

lsergio commented Dec 7, 2023 • edited Loading

lsergio commented Dec 7, 2023 •

edited

Loading

lsergio Dec 7, 2023 •

edited

Loading

lsergio commented Dec 7, 2023 •

edited

Loading