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 operator describe command #19

Merged
merged 6 commits into from
Oct 5, 2020

Conversation

komish
Copy link
Contributor

@komish komish commented Aug 19, 2020

Signed-off-by: Jose R. Gonzalez josegonzalez89@gmail.com

This PR would add the ability to show details about operators available via packagemanifests in a cluster, similar to what one might see when browsing operators in the openshift console. Didn't see the functionality already there, so figured I'd hack together a proof of concept to see if it's something kubectl-operator would want to include.

Let me know what changes I can make.

examples

By default, the command shows some basic information about the operator.

$ ./kubectl-operator show argocd-operator 
== Package ==
Argo CD 0.0.12 (by Argo CD Community)

== Repository ==
https://github.com/argoproj-labs/argocd-operator

== Catalog ==
Community Operators

== Channels ==
alpha (default) (shown)

== Install Modes ==
OwnNamespace
SingleNamespace

== Description ==
Argo CD is a declarative, GitOps continuous delivery tool for Kubernetes.

You can add the --with-long-description flag to see the operator's description, which is in markdown format and commonly used as the "Readme" to render in the OpenShift Console. Since GitHub uses markdown to render content, I've added some escape ("\") characters to try to prevent this from rendering strangely here on GitHub.

$ ./kubectl-operator show argocd-operator --with-long-description
== Package ==
Argo CD 0.0.12 (by Argo CD Community)

== Repository ==
https://github.com/argoproj-labs/argocd-operator

== Catalog ==
Community Operators

== Channels ==
alpha (default) (shown)

== Install Modes ==
OwnNamespace
SingleNamespace

== Description ==
Argo CD is a declarative, GitOps continuous delivery tool for Kubernetes.

== Long Description ==
## Overview

The Argo CD Operator manages the full lifecycle for [Argo CD](https://argoproj.github.io/argo-cd/) and it's
components. The operator's goal is to automate the tasks required when operating an Argo CD cluster.

Beyond installation, the operator helps to automate the process of upgrading, backing up and restoring as needed and
remove the human as much as possible. In addition, the operator aims to provide deep insights into the Argo CD
environment by configuring Prometheus and Grafana to aggregate, visualize and expose the metrics already exported by
Argo CD.

The operator aims to provide the following, and is a work in progress.

* Easy configuration and installation of the Argo CD components with sane defaults to get up and running quickly.
* Provide seamless upgrades to the Argo CD components.
* Ability to back up and restore an Argo CD cluster from a point in time or on a recurring schedule.
* Aggregate and expose the metrics for Argo CD and the operator itself using Prometheus and Grafana.
* Autoscale the Argo CD components as necessary to handle variability in demand.

## Usage

Deploy a basic Argo CD cluster by creating a new ArgoCD resource in the namespace where the operator is installed.

\```
apiVersion: argoproj.io/v1alpha1
kind: ArgoCD
metadata:
  name: example-argocd
spec: {}
\```

## Backup

Backup the cluster above by creating a new ArgoCDExport resource in the namespace where the operator is installed.

\```
apiVersion: argoproj.io/v1alpha1
kind: ArgoCDExport
metadata:
  name: example-argocdexport
spec:
  argocd: example-argocd
\```

See the [documentation](https://argocd-operator.readthedocs.io) and examples on
[github](https://github.com/argoproj-labs/argocd-operator) for more information.

If the operator has multiple channels, the --channel flag helps render the data from the requested channel. Note the (shown) annotation.

$ ./kubectl-operator show kiali --channel alpha
== Package ==
Kiali Operator 1.22.0 (by Kiali)

== Repository ==
https://github.com/kiali/kiali

== Catalog ==
Community Operators

== Channels ==
alpha (shown)
stable (default)

== Install Modes ==
OwnNamespace
SingleNamespace
AllNamespaces

== Description ==
Kiali project provides answers to the questions: What microservices are part of my Istio service mesh and how are they connected?

@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 29, 2020
@akram
Copy link

akram commented Sep 9, 2020

what about using describe operator (or just describe) instead of show ?

kubernetes-operator describe operator kiali

and with kubectl

kubectl operator describe operator kiali -n mynamespace

@komish
Copy link
Contributor Author

komish commented Sep 9, 2020

@akram Sure! I don't see a reason why it can't be changed from show to describe. Just pending confirmation that this is something that would be considered for addition into the plugin and I'll make the changes once I know.

@joelanford
Copy link
Member

I think this would be a great addition! Thanks for taking the time to propose this and put it together!

+1 to describe as well.

To keep with the theme of the cmd struct doing the output and the pkg/action collecting the information and returning it via a struct (e.g. list, list-available, catalog list, etc.), what do you think of refactoring the code that prints to the console into the cmd package?

Also, do you think it be possible to re-use action.OperatorListAvailable instead of adding action.OperatorShow? Is there anything missing from action.OperatorListAvailable that we would need for a describe command to work?

@joelanford
Copy link
Member

kubectl operator describe operator kiali -n mynamespace

@akram Correct me if I'm wrong, but it sounds like you're suggesting to align the name of this subcommand based on your RFE in #22.

Typing operator twice seems like unnecessary repetition to me. WDYT of kubectl operator describe kiali -n mynamespace?

With this plugin, it is difficult to totally align to existing kubectl idioms without introducing this repetition. kubectl subcommands are typically <verb> <noun>, but because this plugin contains so many different verbs, it made more sense to us to flip that and make this plugin use the <noun> <verb> style.

Let's continue this conversation in #22. (I'll copy this response there)

Signed-off-by: Jose R. Gonzalez <josegonzalez89@gmail.com>
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 9, 2020
@komish
Copy link
Contributor Author

komish commented Sep 9, 2020

Latest push is to rebase on main

@komish
Copy link
Contributor Author

komish commented Sep 9, 2020

@joelanford

+1 to describe as well.

Done!

To keep with the theme of the cmd struct doing the output and the pkg/action collecting the information and returning it via a struct (e.g. list, list-available, catalog list, etc.), what do you think of refactoring the code that prints to the console into the cmd package?

Good catch, apologies for missing that theme! I've made this change roughly to what I think it should look like to keep on theme here. Probably more changes to come regarding this based on what's next, though.

Also, do you think it be possible to re-use action.OperatorListAvailable instead of adding action.OperatorShow? Is there anything missing from action.OperatorListAvailable that we would need for a describe command to

Yup, I see the overlap you're mentioning here. The only thing that I see missing in action.OperatorListAvailabel is Channel and LongDescription, which are both input flags coming from the user to help guide what Describe actually presents.

Also a couple of helper functions, GetAvailableChannels() and GetSupportedInstallModes(). The latter can probably be moved away from being a method on action.OperatorShow into a standalone function somewhere so that really only leaves GetAvailableChannels() which I might want to include in action.OperatorListAvailable.

What do you think?

As an aside, I'm keeping things as separate commits for the time being but I plan on squashing them down the line once we've finished cleaning this up!

@joelanford
Copy link
Member

The only thing that I see missing in action.OperatorListAvailabel is Channel and LongDescription, which are both input flags coming from the user to help guide what Describe actually presents.

If I understand correctly, I think we can define and consume those flags entirely on the cmd side since OperatorListAvailable will return all the channels for a given package. Then we can filter the right channel on the cmd side and optionally decide to print the long description when outputting to the console?

@komish
Copy link
Contributor Author

komish commented Sep 10, 2020

@joelanford Ah, yup! Last two commits should reflect those changes. Here's a quick breakdown of what changed in the latest commits.

  1. Move command line flags to the internal/cmd/operator_describe.go package from pkg/action/operator_describe.go.
  2. Utilize the action.OperatorListAvailable struct instead of the action.OperatorDescribe struct. No changes required on the action.OperatorListAvailable struct required for this to work 👍
  3. Removed pkg/action/operator_describe.go as relevant methods were moved and nothing else is needed from it.

Should be working as we described here. Let me know how this looks, any further changes, and if OK I can go through and do some commit squashing as needed.

@komish
Copy link
Contributor Author

komish commented Sep 23, 2020

Hi folks! Just a friendly ping here to see what might be next for this PR.

@joelanford
Copy link
Member

Sorry for the delay @komish!

I've been a bit busy lately on some other priorities. At a glance, I think this looks pretty good, but I need to find some time to review a little more in-depth one more time. I should be able to find some time in the next week or two.

Copy link
Member

@joelanford joelanford left a comment

Choose a reason for hiding this comment

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

Also a couple of helper functions, GetAvailableChannels() and GetSupportedInstallModes(). The latter can probably be moved away from being a method on action.OperatorShow into a standalone function somewhere so that really only leaves GetAvailableChannels() which I might want to include in action.OperatorListAvailable.

Yeah I agree we can consolidate some of this. There is some other potential minor refactoring I'm thinking of doing, so I can handle this in a follow-up.

Overall, looks basically ready to merge. Just one question about asNewlineDelimString vs strings.Join.

internal/cmd/operator_describe.go Outdated Show resolved Hide resolved
Signed-off-by: Jose R. Gonzalez <josegonzalez89@gmail.com>
@komish
Copy link
Contributor Author

komish commented Oct 5, 2020

Overall, looks basically ready to merge. Just one question about asNewlineDelimString vs strings.Join.
Can this function replaced by strings.Join(stringItems, "\n")?

Whoops! It absolutely can. Fixed.

@joelanford joelanford changed the title add operator show command add operator describe command Oct 5, 2020
@joelanford joelanford merged commit 6793f28 into operator-framework:main Oct 5, 2020
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.

None yet

4 participants