From 4a226568a0694224c8d2ec56b0f2801213fa80f1 Mon Sep 17 00:00:00 2001 From: Zsolt Date: Tue, 12 Apr 2022 13:44:03 +1200 Subject: [PATCH] Fix tolerations not parsing its options correctly, add tests Signed-off-by: Zsolt --- commands/create_test.go | 26 ++++ docs/reference/buildx_create.md | 7 +- driver/kubernetes/factory.go | 233 +++++++++++++++--------------- driver/kubernetes/factory_test.go | 230 +++++++++++++++++++++++++++++ 4 files changed, 379 insertions(+), 117 deletions(-) create mode 100644 commands/create_test.go create mode 100644 driver/kubernetes/factory_test.go diff --git a/commands/create_test.go b/commands/create_test.go new file mode 100644 index 00000000..9bd57d2f --- /dev/null +++ b/commands/create_test.go @@ -0,0 +1,26 @@ +package commands + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestCsvToMap(t *testing.T) { + d := []string{ + "\"tolerations=key=foo,value=bar;key=foo2,value=bar2\",replicas=1", + "namespace=default", + } + r, err := csvToMap(d) + + require.NoError(t, err) + + require.Contains(t, r, "tolerations") + require.Equal(t, r["tolerations"], "key=foo,value=bar;key=foo2,value=bar2") + + require.Contains(t, r, "replicas") + require.Equal(t, r["replicas"], "1") + + require.Contains(t, r, "namespace") + require.Equal(t, r["namespace"], "default") +} diff --git a/docs/reference/buildx_create.md b/docs/reference/buildx_create.md index 35400499..bd9ea1c4 100644 --- a/docs/reference/buildx_create.md +++ b/docs/reference/buildx_create.md @@ -139,13 +139,16 @@ Passes additional driver-specific options. Details for each driver: - `requests.memory` - Sets the request memory value specified in bytes or with a valid suffix. Example `requests.memory=500Mi`, `requests.memory=4G` - `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;key=foo2,operator=exists;key=foo3,effect=NoSchedule"` - 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` + - `"nodeselector=label1=value1,label2=value2"` - Sets the kv of `Pod` nodeSelector. No Defaults. Example `nodeselector=kubernetes.io/arch=arm64` + - `"tolerations=key=foo,value=bar;key=foo2,operator=exists;key=foo3,effect=NoSchedule"` - 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` - `rootless=(true|false)` - Run the container as a non-root user without `securityContext.privileged`. Needs Kubernetes 1.19 or later. [Using Ubuntu host kernel is recommended](https://github.com/moby/buildkit/blob/master/docs/rootless.md). Defaults to false. - `loadbalance=(sticky|random)` - Load-balancing strategy. If set to "sticky", the pod is chosen using the hash of the context path. Defaults to "sticky" - `qemu.install=(true|false)` - Install QEMU emulation for multi platforms support. - `qemu.image=IMAGE` - Sets the QEMU emulation image. Defaults to `tonistiigi/binfmt:latest` +Note: When using quoted values for example for the `nodeselector` or `tolerations` options, ensure that quotes are escaped +correctly for your shell. + ### Remove a node from a builder (--leave) The `--leave` flag changes the action of the command to remove a node from a diff --git a/driver/kubernetes/factory.go b/driver/kubernetes/factory.go index a723ebda..04d8e105 100644 --- a/driver/kubernetes/factory.go +++ b/driver/kubernetes/factory.go @@ -68,121 +68,9 @@ func (f *factory) New(ctx context.Context, cfg driver.InitConfig) (driver.Driver clientset: clientset, } - deploymentOpt := &manifest.DeploymentOpt{ - Name: deploymentName, - Image: bkimage.DefaultImage, - Replicas: 1, - BuildkitFlags: cfg.BuildkitFlags, - Rootless: false, - Platforms: cfg.Platforms, - ConfigFiles: cfg.Files, - } - - deploymentOpt.Qemu.Image = bkimage.QemuImage - - loadbalance := LoadbalanceSticky - - for k, v := range cfg.DriverOpts { - switch k { - case "image": - if v != "" { - deploymentOpt.Image = v - } - case "namespace": - namespace = v - case "replicas": - deploymentOpt.Replicas, err = strconv.Atoi(v) - if err != nil { - return nil, err - } - case "requests.cpu": - deploymentOpt.RequestsCPU = v - case "requests.memory": - deploymentOpt.RequestsMemory = v - case "limits.cpu": - deploymentOpt.LimitsCPU = v - case "limits.memory": - deploymentOpt.LimitsMemory = v - case "rootless": - deploymentOpt.Rootless, err = strconv.ParseBool(v) - if err != nil { - return nil, err - } - if _, isImage := cfg.DriverOpts["image"]; !isImage { - deploymentOpt.Image = bkimage.DefaultRootlessImage - } - case "nodeselector": - kvs := strings.Split(strings.Trim(v, `"`), ",") - s := map[string]string{} - for i := range kvs { - kv := strings.Split(kvs[i], "=") - if len(kv) == 2 { - s[kv[0]] = kv[1] - } - } - deploymentOpt.NodeSelector = s - case "tolerations": - u, err := strconv.Unquote(v) - if nil != err { - return nil, err - } - ts := strings.Split(u, ";") - deploymentOpt.Tolerations = []corev1.Toleration{} - for i := range ts { - kvs := strings.Split(ts[i], ",") - if len(kvs) == 0 { - return nil, errors.Errorf("invalid tolaration %q", v) - } - - t := corev1.Toleration{} - - for j := range kvs { - kv := strings.Split(kvs[j], "=") - if len(kv) == 2 { - switch kv[0] { - case "key": - t.Key = kv[1] - case "operator": - t.Operator = corev1.TolerationOperator(kv[1]) - case "value": - t.Value = kv[1] - case "effect": - t.Effect = corev1.TaintEffect(kv[1]) - case "tolerationSeconds": - c, err := strconv.Atoi(kv[1]) - if nil != err { - return nil, err - } - c64 := int64(c) - t.TolerationSeconds = &c64 - default: - return nil, errors.Errorf("invalid tolaration %q", v) - } - } - } - - deploymentOpt.Tolerations = append(deploymentOpt.Tolerations, t) - } - case "loadbalance": - switch v { - case LoadbalanceSticky: - case LoadbalanceRandom: - default: - return nil, errors.Errorf("invalid loadbalance %q", v) - } - loadbalance = v - case "qemu.install": - deploymentOpt.Qemu.Install, err = strconv.ParseBool(v) - if err != nil { - return nil, err - } - case "qemu.image": - if v != "" { - deploymentOpt.Qemu.Image = v - } - default: - return nil, errors.Errorf("invalid driver option %s for driver %s", k, DriverName) - } + deploymentOpt, loadbalance, namespace, err := f.processDriverOpts(deploymentName, namespace, cfg) + if nil != err { + return nil, err } d.deployment, d.configMaps, err = manifest.NewDeployment(deploymentOpt) @@ -212,6 +100,121 @@ func (f *factory) New(ctx context.Context, cfg driver.InitConfig) (driver.Driver return d, nil } +func (f *factory) processDriverOpts(deploymentName string, namespace string, cfg driver.InitConfig) (*manifest.DeploymentOpt, string, string, error) { + deploymentOpt := &manifest.DeploymentOpt{ + Name: deploymentName, + Image: bkimage.DefaultImage, + Replicas: 1, + BuildkitFlags: cfg.BuildkitFlags, + Rootless: false, + Platforms: cfg.Platforms, + ConfigFiles: cfg.Files, + } + + deploymentOpt.Qemu.Image = bkimage.QemuImage + + loadbalance := LoadbalanceSticky + var err error + + for k, v := range cfg.DriverOpts { + switch k { + case "image": + if v != "" { + deploymentOpt.Image = v + } + case "namespace": + namespace = v + case "replicas": + deploymentOpt.Replicas, err = strconv.Atoi(v) + if err != nil { + return nil, "", "", err + } + case "requests.cpu": + deploymentOpt.RequestsCPU = v + case "requests.memory": + deploymentOpt.RequestsMemory = v + case "limits.cpu": + deploymentOpt.LimitsCPU = v + case "limits.memory": + deploymentOpt.LimitsMemory = v + case "rootless": + deploymentOpt.Rootless, err = strconv.ParseBool(v) + if err != nil { + return nil, "", "", err + } + if _, isImage := cfg.DriverOpts["image"]; !isImage { + deploymentOpt.Image = bkimage.DefaultRootlessImage + } + case "nodeselector": + kvs := strings.Split(strings.Trim(v, `"`), ",") + s := map[string]string{} + for i := range kvs { + kv := strings.Split(kvs[i], "=") + if len(kv) == 2 { + s[kv[0]] = kv[1] + } + } + deploymentOpt.NodeSelector = s + case "tolerations": + ts := strings.Split(v, ";") + deploymentOpt.Tolerations = []corev1.Toleration{} + for i := range ts { + kvs := strings.Split(ts[i], ",") + + t := corev1.Toleration{} + + for j := range kvs { + kv := strings.Split(kvs[j], "=") + if len(kv) == 2 { + switch kv[0] { + case "key": + t.Key = kv[1] + case "operator": + t.Operator = corev1.TolerationOperator(kv[1]) + case "value": + t.Value = kv[1] + case "effect": + t.Effect = corev1.TaintEffect(kv[1]) + case "tolerationSeconds": + c, err := strconv.Atoi(kv[1]) + if nil != err { + return nil, "", "", err + } + c64 := int64(c) + t.TolerationSeconds = &c64 + default: + return nil, "", "", errors.Errorf("invalid tolaration %q", v) + } + } + } + + deploymentOpt.Tolerations = append(deploymentOpt.Tolerations, t) + } + case "loadbalance": + switch v { + case LoadbalanceSticky: + case LoadbalanceRandom: + default: + return nil, "", "", errors.Errorf("invalid loadbalance %q", v) + } + loadbalance = v + case "qemu.install": + deploymentOpt.Qemu.Install, err = strconv.ParseBool(v) + if err != nil { + return nil, "", "", err + } + case "qemu.image": + if v != "" { + deploymentOpt.Qemu.Image = v + } + default: + return nil, "", "", errors.Errorf("invalid driver option %s for driver %s", k, DriverName) + } + } + + return deploymentOpt, loadbalance, namespace, nil +} + func (f *factory) AllowsInstances() bool { return true } diff --git a/driver/kubernetes/factory_test.go b/driver/kubernetes/factory_test.go new file mode 100644 index 00000000..9e33f257 --- /dev/null +++ b/driver/kubernetes/factory_test.go @@ -0,0 +1,230 @@ +package kubernetes + +import ( + "testing" + + "github.com/docker/buildx/driver" + "github.com/docker/buildx/driver/bkimage" + "github.com/stretchr/testify/require" + v1 "k8s.io/api/core/v1" + "k8s.io/client-go/rest" +) + +type mockKubeClientConfig struct { + clientConfig *rest.Config + namespace string +} + +func (r *mockKubeClientConfig) ClientConfig() (*rest.Config, error) { + return r.clientConfig, nil +} + +func (r *mockKubeClientConfig) Namespace() (string, bool, error) { + return r.namespace, true, nil +} + +func TestFactory_processDriverOpts(t *testing.T) { + kcc := mockKubeClientConfig{ + clientConfig: &rest.Config{}, + } + + cfg := driver.InitConfig{ + Name: "buildx_buildkit_test", + KubeClientConfig: &kcc, + } + f := factory{} + + t.Run( + "ValidOptions", func(t *testing.T) { + cfg.DriverOpts = map[string]string{ + "namespace": "test-ns", + "image": "test:latest", + "replicas": "2", + "requests.cpu": "100m", + "requests.memory": "32Mi", + "limits.cpu": "200m", + "limits.memory": "64Mi", + "rootless": "true", + "nodeselector": "selector1=value1,selector2=value2", + "tolerations": "key=tolerationKey1,value=tolerationValue1,operator=Equal,effect=NoSchedule,tolerationSeconds=60;key=tolerationKey2,operator=Exists", + "loadbalance": "random", + "qemu.install": "true", + "qemu.image": "qemu:latest", + } + ns := "test" + + r, loadbalance, ns, err := f.processDriverOpts(cfg.Name, ns, cfg) + + nodeSelectors := map[string]string{ + "selector1": "value1", + "selector2": "value2", + } + + ts := int64(60) + tolerations := []v1.Toleration{ + { + Key: "tolerationKey1", + Operator: v1.TolerationOpEqual, + Value: "tolerationValue1", + Effect: v1.TaintEffectNoSchedule, + TolerationSeconds: &ts, + }, + { + Key: "tolerationKey2", + Operator: v1.TolerationOpExists, + }, + } + + require.NoError(t, err) + + require.Equal(t, "test-ns", ns) + require.Equal(t, "test:latest", r.Image) + require.Equal(t, 2, r.Replicas) + require.Equal(t, "100m", r.RequestsCPU) + require.Equal(t, "32Mi", r.RequestsMemory) + require.Equal(t, "200m", r.LimitsCPU) + require.Equal(t, "64Mi", r.LimitsMemory) + require.True(t, r.Rootless) + require.Equal(t, nodeSelectors, r.NodeSelector) + require.Equal(t, tolerations, r.Tolerations) + require.Equal(t, LoadbalanceRandom, loadbalance) + require.True(t, r.Qemu.Install) + require.Equal(t, "qemu:latest", r.Qemu.Image) + }, + ) + + t.Run( + "NoOptions", func(t *testing.T) { + cfg.DriverOpts = map[string]string{} + + r, loadbalance, ns, err := f.processDriverOpts(cfg.Name, "test", cfg) + + require.NoError(t, err) + + require.Equal(t, "test", ns) + require.Equal(t, bkimage.DefaultImage, r.Image) + require.Equal(t, 1, r.Replicas) + require.Equal(t, "", r.RequestsCPU) + require.Equal(t, "", r.RequestsMemory) + require.Equal(t, "", r.LimitsCPU) + require.Equal(t, "", r.LimitsMemory) + require.False(t, r.Rootless) + require.Empty(t, r.NodeSelector) + require.Empty(t, r.Tolerations) + require.Equal(t, LoadbalanceSticky, loadbalance) + require.False(t, r.Qemu.Install) + require.Equal(t, bkimage.QemuImage, r.Qemu.Image) + }, + ) + + t.Run( + "RootlessOverride", func(t *testing.T) { + cfg.DriverOpts = map[string]string{ + "rootless": "true", + "loadbalance": "sticky", + } + + r, loadbalance, ns, err := f.processDriverOpts(cfg.Name, "test", cfg) + + require.NoError(t, err) + + require.Equal(t, "test", ns) + require.Equal(t, bkimage.DefaultRootlessImage, r.Image) + require.Equal(t, 1, r.Replicas) + require.Equal(t, "", r.RequestsCPU) + require.Equal(t, "", r.RequestsMemory) + require.Equal(t, "", r.LimitsCPU) + require.Equal(t, "", r.LimitsMemory) + require.True(t, r.Rootless) + require.Empty(t, r.NodeSelector) + require.Empty(t, r.Tolerations) + require.Equal(t, LoadbalanceSticky, loadbalance) + require.False(t, r.Qemu.Install) + require.Equal(t, bkimage.QemuImage, r.Qemu.Image) + }, + ) + + t.Run( + "InvalidReplicas", func(t *testing.T) { + cfg.DriverOpts = map[string]string{ + "replicas": "invalid", + } + + _, _, _, err := f.processDriverOpts(cfg.Name, "test", cfg) + + require.Error(t, err) + }, + ) + + t.Run( + "InvalidRootless", func(t *testing.T) { + cfg.DriverOpts = map[string]string{ + "rootless": "invalid", + } + + _, _, _, err := f.processDriverOpts(cfg.Name, "test", cfg) + + require.Error(t, err) + }, + ) + + t.Run( + "InvalidTolerationKeyword", func(t *testing.T) { + cfg.DriverOpts = map[string]string{ + "tolerations": "key=foo,value=bar,invalid=foo2", + } + + _, _, _, err := f.processDriverOpts(cfg.Name, "test", cfg) + + require.Error(t, err) + }, + ) + + t.Run( + "InvalidTolerationSeconds", func(t *testing.T) { + cfg.DriverOpts = map[string]string{ + "tolerations": "key=foo,value=bar,tolerationSeconds=invalid", + } + + _, _, _, err := f.processDriverOpts(cfg.Name, "test", cfg) + + require.Error(t, err) + }, + ) + + t.Run( + "InvalidLoadBalance", func(t *testing.T) { + cfg.DriverOpts = map[string]string{ + "loadbalance": "invalid", + } + + _, _, _, err := f.processDriverOpts(cfg.Name, "test", cfg) + + require.Error(t, err) + }, + ) + + t.Run( + "InvalidQemuInstall", func(t *testing.T) { + cfg.DriverOpts = map[string]string{ + "qemu.install": "invalid", + } + + _, _, _, err := f.processDriverOpts(cfg.Name, "test", cfg) + + require.Error(t, err) + }, + ) + + t.Run( + "InvalidOption", func(t *testing.T) { + cfg.DriverOpts = map[string]string{ + "invalid": "foo", + } + + _, _, _, err := f.processDriverOpts(cfg.Name, "test", cfg) + + require.Error(t, err) + }, + ) +}