From 11653b11c1fd8321a8cb04effe250c8b574917cc Mon Sep 17 00:00:00 2001 From: ymqytw Date: Fri, 18 Nov 2016 15:00:14 -0800 Subject: [PATCH] add a unit test --- pkg/kubectl/cmd/BUILD | 1 + pkg/kubectl/cmd/apply_test.go | 113 ++++++++++++++++++++ pkg/kubectl/cmd/testing/fake.go | 8 +- pkg/util/strategicpatch/patch.go | 18 ++-- test/fixtures/pkg/kubectl/cmd/apply/rc.yaml | 2 + 5 files changed, 131 insertions(+), 11 deletions(-) diff --git a/pkg/kubectl/cmd/BUILD b/pkg/kubectl/cmd/BUILD index ba6a9ef1a9..f82463d870 100644 --- a/pkg/kubectl/cmd/BUILD +++ b/pkg/kubectl/cmd/BUILD @@ -191,6 +191,7 @@ go_test( "//pkg/runtime/serializer/streaming:go_default_library", "//pkg/types:go_default_library", "//pkg/util/intstr:go_default_library", + "//pkg/util/strategicpatch:go_default_library", "//pkg/util/strings:go_default_library", "//pkg/util/term:go_default_library", "//pkg/util/wait:go_default_library", diff --git a/pkg/kubectl/cmd/apply_test.go b/pkg/kubectl/cmd/apply_test.go index ab86ef0e10..2918f7894d 100644 --- a/pkg/kubectl/cmd/apply_test.go +++ b/pkg/kubectl/cmd/apply_test.go @@ -23,6 +23,7 @@ import ( "io/ioutil" "net/http" "os" + "strings" "testing" "github.com/spf13/cobra" @@ -37,6 +38,7 @@ import ( cmdtesting "k8s.io/kubernetes/pkg/kubectl/cmd/testing" cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util" "k8s.io/kubernetes/pkg/runtime" + "k8s.io/kubernetes/pkg/util/strategicpatch" ) func TestApplyExtraArgsFail(t *testing.T) { @@ -142,6 +144,58 @@ func readAndAnnotateService(t *testing.T, filename string) (string, []byte) { return annotateRuntimeObject(t, svc1, svc2, "Service") } +func setFinalizersRuntimeObject(t *testing.T, originalObj, currentObj runtime.Object) (string, []byte) { + originalAccessor, err := meta.Accessor(originalObj) + if err != nil { + t.Fatal(err) + } + + originalFinalizers := []string{"a/a"} + originalAccessor.SetFinalizers(originalFinalizers) + original, err := runtime.Encode(testapi.Default.Codec(), originalObj) + if err != nil { + t.Fatal(err) + } + + currentAccessor, err := meta.Accessor(currentObj) + if err != nil { + t.Fatal(err) + } + + currentFinalizers := []string{"b/b"} + currentAccessor.SetFinalizers(currentFinalizers) + + currentAnnotations := currentAccessor.GetAnnotations() + if currentAnnotations == nil { + currentAnnotations = make(map[string]string) + } + currentAnnotations[annotations.LastAppliedConfigAnnotation] = string(original) + currentAccessor.SetAnnotations(currentAnnotations) + current, err := runtime.Encode(testapi.Default.Codec(), currentObj) + if err != nil { + t.Fatal(err) + } + + return currentAccessor.GetName(), current +} + +func readAndSetFinalizersReplicationController(t *testing.T, filename string) (string, []byte) { + rc1 := readReplicationControllerFromFile(t, filename) + rc2 := readReplicationControllerFromFile(t, filename) + name, rcBytes := setFinalizersRuntimeObject(t, rc1, rc2) + return name, rcBytes +} + +func isSMPatchVersion_1_5(t *testing.T, req *http.Request) bool { + patch, err := ioutil.ReadAll(req.Body) + if err != nil { + t.Fatal(err) + } + + // SMPatchVersion_1_5 patch should has string "mergeprimitiveslist" + return strings.Contains(string(patch), strategicpatch.MergePrimitivesListDirective) +} + func validatePatchApplication(t *testing.T, req *http.Request) { patch, err := ioutil.ReadAll(req.Body) if err != nil { @@ -223,6 +277,65 @@ func TestApplyObject(t *testing.T) { } } +func TestApplyRetryWithSMPatchVersion_1_5(t *testing.T) { + initTestErrorHandler(t) + nameRC, currentRC := readAndSetFinalizersReplicationController(t, filenameRC) + pathRC := "/namespaces/test/replicationcontrollers/" + nameRC + + firstPatch := true + retry := false + f, tf, _, ns := cmdtesting.NewAPIFactory() + tf.Printer = &testPrinter{} + tf.Client = &fake.RESTClient{ + NegotiatedSerializer: ns, + Client: fake.CreateHTTPClient(func(req *http.Request) (*http.Response, error) { + switch p, m := req.URL.Path, req.Method; { + case p == pathRC && m == "GET": + bodyRC := ioutil.NopCloser(bytes.NewReader(currentRC)) + return &http.Response{StatusCode: 200, Header: defaultHeader(), Body: bodyRC}, nil + case p == pathRC && m == "PATCH": + if firstPatch { + if !isSMPatchVersion_1_5(t, req) { + t.Fatalf("apply didn't try to send SMPatchVersion_1_5 for the first time") + } + firstPatch = false + statusErr := kubeerr.NewInternalError(fmt.Errorf("Server encountered internal error.")) + bodyBytes, _ := json.Marshal(statusErr) + bodyErr := ioutil.NopCloser(bytes.NewReader(bodyBytes)) + return &http.Response{StatusCode: http.StatusInternalServerError, Header: defaultHeader(), Body: bodyErr}, nil + } + retry = true + if isSMPatchVersion_1_5(t, req) { + t.Fatalf("apply didn't try to send SMPatchVersion_1_0 after SMPatchVersion_1_5 patch encounter an Internal Error (500)") + } + bodyRC := ioutil.NopCloser(bytes.NewReader(currentRC)) + return &http.Response{StatusCode: 200, Header: defaultHeader(), Body: bodyRC}, 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{}) + + cmd := NewCmdApply(f, buf) + cmd.Flags().Set("filename", filenameRC) + cmd.Flags().Set("output", "name") + cmd.Run(cmd, []string{}) + + if !retry { + t.Fatalf("apply didn't retry when get Internal Error (500)") + } + + // uses the name from the file, not the response + expectRC := "replicationcontroller/" + nameRC + "\n" + if buf.String() != expectRC { + t.Fatalf("unexpected output: %s\nexpected: %s", buf.String(), expectRC) + } +} + func TestApplyRetry(t *testing.T) { initTestErrorHandler(t) nameRC, currentRC := readAndAnnotateReplicationController(t, filenameRC) diff --git a/pkg/kubectl/cmd/testing/fake.go b/pkg/kubectl/cmd/testing/fake.go index d5452846c6..ad85354cca 100644 --- a/pkg/kubectl/cmd/testing/fake.go +++ b/pkg/kubectl/cmd/testing/fake.go @@ -427,8 +427,12 @@ func (f *fakeAPIFactory) UnstructuredObject() (meta.RESTMapper, runtime.ObjectTy return cmdutil.NewShortcutExpander(mapper, nil), typer, nil } -func (f *fakeAPIFactory) Decoder(bool) runtime.Decoder { - return testapi.Default.Codec() +func (f *fakeAPIFactory) Decoder(toInternal bool) runtime.Decoder { + if toInternal { + return api.Codecs.UniversalDecoder() + } else { + return api.Codecs.UniversalDeserializer() + } } func (f *fakeAPIFactory) JSONEncoder() runtime.Encoder { diff --git a/pkg/util/strategicpatch/patch.go b/pkg/util/strategicpatch/patch.go index 442a176e94..b9c2c3ab88 100644 --- a/pkg/util/strategicpatch/patch.go +++ b/pkg/util/strategicpatch/patch.go @@ -45,7 +45,7 @@ const ( deleteDirective = "delete" replaceDirective = "replace" mergeDirective = "merge" - mergePrimitivesListDirective = "mergeprimitiveslist" + MergePrimitivesListDirective = "mergeprimitiveslist" // different versions of StrategicMergePatch SMPatchVersion_1_0 StrategicMergePatchVersion = "v1.0.0" @@ -393,7 +393,7 @@ loopB: func diffListsOfScalarsIntoMap(originalScalars, modifiedScalars []interface{}, ignoreChangesAndAdditions, ignoreDeletions bool) (map[string]interface{}, error) { originalIndex, modifiedIndex := 0, 0 patch := map[string]interface{}{} - patch[directiveMarker] = mergePrimitivesListDirective + patch[directiveMarker] = MergePrimitivesListDirective for originalIndex < len(originalScalars) && modifiedIndex < len(modifiedScalars) { originalString := fmt.Sprintf("%v", originalScalars[originalIndex]) @@ -627,7 +627,7 @@ func mergeMap(original, patch map[string]interface{}, t reflect.Type) (map[strin return map[string]interface{}{}, nil } - if v == mergePrimitivesListDirective { + if v == MergePrimitivesListDirective { // delete the directiveMarker's key-value pair to avoid delta map and delete map // overlaping with each other when calculating a ThreeWayDiff for list of Primitives. // Otherwise, the overlaping will cause it calling LookupPatchMetadata() which will @@ -718,7 +718,7 @@ func mergeMap(original, patch map[string]interface{}, t reflect.Type) (map[strin // the patch because getting a deep copy of a slice in golang is highly // non-trivial. // The patch could be a map[string]interface{} representing a slice of primitives. -// If the patch map doesn't has the specific directiveMarker (mergePrimitivesListDirective), +// If the patch map doesn't has the specific directiveMarker (MergePrimitivesListDirective), // it returns an error. Please check patch_test.go and find the test case named // "merge lists of scalars for list of primitives" to see what the patch looks like. // Patch is still []interface{} for all the other types. @@ -731,7 +731,7 @@ func mergeSlice(original []interface{}, patch interface{}, elemType reflect.Type if patchMap, ok := patch.(map[string]interface{}); ok { // We try to merge the original slice with a patch map only when the map has // a specific directiveMarker. Otherwise, this patch will be treated as invalid. - if directiveValue, ok := patchMap[directiveMarker]; ok && directiveValue == mergePrimitivesListDirective { + if directiveValue, ok := patchMap[directiveMarker]; ok && directiveValue == MergePrimitivesListDirective { return mergeSliceOfScalarsWithPatchMap(original, patchMap) } else { return nil, fmt.Errorf("Unable to merge a slice with an invalid map") @@ -838,10 +838,10 @@ func mergeSlice(original []interface{}, patch interface{}, elemType reflect.Type // mergeSliceOfScalarsWithPatchMap merges the original slice with a patch map and // returns an uniqified and sorted slice of primitives. -// The patch map must have the specific directiveMarker (mergePrimitivesListDirective). +// The patch map must have the specific directiveMarker (MergePrimitivesListDirective). func mergeSliceOfScalarsWithPatchMap(original []interface{}, patch map[string]interface{}) ([]interface{}, error) { // make sure the patch has the specific directiveMarker () - if directiveValue, ok := patch[directiveMarker]; ok && directiveValue != mergePrimitivesListDirective { + if directiveValue, ok := patch[directiveMarker]; ok && directiveValue != MergePrimitivesListDirective { return nil, fmt.Errorf("Unable to merge a slice with an invalid map") } delete(patch, directiveMarker) @@ -1181,7 +1181,7 @@ func mergingMapFieldsHaveConflicts( return true, nil } - if leftMarker == mergePrimitivesListDirective && rightMarker == mergePrimitivesListDirective { + if leftMarker == MergePrimitivesListDirective && rightMarker == MergePrimitivesListDirective { return false, nil } } @@ -1209,7 +1209,7 @@ func mapsHaveConflicts(typedLeft, typedRight map[string]interface{}, structType isForListOfPrimitives := false if leftDirective, ok := typedLeft[directiveMarker]; ok { if rightDirective, ok := typedRight[directiveMarker]; ok { - if leftDirective == mergePrimitivesListDirective && rightDirective == rightDirective { + if leftDirective == MergePrimitivesListDirective && rightDirective == rightDirective { isForListOfPrimitives = true } } diff --git a/test/fixtures/pkg/kubectl/cmd/apply/rc.yaml b/test/fixtures/pkg/kubectl/cmd/apply/rc.yaml index 1c1bf15be7..267c7148fb 100644 --- a/test/fixtures/pkg/kubectl/cmd/apply/rc.yaml +++ b/test/fixtures/pkg/kubectl/cmd/apply/rc.yaml @@ -1,6 +1,8 @@ apiVersion: v1 kind: ReplicationController metadata: + finalizers: + - b/b name: test-rc labels: name: test-rc