prevent ctx-switching from failing on server err

[juanvallejo]

Fixes #12343

This patch fixes an issue where the config context will fail to switch if the server in the current context returns an error, or cannot be reached.

Before

# log in to local server
$ oc login https://10.13.137.149:8443 -u test-user

# confirm that current context is that of the local server
$ oc config current-context
default/10-13-137-149:8443/test-user

# kill the local server from previous steps
$ sudo kill <local_server_pid>

# confirm that local server is down
$ oc whoami
The connection to the server 10.13.137.149:8443 was refused - did you specify the right host or port?

# attempt to switch to a new context for a different server using `oc project`
$ oc project <new_context>

error: Get https://10.13.137.149:8443/api: dial tcp 10.13.137.149:8443: getsockopt: connection refused
See 'oc project -h' for help and examples.

# context failed to switch due to error from server / server no longer being available
$ oc config current-context
default/10-13-137-149:8443/system:admin

After

$ oc project <new_context>

error: Get https://10.13.137.149:8443/api: dial tcp 10.13.137.149:8443: getsockopt: connection refused
See 'oc project -h' for help and examples.

$ oc config current-context
<new_context>

cc @openshift/cli-review @smarterclayton

[juanvallejo]

@fabianofranz or @smarterclayton would a message to the user be appropriate here, indicating that the local context was switched anyway? (they would still see the error caused by f.ClientConfig() failing)

[fabianofranz]

Hrm, is this error typed? If we can detect the exact reason I think we should bypass it and switch cleanly, without reporting the error. It's related to the actual context cluster anyway, not the one we're trying to switch to.

[juanvallejo]

Hrm, is this error typed? If we can detect the exact reason I think we should bypass it and switch cleanly, without reporting the error

Sounds good, we could check to see if error is errors.IsNotFound or errors.IsInternalError, and skip to switching the context if this is the case. However, the last few messages that are outputted to a user after successfully switching contexts are usually created with the host of the new context, using the ClientConfig that caused this error to begin with:

$ oc project sample-app-2/api-preview-openshift-com:443/juanvallejo
Already on project "sample-app-2" on server "https://api.preview.openshift.com:443".

Maybe in the case of this error happening, we could modify the message to use the new "context name" instead of its hostname?

[fabianofranz]

I think it makes sense, even because you are switching by using the context name here.

[juanvallejo]

[test]

[juanvallejo]

integration flaked on #12540 re[test]

[juanvallejo]

@fabianofranz thanks for the feedback, review comments addressed.

I updated error checking to only attempt to switch contexts cleanly if the error returned while fetching ClientConfig from the server failed with a url.Error, or errors.IsInternalError. If it fails for these reasons, and the argument passed by the user is a valid context name, it assigns f.OpenShiftClientConfig().ClientConfig() as the value of o.ClientConfig (does not fail if server can't be reached / server is returning 500 error codes), and executes the rest of the command normally.

[juanvallejo]

integration flaked on #12558 re[test]

[fabianofranz]

LGTM, squash?

[juanvallejo]

@fabianofranz thanks, done!

[fabianofranz]

[merge]

[openshift-bot]

Evaluated for origin test up to daa62d5

[openshift-bot]

Evaluated for origin merge up to daa62d5

[openshift-bot]

continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/13257/) (Base Commit: 308e2d4)

[openshift-bot]

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/13257/) (Base Commit: dced7f7) (Image: devenv-rhel7_5774)