From 4bdcc03c0b0f61f84fdb927af98463896e4992aa Mon Sep 17 00:00:00 2001 From: Anastasis Andronidis Date: Mon, 24 Oct 2016 21:34:28 -0700 Subject: [PATCH] test for explicit null value propagation in apply --- hack/make-rules/test-cmd-util.sh | 31 +++++++ .../null-propagation/deployment-l1.yaml | 13 +++ .../null-propagation/deployment-l2.yaml | 17 ++++ pkg/kubectl/cmd/apply_test.go | 87 +++++++++++++++++++ pkg/kubectl/cmd/testing/fake.go | 21 +++++ .../kubectl/cmd/apply/deploy-clientside.yaml | 16 ++++ .../kubectl/cmd/apply/deploy-serverside.yaml | 46 ++++++++++ 7 files changed, 231 insertions(+) create mode 100644 hack/testdata/null-propagation/deployment-l1.yaml create mode 100644 hack/testdata/null-propagation/deployment-l2.yaml create mode 100644 test/fixtures/pkg/kubectl/cmd/apply/deploy-clientside.yaml create mode 100644 test/fixtures/pkg/kubectl/cmd/apply/deploy-serverside.yaml diff --git a/hack/make-rules/test-cmd-util.sh b/hack/make-rules/test-cmd-util.sh index 708eedaac83..8fc72ae5333 100644 --- a/hack/make-rules/test-cmd-util.sh +++ b/hack/make-rules/test-cmd-util.sh @@ -917,6 +917,33 @@ run_kubectl_create_filter_tests() { kubectl delete pods selector-test-pod } +run_kubectl_apply_deployments_tests() { + ## kubectl apply should propagate user defined null values + # Pre-Condition: no Deployments exists + kube::test::get_object_assert deployments "{{range.items}}{{$id_field}}:{{end}}" '' + # apply base deployment + kubectl apply -f hack/testdata/null-propagation/deployment-l1.yaml "${kube_flags[@]}" + # check right deployment exists + kube::test::get_object_assert 'deployments my-depl' "{{${id_field}}}" 'my-depl' + # check right labels exists + kube::test::get_object_assert 'deployments my-depl' "{{.spec.template.metadata.labels.l1}}" 'l1' + kube::test::get_object_assert 'deployments my-depl' "{{.spec.selector.matchLabels.l1}}" 'l1' + kube::test::get_object_assert 'deployments my-depl' "{{.metadata.labels.l1}}" 'l1' + + # apply new deployment with new template labels + kubectl apply -f hack/testdata/null-propagation/deployment-l2.yaml "${kube_flags[@]}" + # check right labels exists + kube::test::get_object_assert 'deployments my-depl' "{{.spec.template.metadata.labels.l1}}" '' + kube::test::get_object_assert 'deployments my-depl' "{{.spec.selector.matchLabels.l1}}" '' + kube::test::get_object_assert 'deployments my-depl' "{{.metadata.labels.l1}}" '' + kube::test::get_object_assert 'deployments my-depl' "{{.spec.template.metadata.labels.l2}}" 'l2' + kube::test::get_object_assert 'deployments my-depl' "{{.spec.selector.matchLabels.l2}}" 'l2' + kube::test::get_object_assert 'deployments my-depl' "{{.metadata.labels.l2}}" 'l2' + + # cleanup + kubectl delete deployments my-depl +} + # Runs tests for --save-config tests. run_save_config_tests() { ## Configuration annotations should be set when --save-config is enabled @@ -2718,6 +2745,10 @@ runTests() { run_kubectl_create_filter_tests fi + if kube::test::if_supports_resource "${deployments}" ; then + run_kubectl_apply_deployments_tests + fi + ############### # Kubectl get # ############### diff --git a/hack/testdata/null-propagation/deployment-l1.yaml b/hack/testdata/null-propagation/deployment-l1.yaml new file mode 100644 index 00000000000..c5123abc5ee --- /dev/null +++ b/hack/testdata/null-propagation/deployment-l1.yaml @@ -0,0 +1,13 @@ +apiVersion: extensions/v1beta1 +kind: Deployment +metadata: + name: my-depl +spec: + template: + metadata: + labels: + l1: l1 + spec: + containers: + - name: nginx + image: gcr.io/google-containers/nginx:1.7.9 diff --git a/hack/testdata/null-propagation/deployment-l2.yaml b/hack/testdata/null-propagation/deployment-l2.yaml new file mode 100644 index 00000000000..6bef22f8815 --- /dev/null +++ b/hack/testdata/null-propagation/deployment-l2.yaml @@ -0,0 +1,17 @@ +apiVersion: extensions/v1beta1 +kind: Deployment +metadata: + name: my-depl + # We expect this field to be defaulted to the new label l2 + labels: null +spec: + # We expect this field to be defaulted to the new label l2 + selector: null + template: + metadata: + labels: + l2: l2 + spec: + containers: + - name: nginx + image: gcr.io/google-containers/nginx:1.7.9 diff --git a/pkg/kubectl/cmd/apply_test.go b/pkg/kubectl/cmd/apply_test.go index c41de7aa158..62564c74789 100644 --- a/pkg/kubectl/cmd/apply_test.go +++ b/pkg/kubectl/cmd/apply_test.go @@ -20,6 +20,7 @@ import ( "bytes" "encoding/json" "fmt" + "io" "io/ioutil" "net/http" "os" @@ -35,6 +36,8 @@ import ( "k8s.io/kubernetes/pkg/api" "k8s.io/kubernetes/pkg/api/annotations" "k8s.io/kubernetes/pkg/api/testapi" + "k8s.io/kubernetes/pkg/apis/extensions" + "k8s.io/kubernetes/pkg/client/restclient/fake" cmdtesting "k8s.io/kubernetes/pkg/kubectl/cmd/testing" cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util" ) @@ -443,3 +446,87 @@ func testApplyMultipleObjects(t *testing.T, asList bool) { t.Fatalf("unexpected output: %s\nexpected: %s OR %s", buf.String(), expectOne, expectTwo) } } + +const ( + filenameDeployObjServerside = "../../../test/fixtures/pkg/kubectl/cmd/apply/deploy-serverside.yaml" + filenameDeployObjClientside = "../../../test/fixtures/pkg/kubectl/cmd/apply/deploy-clientside.yaml" +) + +// validateNULLPatchApplication retrieves the StrategicMergePatch created by kubectl apply and checks that this patch +// contains the null value defined by the user. +func validateNULLPatchApplication(t *testing.T, req *http.Request) { + patch, err := ioutil.ReadAll(req.Body) + if err != nil { + t.Fatal(err) + } + + patchMap := map[string]interface{}{} + if err := json.Unmarshal(patch, &patchMap); err != nil { + t.Fatal(err) + } + + annotationsMap := walkMapPath(t, patchMap, []string{"metadata", "annotations"}) + if _, ok := annotationsMap[annotations.LastAppliedConfigAnnotation]; !ok { + t.Fatalf("patch does not contain annotation:\n%s\n", patch) + } + + labelMap := walkMapPath(t, patchMap, []string{"spec", "strategy"}) + // Checks if `rollingUpdate = null` exists in the patch + if deleteMe, ok := labelMap["rollingUpdate"]; !ok || deleteMe != nil { + t.Fatalf("patch did not retain null value in key: rollingUpdate:\n%s\n", patch) + } +} + +func getServersideObject(t *testing.T) io.ReadCloser { + raw := readBytesFromFile(t, filenameDeployObjServerside) + obj := &extensions.Deployment{} + if err := runtime.DecodeInto(testapi.Extensions.Codec(), raw, obj); err != nil { + t.Fatal(err) + } + objJSON, err := runtime.Encode(testapi.Extensions.Codec(), obj) + if err != nil { + t.Fatal(err) + } + return ioutil.NopCloser(bytes.NewReader(objJSON)) +} + +func TestApplyNULLPreservation(t *testing.T) { + deploymentName := "nginx-deployment" + deploymentPath := "/namespaces/test/deployments/" + deploymentName + + f, tf, _, ns := cmdtesting.NewExtensionsAPIFactory() + tf.Client = &fake.RESTClient{ + NegotiatedSerializer: ns, + GroupName: "extensions", + Client: fake.CreateHTTPClient(func(req *http.Request) (*http.Response, error) { + switch p, m := req.URL.Path, req.Method; { + case p == deploymentPath && m == "GET": + return &http.Response{StatusCode: 200, Header: defaultHeader(), Body: getServersideObject(t)}, nil + case p == deploymentPath && m == "PATCH": + validateNULLPatchApplication(t, req) + // The real API server would had returned the patched object but Kubectl + // is ignoring the actual return object. + // TODO: Make this match actual server behavior by returning the patched object. + return &http.Response{StatusCode: 200, Header: defaultHeader(), Body: getServersideObject(t)}, nil + default: + t.Fatalf("unexpected request: %#v\n%#v", req.URL, req) + return nil, nil + } + }), + } + tf.Namespace = "test" + tf.ClientConfig = defaultClientConfig() + buf := bytes.NewBuffer([]byte{}) + errBuf := bytes.NewBuffer([]byte{}) + + cmd := NewCmdApply(f, buf, errBuf) + cmd.Flags().Set("filename", filenameDeployObjClientside) + cmd.Flags().Set("output", "name") + + cmd.Run(cmd, []string{}) + + expected := "deployment/" + deploymentName + "\n" + if buf.String() != expected { + t.Fatalf("unexpected output: %s\nexpected: %s", buf.String(), expected) + } +} diff --git a/pkg/kubectl/cmd/testing/fake.go b/pkg/kubectl/cmd/testing/fake.go index 320a5d5e4ff..a9929b96df7 100644 --- a/pkg/kubectl/cmd/testing/fake.go +++ b/pkg/kubectl/cmd/testing/fake.go @@ -646,6 +646,27 @@ func NewAPIFactory() (cmdutil.Factory, *TestFactory, runtime.Codec, runtime.Nego }, t, testapi.Default.Codec(), testapi.Default.NegotiatedSerializer() } +type fakeExtensionsAPIFactory struct { + fakeAPIFactory +} + +func (f *fakeExtensionsAPIFactory) JSONEncoder() runtime.Encoder { + return testapi.Extensions.Codec() +} + +func NewExtensionsAPIFactory() (cmdutil.Factory, *TestFactory, runtime.Codec, runtime.NegotiatedSerializer) { + t := &TestFactory{ + Validator: validation.NullSchema{}, + } + rf := cmdutil.NewFactory(nil) + return &fakeExtensionsAPIFactory{ + fakeAPIFactory: fakeAPIFactory{ + Factory: rf, + tf: t, + }, + }, t, testapi.Default.Codec(), testapi.Default.NegotiatedSerializer() +} + func testDynamicResources() []*discovery.APIGroupResources { return []*discovery.APIGroupResources{ { diff --git a/test/fixtures/pkg/kubectl/cmd/apply/deploy-clientside.yaml b/test/fixtures/pkg/kubectl/cmd/apply/deploy-clientside.yaml new file mode 100644 index 00000000000..b3aa9071c4b --- /dev/null +++ b/test/fixtures/pkg/kubectl/cmd/apply/deploy-clientside.yaml @@ -0,0 +1,16 @@ +apiVersion: extensions/v1beta1 +kind: Deployment +metadata: + name: nginx-deployment +spec: + strategy: + type: Recreate + rollingUpdate: null + template: + metadata: + labels: + name: nginx + spec: + containers: + - name: nginx + image: nginx diff --git a/test/fixtures/pkg/kubectl/cmd/apply/deploy-serverside.yaml b/test/fixtures/pkg/kubectl/cmd/apply/deploy-serverside.yaml new file mode 100644 index 00000000000..ffc11aa9df4 --- /dev/null +++ b/test/fixtures/pkg/kubectl/cmd/apply/deploy-serverside.yaml @@ -0,0 +1,46 @@ +apiVersion: extensions/v1beta1 +kind: Deployment +metadata: + annotations: + deployment.kubernetes.io/revision: "1" + kubectl.kubernetes.io/last-applied-configuration: '{"kind":"Deployment","apiVersion":"extensions/v1beta1","metadata":{"name":"nginx-deployment","creationTimestamp":null},"spec":{"template":{"metadata":{"creationTimestamp":null,"labels":{"name":"nginx"}},"spec":{"containers":[{"name":"nginx","image":"nginx","resources":{}}]}},"strategy":{}},"status":{}}' + creationTimestamp: 2016-10-24T22:15:06Z + generation: 6 + labels: + name: nginx + name: nginx-deployment + namespace: test + resourceVersion: "355959" + selfLink: /apis/extensions/v1beta1/namespaces/test/deployments/nginx-deployment + uid: 51ac266e-9a37-11e6-8738-0800270c4edc +spec: + replicas: 1 + selector: + matchLabels: + name: nginx + strategy: + rollingUpdate: + maxSurge: 1 + maxUnavailable: 1 + type: RollingUpdate + template: + metadata: + creationTimestamp: null + labels: + name: nginx + spec: + containers: + - image: nginx + imagePullPolicy: Always + name: nginx + resources: {} + terminationMessagePath: /dev/termination-log + dnsPolicy: ClusterFirst + restartPolicy: Always + securityContext: {} + terminationGracePeriodSeconds: 30 +status: + availableReplicas: 1 + observedGeneration: 6 + replicas: 1 + updatedReplicas: 1