Skip to content

OLM 1.0 OCP 4.15 clean up tasks #71400

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

Merged

Conversation

michaelryanpeter
Copy link
Contributor

@michaelryanpeter michaelryanpeter commented Feb 8, 2024

Version(s): 4.15

Issue: OSDOCS-8881

Link to docs preview:

QE review:

  • QE has approved this change.

Additional information:

@michaelryanpeter michaelryanpeter added this to the Planned for 4.15 GA milestone Feb 8, 2024
@openshift-ci openshift-ci bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Feb 8, 2024
@ocpdocs-previewbot
Copy link

ocpdocs-previewbot commented Feb 8, 2024

🤖 Fri Feb 16 15:01:30 - Prow CI generated the docs preview: https://71400--ocpdocs-pr.netlify.app

@michaelryanpeter
Copy link
Contributor Author

@jianzhangbjz or @Xia-Zhao-rh PTAL.

@michaelryanpeter
Copy link
Contributor Author

@bandrade PTAL

@michaelryanpeter michaelryanpeter force-pushed the 4.15-clean-up-tasks branch 2 times, most recently from 7d9b600 to b90e71e Compare February 13, 2024 20:34
@kevinrizza
Copy link
Member

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Feb 13, 2024
Copy link

@everettraven everettraven left a comment

Choose a reason for hiding this comment

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

A couple typos but other than that LGTM

@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Feb 13, 2024
Copy link

@m1kola m1kola left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Feb 14, 2024
Copy link

@bandrade bandrade left a comment

Choose a reason for hiding this comment

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

/lgtm

@michaelryanpeter
Copy link
Contributor Author

/label peer-review-needed

@openshift-ci openshift-ci bot added the peer-review-needed Signifies that the peer review team needs to review this PR label Feb 15, 2024
@snarayan-redhat snarayan-redhat added peer-review-in-progress Signifies that the peer review team is reviewing this PR and removed peer-review-needed Signifies that the peer review team needs to review this PR labels Feb 16, 2024
Copy link
Contributor

@snarayan-redhat snarayan-redhat left a comment

Choose a reason for hiding this comment

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

great work! Couple of suggestions. LGTM!

@@ -35,9 +35,11 @@ spec:
image:
ref: registry.redhat.io/redhat/redhat-operator-index:v{product-version} <1>
pullSecret: <pull_secret_name> <2>
pollInterval: 30m <3>
Copy link
Contributor

Choose a reason for hiding this comment

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

IIUC, this is a variable/user-defined value. Hence can be represented as a placeholder value, e.g. <poll_interval_duration>

----
<1> Specify the catalog's image in the `spec.source.image` field.
<2> If your catalog is hosted on a secure registry, such as `registry.redhat.io`, you must create a pull secret scoped to the `openshift-catalog` namespace.
<3> Specify the interval for polling the remote registry for newer image digests. The default value is `24h`. Valid units include `s`, `m`, and `h`. To disable polling, set a zero value, such as `0s`.
Copy link
Contributor

@snarayan-redhat snarayan-redhat Feb 16, 2024

Choose a reason for hiding this comment

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

Though it is implied, do you think it could be a good idea to specify seconds, minutes, hours? Up to you :)

----
<1> Specify the interval for polling the remote registry for newer image digests. The default value is `24h`. Valid units include `s`, `m`, and `h`. To disable polling, set a zero value, such as `0s`.
Copy link
Contributor

@snarayan-redhat snarayan-redhat Feb 16, 2024

Choose a reason for hiding this comment

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

same comment as previous

. Edit your CR to update the version to `1.12.1`, as shown in the following example:
. Edit your CR by using one of the following methods:

** If you want to pin your Operator or extension to specific version, such as `1.12.1`, edit your CR similar to the following example:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
** If you want to pin your Operator or extension to specific version, such as `1.12.1`, edit your CR similar to the following example:
** If you want to pin your Operator or extension to specific version, such as `1.12.1`, edit your CR similar to the following example:

@@ -7,7 +7,7 @@
[id="olmv1-version-range-comparisons_{context}"]
= Version comparison strings

