Skip to content

doc: add missing info in the installation guide and olm repo content #122

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

Closed
wants to merge 1 commit into from
Closed

doc: add missing info in the installation guide and olm repo content #122

wants to merge 1 commit into from

Conversation

camilamacedo86
Copy link
Contributor

@camilamacedo86 camilamacedo86 commented Mar 16, 2021

Description

  • Provide the info over how users can use the sdk commands to install/uninstall and run a bundle locally for development purpose
  • Move relevant content from olm repo readme and install guide to here
  • remove pkg instruction since we would like to re-enforce the bundle format usage instead.

Preview: https://deploy-preview-122--operator-lifecycle-manager.netlify.app/docs/getting-started/

Closes: operator-framework/operator-lifecycle-manager#1929

Copy link
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.

A few changes requested.


## Customizing OLM installation

Deployments of OLM can be stamped out with different configurations by writing a `values.yaml` file and running commands to generate resources.
Copy link
Member

Choose a reason for hiding this comment

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

What does it mean "can be stamped out"?

Copy link
Contributor Author

@camilamacedo86 camilamacedo86 Mar 16, 2021

Choose a reason for hiding this comment

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

Just copy and paste. The goal is we move forward with operator-framework/operator-lifecycle-manager#2046

So, it adds the OLM info from the current installation guide to centralize here. I do not think that we should change that now and it is the current info provided. However, some OLM team member might able to improve that after a follow-up.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe changing it to "Default values used while installing OLM with operator-sdk olm install can be overridden by providing the custom configurations in a values.yaml file, and then using that file to generate the new manifests for deploying custom configured OLM." might help?

Copy link
Contributor Author

@camilamacedo86 camilamacedo86 Mar 20, 2021

Choose a reason for hiding this comment

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

It has no relation with the operator-sdk olm install. The operator-sdk olm install will not use it.
this info is in the install guide https://github.com/operator-framework/operator-lifecycle-manager/blob/master/doc/install/install.md#customizing-olm-installation which is used in the README in the Getting started of the OLM README see : https://github.com/operator-framework/operator-lifecycle-manager#getting-started.

The idea of this PR is to move the Getting Started instructions from the OLM README and add them in the Getting Started of the olm docs. So that, we can link the OLM docs webside in the readme and replace that by the Getting Started here.

@openshift-ci-robot openshift-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Mar 16, 2021
@camilamacedo86 camilamacedo86 requested a review from jmrodri March 16, 2021 19:52
Copy link
Member

@timflannagan timflannagan left a comment

Choose a reason for hiding this comment

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

Awesome, this reads a bit clearer with the updated descriptions and examples. I had a couple of suggestions, minor nits, etc., but nothing blocking 👍.

@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: camilamacedo86
To complete the pull request process, please assign ecordell after the PR has been reviewed.
You can assign the PR to them by writing /assign @ecordell 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

@camilamacedo86
Copy link
Contributor Author

camilamacedo86 commented Mar 19, 2021

H @timflannagan,

all done. Please let me know wdyt?
Btw thank you for the help.

@timflannagan
Copy link
Member

Awesome, this reads pretty well to me. Thanks!

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 19, 2021
Copy link
Contributor

@anik120 anik120 left a comment

Choose a reason for hiding this comment

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

