make kubectl edit handle lists again

pkg/kubectl/cmd/edit.go

@@ -23,11 +23,13 @@ import (
 	"io"
 	"os"
 	"path/filepath"
+	"reflect"
 	gruntime "runtime"
 	"strings"
 
 	"k8s.io/kubernetes/pkg/api"
 	"k8s.io/kubernetes/pkg/api/errors"
+	"k8s.io/kubernetes/pkg/api/meta"
 	"k8s.io/kubernetes/pkg/api/unversioned"
 	"k8s.io/kubernetes/pkg/kubectl"
 	cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
@@ -159,8 +161,8 @@ func RunEdit(f *cmdutil.Factory, out, errOut io.Writer, cmd *cobra.Command, args
 		NamespaceParam(cmdNamespace).DefaultNamespace().
 		FilenameParam(enforceNamespace, options.Recursive, options.Filenames...).
 		ResourceTypeOrNameArgs(true, args...).
-		Latest().
 		Flatten().
+		Latest().
 		Do()
 	err = r.Err()
 	if err != nil {
@@ -182,7 +184,7 @@ func RunEdit(f *cmdutil.Factory, out, errOut io.Writer, cmd *cobra.Command, args
 	if err != nil {
 		return err
 	}
-	objs, err := resource.AsVersionedObjects(infos, defaultVersion, encoder)
+	originalObj, err := resource.AsVersionedObject(infos, false, defaultVersion, encoder)
 	if err != nil {
 		return err
 	}
@@ -196,14 +198,16 @@ func RunEdit(f *cmdutil.Factory, out, errOut io.Writer, cmd *cobra.Command, args
 		file               string
 	)
 
-outter:
-	for i := range objs {
-		obj := objs[i]
-		// some bookkeeping
-		results.header.flush()
 	containsError := false
 
 	for {
+		// infos mutates over time to be the list of things we've tried and failed to edit
+		// this means that our overall list changes over time.
+		objToEdit, err := resource.AsVersionedObject(infos, false, defaultVersion, encoder)
+		if err != nil {
+			return err
+		}
+
 		// generate the file to edit
 		buf := &bytes.Buffer{}
 		var w io.Writer = buf
@@ -214,7 +218,7 @@ outter:
 			return preservedFile(err, results.file, errOut)
 		}
 		if !containsError {
-				if err := printer.PrintObj(obj, w); err != nil {
+			if err := printer.PrintObj(objToEdit, w); err != nil {
 				return preservedFile(err, results.file, errOut)
 			}
 			original = buf.Bytes()
@@ -238,7 +242,7 @@ outter:
 			// time. The second case is more usual so we can probably live with it.
 			// TODO: A less hacky fix would be welcome :)
 			fmt.Fprintln(errOut, "Edit cancelled, no valid changes were saved.")
-				continue outter
+			return nil
 		}
 
 		// cleanup any file from the previous pass
@@ -251,7 +255,7 @@ outter:
 		if bytes.Equal(stripComments(original), stripComments(edited)) {
 			os.Remove(file)
 			fmt.Fprintln(errOut, "Edit cancelled, no changes made.")
-				continue outter
+			return nil
 		}
 		lines, err := hasLines(bytes.NewBuffer(edited))
 		if err != nil {
@@ -260,7 +264,7 @@ outter:
 		if !lines {
 			os.Remove(file)
 			fmt.Fprintln(errOut, "Edit cancelled, saved file was empty.")
-				continue outter
+			return nil
 		}
 
 		results = editResults{
@@ -278,44 +282,105 @@ outter:
 		// not a syntax error as it turns out...
 		containsError = false
 
-			// put configuration annotation in "updates"
+		namespaceVisitor := resource.NewFlattenListVisitor(updates, resourceMapper)
-			if err := kubectl.CreateOrUpdateAnnotation(cmdutil.GetFlagBool(cmd, cmdutil.ApplyAnnotationsFlag), updates, encoder); err != nil {
+		// need to make sure the original namespace wasn't changed while editing
+		if err = namespaceVisitor.Visit(resource.RequireNamespace(cmdNamespace)); err != nil {
 			return preservedFile(err, file, errOut)
 		}
-			if cmdutil.ShouldRecord(cmd, updates) {
+
-				err = cmdutil.RecordChangeCause(updates.Object, f.Command())
+		mutatedObjects := []runtime.Object{}
-				if err != nil {
+		annotationVisitor := resource.NewFlattenListVisitor(updates, resourceMapper)
+		// iterate through all items to apply annotations
+		if err = annotationVisitor.Visit(func(info *resource.Info, incomingErr error) error {
+			// put configuration annotation in "updates"
+			if err := kubectl.CreateOrUpdateAnnotation(cmdutil.GetFlagBool(cmd, cmdutil.ApplyAnnotationsFlag), info, encoder); err != nil {
+				return err
+			}
+			if cmdutil.ShouldRecord(cmd, info) {
+				if err := cmdutil.RecordChangeCause(info.Object, f.Command()); err != nil {
 					return err
 				}
 			}
-			editedCopy := edited
+			mutatedObjects = append(mutatedObjects, info.Object)
-			if editedCopy, err = runtime.Encode(encoder, updates.Object); err != nil {
+
+			return nil
+
+		}); err != nil {
 			return preservedFile(err, file, errOut)
 		}
 
-			visitor := resource.NewFlattenListVisitor(updates, resourceMapper)
+		// if we mutated a list in the visitor, persist the changes on the overall object
-
+		if meta.IsListType(updates.Object) {
-			// need to make sure the original namespace wasn't changed while editing
+			meta.SetList(updates.Object, mutatedObjects)
-			if err = visitor.Visit(resource.RequireNamespace(cmdNamespace)); err != nil {
-				return preservedFile(err, file, errOut)
 		}
 
+		patchVisitor := resource.NewFlattenListVisitor(updates, resourceMapper)
+		err = patchVisitor.Visit(func(info *resource.Info, incomingErr error) error {
+			currOriginalObj := originalObj
+
+			// if we're editing a list, then navigate the list to find the item that we're currently trying to edit
+			if meta.IsListType(originalObj) {
+				currOriginalObj = nil
+				editObjUID, err := meta.NewAccessor().UID(info.Object)
+				if err != nil {
+					return err
+				}
+
+				listItems, err := meta.ExtractList(originalObj)
+				if err != nil {
+					return err
+				}
+
+				// iterate through the list to find the item with the matching UID
+				for i := range listItems {
+					originalObjUID, err := meta.NewAccessor().UID(listItems[i])
+					if err != nil {
+						return err
+					}
+					if editObjUID == originalObjUID {
+						currOriginalObj = listItems[i]
+						break
+					}
+				}
+				if currOriginalObj == nil {
+					return fmt.Errorf("no original object found for %#v", info.Object)
+				}
+
+			}
+
+			originalSerialization, err := runtime.Encode(encoder, currOriginalObj)
+			if err != nil {
+				return err
+			}
+			editedSerialization, err := runtime.Encode(encoder, info.Object)
+			if err != nil {
+				return err
+			}
+
+			// compute the patch on a per-item basis
 			// use strategic merge to create a patch
-			originalJS, err := yaml.ToJSON(original)
+			originalJS, err := yaml.ToJSON(originalSerialization)
 			if err != nil {
-				return preservedFile(err, file, errOut)
+				return err
 			}
-			editedJS, err := yaml.ToJSON(editedCopy)
+			editedJS, err := yaml.ToJSON(editedSerialization)
 			if err != nil {
-				return preservedFile(err, file, errOut)
+				return err
 			}
-			patch, err := strategicpatch.CreateStrategicMergePatch(originalJS, editedJS, obj)
+
+			if reflect.DeepEqual(originalJS, editedJS) {
+				// no edit, so just skip it.
+				cmdutil.PrintSuccess(mapper, false, out, info.Mapping.Resource, info.Name, "skipped")
+				return nil
+			}
+
+			patch, err := strategicpatch.CreateStrategicMergePatch(originalJS, editedJS, currOriginalObj)
 			// TODO: change all jsonmerge to strategicpatch
 			// for checking preconditions
 			preconditions := []jsonmerge.PreconditionFunc{}
 			if err != nil {
 				glog.V(4).Infof("Unable to calculate diff, no merge is possible: %v", err)
-				return preservedFile(err, file, errOut)
+				return err
 			} else {
 				preconditions = append(preconditions, jsonmerge.RequireKeyUnchanged("apiVersion"))
 				preconditions = append(preconditions, jsonmerge.RequireKeyUnchanged("kind"))
@@ -324,45 +389,49 @@ outter:
 			}
 
 			if hold, msg := jsonmerge.TestPreconditionsHold(patch, preconditions); !hold {
-				fmt.Fprintf(errOut, "error: %s\n", msg)
+				fmt.Fprintf(errOut, "error: %s", msg)
 				return preservedFile(nil, file, errOut)
 			}
 
-			errorMsg := ""
-			err = visitor.Visit(func(info *resource.Info, err error) error {
 			patched, err := resource.NewHelper(info.Client, info.Mapping).Patch(info.Namespace, info.Name, api.StrategicMergePatchType, patch)
 			if err != nil {
-					errorMsg = results.addError(err, info)
+				fmt.Fprintln(out, results.addError(err, info))
-					return err
+				return nil
 			}
 			info.Refresh(patched, true)
 			cmdutil.PrintSuccess(mapper, false, out, info.Mapping.Resource, info.Name, "edited")
 			return nil
 		})
-			if err == nil {
+		if err != nil {
-				os.Remove(file)
+			return preservedFile(err, results.file, errOut)
-				continue outter
 		}
+
 		// Handle all possible errors
 		//
 		// 1. retryable: propose kubectl replace -f
 		// 2. notfound: indicate the location of the saved configuration of the deleted resource
 		// 3. invalid: retry those on the spot by looping ie. reloading the editor
 		if results.retryable > 0 {
-				fmt.Fprintln(errOut, errorMsg)
 			fmt.Fprintf(errOut, "You can run `%s replace -f %s` to try this update again.\n", filepath.Base(os.Args[0]), file)
-				continue outter
+			return errExit
 		}
 		if results.notfound > 0 {
-				fmt.Fprintln(errOut, errorMsg)
 			fmt.Fprintf(errOut, "The edits you made on deleted resources have been saved to %q\n", file)
-				continue outter
+			return errExit
-			}
-			// validation error
-			containsError = true
 		}
+
+		if len(results.edit) == 0 {
+			if results.notfound == 0 {
+				os.Remove(file)
+			} else {
+				fmt.Fprintf(out, "The edits you made on deleted resources have been saved to %q\n", file)
 			}
 			return nil
+		}
+
+		// loop again and edit the remaining items
+		infos = results.edit
+	}
 }
 
 // editReason preserves a message about the reason this file must be edited again