From a5e6dcb43410dc548558fed112a9b5e9dc1d8902 Mon Sep 17 00:00:00 2001 From: Alejandro Escobar Date: Thu, 12 Jan 2017 23:55:37 -0800 Subject: [PATCH] addressing issue #39427 adding a flag --output to allow for either json or yaml. updating with PR changes requested. latest changes to having short for human readable only, and error cases moved a bit to the end. rebase fixes latest pr. changes. small change moving return nil out of switch. updated the nil check for the error in the humanreadable case. more optimization in humanreadable code. pushed up current test changes, this is purely temporary finished writing tests updated test and function names. changed output extensions from .sh to output. updated version, version struct now just called Version and not VersionObj. made a few changes to testing. fixed testing issues, created better test and cleanup go format change. --- hack/lib/test.sh | 97 ++++++++++++++++++++++++++++++++ hack/make-rules/test-cmd-util.sh | 47 ++++++++++++++++ pkg/kubectl/cmd/BUILD | 1 + pkg/kubectl/cmd/version.go | 88 ++++++++++++++++++++++------- 4 files changed, 213 insertions(+), 20 deletions(-) diff --git a/hack/lib/test.sh b/hack/lib/test.sh index 901777876d6..575664a4dc0 100644 --- a/hack/lib/test.sh +++ b/hack/lib/test.sh @@ -303,3 +303,100 @@ kube::test::if_supports_resource() { done return 1 } + + +kube::test::version::object_to_file() { + name=$1 + flags=${2:-""} + file=$3 + kubectl version $flags | grep "$name Version:" | sed -e s/"$name Version: version.Info{"/'/' -e s/'}'/'/' -e s/', '/','/g -e s/':'/'=/g' -e s/'"'/""/g | tr , '\n' > "${file}" +} + +kube::test::version::json_object_to_file() { + flags=$1 + file=$2 + kubectl version $flags --output json | sed -e s/'\"'/''/g -e s/'}'/''/g -e s/'{'/''/g -e s/'clientVersion:'/'clientVersion:,'/ -e s/'serverVersion:'/'serverVersion:,'/ | tr , '\n' > "${file}" +} + +kube::test::version::json_client_server_object_to_file() { + flags=$1 + name=$2 + file=$3 + kubectl version $flags --output json | jq -r ".${name}" | sed -e s/'\"'/''/g -e s/'}'/''/g -e s/'{'/''/g -e /^$/d -e s/','/''/g -e s/':'/'='/g > "${file}" +} + +kube::test::version::yaml_object_to_file() { + flags=$1 + file=$2 + kubectl version $flags --output yaml | sed -e s/' '/''/g -e s/'\"'/''/g -e /^$/d > "${file}" +} + +kube::test::version::diff_assert() { + local original=$1 + local comparator=${2:-"eq"} + local latest=$3 + local diff_msg=${4:-""} + local res="" + + if [ ! -f $original ]; then + echo ${bold}${red} + echo "FAIL! ${diff_msg}" + echo "the file '${original}' does not exit" + echo ${reset}${red} + caller + echo ${reset} + return 1 + fi + + if [ ! -f $latest ]; then + echo ${bold}${red} + echo "FAIL! ${diff_msg}" + echo "the file '${latest}' does not exit" + echo ${reset}${red} + caller + echo ${reset} + return 1 + fi + + sort ${original} > "${original}.sorted" + sort ${latest} > "${latest}.sorted" + + if [ "$comparator" == "eq" ]; then + if [ "$(diff -iwB ${original}.sorted ${latest}.sorted)" == "" ] ; then + echo -n ${green} + echo "Successful: ${diff_msg}" + echo -n ${reset} + return 0 + else + echo ${bold}${red} + echo "FAIL! ${diff_msg}" + echo " Expected: " + echo "$(cat ${original})" + echo " Got: " + echo "$(cat ${latest})" + echo ${reset}${red} + caller + echo ${reset} + return 1 + fi + else + if [ ! -z "$(diff -iwB ${original}.sorted ${latest}.sorted)" ] ; then + echo -n ${green} + echo "Successful: ${diff_msg}" + echo -n ${reset} + return 0 + else + echo ${bold}${red} + echo "FAIL! ${diff_msg}" + echo " Expected: " + echo "$(cat ${original})" + echo " Got: " + echo "$(cat ${latest})" + echo ${reset}${red} + caller + echo ${reset} + return 1 + fi + fi +} + diff --git a/hack/make-rules/test-cmd-util.sh b/hack/make-rules/test-cmd-util.sh index 903b8b0ac69..d79792e777c 100644 --- a/hack/make-rules/test-cmd-util.sh +++ b/hack/make-rules/test-cmd-util.sh @@ -223,6 +223,48 @@ setup() { kube::log::status "Setup complete" } +######################################################## +# Kubectl version (--short, --client, --output) # +######################################################## +run_kubectl_version_tests() { + kube::log::status "Testing kubectl version" + TEMP="${KUBE_TEMP}" + + # create version files, one for the client, one for the server. + # these are the files we will use to ensure that the remainder output is correct + kube::test::version::object_to_file "Client" "" "${TEMP}/client_version_test" + kube::test::version::object_to_file "Server" "" "${TEMP}/server_version_test" + + kube::log::status "Testing kubectl version: check client only output matches expected output" + kube::test::version::object_to_file "Client" "--client" "${TEMP}/client_only_version_test" + kube::test::version::object_to_file "Client" "--client" "${TEMP}/server_client_only_version_test" + kube::test::version::diff_assert "${TEMP}/client_version_test" "eq" "${TEMP}/client_only_version_test" "the flag '--client' shows correct client info" + kube::test::version::diff_assert "${TEMP}/server_version_test" "ne" "${TEMP}/server_client_only_version_test" "the flag '--client' correctly has no server version info" + + kube::log::status "Testing kubectl version: verify json output" + kube::test::version::json_client_server_object_to_file "" "clientVersion" "${TEMP}/client_json_version_test" + kube::test::version::json_client_server_object_to_file "" "serverVersion" "${TEMP}/server_json_version_test" + kube::test::version::diff_assert "${TEMP}/client_version_test" "eq" "${TEMP}/client_json_version_test" "--output json has correct client info" + kube::test::version::diff_assert "${TEMP}/server_version_test" "eq" "${TEMP}/server_json_version_test" "--output json has correct server info" + + kube::log::status "Testing kubectl version: verify json output using additional --client flag does not contain serverVersion" + kube::test::version::json_client_server_object_to_file "--client" "clientVersion" "${TEMP}/client_only_json_version_test" + kube::test::version::json_client_server_object_to_file "--client" "serverVersion" "${TEMP}/server_client_only_json_version_test" + kube::test::version::diff_assert "${TEMP}/client_version_test" "eq" "${TEMP}/client_only_json_version_test" "--client --output json has correct client info" + kube::test::version::diff_assert "${TEMP}/server_version_test" "ne" "${TEMP}/server_client_only_json_version_test" "--client --output json has no server info" + + kube::log::status "Testing kubectl version: compare json output using additional --short flag" + kube::test::version::json_client_server_object_to_file "--short" "clientVersion" "${TEMP}/client_short_json_version_test" + kube::test::version::json_client_server_object_to_file "--short" "serverVersion" "${TEMP}/server_short_json_version_test" + kube::test::version::diff_assert "${TEMP}/client_version_test" "eq" "${TEMP}/client_short_json_version_test" "--short --output client json info is equal to non short result" + kube::test::version::diff_assert "${TEMP}/server_version_test" "eq" "${TEMP}/server_short_json_version_test" "--short --output server json info is equal to non short result" + + kube::log::status "Testing kubectl version: compare json output with yaml output" + kube::test::version::json_object_to_file "" "${TEMP}/client_server_json_version_test" + kube::test::version::yaml_object_to_file "" "${TEMP}/client_server_yaml_version_test" + kube::test::version::diff_assert "${TEMP}/client_server_json_version_test" "eq" "${TEMP}/client_server_yaml_version_test" "--output json/yaml has identical information" +} + # Runs all pod related tests. run_pod_tests() { kube::log::status "Testing kubectl(v1:pods)" @@ -2819,6 +2861,11 @@ runTests() { kubectl get "${kube_flags[@]}" -f hack/testdata/kubernetes-service.yaml fi + ######################### + # Kubectl version # + ######################### + run_kubectl_version_tests + # Passing no arguments to create is an error ! kubectl create diff --git a/pkg/kubectl/cmd/BUILD b/pkg/kubectl/cmd/BUILD index 2223c6ef3d7..ef6f14f1362 100644 --- a/pkg/kubectl/cmd/BUILD +++ b/pkg/kubectl/cmd/BUILD @@ -133,6 +133,7 @@ go_library( "//vendor:k8s.io/apimachinery/pkg/util/validation/field", "//vendor:k8s.io/apimachinery/pkg/util/wait", "//vendor:k8s.io/apimachinery/pkg/util/yaml", + "//vendor:k8s.io/apimachinery/pkg/version", "//vendor:k8s.io/apimachinery/pkg/watch", "//vendor:k8s.io/apiserver/pkg/util/flag", "//vendor:k8s.io/client-go/discovery", diff --git a/pkg/kubectl/cmd/version.go b/pkg/kubectl/cmd/version.go index b38a3eb84ba..25bad6f67bc 100644 --- a/pkg/kubectl/cmd/version.go +++ b/pkg/kubectl/cmd/version.go @@ -17,17 +17,26 @@ limitations under the License. package cmd import ( + "encoding/json" + "errors" "fmt" "io" + "github.com/ghodss/yaml" "github.com/spf13/cobra" + apimachineryversion "k8s.io/apimachinery/pkg/version" "k8s.io/kubernetes/pkg/kubectl/cmd/templates" cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util" "k8s.io/kubernetes/pkg/util/i18n" "k8s.io/kubernetes/pkg/version" ) +type Version struct { + ClientVersion *apimachineryversion.Info `json:"clientVersion,omitempty" yaml:"clientVersion,omitempty"` + ServerVersion *apimachineryversion.Info `json:"serverVersion,omitempty" yaml:"serverVersion,omitempty"` +} + var ( version_example = templates.Examples(` # Print the client and server versions for the current context @@ -47,36 +56,75 @@ func NewCmdVersion(f cmdutil.Factory, out io.Writer) *cobra.Command { } cmd.Flags().BoolP("client", "c", false, "Client version only (no server required).") cmd.Flags().BoolP("short", "", false, "Print just the version number.") + cmd.Flags().String("output", "", "output format, options available are yaml and json") cmd.Flags().MarkShorthandDeprecated("client", "please use --client instead.") return cmd } func RunVersion(f cmdutil.Factory, out io.Writer, cmd *cobra.Command) error { - v := fmt.Sprintf("%#v", version.Get()) - if cmdutil.GetFlagBool(cmd, "short") { - v = version.Get().GitVersion + var serverVersion *apimachineryversion.Info = nil + var serverErr error = nil + vo := Version{nil, nil} + + clientVersion := version.Get() + vo.ClientVersion = &clientVersion + + if !cmdutil.GetFlagBool(cmd, "client") { + serverVersion, serverErr = retrieveServerVersion(f) + vo.ServerVersion = serverVersion } - fmt.Fprintf(out, "Client Version: %s\n", v) - if cmdutil.GetFlagBool(cmd, "client") { - return nil + switch of := cmdutil.GetFlagString(cmd, "output"); of { + case "": + if cmdutil.GetFlagBool(cmd, "short") { + fmt.Fprintf(out, "Client Version: %s\n", clientVersion.GitVersion) + + if serverVersion != nil { + fmt.Fprintf(out, "Server Version: %s\n", serverVersion.GitVersion) + } + } else { + fmt.Fprintf(out, "Client Version: %s\n", fmt.Sprintf("%#v", clientVersion)) + + if serverVersion != nil { + fmt.Fprintf(out, "Server Version: %s\n", fmt.Sprintf("%#v", *serverVersion)) + } + } + case "yaml": + y, err := yaml.Marshal(&vo) + if err != nil { + return err + } + + fmt.Fprintln(out, string(y)) + case "json": + y, err := json.Marshal(&vo) + if err != nil { + return err + } + fmt.Fprintln(out, string(y)) + default: + return errors.New("invalid output format: " + of) + } - discoveryclient, err := f.DiscoveryClient() - if err != nil { - return err + if serverErr != nil { + return serverErr } - serverVersion, err := discoveryclient.ServerVersion() - if err != nil { - return err - } - - v = fmt.Sprintf("%#v", *serverVersion) - if cmdutil.GetFlagBool(cmd, "short") { - v = serverVersion.GitVersion - } - - fmt.Fprintf(out, "Server Version: %s\n", v) return nil } + +func retrieveServerVersion(f cmdutil.Factory) (*apimachineryversion.Info, error) { + discoveryClient, err := f.DiscoveryClient() + + if err != nil { + return nil, err + } + + serverVersion, err := discoveryClient.ServerVersion() + if err != nil { + return nil, err + } + + return serverVersion, nil +}