-
Notifications
You must be signed in to change notification settings - Fork 252
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 opm
docs
#149
Add opm
docs
#149
Conversation
/hold |
/hold cancel |
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.
Everything looks good. A few nits and questions.
README.md
Outdated
│ └── servicemonitor.crd.yaml | ||
└── prometheus.package.yaml | ||
|
||
The operator-sdk also provides tooling to generate the metadata, Dockerfile, and even build the container image itself. Some basic documentation on how this works is located in [this repository's documentation](docs/design/operator-bundle.md#Operator-SDK-CLI). |
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.
Note here that "operator-sdk" reference is being removed on the bundle content validation PR. So we may need to update this later after the PR merged.
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.
sure, that sounds good. I think we can just remove this and then update it once the sdk pushes their own doc we can link to.
@dinhxuanvu updated based on your feedback. |
|
||
# Manifest format | ||
|
||
|
||
We refer to a directory of files with one ClusterServiceVersion as a "bundle". A bundle is typically just a ClusterServiceVersion and the CRDs that define the owned APIs of the CSV, though additional objects may be included. | ||
We refer to a directory of files with one ClusterServiceVersion as a "bundle". A bundle typically includes a ClusterServiceVersion and the CRDs that define the owned APIs of the CSV in its manifest directory, though additional objects may be included. It also includes an annotations file in its metadata folder which defines some higher level aggregate data that helps to describe the format and package information about how the bundle should be added into an index of bundles. |
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.
This says "typically includes", but if the invariants below are accurate, then this is what a bundle minimally includes, right?
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.
CRDs are not mandatory, actually. It minimally includes just the CSV.
|
||
# Bundle images | ||
|
||
Using [OCI spec](https://github.com/opencontainers/image-spec/blob/master/spec.md) container images as a method of storing the manifest and metadata contents of individual bundles, `opm` interacts directly with these images to generate and incrementally update the database. Once you have your [manifests defined](https://operator-framework.github.io/olm-book/docs/packaging-an-operator.html#writing-your-operator-manifests) and have created a directory in the format defined above, building the image is as simple as defining a [Dockerfile](docs/design/operator-bundle.md#Bundle-Dockerfile) and building that image: |
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'm unclear on how flexible the directory structure is intended to be. Is a distinction made between a bundle directory structure described above and the layout of files in a bundle image?
I see the statement "bundle directories are identified solely by the fact that they contain a ClusterServiceVersion, which provides an amount of freedom for layout of manifests," and I'm not sure how that aligns with the proscribed metadata and manifests directories mentioned above.
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.
The only directory we care about is the one on the image. You could, in theory, write your own Dockerfile that pulls files from lots of places to generate the directory structure in place. The flexibility is that the manifest directory can have certain other kube manifests optionally (currently that list includes rbac files and some prometheus stuff aside from the CSVs and CRDs). In future iterations this list can (and will be) extended. But all of that does need to live in the manifests/
subdirectory on your image.
Right now metadata/
has just the annotations.yaml
file, but we are intending for this to be extended in the same fashion.
/lgtm |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dinhxuanvu, kevinrizza 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 |
/retest |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest |
/retest Please review the full test history for this PR and help us cut down flakes. |
1 similar comment
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest |
commands
Description of the change:
Motivation for the change:
Reviewer Checklist
/docs