enhancement to add test metadata into the bundle #18

jmccormick2001 · 2020-04-29T17:07:16Z

This PR is an enhancement to allow test metadata to be added into a bundle, this will enable testing against a bundle. For operator-sdk scorecard development underway, we would make use of this feature to co-locate tests with the operator metadata. End users would use this feature to add test metadata, update test metadata within their operator bundles. Test frameworks would need an ability to extract test metadata from bundles when executing tests.

ecordell · 2020-04-29T17:45:20Z

enhancements/scorecard-metadata.md

+            └── example2
+```
+
+In the above proposed layout, the test metadata resides under a new


The spec for labels/annotations should be used here as well (see the bundle enhancement)

For these tests, I would think something along these lines:

annotations: operators.operatorframework.io.test.mediatype.v1: "scorecard+v1" operators.operatorframework.io.test.config.v1: "/tests/scorecard/config.yaml" # optional operators.operatorframework.io.test.type.v1: "kuttl"

For reference, this is what they look like for an operator bundle:

annotations: operators.operatorframework.io.bundle.mediatype.v1: "registry+v1" operators.operatorframework.io.bundle.manifests.v1: "path/to/manifests/" operators.operatorframework.io.bundle.metadata.v1: "path/to/metadata/" # optional operators.operatorframework.io.bundle.package.v1: "$packageName" operators.operatorframework.io.bundle.channels.v1: "alpha,stable" operators.operatorframework.io.bundle.channel.default.v1: "stable"

The advantage of defining the test bundle this way is that we are free to combine / split these as needed.

A combined manifest / test bundle might look like this:

annotations: operators.operatorframework.io.bundle.mediatype.v1: "registry+v1" operators.operatorframework.io.bundle.manifests.v1: "/manifests/" operators.operatorframework.io.bundle.metadata.v1: "/metadata/" operators.operatorframework.io.bundle.package.v1: "couchbase" operators.operatorframework.io.bundle.channels.v1: "alpha,stable" operators.operatorframework.io.bundle.channel.default.v1: "stable" operators.operatorframework.io.test.mediatype.v1: "scorecard+v1" operators.operatorframework.io.test.config.v1: "/tests/scorecard/config.yaml" operators.operatorframework.io.test.type.v1: "kuttl"

or they could be two separate images, each with their own metadata.

A few questions

Would it make sense to leave room in the bundle spec for multiple sets of test metadata to exist simultaneously in the same image?

Or would we just have two separate test bundles, each with their own mediatype?

I don't think SDK or scorecard has this use case at the moment, but once the community sees scorecard test metadata packaged into a bundle image, I could see other test frameworks and tools jumping into the fold.

Does the mediatype define the other label keys that are expected to exist?

So in the scorecard case, scorecard could query the image labels, and check to see if

operators.operatorframework.io.test.mediatype.v1 == "scorecard+v1"

If it finds that, it knows look for operators.operatorframework.io.test.config.v1 and operators.operatorframework.io.test.type.v1 because the scorecard+v1 mediatype specifies the keys?

Some scorecard tests verify bundle contents, and other scorecard tests run kuttl. So in the case where the registry+v1 files are in a different image than the scorecard+v1 files, will the bundle image reference handed to scorecard be a manifest list that contains pointers to the separate images? And then scorecard would iterate those images querying the labels to see if it needs to pull the image contents?

As an MVP, is it reasonable for scorecard to assume that registry and scorecard files will be bundled together in a single image?

What about a operators.operatorframework.io.test.mediatypes.v1 label that holds a list of test mediatypes?

I think so based on how I'm interpreting the bundle enhancement, and it would make sense to assume it does. @ecordell @kevinrizza confirm?

Good question.

Probably, assuming we cannot run any of the current alpha scorecard tests without registry contents.

What about a operators.operatorframework.io.test.mediatypes.v1 label that holds a list of test mediatypes?

That strikes me as something that could get unweidly, but not sure. Looking at the bundle spec, it isn't clear to me if there is a specific syntax to the label keys. Can the labels be parsed into a map[string]interface{} such that array indexing would work? Or is it purely dot-delimited?

For example, can I define an array of tests via these labels?

operators.operatorframework.io.test[0].mediatype.v1: "scorecard+v1" operators.operatorframework.io.test[0].scorecard.config.v1: "/tests/scorecard/config.yaml" operators.operatorframework.io.test[1].mediatype.v1: "othertestframework+v1" operators.operatorframework.io.test[1].othertestframework.foo.v1: "bar"

Another option could be to give scorecard it's own special key:

operators.operatorframework.io.scorecard.mediatype.v1: "scorecard+v1" operators.operatorframework.io.scorecard.config.v1: "/tests/scorecard/config.yaml"

An array is weird because you can't "index" on labels since they're just a map key. Having a scorecard-only key makes more sense.

Would it make sense to leave room in the bundle spec for multiple sets of test metadata to exist simultaneously in the same image?

+1 I think it does

As an MVP, is it reasonable for scorecard to assume that registry and scorecard files will be bundled together in a single image?

I actually think that we do not want to support a "test" only bundle. That manifests and metadata are required for the scorecard config

Rather then a list of labels, could we have a "test" config that describes different types of tests that is versioned. ....test.mediatype.v1: "operatorframework+v1" and a location label associated like config.

Then we create a new config that can describe tests. This file will assume that it is in the top-level directory for the rest of the tests. Maybe a little easier way of managing this?

@shawn-hurley defining another config for tests sounds like overkill. We only really need the mediaType and config path for each test anyway. Can we leverage the fact that operator-framework owns the operators.operatorframework.io portion of label keys instead?

Then we could do something like what we originally had and force users to set different domains:

# Ours operators.operatorframework.io.test.mediatype.v1: scorecard+v1 operators.operatorframework.io.test.config.v1: tests/scorecard/config.yaml # Custom operators.other.domain.test.mediatype.v1: custom+v1 operators.other.domain.test.config.v1: tests/custom/config.yaml ...

We control the scorecard+v1 mediaType, so if other operator-framework-owned test types need to be added (ex. kuttl) we can increment versions or change mediaType names.

Another option is to set a different subdomain test on label keys:

# Ours test.operatorframework.io.scorecard.mediatype.v1: scorecard+v1 test.operatorframework.io.scorecard.config.v1: tests/scorecard/config.yaml # Custom test.operatorframework.io.custom.mediatype.v1: custom+v1 test.operatorframework.io.custom.config.v1: tests/scorecard/config.yaml ...

Personally I prefer the former because it distinguishes ownership of labels and is flexible enough for our needs.

We control the scorecard+v1 mediaType, so if other operator-framework-owned test types need to be added (ex. kuttl) we can increment versions or change mediaType names.

Makes sense to me.

@jmccormick2001 this never got addressed

@shawn-hurley if you are ok with Eric's suggestion I'll update the enhancement to include, if not, then I can re-open this PR where we can discuss it further.

kensipe

I have similar questions to @joelanford

is this in the same image as operator?
if separate, how are they associated?
if together, is the image running as operator in cluster and a separate instance running tests? or running tests from the cluster (which seems odd)

estroz

Looks good so far. Some nits and a question about generalizing this.

enhancements/scorecard-metadata.md

estroz · 2020-05-12T02:46:52Z

enhancements/scorecard-metadata.md

+
+### Non-Goals
+
+This enhancement is scoped to define the location within the bundle for


A thought: generalizing this PR to provide a spec for adding arbitrary items to a bundle image (what are the requirements, labels, etc), and using scorecard test definitions as a living example, would allow us to write more succinct bundle-modifying EPs in the future. This is assuming there isn't already a spec for doing so.

Co-authored-by: Eric Stroczynski <estroczy@redhat.com>

estroz · 2020-05-13T17:09:22Z

From offline discussion:

This PR should act as a guide for both future additive bundle specs and a set of "rules" for creating such a spec. The latter could be an addendum to the enhancement, or a separate doc in its own right.
Because the bundle format is extensible, there should be an abstraction over bundle.Dockerfile and annotations.yaml that allows label modification without giving users too much freedom to modify bundles. Not sure what that looks like yet, but it will likely involve some interface work in operator-registry.

jmccormick2001 · 2020-05-14T18:37:39Z

I updated the enhancement to add the proposed annotations plus mention that operator-registry would need to have an API to add this sort of metadata into a bundle.

joelanford · 2020-05-15T19:41:57Z

enhancements/scorecard-metadata.md

+  operators.operatorframework.io.test.mediatype.v1: "scorecard+v1"
+  operators.operatorframework.io.test.config.v1: "/tests/scorecard/config.yaml"
+  # optional
+  operators.operatorframework.io.test.type.v1: "kuttl"


What does operators.operatorframework.io.test.type.v1: "kuttl" convey and how would we act on it?

joelanford · 2020-05-15T19:55:37Z

enhancements/scorecard-metadata.md

+            └── example2
+```
+
+In the above proposed layout, the test metadata resides under a new


