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

addressing issue #39427 adding a flag --output to 'kubectl version' #39858

Merged
merged 1 commit into from
Mar 28, 2017
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
97 changes: 97 additions & 0 deletions hack/lib/test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -303,3 +303,100 @@ kube::test::if_supports_resource() {
done
return 1
}


kube::test::version::object_to_file() {
Copy link
Member

Choose a reason for hiding this comment

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

this file is for generic test helper funcs, not specific tests

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the function isnt a test, its a helper function. I suppose i can rename it to make that clear.

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
}

47 changes: 47 additions & 0 deletions hack/make-rules/test-cmd-util.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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)"
Expand Down Expand Up @@ -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

Expand Down
1 change: 1 addition & 0 deletions pkg/kubectl/cmd/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
80 changes: 64 additions & 16 deletions pkg/kubectl/cmd/version.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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") {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe check outside of the switch if both --short and --output were specified?
An error could be returned if both --short and --output are specified

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I dont think there would be much gained by adding an error if both flags are used. The suggestion goes back to my disagreement regarding expected/unexpected behavior using flags i had prior, please look at the history, regarding how it is implemented now. If you feel strongly about this, I think it would be best we get together and discuss it in detail and perhaps discuss some enforcement throughout the code, or what exists atm.

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
}