Merge pull request #36148 from kargakis/edit-list

Automatic merge from submit-queue

kubectl: make edit work with lists again

@kubernetes/kubectl this is fixing https://github.com/kubernetes/kubernetes/issues/20519 and slightly changes the behavior of --recursive when the directory that is being edited has files with errors. Previously since `edit` was working on an object basis, bad objects would be skipped and the editor would load the next object. We want to load multiple objects in the same list and it's impossible to load invalid objects in a list so --recursive will not work if there is any error in the directory. I think this is an acceptable trade-off.

Review here: https://github.com/kubernetes/kubernetes/pull/36148/files?w=1
This commit is contained in:
Kubernetes Submit Queue 2016-11-03 17:27:13 -07:00 committed by GitHub
commit 6ac5887e8a
2 changed files with 160 additions and 157 deletions

View File

@ -865,6 +865,7 @@ __EOF__
[ "$(EDITOR=cat kubectl edit pod/valid-pod | grep 'name: valid-pod')" ] [ "$(EDITOR=cat kubectl edit pod/valid-pod | grep 'name: valid-pod')" ]
[ "$(EDITOR=cat kubectl edit --windows-line-endings pod/valid-pod | file - | grep CRLF)" ] [ "$(EDITOR=cat kubectl edit --windows-line-endings pod/valid-pod | file - | grep CRLF)" ]
[ ! "$(EDITOR=cat kubectl edit --windows-line-endings=false pod/valid-pod | file - | grep CRLF)" ] [ ! "$(EDITOR=cat kubectl edit --windows-line-endings=false pod/valid-pod | file - | grep CRLF)" ]
[ "$(EDITOR=cat kubectl edit ns | grep 'kind: List')" ]
### Label POD YAML file locally without effecting the live pod. ### Label POD YAML file locally without effecting the live pod.
# Pre-condition: name is valid-pod # Pre-condition: name is valid-pod
@ -1377,8 +1378,10 @@ __EOF__
echo -e '#!/bin/bash\nsed -i "s/image: busybox/image: prom\/busybox/g" $1' > /tmp/tmp-editor.sh echo -e '#!/bin/bash\nsed -i "s/image: busybox/image: prom\/busybox/g" $1' > /tmp/tmp-editor.sh
chmod +x /tmp/tmp-editor.sh chmod +x /tmp/tmp-editor.sh
output_message=$(! EDITOR=/tmp/tmp-editor.sh kubectl edit -f hack/testdata/recursive/pod --recursive 2>&1 "${kube_flags[@]}") output_message=$(! EDITOR=/tmp/tmp-editor.sh kubectl edit -f hack/testdata/recursive/pod --recursive 2>&1 "${kube_flags[@]}")
# Post-condition: busybox0 & busybox1 PODs are edited, and since busybox2 is malformed, it should error # Post-condition: busybox0 & busybox1 PODs are not edited, and since busybox2 is malformed, it should error
kube::test::get_object_assert pods "{{range.items}}{{$image_field}}:{{end}}" 'prom/busybox:prom/busybox:' # The reason why busybox0 & busybox1 PODs are not edited is because the editor tries to load all objects in
# a list but since it contains invalid objects, it will never open.
kube::test::get_object_assert pods "{{range.items}}{{$image_field}}:{{end}}" 'busybox:busybox:'
kube::test::if_has_string "${output_message}" "Object 'Kind' is missing" kube::test::if_has_string "${output_message}" "Object 'Kind' is missing"
# cleaning # cleaning
rm /tmp/tmp-editor.sh rm /tmp/tmp-editor.sh

View File