You can define a version range by adding a comparison string to the `spec.version` field in an Operator or extension's custom resource (CR). A comparison string is a list of space- or comma-separated values and one or more comparison operators. You can add another comparison string by including an `OR`, or double vertical bar (`||`), comparison operator between the strings.
You can define a version range by adding a comparison string to the `spec.version` field in an Operator or extension's custom resource (CR). A comparison string is a list of space- or comma-separated values and one or more comparison operators surrounded by double quotation marks (`"`). You can add another comparison string by including an `OR`, or double vertical bar (`||`), comparison operator between the strings.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
You can define a version range by adding a comparison string to the `spec.version` field in an Operator or extension's custom resource (CR). A comparison string is a list of space- or comma-separated values and one or more comparison operators surrounded by double quotation marks (`"`). You can add another comparison string by including an `OR`, or double vertical bar (`||`), comparison operator between the strings.
You can define a version range by adding a comparison string to the `spec.version` field in an Operator or extension's custom resource (CR). A comparison string is a list of space- or comma-separated values and one or more comparison operators enclosed in double quotation marks (`"`). You can add another comparison string by including an `OR`, or double vertical bar (`||`), comparison operator between the strings.

@@ -28,7 +28,9 @@ spec:
image:
ref: registry.redhat.io/redhat/redhat-operator-index:v{product-version}
pullSecret: <pull_secret_name>
pollInterval: 24h <1>
Copy link
Contributor

Choose a reason for hiding this comment

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

same as previous suggestion for using placeholder unless this is for a specific scenario.

name: pipelines-operator
spec:
packageName: openshift-pipelines-operator-rh
channel: latest
Copy link
Contributor

@snarayan-redhat snarayan-redhat Feb 16, 2024

Choose a reason for hiding this comment

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

IMO this example might need a placeholder and a corresponding callout to highlight the field specifying the channel?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Heard. I did a draft of the doc where I took that approach. But the goal here is to give an actual example from the RH Operators catalog. So, using a placeholder goes against the goal of showing a customer a working CR.

version: "<1.13"
----
+
For more information, see "Example custom resources that specify a target version".
Copy link
Contributor

Choose a reason for hiding this comment

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

Based on the text in Additional resources section.

Suggested change
For more information, see "Example custom resources that specify a target version".
For more information, see "Example custom resources (CRs) that specify a target version".


** If you want to specify a channel and version or version range, edit your CR similar to the following example:
+
.Example CR with a specified channel
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be specified channel and version?

Copy link
Contributor

@snarayan-redhat snarayan-redhat left a comment

Choose a reason for hiding this comment

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

Sorry for adding another comment. Looking at the preview, I had a question:
Step 3 here seems to have scenarios similar to this section. Can we create sections instead of putting them within the procedure? The procedure could have a single yaml with placeholders for channel and version fields and callouts explaining those.

@snarayan-redhat snarayan-redhat added peer-review-done Signifies that the peer review team has reviewed this PR and removed peer-review-in-progress Signifies that the peer review team is reviewing this PR labels Feb 16, 2024
@michaelryanpeter
Copy link
Contributor Author

@snarayan-redhat Thank you for the review! Great suggestions, and I applied most of them. In terms of the break down of step three in the procedure, this was a specific ask from my PM to format the text and include an additional section that goes over the content in a bit more detail later.

@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Feb 16, 2024
Copy link

openshift-ci bot commented Feb 16, 2024

New changes are detected. LGTM label has been removed.

Copy link

openshift-ci bot commented Feb 16, 2024

@michaelryanpeter: all tests passed!

Full PR test history. Your PR dashboard.

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.

@michaelryanpeter michaelryanpeter merged commit f124014 into openshift:main Feb 16, 2024
@michaelryanpeter
Copy link
Contributor Author

/cherrypick enterprise-4.15

@openshift-cherrypick-robot

@michaelryanpeter: new pull request created: #71741

In response to this:

/cherrypick enterprise-4.15

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.

aireilly pushed a commit to aireilly/openshift-docs that referenced this pull request Feb 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
branch/enterprise-4.15 peer-review-done Signifies that the peer review team has reviewed this PR size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants