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

Documentation for Odo integration with Operator Hub #3029

Merged
merged 6 commits into from
May 11, 2020
Merged

Documentation for Odo integration with Operator Hub #3029

merged 6 commits into from
May 11, 2020

Conversation

dharmit
Copy link
Member

@dharmit dharmit commented Apr 28, 2020

What type of PR is this?
/kind documentation
[skip ci]

What does does this PR do / why we need it:
Adds documentation explaining how to use Odo and Operator Hub

Which issue(s) this PR fixes:

Fixes #2779

How to test changes / Special notes to the reviewer:

# Odo integration with Operator Hub
---

When working in experimental mode, Odo provides the abilitiy to work with
Copy link

Choose a reason for hiding this comment

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

abilitiy -> ability

In this document we will cover the following:

. <<list-operators,List the operators installed in the current project>>
. <<dry-run,Print the YAML specification that wil be used to start the service from a CRD>>
Copy link

Choose a reason for hiding this comment

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

wil -> will

and the
link:https://docs.openshift.com/container-platform/4.3/operators/crds/crd-extending-api-with-crds.html#crd-custom-resource-definitions_crd-extending-api-with-crds[CRD
(Custom Resource Definitions)] provided by these operators. For example, we
have installed Etcd and MongoDB opertors and the output we get is like below:
Copy link

Choose a reason for hiding this comment

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

opertors -> operators

Copy link
Contributor

Choose a reason for hiding this comment

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

We generally capitalize Operators.

@@ -0,0 +1,253 @@
:source-highlighter: pygments

# Odo integration with Operator Hub
Copy link

Choose a reason for hiding this comment

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

Should Odo/odo be capitalized?

Copy link
Contributor

Choose a reason for hiding this comment

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

No, odo should not be capitalized. Good catch Yana.

Copy link

@yhontyk yhontyk left a comment

Choose a reason for hiding this comment

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

Found a few typos.

Do you want me to make stylistic suggestions to the language (the many very rigid lang rules we use for downstream...) or do you prefer to have these upstream docs to be written in a less strict/more relaxed language?

@kadel
Copy link
Member

kadel commented Apr 28, 2020

/approve

@openshift-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kadel

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

The pull request process is described 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

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. Required by Prow. label Apr 28, 2020
@codecov
Copy link

codecov bot commented Apr 28, 2020

Codecov Report

Merging #3029 into master will decrease coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3029      +/-   ##
==========================================
- Coverage   45.43%   45.42%   -0.01%     
==========================================
  Files         109      109              
  Lines       10400    10408       +8     
