Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed the error for kubectl edit multiple resources #15980

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
20 changes: 20 additions & 0 deletions hack/test-cmd.sh
Original file line number Diff line number Diff line change
Expand Up @@ -910,6 +910,26 @@ __EOF__
kube::test::get_object_assert 'rc mock2' "{{${labels_field}.status}}" 'replaced'
fi
fi
# Command: kubectl edit multiple resources
temp_editor="${KUBE_TEMP}/tmp-editor.sh"
echo -e '#!/bin/bash\nsed -i "s/status\:\ replaced/status\:\ edited/g" $@' > "${temp_editor}"
chmod +x "${temp_editor}"
EDITOR="${temp_editor}" kubectl edit "${kube_flags[@]}" -f "${file}"
# Post-condition: mock service (and mock2) and mock rc (and mock2) are edited
if [ "$has_svc" = true ]; then
kube::test::get_object_assert 'services mock' "{{${labels_field}.status}}" 'edited'
if [ "$two_svcs" = true ]; then
kube::test::get_object_assert 'services mock2' "{{${labels_field}.status}}" 'edited'
fi
fi
if [ "$has_rc" = true ]; then
kube::test::get_object_assert 'rc mock' "{{${labels_field}.status}}" 'edited'
if [ "$two_rcs" = true ]; then
kube::test::get_object_assert 'rc mock2' "{{${labels_field}.status}}" 'edited'
fi
fi
# cleaning
rm "${temp_editor}"
# Command
# We need to set --overwrite, because otherwise, if the first attempt to run "kubectl label"
# fails on some, but not all, of the resources, retries will fail because it tries to modify
Expand Down
4 changes: 4 additions & 0 deletions pkg/api/meta/meta.go
Original file line number Diff line number Diff line change
Expand Up @@ -402,6 +402,10 @@ func (a genericAccessor) Annotations() map[string]string {
}

func (a genericAccessor) SetAnnotations(annotations map[string]string) {
if a.annotations == nil {
emptyAnnotations := make(map[string]string)
a.annotations = &emptyAnnotations
}
*a.annotations = annotations
}

