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

4.14 OLM 1.0 Tech Preview procedures #66036

Merged

Conversation

michaelryanpeter
Copy link
Contributor

@michaelryanpeter michaelryanpeter commented Oct 10, 2023

Version(s): 4.14

Issues:

Link to docs preview: Installing an Operator from a catalog

QE review:

  • QE has approved this change.

Additional information:

@michaelryanpeter michaelryanpeter added this to the Planned for 4.14 GA milestone Oct 10, 2023
@openshift-ci openshift-ci bot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Oct 10, 2023
@ocpdocs-previewbot
Copy link

ocpdocs-previewbot commented Oct 10, 2023

🤖 Updated build preview is available at:
https://66036--docspreview.netlify.app

Build log: https://circleci.com/gh/ocpdocs-previewbot/openshift-docs/29262

@michaelryanpeter michaelryanpeter force-pushed the 4.14-olm-1.0-tech-preview branch 2 times, most recently from fab5877 to 43b3f35 Compare October 12, 2023 20:30
Copy link
Contributor Author

@michaelryanpeter michaelryanpeter left a comment

Choose a reason for hiding this comment

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

Notes from the engineering read through.

modules/olmv1-about-catalogs.adoc Outdated Show resolved Hide resolved
modules/olmv1-deleting-an-operator.adoc Show resolved Hide resolved
modules/olmv1-adding-a-catalog.adoc Outdated Show resolved Hide resolved
modules/olmv1-about-target-versions.adoc Outdated Show resolved Hide resolved
modules/olmv1-updating-an-operator.adoc Show resolved Hide resolved
modules/olmv1-about-target-versions.adoc Outdated Show resolved Hide resolved
modules/olmv1-about-target-versions.adoc Outdated Show resolved Hide resolved
@michaelryanpeter michaelryanpeter force-pushed the 4.14-olm-1.0-tech-preview branch 5 times, most recently from fa7cc57 to c91051b Compare October 16, 2023 16:14
@michaelryanpeter
Copy link
Contributor Author

@jianzhangbjz or @Xia-Zhao-rh PTAL.

@jianzhangbjz
Copy link

Hi @kuiwang02 , could you help review it? Thanks!
/assign @kuiwang02

@kuiwang02
Copy link

/assign @Xia-Zhao-rh

@jldohmann jldohmann 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 peer-review-needed Signifies that the peer review team needs to review this PR labels Oct 18, 2023
@michaelryanpeter michaelryanpeter force-pushed the 4.14-olm-1.0-tech-preview branch from b4c086b to 8c7c6e1 Compare October 18, 2023 20:50
@Xia-Zhao-rh
Copy link

@Xia-Zhao-rh I think I have addressed your comments.

  • I removed the section on what happens if you leave the target version unspecified. If the bug fix is backported, I will add the content back in.
  • Kevin clarified the issue with the Marketplace catalog.

Would you PTAL?

Hi, @michaelryanpeter LGTM, Thanks.

@michaelryanpeter michaelryanpeter force-pushed the 4.14-olm-1.0-tech-preview branch 2 times, most recently from 31e0937 to 2620a54 Compare October 19, 2023 18:53
@kevinrizza
Copy link
Member

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Oct 19, 2023
@kuiwang02
Copy link

/label qe-approved

@openshift-ci openshift-ci bot added the qe-approved Signifies that QE has signed off on this PR label Oct 20, 2023
@kuiwang02
Copy link

/lgtm

@Xia-Zhao-rh
Copy link

/label qe-approved

@Xia-Zhao-rh
Copy link

/lgtm

modules/olmv1-about-target-versions.adoc Outdated Show resolved Hide resolved
+
[source,terminal]
----
$ oc get package <catalog_name>-<operator_name> -o yaml

Choose a reason for hiding this comment

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

As the same as above, the package_name is better than the operator_name here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

addressed.

type: image
image:
ref: registry.redhat.io/redhat/certified-operator-index:v{product-version}
----

Choose a reason for hiding this comment

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

I guess we also need to list the marketplace operators here, such as,

apiVersion: catalogd.operatorframework.io/v1alpha1
kind: Catalog
metadata:
  name: redhat-marketplace
spec:
  source:
    type: image
    image:
      ref: registry.redhat.io/redhat/redhat-marketplace-index:v{product-version}

Copy link
Contributor Author

@michaelryanpeter michaelryanpeter Oct 20, 2023

Choose a reason for hiding this comment

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

@jianzhangbjz PTAL at @kevinrizza's comment here: #66036 (comment)

I believe that engineering's preference is to leave off the Marketplace catalog, since it is built by IBM and we have less control if something doesn't work with OLM 1.0 at this stage of development.

If you feel strongly that this should be included, I will open a new ticket and PR for us to discuss the issue. We have until Friday 10/27 for docs merge freeze, so we have a bit of time to discuss this.

Copy link
Contributor Author

@michaelryanpeter michaelryanpeter Oct 20, 2023

Choose a reason for hiding this comment

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

I created issue OSDOCS-8299. After this PR merges, I will create a follow up PR with the Marketplace catalog, so that we can get the change published before Docs Merge Freeze on 10/27.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Follow up Marketplace catalog PR: #66647

Choose a reason for hiding this comment

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

ok, thanks!

@michaelryanpeter michaelryanpeter force-pushed the 4.14-olm-1.0-tech-preview branch from 2620a54 to 996ed2c Compare October 20, 2023 17:05
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Oct 20, 2023
@openshift-ci
Copy link

openshift-ci bot commented Oct 20, 2023

New changes are detected. LGTM label has been removed.

@michaelryanpeter michaelryanpeter force-pushed the 4.14-olm-1.0-tech-preview branch from 996ed2c to b72b4d9 Compare October 20, 2023 17:18
@michaelryanpeter michaelryanpeter merged commit 5c84b3d into openshift:main Oct 20, 2023
1 check passed
@michaelryanpeter
Copy link
Contributor Author

/cherrypick enterprise-4.14

@openshift-cherrypick-robot

@michaelryanpeter: new pull request created: #66645

In response to this:

/cherrypick enterprise-4.14

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
branch/enterprise-4.14 peer-review-done Signifies that the peer review team has reviewed this PR qe-approved Signifies that QE has signed off on this PR size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.