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

Add oc command to open the console #23323

Closed
wants to merge 4 commits into from

Conversation

Projects
None yet
3 participants
@cblecker
Copy link
Member

commented Jul 5, 2019

This PR adds an experimental command to open the OpenShift console from the CLI. This might be useful enough to add in as actual top-level command (e.g. oc console), but I wasn't sure so I put it under the experimental heading.

It pulls data from the consoles.config.openshift.io cluster object. I previously had this retrieving route objects from the openshift-console namespace, but this may not be available in hosted offerings. By using the consoles.config.openshift.io object, cluster scoped GET permissions can be granted.. however, this means that this command is v4+ only.

By default, this URL will be opened in your default web browser. If the --url flag is passed, it will just print this URL and exit.

I'm handling specific error message from the API, but I'm not sure on verbiage for them, or if instead of directly handling these use cases, it would be better to just return the err directly.

@openshift-ci-robot

This comment has been minimized.

Copy link

commented Jul 5, 2019

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: cblecker
To complete the pull request process, please assign deads2k
You can assign the PR to them by writing /assign @deads2k in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@cblecker

This comment has been minimized.

Copy link
Member Author

commented Jul 6, 2019

Okay, so I think I solved by glide issues:

  • First off was #23327. This was preventing me from touching deps on master.
  • Second was a weird issue with the way that glide is handling dependencies for the new staging repo setup. If I add a new staging package, reference it somewhere in the code base, and then run glide, glide barfs because that new package isn't available in the master branch of the actively published repo.

For example, I created the github.com/openshift/oc/pkg/cli/experimental/console package in this PR. I needed to add it to staging/src/github.com/openshift/oc/pkg/cli/cli.go so that the new cobra commands could be registered. However, because this package doesn't exist on master, glide repeatedly failed with:

[ERROR] Error scanning github.com/openshift/oc/pkg/cli/experimental/console: cannot find package "." in:
        /root/.glide/cache/src/https-github.com-openshift-oc/pkg/cli/experimental/console

I fixed this by commenting out the import statement, running glide, and then uncommenting. This workflow will likely need to be addressed as the staging repo setup is moved forward.

I've rebased this PR based on #23327, but can rebase it once that one is merged.

@cblecker cblecker force-pushed the cblecker:ex-console branch from 67ed09d to 7260a2b Jul 6, 2019

@openshift-ci-robot openshift-ci-robot added size/XXL and removed size/L labels Jul 6, 2019

@cblecker

This comment has been minimized.

Copy link
Member Author

commented Jul 6, 2019

This is ready for review, but holding so that #23327 can be reviewed and merged first.
/hold

@cblecker

This comment has been minimized.

Copy link
Member Author

commented Jul 6, 2019

/retest

@cblecker cblecker force-pushed the cblecker:ex-console branch from d797248 to 44e4edb Jul 15, 2019

@openshift-ci-robot openshift-ci-robot added size/L and removed size/XXL labels Jul 15, 2019

@cblecker

This comment has been minimized.

Copy link
Member Author

commented Jul 15, 2019

/hold cancel
#23327 has been merged, so I rebased this one cleanly now.

Should be ready for review.

@cblecker cblecker force-pushed the cblecker:ex-console branch from 44e4edb to 10166be Jul 16, 2019

@cblecker

This comment has been minimized.

Copy link
Member Author

commented Jul 16, 2019

/retest

@cblecker cblecker force-pushed the cblecker:ex-console branch from 10166be to d2b44fd Jul 16, 2019

@cblecker

This comment has been minimized.

Copy link
Member Author

commented Jul 16, 2019

/retest

@cblecker cblecker force-pushed the cblecker:ex-console branch from d2b44fd to c52b301 Jul 16, 2019

@openshift-ci-robot openshift-ci-robot added size/XXL and removed size/L labels Jul 16, 2019

@cblecker cblecker force-pushed the cblecker:ex-console branch from 1b87193 to f34de17 Jul 16, 2019

@cblecker

This comment has been minimized.

Copy link
Member Author

commented Jul 16, 2019

/test unit

cblecker added some commits Jul 16, 2019

@cblecker cblecker force-pushed the cblecker:ex-console branch from f34de17 to 5d62aef Jul 17, 2019

@deads2k

This comment has been minimized.

Copy link
Contributor

commented Jul 17, 2019

I'm a -1 on this. the kube-apiserver can redirect to the console already. I'd expect the console team to have a config observer that writes to consolePublicURL to enable that functionality.

/hold

@cblecker

This comment has been minimized.

Copy link
Member Author

commented Jul 17, 2019

Talked to @deads2k on Slack about this.

The API server should redirect to the console, if installed. It's not doing that right now. Opened up a BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1730905

As for the "open the URL directly in the browser" pieces, this may be better served by a oc/kubectl plugin. Going to take a look at that separately.

Going to close this PR for now, as there are better ways to do this that don't require cluster-admin or rbac changes.

@cblecker cblecker closed this Jul 17, 2019

@openshift-ci-robot

This comment has been minimized.

Copy link

commented Jul 17, 2019

@cblecker: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
ci/prow/cmd 731072a link /test cmd
ci/prow/e2e-aws 731072a link /test e2e-aws

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.