Display oc and kube server versions as part of oc version #9785

juanvallejo · 2016-07-11T20:16:06Z

Before

oc version

oc v3.2.1.4-1-g1864c8f
kubernetes v1.2.0-36-g4a3f9c5

After

oc version

CLIENTS
oc v1.3.0-alpha.0+005dbbb-dirty
kubernetes v1.3.0+57fb9ac
features: Basic-Auth

SERVERS
OpenShift Master v1.3.0-alpha.0+005dbbb-dirty (https://10.13.137.149:8443)
Kubernetes Master v1.3.0+57fb9ac (https://10.13.137.149:8443)

On Error Connecting to Server

oc version

CLIENTS
oc v1.3.0-alpha.0+005dbbb-dirty
kubernetes v1.3.0+57fb9ac
features: Basic-Auth

SERVERS
error: server took too long to respond with version information.

juanvallejo · 2016-07-11T20:16:16Z

[test]

juanvallejo · 2016-07-11T20:16:24Z

@fabianofranz Could you please take a look?

liggitt · 2016-07-11T20:26:00Z

that is a surprisingly large number of completions changes for this

liggitt · 2016-07-11T20:26:13Z

newline between client a server versions

deads2k · 2016-07-11T20:26:57Z

any idea what happened to the generation?

liggitt · 2016-07-11T20:26:58Z

add a header or something indicating the version information is switching to server info, and maybe include the host that is being shown? (e.g. server version (https://localhost:8443):

deads2k · 2016-07-11T20:28:40Z

pkg/cmd/cli/cmd/version.go

+	for i := 0; i < 2; i++ {
+		select {
+		case ocVersion := <-ocServerVersionChan:
+			fmt.Fprintf(o.Out, "OpenShift Master %v\n", ocVersion)


random order of output is going to bother me, make the output order consistent.

juanvallejo · 2016-07-12T19:33:30Z

@deads2k

You'll need to interpret this error to provide reasonable output for when this run against a kube server.

This could be a function setting values and closing a channel to indicate, "done", right? You wait for "done" then, inspect the values to limit the number of channels you have.

Thanks for the feedback! e85107a

deads2k · 2016-07-12T19:48:49Z

pkg/cmd/cli/cmd/version.go

+	var maxRequestDuration int32 = 10
+
+	// start goroutine to fetch openshift / kubernetes server version
+	go func(f *clientcmd.Factory, timeout int32) {


done := make(chan struct{}) var serverVersionErr error oVersion := "" kVersion := "" go func{ defer close(done) oversion, err := oclient.Get().AbsPath("/version/openshift").Do().Raw() if err != nil{ serverVersionErr = err return } //parse //repeat for kube version } select { case <- done: case <- time.After(timeout): return fmt.Errorf("error getting server version) } fmt.Fprintf("server vesrion stuf")

looks a little tighter I think

juanvallejo · 2016-07-12T22:00:45Z

re[test]

deads2k · 2016-07-13T12:55:52Z

@juanvallejo what is going on with your completions?

juanvallejo · 2016-07-13T15:08:40Z

@deads2k

what is going on with your completions?

I was passing root.PersistentFlags() to NewCmdVersion in openshift.go here: 5e7f3be#diff-1e4f76227f916f9f1ce10a35f8f756a6R120

It should be fixed: 5e7f3be

liggitt · 2016-07-13T15:57:20Z

pkg/cmd/cli/cmd/version.go

+
+const (
+	versionLong = `
+Display client and server versions for Kubernetes and OpenShift.`


I would drop the product names

juanvallejo · 2016-07-13T17:26:43Z

@liggitt

you lost the version.Options struct

Thanks for your feedback, fixed! e33053b

juanvallejo · 2016-07-13T18:55:33Z

flake? re[test]

liggitt · 2016-07-13T18:57:06Z

I would leave the existing client version output alone (no header)

liggitt · 2016-07-13T18:58:31Z

I would include the server host in the header, something like:

Server https://...
openshift v1.3.0-alpha.0+005dbbb-dirty
kubernetes v1.3.0+57fb9ac

deads2k · 2016-07-13T20:01:41Z

pkg/cmd/cli/cmd/version.go

+
+type VersionOptions struct {
+	BaseName         string
+	Config           clientcmdapi.Config


deads2k · 2016-07-14T20:20:16Z

Otherwise looking good, @deads2k do you have anything else?

final comments in.

juanvallejo · 2016-07-14T22:11:23Z

re[test]

deads2k · 2016-07-15T13:49:04Z

hack/test-cmd.sh

@@ -339,6 +339,10 @@ os::cmd::expect_success "oadm completion zsh"
 os::cmd::expect_failure_and_text "oadm completion test_shell" 'Unsupported shell type "test_shell"'
 echo "oc completion: ok"

+# test oc version output
+os::cmd::expect_success_and_text "oc version" "Server"


Better content test. Check both server down and server up.

juanvallejo · 2016-07-15T20:30:25Z

pkg/cmd/cli/cmd/version.go

+
+	select {
+	case err, closed := <-done:
+		if strings.HasSuffix(fmt.Sprintf("%v", err), "connection refused") || clientcmd.IsConfigurationMissing(err) || kclientcmd.IsConfigurationInvalid(err) {


@deads2k
It turns out that the IsConfigurationInvalid function in https://github.com/openshift/origin/blob/master/vendor/k8s.io/kubernetes/pkg/client/unversioned/clientcmd/validation.go#L95 does not take into account config files that are completely missing, just ones with invalid values. I also tried using the method IsEmptyConfig instead but that one doesn't take into account a config location that is missing altogether. Since the error for a missing config file is located in our clientcmd package, I went ahead and placed the method IsConfigurationMissing there.

deads2k · 2016-07-18T11:42:29Z

lgtm, squash for merge.

juanvallejo · 2016-07-18T16:05:10Z

re[test]

juanvallejo · 2016-07-19T22:40:10Z

@fabianofranz @liggitt @deads2k While working on PR #9944, I ended up needing to add an error type for missing config file (like I did with this one) upstream. I have updated this PR so that it implements the new error type upstream as well, instead of in pkg/cmd/util/clientcmd/factory.go

EDIT: PR #9944 did not end up needing the new type of error, reverted changes to use clientcmd.IsConfigurationMissing error type check

juanvallejo · 2016-07-21T13:42:14Z

re[test]

juanvallejo · 2016-07-21T15:04:24Z

flake re[test]

juanvallejo · 2016-07-22T20:16:47Z

re[test]

juanvallejo · 2016-07-25T15:16:45Z

re[test]

juanvallejo · 2016-07-26T18:29:31Z

re[test]

juanvallejo · 2016-07-27T18:25:16Z

re[test]

openshift-bot · 2016-07-27T18:25:32Z

Evaluated for origin test up to 131728a

openshift-bot · 2016-07-27T19:40:51Z

continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/7003/)

fabianofranz · 2016-07-27T19:53:15Z

[merge]

openshift-bot · 2016-07-27T19:55:39Z

Evaluated for origin merge up to 131728a

openshift-bot · 2016-07-27T22:40:36Z

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/7003/) (Image: devenv-rhel7_4688)

deads2k reviewed Jul 11, 2016
View reviewed changes

deads2k reviewed Jul 12, 2016
View reviewed changes

juanvallejo force-pushed the jvallejo_display-server-version-oc-version branch 2 times, most recently from 4efb92b to bec40dd Compare July 12, 2016 21:49

juanvallejo force-pushed the jvallejo_display-server-version-oc-version branch from bec40dd to 005dbbb Compare July 13, 2016 15:07

liggitt reviewed Jul 13, 2016
View reviewed changes

juanvallejo force-pushed the jvallejo_display-server-version-oc-version branch from 005dbbb to ea02ef2 Compare July 13, 2016 17:26

deads2k reviewed Jul 13, 2016
View reviewed changes

pkg/cmd/cli/cmd/version.go

type VersionOptions struct {

BaseName string

Config clientcmdapi.Config

Copy link

Contributor

deads2k Jul 13, 2016

Choose a reason for hiding this comment

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

dead?

juanvallejo force-pushed the jvallejo_display-server-version-oc-version branch from c01ec83 to d268f24 Compare July 14, 2016 22:10

deads2k reviewed Jul 15, 2016
View reviewed changes

juanvallejo force-pushed the jvallejo_display-server-version-oc-version branch from d268f24 to 9e415fc Compare July 15, 2016 20:23

juanvallejo reviewed Jul 15, 2016
View reviewed changes

juanvallejo force-pushed the jvallejo_display-server-version-oc-version branch 2 times, most recently from 0aeeaaa to 658de10 Compare July 18, 2016 16:03

juanvallejo force-pushed the jvallejo_display-server-version-oc-version branch from 658de10 to 7ea6811 Compare July 19, 2016 22:35

juanvallejo force-pushed the jvallejo_display-server-version-oc-version branch from 7ea6811 to fac96a8 Compare July 20, 2016 19:57

update oc version to display client and server versions

131728a

juanvallejo force-pushed the jvallejo_display-server-version-oc-version branch from fac96a8 to 131728a Compare July 27, 2016 16:42

openshift-bot merged commit 5030b88 into openshift:master Jul 27, 2016

juanvallejo deleted the jvallejo_display-server-version-oc-version branch July 29, 2016 13:13

Display oc and kube server versions as part of oc version #9785

Display oc and kube server versions as part of oc version #9785

Uh oh!

Conversation

juanvallejo commented Jul 11, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Before

After

On Error Connecting to Server

Uh oh!

juanvallejo commented Jul 11, 2016

Uh oh!

juanvallejo commented Jul 11, 2016

Uh oh!

liggitt commented Jul 11, 2016

Uh oh!

liggitt commented Jul 11, 2016

Uh oh!

deads2k commented Jul 11, 2016

Uh oh!

liggitt commented Jul 11, 2016

Uh oh!

deads2k Jul 11, 2016

Choose a reason for hiding this comment

Uh oh!

juanvallejo commented Jul 12, 2016

Uh oh!

deads2k Jul 12, 2016

Choose a reason for hiding this comment

Uh oh!

juanvallejo commented Jul 12, 2016

Uh oh!

deads2k commented Jul 13, 2016

Uh oh!

juanvallejo commented Jul 13, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

liggitt Jul 13, 2016

Choose a reason for hiding this comment

Uh oh!

juanvallejo commented Jul 13, 2016

Uh oh!

juanvallejo commented Jul 13, 2016

Uh oh!

liggitt commented Jul 13, 2016

Uh oh!

liggitt commented Jul 13, 2016

Uh oh!

deads2k Jul 13, 2016

Choose a reason for hiding this comment

Uh oh!

deads2k commented Jul 14, 2016

Uh oh!

juanvallejo commented Jul 14, 2016

Uh oh!

deads2k Jul 15, 2016

Choose a reason for hiding this comment

Uh oh!

juanvallejo Jul 15, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

deads2k commented Jul 18, 2016

Uh oh!

juanvallejo commented Jul 18, 2016

Uh oh!

juanvallejo commented Jul 19, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

juanvallejo commented Jul 21, 2016

Uh oh!

juanvallejo commented Jul 21, 2016

Uh oh!

juanvallejo commented Jul 22, 2016

Uh oh!

juanvallejo commented Jul 25, 2016

Uh oh!

juanvallejo commented Jul 26, 2016

Uh oh!

juanvallejo commented Jul 27, 2016

juanvallejo commented Jul 11, 2016 •

edited

Loading

juanvallejo commented Jul 13, 2016 •

edited

Loading

juanvallejo Jul 15, 2016 •

edited

Loading

juanvallejo commented Jul 19, 2016 •

edited

Loading

openshift-bot commented Jul 27, 2016 •

edited

Loading