@ -143,181 +143,181 @@ func runEdit(f cmdutil.Factory, out, errOut io.Writer, cmd *cobra.Command, args
return err return err
} }
infos, err := r.Infos()
if err != nil {
return err
}
var ( var (
windowsLineEndings = cmdutil.GetFlagBool(cmd, "windows-line-endings") windowsLineEndings = cmdutil.GetFlagBool(cmd, "windows-line-endings")
edit = editor.NewDefaultEditor(f.EditorEnvs()) edit = editor.NewDefaultEditor(f.EditorEnvs())
) )
err = r.Visit(func(info *resource.Info, err error) error { var (
var ( results = editResults{}
results = editResults{} original = []byte{}
original = []byte{} edited = []byte{}
edited = []byte{} file string
file string )
)
containsError := false containsError := false
for {
originalObj, err := resource.AsVersionedObject(infos, false, defaultVersion, encoder)
if err != nil {
return err
}
for { objToEdit := originalObj
infos := []*resource.Info{info}
originalObj, err := resource.AsVersionedObject(infos, false, defaultVersion, encoder)
if err != nil {
return err
}
objToEdit := originalObj // generate the file to edit
buf := &bytes.Buffer{}
var w io.Writer = buf
if windowsLineEndings {
w = crlf.NewCRLFWriter(w)
}
// generate the file to edit if o.addHeader {
buf := &bytes.Buffer{} results.header.writeTo(w)
var w io.Writer = buf }
if windowsLineEndings {
w = crlf.NewCRLFWriter(w)
}
if o.addHeader { if !containsError {
results.header.writeTo(w) if err := o.printer.PrintObj(objToEdit, w); err != nil {
}
if !containsError {
if err := o.printer.PrintObj(objToEdit, w); err != nil {
return preservedFile(err, results.file, errOut)
}
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))
}
// launch the editor
editedDiff := edited
edited, file, err = edit.LaunchTempFile(fmt.Sprintf("%s-edit-", filepath.Base(os.Args[0])), o.ext, buf)
if err != nil {
return preservedFile(err, results.file, errOut) return preservedFile(err, results.file, errOut)
} }
if editMode == NormalEditMode || containsError { original = buf.Bytes()
if bytes.Equal(stripComments(editedDiff), stripComments(edited)) { } else {
// Ugly hack right here. We will hit this either (1) when we try to // In case of an error, preserve the edited file.
// save the same changes we tried to save in the previous iteration // Remove the comments (header) from it since we already
// which means our changes are invalid or (2) when we exit the second // have included the latest header in the buffer above.
// time. The second case is more usual so we can probably live with it. buf.Write(manualStrip(edited))
// TODO: A less hacky fix would be welcome :) }
return preservedFile(fmt.Errorf("%s", "Edit cancelled, no valid changes were saved."), file, errOut)
}
}
// cleanup any file from the previous pass // launch the editor
if len(results.file) > 0 { editedDiff := edited
os.Remove(results.file) edited, file, err = edit.LaunchTempFile(fmt.Sprintf("%s-edit-", filepath.Base(os.Args[0])), o.ext, buf)
if err != nil {
return preservedFile(err, results.file, errOut)
}
if editMode == NormalEditMode || containsError {
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 :)
return preservedFile(fmt.Errorf("%s", "Edit cancelled, no valid changes were saved."), file, errOut)
} }
glog.V(4).Infof("User edited:\n%s", string(edited)) }
// Apply validation // cleanup any file from the previous pass
schema, err := f.Validator(cmdutil.GetFlagBool(cmd, "validate"), cmdutil.GetFlagString(cmd, "schema-cache-dir")) if len(results.file) > 0 {
if err != nil { os.Remove(results.file)
return preservedFile(err, file, errOut) }
} glog.V(4).Infof("User edited:\n%s", string(edited))
err = schema.ValidateBytes(stripComments(edited))
if err != nil {
results = editResults{
file: file,
}
containsError = true
fmt.Fprintln(out, results.addError(errors.NewInvalid(api.Kind(""), "", field.ErrorList{field.Invalid(nil, "The edited file failed validation", fmt.Sprintf("%v", err))}), info))
continue
}
// Compare content without comments
if bytes.Equal(stripComments(original), stripComments(edited)) {
os.Remove(file)
fmt.Fprintln(errOut, "Edit cancelled, no changes made.")
return nil
}
lines, err := hasLines(bytes.NewBuffer(edited))
if err != nil {
return preservedFile(err, file, errOut)
}
if !lines {
os.Remove(file)
fmt.Fprintln(errOut, "Edit cancelled, saved file was empty.")
return nil
}
// Apply validation
schema, err := f.Validator(cmdutil.GetFlagBool(cmd, "validate"), cmdutil.GetFlagString(cmd, "schema-cache-dir"))
if err != nil {
return preservedFile(err, file, errOut)
}
err = schema.ValidateBytes(stripComments(edited))
if err != nil {
results = editResults{ results = editResults{
file: file, file: file,
} }
containsError = true
// parse the edited file fmt.Fprintln(out, results.addError(errors.NewInvalid(api.Kind(""), "", field.ErrorList{field.Invalid(nil, "The edited file failed validation", fmt.Sprintf("%v", err))}), infos[0]))
updates, err := resourceMapper.InfoForData(edited, "edited-file") continue
if err != nil {
// 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
namespaceVisitor := resource.NewFlattenListVisitor(updates, resourceMapper)
// 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)
}
// iterate through all items to apply annotations
mutatedObjects, err := visitAnnotation(cmd, f, updates, resourceMapper, encoder)
if err != nil {
return preservedFile(err, file, errOut)
}
// if we mutated a list in the visitor, persist the changes on the overall object
if meta.IsListType(updates.Object) {
meta.SetList(updates.Object, mutatedObjects)
}
switch editMode {
case NormalEditMode:
err = visitToPatch(originalObj, updates, mapper, resourceMapper, encoder, out, errOut, defaultVersion, &results, file)
case EditBeforeCreateMode:
err = visitToCreate(updates, mapper, resourceMapper, out, errOut, defaultVersion, &results, file)
default:
err = fmt.Errorf("Not supported edit mode %q", editMode)
}
if err != nil {
return preservedFile(err, results.file, errOut)
}
// 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(errOut, "You can run `%s replace -f %s` to try this update again.\n", filepath.Base(os.Args[0]), file)
return cmdutil.ErrExit
}
if results.notfound > 0 {
fmt.Fprintf(errOut, "The edits you made on deleted resources have been saved to %q\n", file)
return cmdutil.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)
}
return nil
}
if len(results.header.reasons) > 0 {
containsError = true
}
} }
})
return err // Compare content without comments
if bytes.Equal(stripComments(original), stripComments(edited)) {
os.Remove(file)
fmt.Fprintln(errOut, "Edit cancelled, no changes made.")
return nil
}
lines, err := hasLines(bytes.NewBuffer(edited))
if err != nil {
return preservedFile(err, file, errOut)
}
if !lines {
os.Remove(file)
fmt.Fprintln(errOut, "Edit cancelled, saved file was empty.")
return nil
}
results = editResults{
file: file,
}
// parse the edited file
updates, err := resourceMapper.InfoForData(edited, "edited-file")
if err != nil {
// 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
namespaceVisitor := resource.NewFlattenListVisitor(updates, resourceMapper)
// 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)
}
// iterate through all items to apply annotations
mutatedObjects, err := visitAnnotation(cmd, f, updates, resourceMapper, encoder)
if err != nil {
return preservedFile(err, file, errOut)
}
// if we mutated a list in the visitor, persist the changes on the overall object
if meta.IsListType(updates.Object) {
meta.SetList(updates.Object, mutatedObjects)
}
switch editMode {
case NormalEditMode:
err = visitToPatch(originalObj, updates, mapper, resourceMapper, encoder, out, errOut, defaultVersion, &results, file)
case EditBeforeCreateMode:
err = visitToCreate(updates, mapper, resourceMapper, out, errOut, defaultVersion, &results, file)
default:
err = fmt.Errorf("Not supported edit mode %q", editMode)
}
if err != nil {
return preservedFile(err, results.file, errOut)
}
// 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(errOut, "You can run `%s replace -f %s` to try this update again.\n", filepath.Base(os.Args[0]), file)
return cmdutil.ErrExit
}
if results.notfound > 0 {
fmt.Fprintf(errOut, "The edits you made on deleted resources have been saved to %q\n", file)
return cmdutil.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)
}
return nil
}
if len(results.header.reasons) > 0 {
containsError = true
}
}
} }
func getPrinter(cmd *cobra.Command) (*editPrinterOptions, error) { func getPrinter(cmd *cobra.Command) (*editPrinterOptions, error) {