What about a operators.operatorframework.io.test.mediatypes.v1 label that holds a list of test mediatypes?

That strikes me as something that could get unweidly, but not sure. Looking at the bundle spec, it isn't clear to me if there is a specific syntax to the label keys. Can the labels be parsed into a map[string]interface{} such that array indexing would work? Or is it purely dot-delimited?

For example, can I define an array of tests via these labels?

operators.operatorframework.io.test[0].mediatype.v1: "scorecard+v1" operators.operatorframework.io.test[0].scorecard.config.v1: "/tests/scorecard/config.yaml" operators.operatorframework.io.test[1].mediatype.v1: "othertestframework+v1" operators.operatorframework.io.test[1].othertestframework.foo.v1: "bar"

Another option could be to give scorecard it's own special key:

operators.operatorframework.io.scorecard.mediatype.v1: "scorecard+v1" operators.operatorframework.io.scorecard.config.v1: "/tests/scorecard/config.yaml"

shawn-hurley

I would like to make sure we capture:

The ability for other types of tests to be added. The initial version may only contain the scorecard tests, but that is ok if the design is extensible.
The ability to version the tests that are added. Scorecard v1 tests look like X and maybe in a year or two we have a breaking change, we would want to be able to determine if the tests in the bundle are v1 or v2.

shawn-hurley · 2020-05-18T14:24:15Z

enhancements/scorecard-metadata.md

+            └── example2
+```
+
+In the above proposed layout, the test metadata resides under a new


Would it make sense to leave room in the bundle spec for multiple sets of test metadata to exist simultaneously in the same image?

+1 I think it does

As an MVP, is it reasonable for scorecard to assume that registry and scorecard files will be bundled together in a single image?

I actually think that we do not want to support a "test" only bundle. That manifests and metadata are required for the scorecard config

Rather then a list of labels, could we have a "test" config that describes different types of tests that is versioned. ....test.mediatype.v1: "operatorframework+v1" and a location label associated like config.

Then we create a new config that can describe tests. This file will assume that it is in the top-level directory for the rest of the tests. Maybe a little easier way of managing this?

shawn-hurley · 2020-05-18T14:25:22Z

enhancements/scorecard-metadata.md

+  operators.operatorframework.io.test.type.v1: "kuttl"
+```
+
+For the purpose of this enhancement, scorecard would assume that its test


as commented above, I think we want to make this a hard requirement

It's probably worth thinking about stripping test data for disconnected clusters?

I added a note in there into the user story and API description about the need to add/update/remove custom metadata like this to support disconnected environments. Good catch.

jmccormick2001 · 2020-05-19T19:48:44Z

I updated this to show how a test framework other than scorecard would show up in the label nomenclature just as an example.

jmccormick2001 · 2020-05-21T13:03:34Z

I removed the annotation that specified kuttl, I didn't see what its purpose would be in the context of scorecard. Scorecard will just process tests as it finds them within the scorecard metadata essentially, regardless if they are kuttl or not. The only value I could think of in having a kuttl annotation is if some other tooling outside of scorecard cared about what types of scorecard tests were being run, I'm not aware of that being a requirement at this point.

jmccormick2001 · 2020-05-29T15:22:32Z

@ecordell wdyt? did I miss any of your concerns/questions or items I need to address?

estroz

One recommendation on labels and two nits.

enhancements/scorecard-metadata.md

estroz · 2020-05-29T16:25:36Z

enhancements/scorecard-metadata.md

