Make svcat version more in line with kubectl version by always reporting the client version.
Presently svcat reports the following for version info when Kubernetes isn't running:
$ svcat version
Error: unable to get version, Get http://localhost:8080/version: dial tcp [::1]:8080: getsockopt: connection refused
$ svcat version --client
client: v0.1.17-7-gef904a5-dirty
vs kubectl
$ kubectl version
Client Version: version.Info{Major:"1", Minor:"11+", GitVersion:"v1.11.0-alpha.2.8+0d43bdec2b8598-dirty", GitCommit:"0d43bdec2b8598ff542a1afdee876d417b4e7668", GitTreeState:"dirty", BuildDate:"2018-05-16T20:04:10Z", GoVersion:"go1.10.1", Compiler:"gc", Platform:"linux/amd64"}
The connection to the server localhost:8080 was refused - did you specify the right host or port?
To be clear, svcat version _always_ returns the client version, and the --client flag allows us to query the client version only, in instances where perhaps there isn't a configured cluster yet.
The problem is that when we fail to query the server version, svcat exits immediately before the client version is also printed.
The code that needs to be changed is in the version command
It could be fixed in either of two ways:
c.App.ServerVersion() returns an error, so that output.WriteVersion(c.Output, client, server) is still called. The function should still return an error when that call fails however, so that the return code of the svcat version function is still non-zero.There aren't existing tests for the version command. As part of this issue, three new tests should be added (that look similar to these new tests that have yet to be merged), that verify the following scenarios using a fake client:
svcat versionsvcat version --clientsvcat version when the server returns an errorI thought that this would be a good first PR initially, but since the tests don't exist yet this may be easier for someone already familiar with writing tests in service catalog, or a very enterprising new contributor! 馃榾
Hello :) ! Can i work on this this ?
@A-Hilaly Yup! Go for it 馃檶 When you get to the part about adding tests, feel free to ping back for help. As I noted originally, this part doesn't have existing tests so you will need to add them yourself.
Alright, thank you 馃檹 I'll open a PR soon !
closed by #2109
Most helpful comment
To be clear,
svcat version_always_ returns the client version, and the--clientflag allows us to query the client version only, in instances where perhaps there isn't a configured cluster yet.The problem is that when we fail to query the server version, svcat exits immediately before the client version is also printed.
The code that needs to be changed is in the version command
https://github.com/kubernetes-incubator/service-catalog/blob/a514e67a664cff404196158436e6d158a33fc886/cmd/svcat/versions/version_cmd.go#L68-L83
It could be fixed in either of two ways:
c.App.ServerVersion()returns an error, so thatoutput.WriteVersion(c.Output, client, server)is still called. The function should still return an error when that call fails however, so that the return code of thesvcat versionfunction is still non-zero.There aren't existing tests for the version command. As part of this issue, three new tests should be added (that look similar to these new tests that have yet to be merged), that verify the following scenarios using a fake client:
svcat versionsvcat version --clientsvcat versionwhen the server returns an error