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

add kubectl apply diff-last-applied subcommand #49174

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions build/visible_to/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,14 @@ package_group(
],
)

package_group(
name = "pkg_kubectl_cmd_util_diff_CONSUMERS",
packages = [
"//pkg/kubectl/cmd",
"//pkg/kubectl/cmd/util",
],
)

package_group(
name = "pkg_kubectl_cmd_util_jsonmerge_CONSUMERS",
packages = [
Expand Down
2 changes: 2 additions & 0 deletions docs/.generated_docs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ docs/man/man1/kube-scheduler.1
docs/man/man1/kubectl-alpha.1
docs/man/man1/kubectl-annotate.1
docs/man/man1/kubectl-api-versions.1
docs/man/man1/kubectl-apply-diff-last-applied.1
docs/man/man1/kubectl-apply-edit-last-applied.1
docs/man/man1/kubectl-apply-set-last-applied.1
docs/man/man1/kubectl-apply-view-last-applied.1
Expand Down Expand Up @@ -118,6 +119,7 @@ docs/user-guide/kubectl/kubectl.md
docs/user-guide/kubectl/kubectl_annotate.md
docs/user-guide/kubectl/kubectl_api-versions.md
docs/user-guide/kubectl/kubectl_apply.md
docs/user-guide/kubectl/kubectl_apply_diff-last-applied.md
docs/user-guide/kubectl/kubectl_apply_edit-last-applied.md
docs/user-guide/kubectl/kubectl_apply_set-last-applied.md
docs/user-guide/kubectl/kubectl_apply_view-last-applied.md
Expand Down
3 changes: 3 additions & 0 deletions docs/man/man1/kubectl-apply-diff-last-applied.1
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
This file is autogenerated, but we've stopped checking such files into the
repository to reduce the need for rebases. Please run hack/generate-docs.sh to
populate this file.
3 changes: 3 additions & 0 deletions docs/user-guide/kubectl/kubectl_apply_diff-last-applied.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
This file is autogenerated, but we've stopped checking such files into the
repository to reduce the need for rebases. Please run hack/generate-docs.sh to
populate this file.
3 changes: 3 additions & 0 deletions pkg/kubectl/cmd/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ go_library(
"annotate.go",
"apiversions.go",
"apply.go",
"apply_diff_last_applied.go",
"apply_edit_last_applied.go",
"apply_set_last_applied.go",
"apply_view_last_applied.go",
Expand Down Expand Up @@ -83,6 +84,7 @@ go_library(
"//pkg/kubectl/cmd/set:go_default_library",
"//pkg/kubectl/cmd/templates:go_default_library",
"//pkg/kubectl/cmd/util:go_default_library",
"//pkg/kubectl/cmd/util/diff:go_default_library",
"//pkg/kubectl/cmd/util/editor:go_default_library",
"//pkg/kubectl/cmd/util/openapi:go_default_library",
"//pkg/kubectl/explain:go_default_library",
Expand Down Expand Up @@ -212,6 +214,7 @@ go_test(
"//pkg/kubectl/util/term:go_default_library",
"//pkg/printers:go_default_library",
"//pkg/printers/internalversion:go_default_library",
"//pkg/util/file:go_default_library",
"//pkg/util/strings:go_default_library",
"//vendor/github.com/spf13/cobra:go_default_library",
"//vendor/github.com/stretchr/testify/assert:go_default_library",
Expand Down
1 change: 1 addition & 0 deletions pkg/kubectl/cmd/apply.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ func NewCmdApply(baseName string, f cmdutil.Factory, out, errOut io.Writer) *cob
cmd.AddCommand(NewCmdApplyViewLastApplied(f, out, errOut))
cmd.AddCommand(NewCmdApplySetLastApplied(f, out, errOut))
cmd.AddCommand(NewCmdApplyEditLastApplied(f, out, errOut))
cmd.AddCommand(NewCmdApplyDiffLastApplied(f, out, errOut))

return cmd
}
Expand Down
62 changes: 62 additions & 0 deletions pkg/kubectl/cmd/apply_diff_last_applied.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
/*
Copyright 2017 The Kubernetes Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package cmd

import (
"io"

"github.com/spf13/cobra"

"k8s.io/kubernetes/pkg/kubectl"
"k8s.io/kubernetes/pkg/kubectl/cmd/templates"
cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
"k8s.io/kubernetes/pkg/kubectl/cmd/util/diff"
"k8s.io/kubernetes/pkg/kubectl/util/i18n"
)

var (
applyDiffLastAppliedLong = templates.LongDesc(i18n.T(`
Opens up a 2-way diff in the default diff viewer. This should follow the same semantics as 'git diff'.
It should accept an environment variable KUBECTL_EXTERNAL_DIFF=meld to specify a custom diff tool.
Copy link
Member

Choose a reason for hiding this comment

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

It should have flag --diff-viewer as described in the proposal.
It should take either KUBECTL_EXTERNAL_DIFF or --diff-viewer. If they are both specified, it should check if they match. If not, error out.

Copy link
Contributor Author

@shiywang shiywang Jul 19, 2017

Choose a reason for hiding this comment

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

I already add that flag, still need to add some tests and update help information, if there's some scenario I haven't consider in or code could be simplify, please comment on it, thanks

If not specified, the 'git diff' command should be used, if 'git diff' not found, it will degenerate to use diff.
`))

applyDiffLastAppliedExample = templates.Examples(i18n.T(`
# Diff the last-applied-configuration annotations with input file
kubectl apply diff-last-applied -f deploy.yaml
`))
)

func NewCmdApplyDiffLastApplied(f cmdutil.Factory, out, err io.Writer) *cobra.Command {
options := &diff.Options{Out: out, ErrOut: err}
cmd := &cobra.Command{
Use: "diff-last-applied -f FILENAME",
Short: i18n.T("Diff the last-applied-configuration annotation on a live object to match the contents of a file."),
Long: applyDiffLastAppliedLong,
Example: applyDiffLastAppliedExample,
Run: func(cmd *cobra.Command, args []string) {
cmdutil.CheckErr(options.Complete(f, cmd))
cmdutil.CheckErr(options.Validate(f, cmd))
cmdutil.CheckErr(options.RunDiffLastApplied(f, cmd))
},
}
cmdutil.AddOutputVarFlagsForJsonYaml(cmd, &options.Output, "yaml")
usage := "that contains the last-applied-configuration annotations"
kubectl.AddJsonFilenameFlag(cmd, &options.FilenameOptions.Filenames, "Filename, directory, or URL to files "+usage)
Copy link
Contributor

Choose a reason for hiding this comment

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

What is going on with this usage variable?

Copy link
Contributor Author

@shiywang shiywang Aug 1, 2017

Choose a reason for hiding this comment

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

@mengqiy wrote this, my understanding is usage in AddFilenameOptionFlags can support different commands, when you add it to a new command, the only part you need to change is that usage variable, kind of decouple ?

Copy link
Contributor

Choose a reason for hiding this comment

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

I just don't understand why it's necessary to make it a separate variable if it's only used once as part of another string. @mengqiy can you help me understand? Thanks!

Copy link
Member

Choose a reason for hiding this comment

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

I wrote that abut one year ago. It may be not good now. Feel free to refactor it or just remove it if it is not helpful. :)


return cmd
}
2 changes: 1 addition & 1 deletion pkg/kubectl/cmd/apply_edit_last_applied.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ func NewCmdApplyEditLastApplied(f cmdutil.Factory, out, errOut io.Writer) *cobra

usage := "to use to edit the resource"
cmdutil.AddFilenameOptionFlags(cmd, &options.FilenameOptions, usage)
cmd.Flags().StringVarP(&options.Output, "output", "o", "yaml", "Output format. One of: yaml|json.")
cmdutil.AddOutputVarFlagsForJsonYaml(cmd, &options.Output, "yaml")
cmd.Flags().BoolVar(&options.WindowsLineEndings, "windows-line-endings", runtime.GOOS == "windows",
"Defaults to the line ending native to your platform.")
cmdutil.AddRecordVarFlag(cmd, &options.Record)
Expand Down
42 changes: 3 additions & 39 deletions pkg/kubectl/cmd/apply_set_last_applied.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,22 +18,19 @@ package cmd

import (
"bytes"
"encoding/json"
"fmt"
"io"

"github.com/ghodss/yaml"
"github.com/spf13/cobra"

"k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/api/meta"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
apijson "k8s.io/apimachinery/pkg/util/json"
"k8s.io/kubernetes/pkg/api"
"k8s.io/kubernetes/pkg/kubectl"
"k8s.io/kubernetes/pkg/kubectl/cmd/templates"
cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
"k8s.io/kubernetes/pkg/kubectl/cmd/util/diff"
"k8s.io/kubernetes/pkg/kubectl/cmd/util/editor"
"k8s.io/kubernetes/pkg/kubectl/resource"
"k8s.io/kubernetes/pkg/kubectl/util/i18n"
Expand Down Expand Up @@ -201,46 +198,13 @@ func (o *SetLastAppliedOptions) RunSetLastApplied(f cmdutil.Factory, cmd *cobra.
cmdutil.PrintSuccess(o.Mapper, o.ShortOutput, o.Out, info.Mapping.Resource, info.Name, o.DryRun, "configured")

} else {
err := o.formatPrinter(o.Output, patch.Patch, o.Out)
out, err := diff.FormatConvert(patch.Patch, o.Output)
if err != nil {
return err
}
fmt.Fprintf(o.Out, "%s\n", string(out))
cmdutil.PrintSuccess(o.Mapper, o.ShortOutput, o.Out, info.Mapping.Resource, info.Name, o.DryRun, "configured")
}
}
return nil
}

func (o *SetLastAppliedOptions) formatPrinter(output string, buf []byte, w io.Writer) error {
yamlOutput, err := yaml.JSONToYAML(buf)
if err != nil {
return err
}
switch output {
case "json":
jsonBuffer := &bytes.Buffer{}
err = json.Indent(jsonBuffer, buf, "", " ")
if err != nil {
return err
}
fmt.Fprintf(w, "%s\n", jsonBuffer.String())
case "yaml":
fmt.Fprintf(w, "%s\n", string(yamlOutput))
}
return nil
}

func (o *SetLastAppliedOptions) getPatch(info *resource.Info) ([]byte, []byte, error) {
objMap := map[string]map[string]map[string]string{}
metadataMap := map[string]map[string]string{}
annotationsMap := map[string]string{}
localFile, err := runtime.Encode(o.Codec, info.VersionedObject)
if err != nil {
return nil, localFile, err
}
annotationsMap[api.LastAppliedConfigAnnotation] = string(localFile)
metadataMap["annotations"] = annotationsMap
objMap["metadata"] = metadataMap
jsonString, err := apijson.Marshal(objMap)
return jsonString, localFile, err
}
116 changes: 107 additions & 9 deletions pkg/kubectl/cmd/apply_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import (
cmdtesting "k8s.io/kubernetes/pkg/kubectl/cmd/testing"
cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
"k8s.io/kubernetes/pkg/printers"
utilfile "k8s.io/kubernetes/pkg/util/file"
)

func TestApplyExtraArgsFail(t *testing.T) {
Expand All @@ -62,18 +63,24 @@ func validateApplyArgs(cmd *cobra.Command, args []string) error {
}

const (
filenameRC = "../../../test/fixtures/pkg/kubectl/cmd/apply/rc.yaml"
filenameRCNoAnnotation = "../../../test/fixtures/pkg/kubectl/cmd/apply/rc-no-annotation.yaml"
filenameRCLASTAPPLIED = "../../../test/fixtures/pkg/kubectl/cmd/apply/rc-lastapplied.yaml"
filenameSVC = "../../../test/fixtures/pkg/kubectl/cmd/apply/service.yaml"
filenameRCSVC = "../../../test/fixtures/pkg/kubectl/cmd/apply/rc-service.yaml"
filenameNoExistRC = "../../../test/fixtures/pkg/kubectl/cmd/apply/rc-noexist.yaml"
filenameRCPatchTest = "../../../test/fixtures/pkg/kubectl/cmd/apply/patch.json"
dirName = "../../../test/fixtures/pkg/kubectl/cmd/apply/testdir"
filenameRCJSON = "../../../test/fixtures/pkg/kubectl/cmd/apply/rc.json"
filenameRC = "../../../test/fixtures/pkg/kubectl/cmd/apply/rc.yaml"
filenameRCNoAnnotation = "../../../test/fixtures/pkg/kubectl/cmd/apply/rc-no-annotation.yaml"
filenameRCLASTAPPLIED = "../../../test/fixtures/pkg/kubectl/cmd/apply/rc-lastapplied.yaml"
filenameSVC = "../../../test/fixtures/pkg/kubectl/cmd/apply/service.yaml"
filenameRCSVC = "../../../test/fixtures/pkg/kubectl/cmd/apply/rc-service.yaml"
filenameNoExistRC = "../../../test/fixtures/pkg/kubectl/cmd/apply/rc-noexist.yaml"
filenameLocalChangedRC = "../../../test/fixtures/pkg/kubectl/cmd/apply/rc-changed.yaml"
filenameApplyDiffRESP = "../../../test/fixtures/pkg/kubectl/cmd/apply/rc-resp.json"
filenameApplyDiffChangedRESP = "../../../test/fixtures/pkg/kubectl/cmd/apply/rc-changed-resp.json"
filenameRCPatchTest = "../../../test/fixtures/pkg/kubectl/cmd/apply/patch.json"
dirName = "../../../test/fixtures/pkg/kubectl/cmd/apply/testdir"
filenameRCJSON = "../../../test/fixtures/pkg/kubectl/cmd/apply/rc.json"

filenameWidgetClientside = "../../../test/fixtures/pkg/kubectl/cmd/apply/widget-clientside.yaml"
filenameWidgetServerside = "../../../test/fixtures/pkg/kubectl/cmd/apply/widget-serverside.yaml"

fileNameGitDiffResult = "./git-diff.result"
fileNameNormalDiffResutl = "./diff.result"
)

func readBytesFromFile(t *testing.T, filename string) []byte {
Expand Down Expand Up @@ -991,6 +998,97 @@ func TestRunApplySetLastApplied(t *testing.T) {
cmdutil.BehaviorOnFatal(func(str string, code int) {})
}

func TestRunApplyDiffLastApplied(t *testing.T) {
initTestErrorHandler(t)
rcByte := readBytesFromFile(t, filenameApplyDiffRESP)
pathRC := "/namespaces/test/replicationcontrollers/" + "test-rc"

rcChangedByte := readBytesFromFile(t, filenameApplyDiffChangedRESP)

tests := []struct {
name, nameRC, pathRC, filePath, expectedGitDiffOut, expectedDiffOut, expectedErr, output string
resp []byte
}{
{
name: "git-diff objects with no different",
filePath: filenameRC,
expectedErr: "",
expectedGitDiffOut: "",
expectedDiffOut: "",
resp: rcByte,
output: "yaml",
},
{
name: "git-diff after local file changed with json",
filePath: filenameLocalChangedRC,
expectedErr: "",
expectedGitDiffOut: "- \"containerPort\": 80\n+ \"containerPort\": 123\n }\n",
expectedDiffOut: "< \"containerPort\": 80\n> \"containerPort\": 123\n",
resp: rcByte,
output: "json",
},
{
name: "git-diff after annotations changed",
filePath: filenameRC,
expectedErr: "",
expectedGitDiffOut: "- - containerPort: 123\n+ - containerPort: 80\n",
expectedDiffOut: "< - containerPort: 123\n> - containerPort: 80\n",
resp: rcChangedByte,
output: "yaml",
},
}
for _, test := range tests {
f, tf, _, _ := cmdtesting.NewAPIFactory()
tf.Printer = &testPrinter{}
tf.UnstructuredClient = &fake.RESTClient{
APIRegistry: api.Registry,
NegotiatedSerializer: unstructuredSerializer,
Client: fake.CreateHTTPClient(func(req *http.Request) (*http.Response, error) {
switch p, m := req.URL.Path, req.Method; {
case p == pathRC && m == "GET":
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not just use

if req.URL.Path == pathRC && req.Method == "GET" {
     ...
}
t.Fatalf(...)
return nil, nil

Copy link
Contributor Author

@shiywang shiywang Aug 1, 2017

Choose a reason for hiding this comment

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

because I follow the previous coding style, see here, here and here ...

Copy link
Contributor

Choose a reason for hiding this comment

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

Ha, good justification. I'm a little surprised that's our existing style.

bodyRC := ioutil.NopCloser(bytes.NewReader(test.resp))
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, errBuf := bytes.NewBuffer([]byte{}), bytes.NewBuffer([]byte{})

cmd := NewCmdApplyDiffLastApplied(f, buf, errBuf)
os.Setenv("KUBECTL_EXTERNAL_DIFF", "testdata/diff/test_diff.sh")

cmd.Flags().Set("filename", test.filePath)
cmd.Flags().Set("output", test.output)
cmd.Run(cmd, []string{})

pathExist, _ := utilfile.FileExists(fileNameGitDiffResult)
if pathExist {
out := string(readBytesFromFile(t, fileNameGitDiffResult))
if out != test.expectedGitDiffOut {
t.Fatalf("%s: unexpected output: %q\nexpected: %q", test.name, out, test.expectedGitDiffOut)
}
os.Remove(fileNameGitDiffResult)
continue
}

pathExist, _ = utilfile.FileExists(fileNameNormalDiffResutl)
if pathExist {
out := string(readBytesFromFile(t, fileNameNormalDiffResutl))
if out != test.expectedDiffOut {
t.Fatalf("%s: unexpected output: %q\nexpected: %q", test.name, out, test.expectedDiffOut)
}
os.Remove(fileNameNormalDiffResutl)
continue
}

t.Fatalf(`External test script failed. Check that either "diff" or "git diff" is installed.`)
}
}

func checkPatchString(t *testing.T, req *http.Request) {
checkString := string(readBytesFromFile(t, filenameRCPatchTest))
patch, err := ioutil.ReadAll(req.Body)
Expand Down
11 changes: 5 additions & 6 deletions pkg/kubectl/cmd/apply_view_last_applied.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,7 @@ func NewCmdApplyViewLastApplied(f cmdutil.Factory, out, err io.Writer) *cobra.Co
cmdutil.CheckErr(options.RunApplyViewLastApplied())
},
}

cmd.Flags().StringP("output", "o", "", "Output format. Must be one of yaml|json")
cmdutil.AddOutputFlagsForJsonYaml(cmd, "yaml")
cmd.Flags().StringVarP(&options.Selector, "selector", "l", "", "Selector (label query) to filter on, supports '=', '==', and '!='.(e.g. -l key1=value1,key2=value2)")
cmd.Flags().BoolVar(&options.All, "all", false, "Select all resources in the namespace of the specified resource types")
usage := "that contains the last-applied-configuration annotations"
Expand Down Expand Up @@ -138,12 +137,12 @@ func (o *ViewLastAppliedOptions) RunApplyViewLastApplied() error {
for _, str := range o.LastAppliedConfigurationList {
switch o.OutputFormat {
case "json":
jsonBuffer := &bytes.Buffer{}
err := json.Indent(jsonBuffer, []byte(str), "", " ")
prettyJSON := new(bytes.Buffer)
err := json.Indent(prettyJSON, []byte(str), "", " ")
if err != nil {
return err
}
fmt.Fprint(o.Out, string(jsonBuffer.Bytes()))
fmt.Fprint(o.Out, prettyJSON.String())
case "yaml":
yamlOutput, err := yaml.JSONToYAML([]byte(str))
if err != nil {
Expand All @@ -163,7 +162,7 @@ func (o *ViewLastAppliedOptions) ValidateOutputArgs(cmd *cobra.Command) error {
o.OutputFormat = "json"
return nil
// If flag -o is not specified, use yaml as default
case "yaml", "":
case "yaml":
o.OutputFormat = "yaml"
return nil
default:
Expand Down
Loading