==========================================
+ Hits         4725     4728       +3     
- Misses       5220     5223       +3     
- Partials      455      457       +2     
Impacted Files Coverage Δ
pkg/sync/sync.go 48.51% <0.00%> (-1.00%) ⬇️
pkg/devfile/adapters/docker/component/utils.go 71.36% <0.00%> (-0.91%) ⬇️
pkg/devfile/adapters/helper.go 10.34% <0.00%> (-0.37%) ⬇️
pkg/debug/info.go 67.56% <0.00%> (ø)
pkg/occlient/utils.go 11.11% <0.00%> (ø)
pkg/odo/cli/service/ui/ui.go 35.48% <0.00%> (ø)
pkg/odo/cli/service/create.go 11.11% <0.00%> (ø)
pkg/devfile/adapters/common/command.go 97.87% <0.00%> (ø)
pkg/service/service.go 68.30% <0.00%> (+0.54%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e1232ad...22d9114. Read the comment docs.

@dharmit
Copy link
Member Author

dharmit commented Apr 28, 2020

Found a few typos.

Fixed them all. Thanks for the catch!

Do you want me to make stylistic suggestions to the language (the many very rigid lang rules we use for downstream...) or do you prefer to have these upstream docs to be written in a less strict/more relaxed language?

I'd prefer to keep it somewhat relaxed. But how do other teams (for eg: CRC) do this? Also, making changes here would mean that you can use more things as-is, right?

docs/user/operator-hub.adoc Outdated Show resolved Hide resolved
Copy link
Member

@cdrage cdrage left a comment

Choose a reason for hiding this comment

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

Hey @dharmit thank you so much for detailing how to use odo with Operator Hub.

My question is, is there any way you can format it similar to how we do some of the OpenShift tutorials? For example:

  • Showing how to create a project first
  • Using the numeric formatting (1. 2. 3. etc) of the steps

Similar to this example: https://odo.dev/docs/creating-a-multicomponent-application-with-odo/

The source being here: https://github.com/openshift/openshift-docs/blob/master/cli_reference/openshift_developer_cli/creating-a-multicomponent-application-with-odo.adoc

If not, no worries. at the moment, the site and the synchronization to the current docs is a bit messed up.

I know this is a lot to ask. But I can re-organize it if you'd like. I wouldn't be removing any of your notes, but simply putting it in a similar format as our current documentation.

# odo integration with Operator Hub
---

When working in experimental mode, Odo provides the ability to work with
Copy link
Member

Choose a reason for hiding this comment

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

odo should not be capitalized

start from it.
====

To start an `EtcdCluster` service from `etcdoperator.v0.9.4` operator, you need
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe mention that the name of the CRD is case sensitive?


=== [[dry-run]]Print the YAML used to start a service

Odo provides the feature to print the YAML definition of the service (Custom
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: s/Odo/odo

@girishramnani
Copy link
Contributor

The docs are very nice and thorough, the one thing that worries me is that it exposes alot of words like pods, CRDs etc which we want to hide from the user as thats one of the principals of odo I suppose.
I understand that, the docs isn't the only place where this needs to be taken care of. or maybe we cannot solve this problem for the operator hub feature due to the limitations of the metadata accessible to odo. something to think about I guess.

@kadel
Copy link
Member

kadel commented Apr 30, 2020

The docs are very nice and thorough, the one thing that worries me is that it exposes alot of words like pods, CRDs etc which we want to hide from the user as thats one of the principals of odo I suppose.
I understand that, the docs isn't the only place where this needs to be taken care of. or maybe we cannot solve this problem for the operator hub feature due to the limitations of the metadata accessible to odo. something to think about I guess.

I wouldn't be afraid of using terms like Pods or CRDs in the documentation. This is a great place to explain what odo is doing under the hood, for those that are interested, and reveal some of the "odo magic".

@dharmit
Copy link
Member Author

dharmit commented May 5, 2020

Hey @dharmit thank you so much for detailing how to use odo with Operator Hub.

My question is, is there any way you can format it similar to how we do some of the OpenShift tutorials? For example:

* Showing how to create a project first

* Using the numeric formatting (1. 2. 3. etc) of the steps

Similar to this example: https://odo.dev/docs/creating-a-multicomponent-application-with-odo/

The source being here: https://github.com/openshift/openshift-docs/blob/master/cli_reference/openshift_developer_cli/creating-a-multicomponent-application-with-odo.adoc

If not, no worries. at the moment, the site and the synchronization to the current docs is a bit messed up.

I know this is a lot to ask. But I can re-organize it if you'd like. I wouldn't be removing any of your notes, but simply putting it in a similar format as our current documentation.

Sure, I can make the changes that should help this piece use directly on odo website. That would be cool!

@dharmit
Copy link
Member Author

dharmit commented May 5, 2020

The docs are very nice and thorough, the one thing that worries me is that it exposes alot of words like pods, CRDs etc which we want to hide from the user as thats one of the principals of odo I suppose.

I agree with your thought process but I think we've used the corresponding words used in odo terminology all over the place. For example, if we're talking about CSV, we make sure that the word Operator shows up right besides it or at least in the same sentence. Same for CR/CRD and service.

As for pods, it's showing up near the end of the doc where we explain the user that default configuration will bring up three pods of EtcdCluster. I don't see a way to use something else in its place. Three instances of EtcdCluster doesn't seem like entirely right to me. 😞

I understand that, the docs isn't the only place where this needs to be taken care of. or maybe we cannot solve this problem for the operator hub feature due to the limitations of the metadata accessible to odo. something to think about I guess.

I think we should always keep in mind to use the words in odo terminology more liberally than those from k8s terminology. So, if it seems like k8s terminology is used more than odo terminology, it's wrong on our part (here, my part) and we must fix it!

That said, I believe that documentation is a place where we could use words from k8s world so that interested users can know what's happening under the hood.

@dharmit
Copy link
Member Author

dharmit commented May 5, 2020

@cdrage to make changes to this PR to structure the docs like you've suggested, I think I could do below:

  • break the sections in the .adoc file into modules.
  • have a modules directory at a level above docs/user/operator-hub.adoc, so that would be docs/modules

Is there anything else I'd be required to do?

@cdrage
Copy link
Member

cdrage commented May 5, 2020

@dharmit Nope, no need for the modules. What I was suggesting is adding perhaps a "Creating a project" and following a similar format as: https://odo.dev/docs/creating-a-single-component-application-with-odo/

For example, we start with listing pre-requisites and follow a simple step-by-step structure. Have a look at a few of the other examples such as:

https://odo.dev/docs/creating-a-multicomponent-application-with-odo/

and https://odo.dev/docs/creating-an-application-with-a-database/ and notice how similar they are.

@dharmit
Copy link
Member Author

dharmit commented May 6, 2020

@cdrage I've made the changes. Hope they align with your recommendations. Let me know if you'd like me to make any changes. I've kept a separate "Prerequisites" section but let me know if it doesn't make sense to keep it.

Other than that, the docs on https://odo.dev need to change from Log in to an OpenShift cluster: to Login to the Kubernetes/OpenShift cluster:, IMO.

@adisky
Copy link
Contributor

adisky commented May 8, 2020

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label May 8, 2020
@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@dharmit
Copy link
Member Author

dharmit commented May 8, 2020

@cdrage need your input on this comment #3029 (comment). If it looks good, could you please LGTM?

/hold

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. Required by Prow. label May 8, 2020
:source-highlighter: pygments

# odo integration with Operator Hub
---
Copy link
Member

Choose a reason for hiding this comment

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

Have to get rid of the above stuff :) not sure why this is here.

the cluster. It allows listing of Operators, creation of services from CRD
(Custom Resource Definitions) provided by the Operators, printing the YAML
definition and providing custom YAML definition to start the service from a
CRD.
Copy link
Member

Choose a reason for hiding this comment

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

Can you refactor the above sentence? We could instead create it as bullet points? Sorry! Just a very long sentence.

[NOTE]
====
Installation of Operators is not a part of odo workflow. It is something that
your OpenShift/Kubernetes cluster administrator should be able to do for you.
Copy link
Member

Choose a reason for hiding this comment

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

We may have to remove this since you've already got it covered under prerequisites.


=== Prerequisites

- `odo` is installed.
Copy link
Member

Choose a reason for hiding this comment

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

can remove this

=== Prerequisites

- `odo` is installed.
- Required Operators are installed in the project/namespace by cluster
Copy link
Member

Choose a reason for hiding this comment

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

Reword this?

Perhaps something like: Operators are required to be installed in your OpenShift cluster.

And then provide a link?

+
[source,shell]
----
$ export ODO_EXPERIMENTAL=true
Copy link
Member

Choose a reason for hiding this comment

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

See how we do it here: https://odo.dev/docs/deploying-a-devfile-using-odo/#deploying-your-first-devfile

No need to go into detail with regards to adding experimental mode, we can make it just a one-liner under pre-requisites.

Create a project to keep your source code, tests, and libraries organized in a
separate single unit.

1. Log in to the Kubernetes/OpenShift cluster:
Copy link
Member

Choose a reason for hiding this comment

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

Add spacing between /


=== [[list-operators]]Listing the Operators

To list the Operators installed in current project, execute below command:
Copy link
Member

Choose a reason for hiding this comment

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

installed in the current project

Copy link
Member

Choose a reason for hiding this comment

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

No need to have stuff like: execute the command

See examples such as: https://odo.dev/docs/creating-a-single-component-application-with-odo/

You just need to say: List the Operators installed in the current project.

And then show the command.

NAME CRDs
etcdoperator.v0.9.4 EtcdCluster, EtcdBackup, EtcdRestore
mongodb-enterprise.v1.4.5 MongoDB, MongoDBUser, MongoDBOpsManager
----
Copy link
Member

Choose a reason for hiding this comment

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

No need to have this section. Instead, add the output of this into line 75:

https://github.com/openshift/odo/pull/3029/files#diff-15071c5bfeaad296b2a7b29032ce0d56R75

And then go into detail after with the following paragraphs / sentences.

----

In above output, `etcdoperator.v0.9.4` is the Operator while `EtcdCluster`,
`EtcdBackup` and `EtcdRestore` are the CRDs provided by this Operator.
Copy link
Member

Choose a reason for hiding this comment

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

the Operator

@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. Required by Prow. label May 8, 2020
@cdrage
Copy link
Member

cdrage commented May 10, 2020

Can you change this so it's in the

/docs/public folder? Then we can refactor after / LGTM :)

@openshift-ci-robot
Copy link
Collaborator

@dharmit: The following test failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
ci/prow/unit 22d9114 link /test unit

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.

@cdrage
Copy link
Member

cdrage commented May 11, 2020

/lgtm

@cdrage cdrage merged commit 5226dab into redhat-developer:master May 11, 2020
@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label May 11, 2020
@dharmit dharmit deleted the fix-2779 branch May 15, 2020 04:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. Required by Prow. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. Required by Prow. lgtm Indicates that a PR is ready to be merged. Required by Prow.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create documentation for odo & Operator Hub integration
9 participants