models: make kubernetes taint values optional

[etungsten]

Issue number:
Addresses #1302

Description of changes:

Author: Erikson Tung <[email protected]>
Date:   Fri Mar 19 11:55:17 2021 -0700

    models: make kubernetes taint values optional
    
    This modifies the validation for KubernetesTaintValue to make the taint
    value optional for 'Exists' toleration operators.

Testing done:
Built an aws-k8s-1.19 x86_64 AMI and launched it with the following specified in my userdata:

[settings.kubernetes.node-labels]
label1 = "foo"
label2 = "bar"
[settings.kubernetes.node-taints]
testMe = ":NoSchedule"

The node comes up successfully in my cluster with the specified labels and taints:

$ kubectl get nodes --output json
...
                "taints": [
                    {
                        "effect": "NoSchedule",
                        "key": "testMe"
                    }
                ]

I'm also able to create tolerations that uses the Exists operator in my pod specs:

apiVersion: v1
kind: Pod
metadata:
  name: nginx
  labels:
    env: test
spec:
  containers:
  - name: nginx
    image: nginx
    imagePullPolicy: IfNotPresent
  tolerations:
  - key: "MyKey"
    operator: "Exists"
    effect: "NoSchedule"

...and the nginx pod does not get scheduled on the node that has the testMe taint as expected.

$ kubectl get pod nginx --output json
...
    "status": {
        "conditions": [
            {
                "lastProbeTime": null,
                "lastTransitionTime": "2021-03-19T20:35:20Z",
                "message": "0/1 nodes are available: 1 node(s) had taint {testMe: }, that the pod didn't tolerate.",
                "reason": "Unschedulable",
                "status": "False",
                "type": "PodScheduled"
            }
        ],
        "phase": "Pending",
        "qosClass": "BestEffort"
    }

I launched another node without the taint, and the nginx pod gets scheduled onto that node as soon as it became Ready.

If I launch another node with a MyKey=:NoSchedule taint, the pod correctly tolerates it and is able to schedule onto that node as well.

Terms of contribution:

By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.

[zmrow]

🏐

[tjkirch]

I believe this would now mistakenly require the value to be two characters minimum, but I think one is intended to be allowed. Would you please add a test using a single character?

[etungsten]

I believe this would now mistakenly require the value to be two characters minimum, but I think one is intended to be allowed. Would you please add a test using a single character?

You're absolutely right. 🤦. It'll require minimum 2 alphanumeric characters if the value is specified. I'll fix it.

[etungsten]

Push above addresses @tjkirch 's comment #1406 (review):

• Fixes regex so that if a taint value is specified, it needs at least 1 alphanumeric character, and end with an alphanumeric character.
• Adds more unit tests

All unit tests pass.

[zmrow]

🤗

[etungsten]

Push above addresses @tjkirch 's comment: #1406 (comment)

Ran unit tests again and they passed.