Operator Bundle metadata updates#70
Conversation
openshift-ci-robot
added
the
size/M
| - stable | ||
| ``` | ||
|
|
||
| Also note that this file is optional. If it is not specified, the registry tooling will put the bundle in a default channel. |
There was a problem hiding this comment.
by default channel do you mean a new channel named default or is there some other mechanism to specify the default channel?
There was a problem hiding this comment.
sorry, maybe this isn't clear. I'm not suggesting we are defining the default channel. i'm suggesting that if you don't specify a channel then when that operator bundle is added to an index we will have a predefined default channel for you. I was thinking that channel can be called default.
There was a problem hiding this comment.
So what if an older version of the bundle had a default channel defined?
would you:
- Create a new channel
default - Use the existing channel that was marked as default most recently
- Fail(?)
- ? some other option that I can not think of
What happens if I add a default AFTER I have been using this default? do we move the implicit default channel to this newly named default channel? Do we keep the default channel and the new default channel and mark the new one default even if there is a named "defautl" channel?
I think we need to describe the rules around this.
There was a problem hiding this comment.
Yep we do need to define all of it. In this enhancement I was just calling out that this file was optional. In the operator-registry enhancement I am going to create an additional PR that will describe how all of this will work.
There was a problem hiding this comment.
@shawn-hurley I am going to back off on this for now and will define what I am imagining for this defaulting behavior in a future enhancement.
| An additional point of concern is the fact that the current [operator-registry manifest format](https://github.com/operator-framework/operator-registry#manifest-format) also has a multi-version aggregation file `package.yaml` that describes data about the channels that individual versions are subscribed to. Given that we are splitting up these manifests into separate objects, we now need a way to define how an individual version subscribes to a given channel. To do this, we will create a new metadata file that defines the channels explicitly for each version called `channels.yaml` | ||
|
|
||
| *channels.yaml* | ||
| ```yaml |
There was a problem hiding this comment.
Why not have these be labels / annotations.yaml entries?
operators.operatorframework.io.bundle.package=etcd
operators.operatorframework.io.bundle.channels=alpha,stable
It doesn't help much on-cluster because we have to go through crio, but on the registry side it saves us some blob unpacking.
There was a problem hiding this comment.
I actually like this idea given it will simplify things even further for bundle.
There was a problem hiding this comment.
But the registry will still need to unpack all of the other files, so I'm not sure it really saves us anything. @dinhxuanvu @ecordell Could you elaborate on why this will simplify it? Just because don't have to worry about multiple files?
There was a problem hiding this comment.
From my perspective, we're adding a stripped-down version of package.yaml that only contains package name and some channels. Those can be easily added to annotations.yaml as the annotations.yaml itself is a new metadata file that we require as a part of this bundle image enhancement (similar to what you are doing with channels.yaml in this PR). Two new things can be combined into one. As a result, we only require 1 single metadata file. That sounds beneficial.
To be honest, Evan's approach requires some medium changes in the existing PR that I already have compared to your approach, Kevin, which requires only minor adjustment. I'm totally fine to make changes on either way as long as we can agree on one or the other.
There was a problem hiding this comment.
We discussed offline yesterday, but the idea is that, in the future, we may have access to labels directly on a cluster and can deprecate the in-image annotations.yaml.
kevinrizza
force-pushed
the
add-channel-metadata-to-bundle
branch
from
7853504 to
8d56183
Compare
kevinrizza
force-pushed
the
add-channel-metadata-to-bundle
branch
from
8d56183 to
e8548d1
Compare
kevinrizza
changed the title
| @@ -88,6 +93,11 @@ The labels will also be put inside a YAML file, as shown below. | |||
| annotations: | |||
| operators.operatorframework.io.bundle.resources: "manifests+metadata" | |||
There was a problem hiding this comment.
It seems like we don't need this field if we have the other two?
| annotations: | ||
| operators.operatorframework.io.bundle.resources: "manifests+metadata" | ||
| operators.operatorframework.io.bundle.mediatype: "registry+v1" | ||
| operators.operatorframework.io.bundle.manifests: "path/to/manifests/" |
There was a problem hiding this comment.
should we version all of these?
...mediatype.v1: , ...pacakge.v1, etc?
kevinrizza
force-pushed
the
add-channel-metadata-to-bundle
branch
from
e8548d1 to
8209674
Compare
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ecordell, kevinrizza The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci-robot
added
the
approved
6 participants
This PR updates the olm/operator-bundle enhancement to include a reference to an additional (optional) file that can be added to the metadata folder in the bundle image. This file is an attempt to relate the old
package.yamlfile to metadata defined per operator bundle. Additionally, it updates the expected storage format for the metadata input folder and defines the location more arbitrarily so that it can be specified by an additional annotation.