@camilamacedo86 thank you for your PR! Had a few suggestions, and also we can probably get rid of the existing sections "Installing released OLM", "Verify installation". Installing olm with sdk is really cool (with the customization capabilities and everything) and that is what we should be documenting here, not how to install olm from the repo (that's probably the job of the CONTRIBUTING.MD in the repo to document).


When you install OLM, it comes packaged with a number of Operators developed by the community that you can install instantly.
You can use the `pacakagemanifest` api to see the operators available for you to install in your cluster:
## Installing OLM and operator bundle via SDK for development purpose
Copy link
Contributor

Choose a reason for hiding this comment

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

This heading seems like we're mushing two things together. Maybe two headings, one for "Installing OLM with sdk" and "installing an operator bundle with sdk" is better to stick to "one task per function" philosophy?

Also, is the word "development" really necessary here? We may not want to give readers the impression that everything in this document is only is for developers.


```bash
# To install and run locally
$ make run-local
Copy link
Contributor

Choose a reason for hiding this comment

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

Does the make target provide any value for this doc? Looks like this is something to document in the repo, not on our doc site.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is part of getting started - installation docs which now is the OLM README.
Should not the olm docs cover all docs regarding the OLM project? Will not we at some point indeed move the OLM docs to the OLM repo?


## Customizing OLM installation

Deployments of OLM can be stamped out with different configurations by writing a `values.yaml` file and running commands to generate resources.
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe changing it to "Default values used while installing OLM with operator-sdk olm install can be overridden by providing the custom configurations in a values.yaml file, and then using that file to generate the new manifests for deploying custom configured OLM." might help?

# first arg must be a semver-compatible version string
# second arg is the output directory
# third arg is the values.yaml file
./scripts/package_release.sh 1.0.0-myolm ./my-olm-deployment my-values.yaml
Copy link
Contributor

Choose a reason for hiding this comment

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

This script does not exist in this repo, we should probably either copy paste the script in a separate file in the doc site so that users can copy it to create their own file(this is my preference so that users don't have to jump back and forth between the site and the repos), or link to the script at least to begin with.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not sure if I am following that. I understand that:

  • I would be reading the info in https://olm.operatorframework.io/ (which is the doc ref)
  • However, locally I would have the OLM repo cloned since the goal here is to install the OLM project. Then, I would have this script because it is provided there. I mean, I do not think that has a reason for someone that is following the getting started docs to have the docs locally instead of the OLM project.

Then, if we move the script to OLM docs and if changed in the repo it will be providing the wrong info. Is that make sense?


## Overriding the Global Catalog Namespace

It is possible to override the Global Catalog Namespace by setting the `GLOBAL_CATALOG_NAMESPACE` environment variable in the catalog operator deployment.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure what this means? We could probably elaborate on this a little more.

Copy link
Contributor Author

@camilamacedo86 camilamacedo86 Mar 20, 2021

Choose a reason for hiding this comment

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

It is the current info provided by OLM README via the Getting Started section see:

Then, if need to be improved I think would better be done in a follow up for someone that has more context than I.

@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Mar 20, 2021
@openshift-ci-robot
Copy link

New changes are detected. LGTM label has been removed.

@camilamacedo86
Copy link
Contributor Author

Hi @anik120,

@camilamacedo86 thank you for your PR! Had a few suggestions, and also we can probably get rid of the existing sections "Installing released OLM", "Verify installation". Installing olm with sdk is really cool (with the customization capabilities and everything) and that is what we should be documenting here, not how to install olm from the repo (that's probably the job of the CONTRIBUTING.MD in the repo to document).

I am not sure if it would fit well. See that these instructions are provided for those who want to get started with OLM today not necessarily contribute it but only install the project in their clusters/local envs. I tried to update the docs here with your suggestions and answer the questions. Please, let me know if it is OK now and if as anything else that we should do here.

Also, see that the motivation here is we are able to :

  • Move forward with the PR doc: readme add info over olm website and docs operator-lifecycle-manager#2046 which would link the OLM docs and make it available for the OLM users.
  • However, today the README has a section Getting Started -> Installation which has info that was not added to the Getting Started -> Installation of the OLM docs. So, I am just moving from there to here what was missing.

@camilamacedo86 camilamacedo86 requested a review from anik120 March 20, 2021 19:45
@camilamacedo86
Copy link
Contributor Author

camilamacedo86 commented Apr 7, 2021

HI @anik120 and @hasbro17,

All suggestions/comments are addressed. Could we merge this now for we are able to move forward with operator-framework/operator-lifecycle-manager#2046 and have the link in the readme for OLM docs there as we have for SDK?

@camilamacedo86 camilamacedo86 requested a review from hasbro17 April 7, 2021 19:40
@camilamacedo86
Copy link
Contributor Author

HI @anik120.

Can we get this one merged now? WDYT?

@anik120
Copy link
Contributor

anik120 commented Apr 15, 2021

@camilamacedo86 in lieu of operator-framework/operator-lifecycle-manager#2046 (comment)

I think this doc should be simplified to
"Installing latest olm with sdk"
"Installing released version of olm"
(with the installation with sdk coming first)
Also, in the Installing latest olm with sdk section, we should probably just copy paste whatever's there in https://sdk.operatorframework.io/docs/olm-integration/quickstart-bundle/#enabling-olm
I really like the content of the section, but in that doc it appears backward, i.e the doc is for talking about "olm integration with bundles" but then goes on to say "but wait, if you don't have olm installed, do this first".
So this doc can go straight into "install olm with sdk", i.e copy paste of that section

Also, https://sdk.operatorframework.io/docs/olm-integration/quickstart-package-manifests/ seems outdated since we want to move to bundle format instead of packagemanifest format, so we probably need to update https://sdk.operatorframework.io/docs/olm-integration/ soon.

@camilamacedo86
Copy link
Contributor Author

Hi @anik120,

Thank you for the reply. Regarding #122 (comment).

I am not sure if I follow up on all your ideas. My goal here was more only to add the SDK steps. Would you like to get from here what do you think is good and push a PR with your suggestion? WDYT? Would that be easy?

Also, https://sdk.operatorframework.io/docs/olm-integration/quickstart-package-manifests/ seems outdated since we want to move to bundle format instead of packagemanifest format, so we probably need to update https://sdk.operatorframework.io/docs/olm-integration/ soon.

Yes, you are right. We have a task to fix this section on the SDK side see: operator-framework/operator-sdk#4528

However, here we are not linking this point direct. We just link the section https://sdk.operatorframework.io/docs/olm-integration/ in order to avoid this doc gets outdated. In this way, I think that is fine.

@@ -60,73 +60,160 @@ olm-operator 1/1 1 1 5m52s
packageserver 2/2 2 2 5m43s
```

## Installing an Operator using OLM
## Installing OLM with Operator SDK
Copy link
Member

Choose a reason for hiding this comment

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

I think I'd like to see us continue documenting how to install an Operator manually, in addition to how you would achieve that using the operator-sdk functionality. Can we keep the existing content, but add a sub-section here that details how you would do this manually vs. using the operator-sdk commands?

Copy link
Member

Choose a reason for hiding this comment

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

Actually, I think I misread what we're trying to achieve here. Can we move this Installing OLM with Operator SDK further up the documentation as a subheader under the Installing OLM in your cluster?

@openshift-ci
Copy link

openshift-ci bot commented May 13, 2021

@camilamacedo86: PR needs rebase.

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.

@openshift-ci openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 13, 2021
@camilamacedo86
Copy link
Contributor Author

I am closing this one seems outdated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Doc/Website to install/uninstall an operator are not covering the SDK features to help with
6 participants