+            └── example2
+```
+
+In the above proposed layout, the test metadata resides under a new


@shawn-hurley defining another config for tests sounds like overkill. We only really need the mediaType and config path for each test anyway. Can we leverage the fact that operator-framework owns the operators.operatorframework.io portion of label keys instead?

Then we could do something like what we originally had and force users to set different domains:

# Ours operators.operatorframework.io.test.mediatype.v1: scorecard+v1 operators.operatorframework.io.test.config.v1: tests/scorecard/config.yaml # Custom operators.other.domain.test.mediatype.v1: custom+v1 operators.other.domain.test.config.v1: tests/custom/config.yaml ...

We control the scorecard+v1 mediaType, so if other operator-framework-owned test types need to be added (ex. kuttl) we can increment versions or change mediaType names.

Another option is to set a different subdomain test on label keys:

# Ours test.operatorframework.io.scorecard.mediatype.v1: scorecard+v1 test.operatorframework.io.scorecard.config.v1: tests/scorecard/config.yaml # Custom test.operatorframework.io.custom.mediatype.v1: custom+v1 test.operatorframework.io.custom.config.v1: tests/scorecard/config.yaml ...

Personally I prefer the former because it distinguishes ownership of labels and is flexible enough for our needs.

Co-authored-by: Eric Stroczynski <estroczy@redhat.com>

removed the kuttl files from the example, here we assume that we control what is under tests/scorecard so an example of that is not useful to call out.

…upport disconnected environments

ecordell · 2020-06-11T12:50:46Z

enhancements/scorecard-metadata.md

+For the purpose of this enhancement, scorecard would assume that its test
+metadata would be in the same bundle image as the operator image.
+
+The operator-registry API would need to be updated to support an ability


Can we quickly clarify what this means?

I'm interpreting this as: "the operator-registry github repo will contain a golang API that can be used to add custom metadata to a bundle image".

We could write a separate enhancement for it, but there is some ux to hash out for how this is exposed via libraries or opm.

@ecordell see what you think about the updates I made, I tried to get it more specific to operator-registry repo golang API.

enhancements/scorecard-metadata.md

Co-authored-by: Evan Cordell <cordell.evan@gmail.com>

…r-registry API requirements

ecordell · 2020-06-15T22:13:53Z

/lgtm

joelanford · 2020-06-16T00:45:29Z

/lgtm

…like other catalog images

shawn-hurley · 2020-06-22T15:53:42Z

/lgtm

I am a little worried that the user stories may require more work and design. We should consider updating this enhancement as we implement them.

initial version

7907447

ecordell reviewed Apr 29, 2020

View reviewed changes

kensipe reviewed May 5, 2020

View reviewed changes

estroz suggested changes May 12, 2020

View reviewed changes

Jeff McCormick and others added 3 commits May 12, 2020 16:31

Update enhancements/scorecard-metadata.md

31f7c4c

Co-authored-by: Eric Stroczynski <estroczy@redhat.com>

Update enhancements/scorecard-metadata.md

880f90f

Co-authored-by: Eric Stroczynski <estroczy@redhat.com>

Update enhancements/scorecard-metadata.md

32a7e7a

Co-authored-by: Eric Stroczynski <estroczy@redhat.com>

add updates based on review comments and discussions

bed8520

joelanford reviewed May 15, 2020

View reviewed changes

joelanford mentioned this pull request May 15, 2020

[scorecard] inject scorecard config to bundle image operator-framework/operator-sdk#3045

Merged

shawn-hurley reviewed May 18, 2020

View reviewed changes

estroz mentioned this pull request May 18, 2020

scorecard (alpha): use bundle labels in pod runner operator-framework/operator-sdk#3062

Merged

update to show how other test frameworks could be represented in labels

f2a892b

joelanford mentioned this pull request May 20, 2020

[sdk-metrics] add sdk-stamps to bundle resources #21

Merged

remove kuttl annotations

16e039c

change key to be operators.operatorframework.io.scorecard

ec58de0

estroz suggested changes May 29, 2020

View reviewed changes

Jeff McCormick and others added 2 commits May 29, 2020 11:32

Update enhancements/scorecard-metadata.md

cba3343

Co-authored-by: Eric Stroczynski <estroczy@redhat.com>

Update scorecard-metadata.md

f631f5f

removed the kuttl files from the example, here we assume that we control what is under tests/scorecard so an example of that is not useful to call out.

estroz mentioned this pull request Jun 1, 2020

scorecard new API operator-framework/operator-sdk#3125

Merged

add note on the API needing an ability to remove custom metadata to s…

70904ed

…upport disconnected environments

ecordell reviewed Jun 11, 2020

View reviewed changes

Jeff McCormick and others added 2 commits June 11, 2020 14:27

Update enhancements/scorecard-metadata.md

7d9f3be

Co-authored-by: Evan Cordell <cordell.evan@gmail.com>

address PR comments, add user story 3, be more specific as to operato…

32baea4

…r-registry API requirements

openshift-ci-robot assigned ecordell Jun 15, 2020

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jun 15, 2020

openshift-ci-robot assigned joelanford Jun 16, 2020

add user story 4 about providing the option of mirroring test images …

0fbf9e7

…like other catalog images

openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Jun 16, 2020

openshift-ci-robot assigned shawn-hurley Jun 22, 2020

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jun 22, 2020

jmccormick2001 merged commit f5d0096 into operator-framework:master Jun 22, 2020


		### Non-Goals

		This enhancement is scoped to define the location within the bundle for

enhancement to add test metadata into the bundle #18

enhancement to add test metadata into the bundle #18

Uh oh!

Conversation

jmccormick2001 commented Apr 29, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

estroz May 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kensipe left a comment

Choose a reason for hiding this comment

Uh oh!

estroz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

estroz commented May 13, 2020

Uh oh!

jmccormick2001 commented May 14, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shawn-hurley left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jmccormick2001 commented May 19, 2020

Uh oh!

jmccormick2001 commented May 21, 2020

Uh oh!

jmccormick2001 commented May 29, 2020

Uh oh!

estroz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

estroz May 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ecordell commented Jun 15, 2020

estroz May 29, 2020 •

edited

Loading

estroz May 29, 2020 •

edited

Loading