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 a note about Operators. #27668

Closed
wants to merge 1 commit into from
Closed

Add a note about Operators. #27668

wants to merge 1 commit into from

Conversation

yhontyk
Copy link
Contributor

@yhontyk yhontyk commented Nov 25, 2020

Branch 4.6 and 4.7

@openshift-ci-robot openshift-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Nov 25, 2020
@openshift-docs-preview-bot

The preview will be available shortly at:

Copy link

@dharmit dharmit left a comment

Choose a reason for hiding this comment

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

This change looks good to me from odo side.

@pneedle-rh pneedle-rh self-requested a review December 3, 2020 10:05
@pneedle-rh pneedle-rh added branch/enterprise-4.6 branch/enterprise-4.7 peer-review-needed Signifies that the peer review team needs to review this PR labels Dec 4, 2020
@pneedle-rh pneedle-rh added this to the Next Release milestone Dec 4, 2020
Copy link
Contributor

@pneedle-rh pneedle-rh left a comment

Choose a reason for hiding this comment

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

@boczkowska I have now completed my review. Please let me know if you have any questions.

the service. If this YAML has placeholder values or sample values, a service cannot start. You can modify the YAML file and start the service with the modified values. To learn how to modify YAML files and start services from it, go to xref:../../cli_reference/developer_cli_odo/creating-instances-of-services-managed-by-operators.adoc#creating-services-from-yaml-files_creating-instances-of-services-managed-by-operators[Creating services from YAML files].
[NOTE]
====
You cannot install Operators with `{odo-title}` on your cluster. To install Operators, contact your cluster administrator or see xref:../../operators/user/olm-installing-operators-in-namespace.adoc#olm-installing-operators-in-namespace-prereqs[Installing Operators in your namespace]
Copy link
Contributor

Choose a reason for hiding this comment

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

You could consider changing "with {odo-title}" to "by using {odo-title}". As the sentence stands at the moment, it could be interpreted to mean that you cannot install an Operator that includes odo.

Copy link
Contributor

@pneedle-rh pneedle-rh Dec 4, 2020

Choose a reason for hiding this comment

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

You could also move the odo aspect to the end of the sentence, so that the statement about the task that you cannot do (install Operators on your cluster) is not interrupted by the method (using odo).

"You cannot install Operators on your cluster by using {odo-title}."

== Prerequisites
To create services from an Operator, you must ensure that the Operator has valid values defined in its `metadata` to start the requested service. `{odo-title}` uses the `metadata.annotations.alm-examples` YAML file of an Operator to start the service. If this YAML has placeholder values or sample values, a service cannot start. You can modify the YAML file and start the service with the modified values. To learn how to modify YAML files and start services from it, go to xref:../../cli_reference/developer_cli_odo/creating-instances-of-services-managed-by-operators.adoc#creating-services-from-yaml-files_creating-instances-of-services-managed-by-operators[Creating services from YAML files].

.Prerequisites
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would be useful for the reader to move this prerequisites section to the start of each of the procedure modules in this assembly. Then the prerequisites will be presented to the reader if any of those modules are reused in other assemblies.

It doesn't feel like it's in the right place at the moment, as a dot title in the introduction.


== Prerequisites
To create services from an Operator, you must ensure that the Operator has valid values defined in its `metadata` to start the requested service. `{odo-title}` uses the `metadata.annotations.alm-examples` YAML file of an Operator to start the service. If this YAML has placeholder values or sample values, a service cannot start. You can modify the YAML file and start the service with the modified values. To learn how to modify YAML files and start services from it, go to xref:../../cli_reference/developer_cli_odo/creating-instances-of-services-managed-by-operators.adoc#creating-services-from-yaml-files_creating-instances-of-services-managed-by-operators[Creating services from YAML files].
Copy link
Contributor

Choose a reason for hiding this comment

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

You could consider swapping the order of the first three sentences. I suggest introducing the metadata.annotations.alm-examples YAML file, then introducing the requirements for the file. For example:

"{odo-title} uses the metadata.annotations.alm-examples YAML file of an Operator to start a service from an Operator. A service cannot start if this YAML file has placeholder or sample values. To start a service, you must ensure that the metadata section of the YAML has valid values."

Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest changing "To learn how to modify YAML files and start services from it" to "To learn how to modify YAML files and start services from them".

@pneedle-rh pneedle-rh added peer-review-done Signifies that the peer review team has reviewed this PR and removed peer-review-needed Signifies that the peer review team needs to review this PR labels Dec 4, 2020
@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 5, 2020
@openshift-ci-robot
Copy link

@boczkowska: 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.

@pneedle-rh
Copy link
Contributor

@boczkowska please let me know if you have any questions about my previous review comments or about what is required to rebase this pull request. Thanks.

@yhontyk yhontyk closed this Feb 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
branch/enterprise-4.6 branch/enterprise-4.7 needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. peer-review-done Signifies that the peer review team has reviewed this PR size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants