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

render: support rendering FBC from bundle directories #748

Merged

Conversation

joelanford
Copy link
Member

@joelanford joelanford commented Aug 13, 2021

Description of the change:
This PR adds support to the opm render command for bundle directories.

When rendering bundle directories, there is no metadata about the bundle image reference, so the FBC rendered from a bundle directory contains inlined olm.bundle.object properties but lacks an image value.

However callers of opm render with bundle directories will generally know the image that will contain the bundle directory, so they can insert the bundle image reference themselves. To facilitate this image reference injection, this PR adds a --image-ref-template flag that supports templating based on package, name, and version of the bundle.

If specified, render will populate olm.bundle objects' image field with the templated value and use the olm.csv.metadata property. If not specified, render will leave the image field empty and will instead include all of the bundle manifests in the catalog as olm.bundle.object properties.

Motivation for the change:
Some catalog pipelines build the bundle images based on the bundle sources that are included in a source repository. This structure works well in imperative flows where bundle sources alone are sufficient to construct a valid catalog.

However with catalogs themselves now being declarative, a new use case has appeared. Operator authors would like to be able to provide the pipeline with their bundle sources and their catalog metadata. But there is a chicken and egg problem because existing opm tooling only supports generating catalog metadata from a bundle image. If a pipeline builds the bundle images and opm expects the bundle images to exist in order to generate catalog metadata, it is impossible for an operator author to provide catalog and bundle sources at the same time.

In order to fully support this style of pipeline, a follow-on PR may be necessary to update opm's template to support using bundle directory references and supplying an image reference template.

Reviewer Checklist

  • Implementation matches the proposed design, or proposal is updated to match implementation
  • Sufficient unit test coverage
  • Sufficient end-to-end test coverage
  • Docs updated or added to /docs
  • Commit messages sensible and descriptive

@openshift-ci openshift-ci bot requested review from anik120 and benluddy August 13, 2021 01:20
@joelanford joelanford force-pushed the render-pm-bundle-dirs branch from 1ab84df to a413d20 Compare August 13, 2021 01:20
@codecov
Copy link

codecov bot commented Aug 13, 2021

Codecov Report

Attention: Patch coverage is 72.22222% with 20 lines in your changes are missing coverage. Please review.

Project coverage is 53.88%. Comparing base (d74ce59) to head (8b20b15).
Report is 6 commits behind head on master.

Files Patch % Lines
alpha/action/render.go 72.22% 10 Missing and 10 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #748      +/-   ##
==========================================
+ Coverage   53.80%   53.88%   +0.08%     
==========================================
  Files         108      108              
  Lines       10368    10436      +68     
==========================================
+ Hits         5578     5623      +45     
- Misses       3808     3823      +15     
- Partials      982      990       +8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@joelanford
Copy link
Member Author

/hold
We might not need this, at least not urgently.

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 18, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Aug 19, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: joelanford

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 openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 19, 2021
@joelanford joelanford force-pushed the render-pm-bundle-dirs branch from fc6a682 to 558f9ed Compare August 19, 2021 01:04
@openshift-ci openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 20, 2021
@joelanford joelanford force-pushed the render-pm-bundle-dirs branch from 558f9ed to e08b50f Compare October 29, 2021 16:30
@openshift-ci openshift-ci bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 29, 2021
@joelanford joelanford force-pushed the render-pm-bundle-dirs branch from e08b50f to 2f2e8c4 Compare November 1, 2021 14:34
@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 8, 2023
@joelanford joelanford closed this Jun 8, 2023
@joelanford joelanford reopened this Jan 24, 2024
@joelanford joelanford changed the title render: support rendering DC from packagemanifest and bundle directories render: support rendering FBC from packagemanifest and bundle directories Jan 24, 2024
@joelanford joelanford force-pushed the render-pm-bundle-dirs branch from 2f2e8c4 to 40436e0 Compare January 24, 2024 19:27
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 24, 2024
alpha/action/render.go Outdated Show resolved Hide resolved
@joelanford joelanford force-pushed the render-pm-bundle-dirs branch from 40436e0 to 2d99bbd Compare January 25, 2024 18:25
@joelanford joelanford changed the title render: support rendering FBC from packagemanifest and bundle directories render: support rendering FBC from bundle directories Jan 25, 2024
…ries

In order to generate expected olm.bundles, this commit also adds a new
--image-ref-template flag to the `render` subcommand, which callers
can use to generate image references from source data based on package
name, bundle name, and bundle version.

Signed-off-by: Joe Lanford <[email protected]>
@joelanford joelanford force-pushed the render-pm-bundle-dirs branch from 2d99bbd to e6f8920 Compare January 25, 2024 21:28
@grokspawn
Copy link
Contributor

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jan 31, 2024
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Feb 16, 2024
@joelanford joelanford force-pushed the render-pm-bundle-dirs branch from a7a0196 to 8b20b15 Compare February 16, 2024 04:37
@grokspawn
Copy link
Contributor

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Feb 27, 2024
@grokspawn grokspawn removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 27, 2024
@openshift-merge-bot openshift-merge-bot bot merged commit a21f962 into operator-framework:master Feb 27, 2024
12 of 13 checks passed
@joelanford joelanford deleted the render-pm-bundle-dirs branch February 28, 2024 13:41
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. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants