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

feat: add tolerations handling to kubernetes driver #1045

Merged
merged 1 commit into from
Apr 7, 2022

Conversation

szeber
Copy link

@szeber szeber commented Apr 5, 2022

This PR is to add support for Kubernetes tolerations. Should support all current tolerations options and adding multiple tolerations. This should cover the use cases listed in #377, but it doesn't add the full manifests support.

Adding full support to manifests would be a much more flexible option, but this should solve the main pain points listed in that ticket.

@tonistiigi
Copy link
Member

@szeber missing DCO

@@ -140,6 +140,7 @@ Passes additional driver-specific options. Details for each driver:
- `limits.cpu` - Sets the limit CPU value specified in units of Kubernetes CPU. Example `limits.cpu=100m`, `limits.cpu=2`
- `limits.memory` - Sets the limit memory value specified in bytes or with a valid suffix. Example `limits.memory=500Mi`, `limits.memory=4G`
- `nodeselector="label1=value1,label2=value2"` - Sets the kv of `Pod` nodeSelector. No Defaults. Example `nodeselector=kubernetes.io/arch=arm64`
- `tolerations="key=foo,value=bar;key2=foo2,operator=exists"` - Sets the `Pod` tolerations. Accepts the same values as the kube manifest tolerations. Key-value pairs are separated by `,`, tolerations are separated by `;`. No Defaults. Example `tolerations=operator=exists`
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we have effect examples?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added an example there. Would it make sense to add a link here to the relevant kube docs? https://kubernetes.io/docs/concepts/scheduling-eviction/taint-and-toleration/

@szeber
Copy link
Author

szeber commented Apr 6, 2022

Thanks @AkihiroSuda @tonistiigi, updated, could you please recheck?

@AkihiroSuda
Copy link
Collaborator

CI failing

#13 98.37 driver/kubernetes/factory.go:5: File is not `goimports`-ed (goimports)
#13 98.37 	corev1 "k8s.io/api/core/v1"
#13 98.37 driver/kubernetes/factory.go:[125](https://github.com/docker/buildx/runs/5842873028?check_suite_focus=true#step:4:125):11: if block ends with a return statement, so drop this else and outdent its block (move short variable declaration to its own line if necessary) (golint)
#13 98.37 			} else {
#13 98.37 			       ^
#13 98.37 driver/kubernetes/factory.go:152:15: if block ends with a return statement, so drop this else and outdent its block (move short variable declaration to its own line if necessary) (golint)
#13 98.37 							} else {
#13 98.37 							       ^
#13 ERROR: process "/bin/sh -c golangci-lint run" did not complete successfully: exit code: 1
------
 > [stage-0 5/6] RUN --mount=target=/go/src/github.com/docker/buildx --mount=target=/root/.cache,type=cache   golangci-lint run:
#13 98.37 driver/kubernetes/factory.go:5: File is not `goimports`-ed (goimports)
#13 98.37 	corev1 "k8s.io/api/core/v1"
#13 98.37 driver/kubernetes/factory.go:125:11: if block ends with a return statement, so drop this else and outdent its block (move short variable declaration to its own line if necessary) (golint)
#13 98.37 			} else {
#13 98.37 			       ^
#13 98.37 driver/kubernetes/factory.go:152:15: if block ends with a return statement, so drop this else and outdent its block (move short variable declaration to its own line if necessary) (golint)
#13 98.37 							} else {
#13 98.37 							       ^
------

@szeber
Copy link
Author

szeber commented Apr 6, 2022

@AkihiroSuda hopefully this fixes the lint

@szeber
Copy link
Author

szeber commented Apr 6, 2022

@AkihiroSuda hopefully the last try, sorry :)

@tonistiigi tonistiigi merged commit a60150c into docker:master Apr 7, 2022
@ktaborski
Copy link

@szeber thanks for creating this change. I tried to use it in my env, but for some reason it's not working:

$ echo FROM scratch > Dockerfile.dummy
$ buildx version
github.com/docker/buildx a60150c a60150cbc65687c7cc921d519a2f35758202edb6
$ buildx create --name test_1 --driver kubernetes --driver-opt image=moby/buildkit:master,namespace=jenkins,requests.cpu=500m,requests.memory=300Mi,tolerations="key=spot,operator=exists;key=dedicated,value=jenkins" --use
test_1
$ buildx build . -f Dockerfile.dummy
[+] Building 0.0s (0/0)                                                                                                                                                                                                                      
Error: no valid drivers found: invalid driver option operator for driver kubernetes
<help message>

$ buildx create --name test_2 --driver kubernetes --driver-opt image=moby/buildkit:master,namespace=jenkins,requests.cpu=500m,requests.memory=300Mi --use
test_2
$  buildx build . -f Dockerfile.dummy
WARNING: No output specified for kubernetes driver. Build result will only remain in the build cache. To push result image into registry use --push or to load image into docker use --load
[+] Building 5.2s (3/3) FINISHED                                                                                                                                                                                                             
 => [internal] booting buildkit                                                                                                                                                                                                         4.3s
 => => waiting for 1 pods to be ready                                                                                                                                                                                                   4.1s
 => [internal] load build definition from Dockerfile.dummy                                                                                                                                                                              0.2s
 => => transferring dockerfile: 56B                                                                                                                                                                                                     0.1s
 => [internal] load .dockerignore                                                                                                                                                                                                       0.2s
 => => transferring context: 2B
 
 <It works>

$ buildx create --name test_3 --driver kubernetes --driver-opt tolerations="key=spot,operator=exists;key=dedicated,value=jenkins" --use
test_3
$ buildx build . -f Dockerfile.dummy
[+] Building 0.0s (0/0)                                                                                                                                                                                                                      
Error: no valid drivers found: invalid driver option operator for driver kubernetes
<help message>

Can you suggest me, what I am doing wrong?

1 similar comment
@ktaborski
Copy link

@szeber thanks for creating this change. I tried to use it in my env, but for some reason it's not working:

$ echo FROM scratch > Dockerfile.dummy
$ buildx version
github.com/docker/buildx a60150c a60150cbc65687c7cc921d519a2f35758202edb6
$ buildx create --name test_1 --driver kubernetes --driver-opt image=moby/buildkit:master,namespace=jenkins,requests.cpu=500m,requests.memory=300Mi,tolerations="key=spot,operator=exists;key=dedicated,value=jenkins" --use
test_1
$ buildx build . -f Dockerfile.dummy
[+] Building 0.0s (0/0)                                                                                                                                                                                                                      
Error: no valid drivers found: invalid driver option operator for driver kubernetes
<help message>

$ buildx create --name test_2 --driver kubernetes --driver-opt image=moby/buildkit:master,namespace=jenkins,requests.cpu=500m,requests.memory=300Mi --use
test_2
$  buildx build . -f Dockerfile.dummy
WARNING: No output specified for kubernetes driver. Build result will only remain in the build cache. To push result image into registry use --push or to load image into docker use --load
[+] Building 5.2s (3/3) FINISHED                                                                                                                                                                                                             
 => [internal] booting buildkit                                                                                                                                                                                                         4.3s
 => => waiting for 1 pods to be ready                                                                                                                                                                                                   4.1s
 => [internal] load build definition from Dockerfile.dummy                                                                                                                                                                              0.2s
 => => transferring dockerfile: 56B                                                                                                                                                                                                     0.1s
 => [internal] load .dockerignore                                                                                                                                                                                                       0.2s
 => => transferring context: 2B
 
 <It works>

$ buildx create --name test_3 --driver kubernetes --driver-opt tolerations="key=spot,operator=exists;key=dedicated,value=jenkins" --use
test_3
$ buildx build . -f Dockerfile.dummy
[+] Building 0.0s (0/0)                                                                                                                                                                                                                      
Error: no valid drivers found: invalid driver option operator for driver kubernetes
<help message>

Can you suggest me, what I am doing wrong?

@szeber
Copy link
Author

szeber commented Apr 8, 2022

@ktaborski I don't think you did anything wrong, there is a bug in how the options are getting parsed.

@AkihiroSuda by the looks of it both this and nodeselector is broken when , is present in the argument list... I'm looking for options to solve this

@szeber
Copy link
Author

szeber commented Apr 8, 2022

So for example --nodeselector="foo=bar,foo2=bar2" is parsed incorrectly. It gets parsed to

{
"nodeselector": "foo=bar",
"foo2": "bar"
}

@tonistiigi
Copy link
Member

You need to either quote it correctly "foo=1,2",bar=3 with proper escaping for the quotes so your shell doesn't eat them.

@szeber
Copy link
Author

szeber commented Apr 8, 2022

@tonistiigi Yes, that is one part, but the CSV parser doesn't like it quoted that way. what seems to work for node selectors is --driver-opts \"nodeselector=foo=bar,foo2=bar2\"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants