From f4658396eb280ac2889cf1f8d82165bca9429e99 Mon Sep 17 00:00:00 2001 From: Michail Kargakis Date: Tue, 2 Feb 2016 13:30:12 +0100 Subject: [PATCH] kubectl: a couple of edit fixes Fix some edit bugs: * Reload the file in case of syntax errors * Fix error format for validation errors * Avoid hotlooping in case of no changes or empty saves --- pkg/kubectl/cmd/edit.go | 174 +++++++++++++++++++++------------------- 1 file changed, 93 insertions(+), 81 deletions(-) diff --git a/pkg/kubectl/cmd/edit.go b/pkg/kubectl/cmd/edit.go index 7543c4a0e54..279cd9ecab0 100644 --- a/pkg/kubectl/cmd/edit.go +++ b/pkg/kubectl/cmd/edit.go @@ -19,7 +19,6 @@ package cmd import ( "bufio" "bytes" - "encoding/json" "fmt" "io" "os" @@ -151,24 +150,32 @@ func RunEdit(f *cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []strin } encoder := f.JSONEncoder() - - windowsLineEndings := cmdutil.GetFlagBool(cmd, "windows-line-endings") - edit := editor.NewDefaultEditor(f.EditorEnvs()) defaultVersion, err := cmdutil.OutputVersion(cmd, clientConfig.GroupVersion) if err != nil { return err } - results := editResults{} - for { - objs, err := resource.AsVersionedObjects(infos, defaultVersion.String(), encoder) - if err != nil { - return preservedFile(err, results.file, out) - } - // if input object is a list, traverse and edit each item one at a time - for _, obj := range objs { - // TODO: add an annotating YAML printer that can print inline comments on each field, - // including descriptions or validation errors + objs, err := resource.AsVersionedObjects(infos, defaultVersion.String(), encoder) + if err != nil { + return err + } + var ( + windowsLineEndings = cmdutil.GetFlagBool(cmd, "windows-line-endings") + edit = editor.NewDefaultEditor(f.EditorEnvs()) + results = editResults{} + original = []byte{} + edited = []byte{} + file string + ) + +outter: + for i := range objs { + obj := objs[i] + // some bookkeeping + results.header.flush() + containsError := false + + for { // generate the file to edit buf := &bytes.Buffer{} var w io.Writer = buf @@ -178,45 +185,54 @@ func RunEdit(f *cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []strin if err := results.header.writeTo(w); err != nil { return preservedFile(err, results.file, out) } - if err := printer.PrintObj(obj, w); err != nil { - return preservedFile(err, results.file, out) + if !containsError { + if err := printer.PrintObj(obj, w); err != nil { + return preservedFile(err, results.file, out) + } + original = buf.Bytes() + } else { + // In case of an error, preserve the edited file. + // Remove the comments (header) from it since we already + // have included the latest header in the buffer above. + buf.Write(manualStrip(edited)) } - original := buf.Bytes() // launch the editor - edited, file, err := edit.LaunchTempFile("kubectl-edit-", ext, buf) + editedDiff := edited + edited, file, err = edit.LaunchTempFile(fmt.Sprintf("%s-edit-", os.Args[0]), ext, buf) if err != nil { return preservedFile(err, results.file, out) } + if bytes.Equal(stripComments(editedDiff), stripComments(edited)) { + // Ugly hack right here. We will hit this either (1) when we try to + // save the same changes we tried to save in the previous iteration + // which means our changes are invalid or (2) when we exit the second + // time. The second case is more usual so we can probably live with it. + // TODO: A less hacky fix would be welcome :) + fmt.Fprintln(out, "Edit cancelled, no valid changes were saved.") + continue outter + } // cleanup any file from the previous pass if len(results.file) > 0 { os.Remove(results.file) } - glog.V(4).Infof("User edited:\n%s", string(edited)) + + // Compare content without comments + if bytes.Equal(stripComments(original), stripComments(edited)) { + os.Remove(file) + fmt.Fprintln(out, "Edit cancelled, no changes made.") + continue outter + } lines, err := hasLines(bytes.NewBuffer(edited)) if err != nil { return preservedFile(err, file, out) } - // Compare content without comments - if bytes.Equal(stripComments(original), stripComments(edited)) { - if len(results.edit) > 0 { - preservedFile(nil, file, out) - } else { - os.Remove(file) - } - fmt.Fprintln(out, "Edit cancelled, no changes made.") - continue - } if !lines { - if len(results.edit) > 0 { - preservedFile(nil, file, out) - } else { - os.Remove(file) - } + os.Remove(file) fmt.Fprintln(out, "Edit cancelled, saved file was empty.") - continue + continue outter } results = editResults{ @@ -226,8 +242,13 @@ func RunEdit(f *cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []strin // parse the edited file updates, err := resourceMapper.InfoForData(edited, "edited-file") if err != nil { - return fmt.Errorf("The edited file had a syntax error: %v", err) + // syntax error + containsError = true + results.header.reasons = append(results.header.reasons, editReason{head: fmt.Sprintf("The edited file had a syntax error: %v", err)}) + continue } + // not a syntax error as it turns out... + containsError = false // put configuration annotation in "updates" if err := kubectl.CreateOrUpdateAnnotation(cmdutil.GetFlagBool(cmd, cmdutil.ApplyAnnotationsFlag), updates, encoder); err != nil { @@ -239,8 +260,8 @@ func RunEdit(f *cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []strin return err } } - // encode updates back to "edited" since we'll only generate patch from "edited" - if edited, err = runtime.Encode(encoder, updates.Object); err != nil { + editedCopy := edited + if editedCopy, err = runtime.Encode(encoder, updates.Object); err != nil { return preservedFile(err, file, out) } @@ -256,7 +277,7 @@ func RunEdit(f *cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []strin if err != nil { return preservedFile(err, file, out) } - editedJS, err := yaml.ToJSON(edited) + editedJS, err := yaml.ToJSON(editedCopy) if err != nil { return preservedFile(err, file, out) } @@ -282,53 +303,37 @@ func RunEdit(f *cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []strin 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 { - fmt.Fprintln(out, results.addError(err, info)) - return nil + glog.V(4).Infof(results.addError(err, info)) + return err } info.Refresh(patched, true) cmdutil.PrintSuccess(mapper, false, out, info.Mapping.Resource, info.Name, "edited") return nil }) - if err != nil { - return preservedFile(err, file, out) + if err == nil { + os.Remove(file) + 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.Fprintf(out, "You can run `kubectl replace -f %s` to try this update again.\n", file) - return errExit + fmt.Fprintf(out, "You can run `%s replace -f %s` to try this update again.\n", os.Args[0], file) + continue outter } - if results.conflict > 0 { - fmt.Fprintf(out, "You must update your local resource version and run `kubectl replace -f %s` to overwrite the remote changes.\n", file) - return errExit - } - 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) - } + if results.notfound > 0 { + fmt.Fprintf(out, "The edits you made on deleted resources have been saved to %q\n", file) + continue outter } + // validation error + containsError = true } - if len(results.edit) == 0 { - return nil - } - - // loop again and edit the remaining items - infos = results.edit } return nil } -// print json file (such as patch file) content for debugging -func printJson(out io.Writer, file []byte) error { - diff := make(map[string]interface{}) - if err := json.Unmarshal(file, &diff); err != nil { - return err - } - fmt.Fprintf(out, "%v\n", diff) - return nil -} - // editReason preserves a message about the reason this file must be edited again type editReason struct { head string @@ -361,12 +366,15 @@ func (h *editHeader) writeTo(w io.Writer) error { return nil } +func (h *editHeader) flush() { + h.reasons = []editReason{} +} + // editResults capture the result of an update type editResults struct { header editHeader retryable int notfound int - conflict int edit []*resource.Info file string @@ -378,23 +386,23 @@ func (r *editResults) addError(err error, info *resource.Info) string { case errors.IsInvalid(err): r.edit = append(r.edit, info) reason := editReason{ - head: fmt.Sprintf("%s %s was not valid", info.Mapping.Kind, info.Name), + head: fmt.Sprintf("%s %q was not valid", info.Mapping.Resource, info.Name), } if err, ok := err.(errors.APIStatus); ok { if details := err.Status().Details; details != nil { for _, cause := range details.Causes { - reason.other = append(reason.other, cause.Message) + reason.other = append(reason.other, fmt.Sprintf("%s: %s", cause.Field, cause.Message)) } } } r.header.reasons = append(r.header.reasons, reason) - return fmt.Sprintf("Error: the %s %s is invalid", info.Mapping.Kind, info.Name) + return fmt.Sprintf("Error: %s %q is invalid", info.Mapping.Resource, info.Name) case errors.IsNotFound(err): r.notfound++ - return fmt.Sprintf("Error: the %s %s could not be found on the server", info.Mapping.Kind, info.Name) + return fmt.Sprintf("Error: %s %q could not be found on the server", info.Mapping.Resource, info.Name) default: r.retryable++ - return fmt.Sprintf("Error: the %s %s could not be patched: %v", info.Mapping.Kind, info.Name, err) + return fmt.Sprintf("Error: %s %q could not be patched: %v", info.Mapping.Resource, info.Name, err) } } @@ -432,7 +440,8 @@ func hasLines(r io.Reader) (bool, error) { // in it. Note that if the given file has a syntax error, the transformation will // fail and we will manually drop all comments from the file. func stripComments(file []byte) []byte { - stripped, err := yaml.ToJSON(file) + stripped := file + stripped, err := yaml.ToJSON(stripped) if err != nil { stripped = manualStrip(file) } @@ -442,12 +451,15 @@ func stripComments(file []byte) []byte { // manualStrip is used for dropping comments from a YAML file func manualStrip(file []byte) []byte { stripped := []byte{} - for _, line := range bytes.Split(file, []byte("\n")) { + lines := bytes.Split(file, []byte("\n")) + for i, line := range lines { if bytes.HasPrefix(bytes.TrimSpace(line), []byte("#")) { continue } stripped = append(stripped, line...) - stripped = append(stripped, '\n') + if i < len(lines)-1 { + stripped = append(stripped, '\n') + } } return stripped }