Expand Down
252 changes: 140 additions & 112 deletions pkg/kubectl/cmd/edit.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,139 +145,143 @@ func RunEdit(f *cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []strin
defaultVersion := cmdutil.OutputVersion(cmd, clientConfig.Version)
results := editResults{}
for {
obj, err := resource.AsVersionedObject(infos, false, defaultVersion)
objs, err := resource.AsVersionedObjects(infos, defaultVersion)
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

// generate the file to edit
buf := &bytes.Buffer{}
if err := results.header.writeTo(buf); err != nil {
return preservedFile(err, results.file, out)
}
if err := printer.PrintObj(obj, buf); err != nil {
return preservedFile(err, results.file, out)
}
original := buf.Bytes()

// TODO: add an annotating YAML printer that can print inline comments on each field,
// including descriptions or validation errors

// generate the file to edit
buf := &bytes.Buffer{}
if err := results.header.writeTo(buf); err != nil {
return preservedFile(err, results.file, out)
}
if err := printer.PrintObj(obj, buf); err != nil {
return preservedFile(err, results.file, out)
}
original := buf.Bytes()

// launch the editor
edit := editor.NewDefaultEditor()
edited, file, err := edit.LaunchTempFile("kubectl-edit-", ext, buf)
if err != nil {
return preservedFile(err, results.file, out)
}
// launch the editor
edit := editor.NewDefaultEditor()
edited, file, err := edit.LaunchTempFile("kubectl-edit-", ext, buf)
if err != nil {
return preservedFile(err, results.file, out)
}

// cleanup any file from the previous pass
if len(results.file) > 0 {
os.Remove(results.file)
}
// 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))
fmt.Printf("User edited:\n%s", string(edited))
lines, err := hasLines(bytes.NewBuffer(edited))
if err != nil {
return preservedFile(err, file, out)
}
if bytes.Equal(original, edited) {
if len(results.edit) > 0 {
preservedFile(nil, file, out)
} else {
os.Remove(file)
glog.V(4).Infof("User edited:\n%s", string(edited))
lines, err := hasLines(bytes.NewBuffer(edited))
if err != nil {
return preservedFile(err, file, out)
}
fmt.Fprintln(out, "Edit cancelled, no changes made.")
return nil
}
if !lines {
if len(results.edit) > 0 {
preservedFile(nil, file, out)
} else {
os.Remove(file)
// 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)
}
fmt.Fprintln(out, "Edit cancelled, saved file was empty.")
continue
}
fmt.Fprintln(out, "Edit cancelled, saved file was empty.")
return nil
}

results = editResults{
file: file,
}
results = editResults{
file: file,
}

// parse the edited file
updates, err := rmap.InfoForData(edited, "edited-file")
if err != nil {
return preservedFile(err, file, out)
}
// parse the edited file
updates, err := rmap.InfoForData(edited, "edited-file")
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When we edit a list of objects (edited is a []byte type List data), InfoForData ignores the changes users made in each Item of the List, and therefore the changes are never patched when the users edit a list. So instead, I parse infos as []runtime.Object in L148 and the users now need to traverse/edit them one by one (not editing the List directly).
@smarterclayton @deads2k Do you also see this in Openshift origin's edit?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@smarterclayton @deads2k Do you also see this in Openshift origin's edit?

I'm been able to edit lists with oc edit.

if err != nil {
return fmt.Errorf("The edited file had a syntax error: %v", err)
}

// annotate the edited object for kubectl apply
if err := kubectl.UpdateApplyAnnotation(updates); err != nil {
return preservedFile(err, file, out)
}
// annotate the edited object for kubectl apply
if err := kubectl.UpdateApplyAnnotation(updates); err != nil {
return preservedFile(err, file, out)
}

visitor := resource.NewFlattenListVisitor(updates, rmap)
visitor := resource.NewFlattenListVisitor(updates, rmap)

// need to make sure the original namespace wasn't changed while editing
if err = visitor.Visit(resource.RequireNamespace(cmdNamespace)); err != nil {
return preservedFile(err, file, out)
}
// need to make sure the original namespace wasn't changed while editing
if err = visitor.Visit(resource.RequireNamespace(cmdNamespace)); err != nil {
return preservedFile(err, file, out)
}

// use strategic merge to create a patch
originalJS, err := yaml.ToJSON(original)
if err != nil {
return preservedFile(err, file, out)
}
editedJS, err := yaml.ToJSON(edited)
if err != nil {
return preservedFile(err, file, out)
}
patch, err := strategicpatch.CreateStrategicMergePatch(originalJS, editedJS, obj)
// 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, out)
} else {
preconditions = append(preconditions, jsonmerge.RequireKeyUnchanged("apiVersion"))
preconditions = append(preconditions, jsonmerge.RequireKeyUnchanged("kind"))
preconditions = append(preconditions, jsonmerge.RequireMetadataKeyUnchanged("name"))
results.version = defaultVersion
}
// use strategic merge to create a patch
originalJS, err := yaml.ToJSON(original)
if err != nil {
return preservedFile(err, file, out)
}
editedJS, err := yaml.ToJSON(edited)
if err != nil {
return preservedFile(err, file, out)
}
patch, err := strategicpatch.CreateStrategicMergePatch(originalJS, editedJS, obj)
// 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, out)
} else {
preconditions = append(preconditions, jsonmerge.RequireKeyUnchanged("apiVersion"))
preconditions = append(preconditions, jsonmerge.RequireKeyUnchanged("kind"))
preconditions = append(preconditions, jsonmerge.RequireMetadataKeyUnchanged("name"))
results.version = defaultVersion
}

if hold, msg := jsonmerge.TestPreconditionsHold(patch, preconditions); !hold {
fmt.Fprintf(out, "error: %s", msg)
return preservedFile(nil, file, out)
}
if hold, msg := jsonmerge.TestPreconditionsHold(patch, preconditions); !hold {
fmt.Fprintf(out, "error: %s", msg)
return preservedFile(nil, file, out)
}

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))
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
}
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)
}
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 results.retryable > 0 {
fmt.Fprintf(out, "You can run `kubectl replace -f %s` to try this update again.\n", file)
return errExit
}
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 results.retryable > 0 {
fmt.Fprintf(out, "You can run `kubectl replace -f %s` to try this update again.\n", file)
return errExit
}
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 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
}

Expand Down Expand Up @@ -395,3 +399,27 @@ func hasLines(r io.Reader) (bool, error) {
}
return false, nil
}

// stripComments will transform a YAML file into JSON, thus dropping any comments
// 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)
if err != nil {
stripped = manualStrip(file)
}
return stripped
}

// 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")) {
if bytes.HasPrefix(bytes.TrimSpace(line), []byte("#")) {
continue
}
stripped = append(stripped, line...)
stripped = append(stripped, '\n')
}
return stripped
}