-
Notifications
You must be signed in to change notification settings - Fork 83
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
doc: add missing info in the installation guide and olm repo content #122
Conversation
There was a problem hiding this 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. |
There was a problem hiding this comment.
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"?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this 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 👍.
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: camilamacedo86 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 |
all done. Please let me know wdyt? |
Awesome, this reads pretty well to me. Thanks! /lgtm |
There was a problem hiding this 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 |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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. |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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. |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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:
- https://github.com/operator-framework/operator-lifecycle-manager#getting-started
- https://github.com/operator-framework/operator-lifecycle-manager/blob/master/doc/install/install.md#overriding-the-global-catalog-namespace
Then, if need to be improved I think would better be done in a follow up for someone that has more context than I.
New changes are detected. LGTM label has been removed. |
Hi @anik120,
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 :
|
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? |
HI @anik120. Can we get this one merged now? WDYT? |
@camilamacedo86 in lieu of operator-framework/operator-lifecycle-manager#2046 (comment) I think this doc should be simplified to 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. |
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?
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 |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
?
@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. |
I am closing this one seems outdated. |
Description
Preview: https://deploy-preview-122--operator-lifecycle-manager.netlify.app/docs/getting-started/
Closes: operator-framework/operator-lifecycle-manager#1929