Fix tolerations not parsing its options correctly, add tests

Signed-off-by: Zsolt <[email protected]>

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")
+}

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.
+
 ### <a name="leave"></a> Remove a node from a builder (--leave)
 
 The `--leave` flag changes the action of the command to remove a node from a

driver/kubernetes/factory.go

@@ -68,6 +68,39 @@ func (f *factory) New(ctx context.Context, cfg driver.InitConfig) (driver.Driver
 		clientset:  clientset,
 	}
 
+	deploymentOpt, loadbalance, namespace, err := f.processDriverOpts(deploymentName, namespace, cfg)
+	if nil != err {
+		return nil, err
+	}
+
+	d.deployment, d.configMaps, err = manifest.NewDeployment(deploymentOpt)
+	if err != nil {
+		return nil, err
+	}
+
+	d.minReplicas = deploymentOpt.Replicas
+
+	d.deploymentClient = clientset.AppsV1().Deployments(namespace)
+	d.podClient = clientset.CoreV1().Pods(namespace)
+	d.configMapClient = clientset.CoreV1().ConfigMaps(namespace)
+
+	switch loadbalance {
+	case LoadbalanceSticky:
+		d.podChooser = &podchooser.StickyPodChooser{
+			Key:        cfg.ContextPathHash,
+			PodClient:  d.podClient,
+			Deployment: d.deployment,
+		}
+	case LoadbalanceRandom:
+		d.podChooser = &podchooser.RandomPodChooser{
+			PodClient:  d.podClient,
+			Deployment: d.deployment,
+		}
+	}
+	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,
@@ -81,6 +114,7 @@ func (f *factory) New(ctx context.Context, cfg driver.InitConfig) (driver.Driver
 	deploymentOpt.Qemu.Image = bkimage.QemuImage
 
 	loadbalance := LoadbalanceSticky
+	var err error
 
 	for k, v := range cfg.DriverOpts {
 		switch k {
@@ -93,7 +127,7 @@ func (f *factory) New(ctx context.Context, cfg driver.InitConfig) (driver.Driver
 		case "replicas":
 			deploymentOpt.Replicas, err = strconv.Atoi(v)
 			if err != nil {
-				return nil, err
+				return nil, "", "", err
 			}
 		case "requests.cpu":
 			deploymentOpt.RequestsCPU = v
@@ -106,9 +140,11 @@ func (f *factory) New(ctx context.Context, cfg driver.InitConfig) (driver.Driver
 		case "rootless":
 			deploymentOpt.Rootless, err = strconv.ParseBool(v)
 			if err != nil {
-				return nil, err
+				return nil, "", "", err
+			}
+			if deploymentOpt.Image == bkimage.DefaultImage {
+				deploymentOpt.Image = bkimage.DefaultRootlessImage
 			}
-			deploymentOpt.Image = bkimage.DefaultRootlessImage
 		case "nodeselector":
 			kvs := strings.Split(strings.Trim(v, `"`), ",")
 			s := map[string]string{}
@@ -120,17 +156,10 @@ func (f *factory) New(ctx context.Context, cfg driver.InitConfig) (driver.Driver
 			}
 			deploymentOpt.NodeSelector = s
 		case "tolerations":
-			u, err := strconv.Unquote(v)
-			if nil != err {
-				return nil, err
-			}
-			ts := strings.Split(u, ";")
+			ts := strings.Split(v, ";")
 			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{}
 
@@ -149,12 +178,12 @@ func (f *factory) New(ctx context.Context, cfg driver.InitConfig) (driver.Driver
 						case "tolerationSeconds":
 							c, err := strconv.Atoi(kv[1])
 							if nil != err {
-								return nil, err
+								return nil, "", "", err
 							}
 							c64 := int64(c)
 							t.TolerationSeconds = &c64
 						default:
-							return nil, errors.Errorf("invalid tolaration %q", v)
+							return nil, "", "", errors.Errorf("invalid tolaration %q", v)
 						}
 					}
 				}
@@ -166,48 +195,24 @@ func (f *factory) New(ctx context.Context, cfg driver.InitConfig) (driver.Driver
 			case LoadbalanceSticky:
 			case LoadbalanceRandom:
 			default:
-				return nil, errors.Errorf("invalid loadbalance %q", v)
+				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
+				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 nil, "", "", errors.Errorf("invalid driver option %s for driver %s", k, DriverName)
 		}
 	}
 
-	d.deployment, d.configMaps, err = manifest.NewDeployment(deploymentOpt)
-	if err != nil {
-		return nil, err
-	}
-
-	d.minReplicas = deploymentOpt.Replicas
-
-	d.deploymentClient = clientset.AppsV1().Deployments(namespace)
-	d.podClient = clientset.CoreV1().Pods(namespace)
-	d.configMapClient = clientset.CoreV1().ConfigMaps(namespace)
-
-	switch loadbalance {
-	case LoadbalanceSticky:
-		d.podChooser = &podchooser.StickyPodChooser{
-			Key:        cfg.ContextPathHash,
-			PodClient:  d.podClient,
-			Deployment: d.deployment,
-		}
-	case LoadbalanceRandom:
-		d.podChooser = &podchooser.RandomPodChooser{
-			PodClient:  d.podClient,
-			Deployment: d.deployment,
-		}
-	}
-	return d, nil
+	return deploymentOpt, loadbalance, namespace, nil
 }
 
 func (f *factory) AllowsInstances() bool {