addressing issue kubernetes#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.
alejandroEsc · Mar 28, 2017 · a5e6dcb · a5e6dcb
1 parent be4452c
commit a5e6dcb
Show file tree

Hide file tree

Showing 4 changed files with 209 additions and 16 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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()
+	return nil
+}
+
+func retrieveServerVersion(f cmdutil.Factory) (*apimachineryversion.Info, error) {
+	discoveryClient, err := f.DiscoveryClient()
+
 	if err != nil {
-		return err
+		return nil, err
 	}
 
-	v = fmt.Sprintf("%#v", *serverVersion)
-	if cmdutil.GetFlagBool(cmd, "short") {
-		v = serverVersion.GitVersion
+	serverVersion, err := discoveryClient.ServerVersion()
+	if err != nil {
+		return nil, err
 	}
 
-	fmt.Fprintf(out, "Server Version: %s\n", v)
-	return nil
+	return serverVersion, nil
 }