-
Notifications
You must be signed in to change notification settings - Fork 253
(plain bundle usage) Documentation on how to add a plain bundle image to a file-based catalog #1127
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
(plain bundle usage) Documentation on how to add a plain bundle image to a file-based catalog #1127
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1127 +/- ##
==========================================
- Coverage 53.78% 53.77% -0.01%
==========================================
Files 108 108
Lines 10163 10163
==========================================
- Hits 5466 5465 -1
- Misses 3736 3738 +2
+ Partials 961 960 -1 |
5b7d36a
to
14b303a
Compare
14b303a
to
1027b4a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks great! There are a few minor changes that need to be made. I've also left a few styling nits:
317130f
to
63e018c
Compare
75e6f43
to
b8afc77
Compare
docs/design/bundle-sources.md
Outdated
|
||
```sh | ||
opm init <operator_name> \ | ||
--default-channel=preview \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this flag required on the opm init
command? If not, I'd suggest removing it because default channel is not an OLMv1 concept.
If it is required, should we consider an alternate instruction that has the user use HEREDOC to cat
an olm.package
directly into a file?
My motivation here is to avoid leaking OLMv0-specific APIs/concepts/behaviors into OLMv1 stuff.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That makes sense. So, the flag itself is not required and I've removed it from the command. However, without the flag too, opm init
generates an empty default channel in the index "defaultChannel": ""
.
I am guessing we should remove this line manually from the index. I think it should be ok for this doc to use the opm init
and then explicitly mention about removing the empty default channel from the index file. Open to knowing about any other alternatives.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Most of these comments address consistency usage. There are a couple of places where the instructions as written did not work for me, and I had to do something different. It could be because I used the Helm memcached-operator
. But I am happy to sync to figure out what we need to change.
Signed-off-by: Rashmi Gottipati <[email protected]>
Signed-off-by: Rashmi Gottipati <[email protected]>
Signed-off-by: Rashmi Gottipati <[email protected]>
Signed-off-by: Rashmi Gottipati <[email protected]>
b8afc77
to
6069ac6
Compare
Signed-off-by: Rashmi Gottipati <[email protected]>
Signed-off-by: Rashmi Gottipati <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left a few nits. Otherwise, LGTM!
…tices for channel naming conventions Signed-off-by: Rashmi Gottipati <[email protected]>
83cdaea
to
2b4c7a8
Compare
/lgtm |
@michaelryanpeter: changing LGTM is restricted to collaborators In response to this:
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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: everettraven, michaelryanpeter, oceanc80, rashmigottipati 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 |
Description of the change:
Write a plain bundle usage document by listing the process required to build a plain bundle image and add it to a file-based catalog.
Motivation for the change:
The goal of this document is to highlight steps to add a plain bundle image to a file-based catalog and enumerate the experimental
olm.bundle.mediatype
field. Currently,opm
does not support plain bundles i.e. plain bundles cannot be rendered throughopm render
as theplain+v0
format is just the manifests andopm render
would require themanifests
andmetadata
in order to be able to render the plain bundle. This documentation provides instructions on how to manually include the plain bundle in a file-based catalog. Please note that this is experimental for now and not recommended for production usage.Reviewer Checklist
/docs
Doc collaboration with @michaelryanpeter