Skip to content

[proposal] Documentation for olm command workflows#2834

Closed
varshaprasad96 wants to merge 1 commit intooperator-framework:masterfrom
varshaprasad96:csv-enhancement
Closed

[proposal] Documentation for olm command workflows#2834
varshaprasad96 wants to merge 1 commit intooperator-framework:masterfrom
varshaprasad96:csv-enhancement

Conversation

@varshaprasad96
Copy link
Copy Markdown
Member

Description of the change:
This PR provides documentation explaining the workflows for proposed OLM commands and a README describing the deployment of operator by following suggested workflows.

Motivation for the change:
Improve OLM commands.

This PR provides documentation explaining the workflows for
proposed olm commands and a README describing
the deployment of operator by following suggested workflows.
@varshaprasad96 varshaprasad96 changed the title [enhancement-proposal] Documentation for olm command workflows [proposal] Documentation for olm command workflows Apr 14, 2020
@estroz
Copy link
Copy Markdown
Member

estroz commented Apr 14, 2020

This can probably go in operator-framework/enhancements since it enhances OLM and registry tooling.

Copy link
Copy Markdown
Member

@jmrodri jmrodri left a comment

Choose a reason for hiding this comment

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

some questions and some typos.

status: provisional
---

# README describing new command workflows for olm enabled operators
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

when referring to the product or project name I think we should use capitals. olm -> OLM


## Motivation

Currently, the `operator-sdk generate csv` command is used for generating CSVs, which sacffolds the project and writes the csv manifest on disk. This process still requires the developers to manually edit the CSV when any of the required fields are missing. Also, with the depreciation in the use of package manifests for olm, generating CSVs and creating operator bundle image does not require two separate commands. Integrating both the functionalities and providing a single command with an interactive input for collecting the required UI metadata to generate CSV as well as for building an operator bundle image will result in a better developer experience.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

typo: sacffolds -> scaffolds
typo: csv manifest -> CSV manifest
did you mean deprecation? instead of "depreciation"?
typo: for olm -> for OLM


## Goals

The proposal is aimed at expalining the resulting process of developing an olm enabled operator from scratch with the above mentioned improvements.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

typo: expalining -> explaining
typo: olm -> OLM


## Proposal

### README
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

is README the correct the subheading?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Not sure completely. But this is a fake README to give an overview about the process.

Comment on lines +101 to +103
$ Provide DisplayName for your operator:
$ Provide version for your operator:
$ Provide minKubeVersion for CSV:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If these are new prompts we are asking the user, are we thinking about internationalizing the strings? We should at least call out what our plan is for i18n.

https://github.com/leonelquinteros/gotext

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Will there be input validation? If so, how do we ask the user to re-enter?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

These are the UI metadata fields which could asked along with the command. An exhaustive list is here: https://docs.google.com/document/d/1dP6Y9DlE3TuoVXaOdYbuKjo97Qxwqy-07A7msf6_cTQ/edit
We haven't discussed about the implementation yet. Should it be included in the doc?

Copy link
Copy Markdown
Contributor

@dmesser dmesser left a comment

Choose a reason for hiding this comment

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

This looks good. We would create even more appeal with developers if we are able to shorten the commands by relying on reasonable defaults, given we are in a Operator-SDK project directory. If we are not or we couldn't determine the information from on-disk data we can fail fast and ask the user to set the value explicitly.

In addition this proposal would benefit from a fake man page that elaborates on the various switches. This allows for the example workflow to use the short versions whereas the fake man page provides the details about further options.


# Generate kustomize base templates with UI metadata and publish operator in a bundle image
# If you would like to write the operator manifests on disk, specify the path using the flag "--output-dir"
$ operator-sdk generate bundle <IMAGE_REGISTRY>/<TAG>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A couple thoughts to make this command even shorter:

--package - could we derive this from the operator (project) name by default?
--kustomize-dir - it's already proposed as optional given we have a default, that's great
--output-dir - it's already optional, so we are good

Given the above, the command would be shortened to: operator-sdk generate bundle <image>:<tag> which looks really appealing in a (repetitive) workflow.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes, as the package name can be derived from the operator project, have made it optional if the user wants a different package name than the project name.


# Deploy and test the operator on the cluster using OLM
# If path to the kustomize base templates is not present, they are generated from scratch inside ./config folder
$ operator-sdk run --olm --kustomize-dir <Path to directory containing kustomize base templates>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Similar to my previous comment, I am looking for ways to make this even shorter.

--kustomize-dir - can we use the same default like we do in generate bundle?
--operator-version - can we use the the version string from CSV as a default?

If both is possible, this command would be a mere operator-sdk run --olm which is really nice.

Copy link
Copy Markdown
Member Author

@varshaprasad96 varshaprasad96 Apr 16, 2020

Choose a reason for hiding this comment

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

Have modified the operator-sdk run --olm command to:

  1. Deploy the operator for the version specified in the kustomize template for generating CSV.
  2. Use the specified --version if present.

...

# Cleanup
$ operator-sdk cleanup --olm --operator-version <Version of the operator>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We could make --operator-version optional by either taking the version string from the CSV or clean up the first operator we find by this name.

@varshaprasad96
Copy link
Copy Markdown
Member Author

@dmesser - I have made the changes and updated the PR (operator-framework/enhancements#16) in the operator-framework/enhancements repository. Please provide your reviews.

@varshaprasad96
Copy link
Copy Markdown
Member Author

Closing this PR, as it has been moved to operator-framework/enhancements repository.
Link to updated PR: operator-framework/enhancements#16

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.

4 participants