Skip to content

enhancement to add test metadata into the bundle #18

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.

Sign up for GitHub

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

Merged
merged 14 commits into from
Jun 22, 2020
Merged

enhancement to add test metadata into the bundle #18

Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
7907447
initial version
Apr 29, 2020
31f7c4c
Update enhancements/scorecard-metadata.md
May 12, 2020
880f90f
Update enhancements/scorecard-metadata.md
May 12, 2020
32a7e7a
Update enhancements/scorecard-metadata.md
May 12, 2020
bed8520
add updates based on review comments and discussions
May 14, 2020
f2a892b
update to show how other test frameworks could be represented in labels
May 19, 2020
16e039c
remove kuttl annotations
May 21, 2020
ec58de0
change key to be operators.operatorframework.io.scorecard
May 28, 2020
cba3343
Update enhancements/scorecard-metadata.md
May 29, 2020
f631f5f
Update scorecard-metadata.md
May 29, 2020
70904ed
add note on the API needing an ability to remove custom metadata to s…
Jun 4, 2020
7d9f3be
Update enhancements/scorecard-metadata.md
Jun 11, 2020
32baea4
address PR comments, add user story 3, be more specific as to operato…
Jun 11, 2020
0fbf9e7
add user story 4 about providing the option of mirroring test images …
Jun 16, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
137 changes: 137 additions & 0 deletions enhancements/scorecard-metadata.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
---
title: scorecard-metadata
authors:
- "@jemccorm"
- "@jlanford"
reviewers:
- TBD
approvers:
- TBD
creation-date: 2020-04-29
last-updated: 2020-05-19
status: provisional
---

# scorecard-metadata

## Release Signoff Checklist

- [ ] Enhancement is `implementable`
- [ ] Design details are appropriately documented from clear requirements
- [ ] Test plan is defined
- [ ] Graduation criteria for dev preview, tech preview, GA

## Summary

This enhancement is to define a place within the bundle format whereby
scorecard metadata could be added. This will allow tests to be co-located
with the normal operator content found in a bundle. This enhancement
suggests naming and paths for the scorecard metadata within the bundle.

## Motivation

The purpose of this enhancement is to define a location for scorecard
and other test assets to be included into the bundle. This definition
informs test frameworks and tools of a well-known location to search for test assets
within a bundle and reduces confusion to test developers as to
where test assets will reside.

### Goals

This enhancement will be used by the scorecard feature within the operator-sdk
initially as part of a major redesign of scorecard functionality. Other
test frameworks apart from operator-sdk or scorecard could leverage this
enhancement as well.

### Non-Goals

This enhancement is scoped to define the location within the bundle for
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

this sort of metadata and get consensus on names and paths. Follow on
work would be required to implement tooling that lets a user inject
their test assets into a bundle image as well as extract this metadata.

## Design Details

The bundle format would allow for test metadata to be
included in the following locations within a bundle:

```
bundle/
├── manifests
│ ├── cache.example.com_memcacheds_crd.yaml
│ └── memcached-operator.clusterserviceversion.yaml
├── metadata
│ └── annotations.yaml
└── tests
└── other-tests
└── scorecard
└── config.yaml

```

In the above proposed layout, the test metadata resides under a new
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few questions

  1. 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.

  2. 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?

  3. 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?

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. What about a operators.operatorframework.io.test.mediatypes.v1 label that holds a list of test mediatypes?
  2. I think so based on how I'm interpreting the bundle enhancement, and it would make sense to assume it does. @ecordell @kevinrizza confirm?
  3. Good question.
  4. Probably, assuming we cannot run any of the current alpha scorecard tests without registry contents.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Member

@estroz estroz May 29, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jmccormick2001 this never got addressed

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

directory called `tests`. Below that directory is free form but
in the case of operator-sdk scorecard, it would populate this
directory with a subdirectory named `scorecard` as depicted.

The scorecard test label annotations for the bundle would be as follows:
```
annotations:
operators.operatorframework.io.scorecard.mediatype.v1: "scorecard+v1"
operators.operatorframework.io.scorecard.config.v1: "/tests/scorecard/config.yaml"
operators.operatorframework.io.other-tests.mediatype.v1: "other-tests+v1"
operators.operatorframework.io.other-tests.config.v1: "/tests/other-tests/setup.yaml"
```

For the purpose of this enhancement, scorecard would assume that its test
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

metadata would be in the same bundle image as the operator image.

This proposal would require the operator-registry github repo to
contain a golang API that can be used by the SDK to add scorecard
metadata into a bundle image.

The SDK scorecard is just one example of the type of metadata
that might be added into the bundle image using the proposed
operator-registry golang API.

The proposed operator-registry golang API would also need to
support updating or removing custom metadata from a bundle image.

## Proposal

### User Stories

#### Story 1

As a developer, I would like to use the operator-registry github repo
golang API to include, replace, and remove test metadata within operator
bundles. The ability to remove test metadata would support
disconnected environments where running tests is not applicable.

#### Story 2

As a developer I would like to use the operator-registry github repo
golang API to extract test metadata from operator bundles so as to
execute tests against the operator and its bundle contents.

#### Story 3

As a developer I would like to use the operator-registry github repo
golang API to extract test metadata from operator bundles but also
have the ability to control what bundle contents are extracted.

#### Story 4

As a openshift user I would like to use the 'oc adm catalog build'
command to optionally include the scorecard test images into the catalog.
This would enable openshift users to mirror the test images like other
operator images using the 'oc adm catalog mirror' command.

### Risks and Mitigations

You would not want to define locations in the bundle that might
conflict with other metadata or usage of the bundle.

Some sort of check would need to be created when inserting test
metadata within a bundle to allow a maximum size of metadata
that needs to be determined.