diff --git a/pkg/kubectl/cmd/BUILD b/pkg/kubectl/cmd/BUILD index f82463d870c..ba6a9ef1a96 100644 --- a/pkg/kubectl/cmd/BUILD +++ b/pkg/kubectl/cmd/BUILD @@ -191,7 +191,6 @@ 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 2918f7894da..ab86ef0e101 100644 --- a/pkg/kubectl/cmd/apply_test.go +++ b/pkg/kubectl/cmd/apply_test.go @@ -23,7 +23,6 @@ import ( "io/ioutil" "net/http" "os" - "strings" "testing" "github.com/spf13/cobra" @@ -38,7 +37,6 @@ 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) { @@ -144,58 +142,6 @@ 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 { @@ -277,65 +223,6 @@ 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 ad85354cca4..d5452846c6b 100644 --- a/pkg/kubectl/cmd/testing/fake.go +++ b/pkg/kubectl/cmd/testing/fake.go @@ -427,12 +427,8 @@ func (f *fakeAPIFactory) UnstructuredObject() (meta.RESTMapper, runtime.ObjectTy return cmdutil.NewShortcutExpander(mapper, nil), typer, nil } -func (f *fakeAPIFactory) Decoder(toInternal bool) runtime.Decoder { - if toInternal { - return api.Codecs.UniversalDecoder() - } else { - return api.Codecs.UniversalDeserializer() - } +func (f *fakeAPIFactory) Decoder(bool) runtime.Decoder { + return testapi.Default.Codec() } func (f *fakeAPIFactory) JSONEncoder() runtime.Encoder { diff --git a/pkg/util/strategicpatch/patch.go b/pkg/util/strategicpatch/patch.go index b9c2c3ab882..442a176e943 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 267c7148fb1..1c1bf15be7f 100644 --- a/test/fixtures/pkg/kubectl/cmd/apply/rc.yaml +++ b/test/fixtures/pkg/kubectl/cmd/apply/rc.yaml @@ -1,8 +1,6 @@ apiVersion: v1 kind: ReplicationController metadata: - finalizers: - - b/b name: test-rc labels: name: test-rc