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

oc cluster console #11355

Closed
wants to merge 2 commits into from
Closed

Conversation

Ladicek
Copy link

@Ladicek Ladicek commented Oct 13, 2016

This PR adds a new command oc cluster console that opens the OpenShift web console of the local cluster in a web browser, or just provides the URL. It's merely for convenience. The idea was stolen from @jimmidyson's Minishift.

This is my first attempt at a contribution to OpenShift, so please forgive me (and warn me) if I'm doing something wrong. It's also the first piece of code I wrote in Go, so if I'm doing something wrong on that side, I'd be happy to fix it.

All that said:

  1. I understand that I'm adding new visible feature to a fairly exposed part of code, with no previous discussion etc. I'd be happy to iterate on design, if needed.
  2. I realize that I didn't add any tests. If you could point me to an example in the existing codebase that I could copy, I'd be happy to add some tests too.
  3. I know that I'm adding yet another dependency. It's really small, though; I believe footprint shouldn't be a problem. I'm not sure if I added the dependency correctly -- fighting with godep actually took me a lot more time than writing the code itself.

Disclosure: I do work for Red Hat, but I'm not on the OpenShift team.

@liggitt
Copy link
Contributor

liggitt commented Oct 13, 2016

This doesn't really have anything to do with cluster up/down... it's just looking at the client config, which could come from anywhere. oc whoami --show-server will show the current URL

@jimmidyson
Copy link
Contributor

This was in minishift to give an equivalent feature to minikube. If this was to be added anywhere it could go in as oc console as it's unrelated to the cluster subcommand. What it does do which is quite nice is launch the user's browser at the console URL - pretty useful rather than using oc whoami --show-server & then copying URL into your browser.

@liggitt
Copy link
Contributor

liggitt commented Oct 13, 2016

there's no guarantee that the server your kubeconfig is pointing to has a web console configured.

@jimmidyson
Copy link
Contributor

That's a very good point & why it makes more sense in mini* because we don't use the kube config context to work out the console/dashboard address.

@Ladicek
Copy link
Author

Ladicek commented Oct 13, 2016

I'm thinking about this as a local developer convenience, which is why I put it under oc cluster. There, it is guaranteed that the console will be running.

But I'm just a beginner, not fully understanding all the consequences, so if it really doesn't make any sense, then please feel free to close the PR.

@fabianofranz
Copy link
Member

@csrwng PTAL

@csrwng
Copy link
Contributor

csrwng commented Oct 14, 2016

@Ladicek @fabianofranz do we want the command to open the console to whatever cluster your cli is pointing to? or do we want it to open the console to a cluster you started with cluster up?

If the former, maybe it should be under 'oc config' or just 'oc', instead of under cluster ? (may be confusing). If the latter, then it should likely use the URL obtained with the status command, see #11171

@jwforres
Copy link
Member

We recently argued against the same type of feature in kubectl in kubernetes (with opening kube dashboard).

I don't want to start making assumptions from the CLI that a web console is always installed when it is not in some clusters.

@Ladicek
Copy link
Author

Ladicek commented Oct 14, 2016

While I still very much like the convenience, I can see now why it's not necessarily the best idea. Thanks all.

@Ladicek Ladicek closed this Oct 14, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants