Skip to content
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 ClusterServiceVersion bundling #682

Merged
merged 24 commits into from
Jul 23, 2020
Merged

Add ClusterServiceVersion bundling #682

merged 24 commits into from
Jul 23, 2020

Conversation

lmm
Copy link
Contributor

@lmm lmm commented Jul 8, 2020

Description

This PR adds plumbing to generate ClusterServiceVersion (CSV) bundles which we use to validate and publish the operator on the RH catalog.

Two new make targets are added:

  • gen-csv creates a CSV for every operator version we want to publish;
  • gen-bundle gathers all generated CSVs, adds them to the ones already committed, and creates a zip file

This PR also removes operator manifests from the deploy directory since they are not used for CSV generation. Instead, users should use the operator manifests from our docs.

Testing

  • The CSV bundling was tested by generating them and uploading them for validation testing. I also locally installed these bundles in my OCP cluster to verify the operator metadata in the OpenShift Console.

For PR author

  • Tests for change.
  • If changing pkg/apis/, run make gen-files
  • If changing versions, run make gen-versions

For PR reviewers

A note for code reviewers - all pull requests must have the following:

  • Milestone set according to targeted release.
  • Appropriate labels:
    • kind/bug if this is a bugfix.
    • kind/enhancement if this is a a new feature.
    • enterprise if this PR applies to Calico Enterprise only.

@lmm lmm force-pushed the lmm-bundle branch 2 times, most recently from 9b75c11 to 16f51a9 Compare July 8, 2020 18:49
Copy link
Member

@tmjd tmjd left a comment

Choose a reason for hiding this comment

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

Some high level thoughts/questions:

  • Should we be committing these bundles to this repo? Did you consider any other options?
  • I'm not sure if bundling the CRDs in this repo is right, I'm wondering if instead of that we should be pulling from the docs. The ones here are more for testing purposes than anything and the 'real' ones should be coming from the docs. Or maybe part of the process needs to be pulling the latest Calico CRDs from the docs.

RELEASING.md Show resolved Hide resolved
RELEASING.md Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
@lmm
Copy link
Contributor Author

lmm commented Jul 8, 2020

Should we be committing these bundles to this repo? Did you consider any other options?

Pre-generating the CSV's as needed was an option that was considered but the trade off is that we'd need some other way to keep track of the operator versions published on RH. The other thing is that the many operators in the operator-sdk ecosystem already check-in their CSVs, and since we already follow a lot of the operator-sdk conventions already, the proposed process in this PR is consistent.

I'm not sure if bundling the CRDs in this repo is right, I'm wondering if instead of that we should be pulling from the docs. The ones here are more for testing purposes than anything and the 'real' ones should be coming from the docs. Or maybe part of the process needs to be pulling the latest Calico CRDs from the docs.

I had a brief chat with @caseydavenport about this and the gist of that was that maybe we want the manifests in deploy/ to be the "real" ones that the docs follow. I think that simplifies the process for CSV bundling but makes it slightly awkward if contributors submit changes to operator manifests in the calico repo. Maybe Casey had some more thoughts on this.

@caseydavenport
Copy link
Member

caseydavenport commented Jul 9, 2020

Should we be committing these bundles to this repo? Did you consider any other options?

Yeah, this strongly weirds me out, but it seems to be the pattern that other RH operators are according to my discussion with @lmm.

The main difference I think, based on my conversation with @lmm was that the other operators have a single release stream (e.g., the only maintain the master branch, not additional release branches).

@lmm lmm mentioned this pull request Jul 9, 2020
Makefile Outdated Show resolved Hide resolved
@tmjd
Copy link
Member

tmjd commented Jul 10, 2020

I'm not sure if bundling the CRDs in this repo is right, I'm wondering if instead of that we should be pulling from the docs. The ones here are more for testing purposes than anything and the 'real' ones should be coming from the docs. Or maybe part of the process needs to be pulling the latest Calico CRDs from the docs.

I had a brief chat with @caseydavenport about this and the gist of that was that maybe we want the manifests in deploy/ to be the "real" ones that the docs follow. I think that simplifies the process for CSV bundling but makes it slightly awkward if contributors submit changes to operator manifests in the calico repo. Maybe Casey had some more thoughts on this.

I think I'd be worried about moving the CRDs into the operator repo without some automation around copying them to the docs and a better understanding of what the process around that would be. Mainly because a lot of integration testing uses the docs so if they get out of sync that would not be good.

I feel like it would be simple enough to download the CRDs as part of the CSV generation step, I guess the caveat with that would be that we should wait until the Calico release is done and the CRDs are 'officially' available.

@caseydavenport WDYT?

@tmjd tmjd added this to the v1.9.0 milestone Jul 16, 2020
@lmm
Copy link
Contributor Author

lmm commented Jul 21, 2020

I feel like it would be simple enough to download the CRDs as part of the CSV generation step

Thanks @tmjd, I've added this with 9d8ca79 and 2d0fea3

Makefile Outdated Show resolved Hide resolved
hack/gen-csv/get-manifests.sh Outdated Show resolved Hide resolved
hack/gen-csv/get-manifests.sh Outdated Show resolved Hide resolved
RELEASING.md Show resolved Hide resolved
RELEASING.md Outdated Show resolved Hide resolved
And remove operator-sdk dependency for the gen-bundle target
Copy link
Member

@tmjd tmjd left a comment

Choose a reason for hiding this comment

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

LGTM, once the tests finish (and pass of course)

@lmm lmm merged commit 21be623 into tigera:master Jul 23, 2020
@lmm lmm deleted the lmm-bundle branch July 23, 2020 22:06
lmm added a commit to lmm/operator that referenced this pull request Jul 24, 2020
lmm added a commit to lmm/operator that referenced this pull request Jul 24, 2020
lmm added a commit to lmm/operator that referenced this pull request Jul 24, 2020
lmm added a commit that referenced this pull request Jul 24, 2020
* Add ClusterServiceVersion bundling (#682)

* Fix compile

* Update CSV template for release-v1.5

- Update CSV template docs links
- Fix manifest urls in gen csv script

* Use operator-sdk v0.10.1 for everything but CSV gen

CSV gen will use v0.18.2
lmm added a commit to lmm/operator that referenced this pull request Aug 7, 2020
lmm added a commit to lmm/operator that referenced this pull request Aug 7, 2020
lmm added a commit that referenced this pull request Aug 7, 2020
* Add ClusterServiceVersion bundling (#682)

* Update docs links in clusterserviceversion template

* Use operator-sdk v0.10.1 for everything but CSV gen

CSV gen will use v0.18.2

* Merge pull request #761 from lmm/lmm-fix-release-script

Exit release script gracefully if not on a tag

* Tag and push release images to RedHat Connect for validation (#688)

Co-authored-by: Erik Stidham <erik@tigera.io>
lmm added a commit to lmm/operator that referenced this pull request Aug 25, 2020
lmm added a commit that referenced this pull request Aug 25, 2020
* Add ClusterServiceVersion bundling (#682)

* Tag and push release images to RedHat Connect for validation (#688)

* Merge pull request #761 from lmm/lmm-fix-release-script

Exit release script gracefully if not on a tag

* Fix version in image tag (#818)

Co-authored-by: Erik Stidham <erik@tigera.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants