Skip to content

[WIP] bundle: add subcommand to build operator index images#2714

Closed
estroz wants to merge 1 commit intooperator-framework:masterfrom
estroz:feature/bundle-index-cli
Closed

[WIP] bundle: add subcommand to build operator index images#2714
estroz wants to merge 1 commit intooperator-framework:masterfrom
estroz:feature/bundle-index-cli

Conversation

@estroz
Copy link
Member

@estroz estroz commented Mar 25, 2020 •
edited
Loading

Description of the change: operator-sdk bundle add shells out to opm to build index images that encapsulate multiple operator versions.

Motivation for the change: users should be able to ship multiple versions of their operator in the index image format after building an operator image with bundle create.

Note: operator-registry sports a Go library to handle index building, but it requires operator-sdk be built with CGO_ENABLED=1. This is currently not possible due to k8s dependency requirements, hence the shell-out to opm.

TODO:

/kind feature

@estroz estroz added the olm-integration Issue relates to the OLM integration label Mar 25, 2020
@estroz estroz added this to the v1.0.0 milestone Mar 25, 2020
@openshift-ci-robot openshift-ci-robot added kind/feature Categorizes issue or PR as related to a new feature. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Mar 25, 2020
@estroz estroz force-pushed the feature/bundle-index-cli branch from fa1c740 to fabd753 Compare March 25, 2020 06:18
kevinrizza
kevinrizza reviewed Mar 25, 2020
View reviewed changes
Copy link
Member

@kevinrizza kevinrizza left a comment

Choose a reason for hiding this comment

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

@estroz I'm assuming that since this PR includes my commit from #2662 that we can just close that one? I'll close it now.

cmd/operator-sdk/bundle/add.go
// buildOPM reates and download 'opm' to a binary directory using 'go get' to
// download a particular version, specified by registryVersion, to install.
// TODO(estroz): shell out to make after kubebuilder integration.
func (c bundleIndexCmd) buildOPM(binaryPath string) error {
Copy link
Member

@kevinrizza kevinrizza Mar 25, 2020

Choose a reason for hiding this comment

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

@ecordell is in the process of creating a downstream build of opm. Does it make more sense for the registry project to just include a release of the opm binary that could be downloaded -- or perhaps even just a container image to extract it from like opm itself does to do the container image build -- rather than compiling from source?

Trying to avoid requiring golang as a dependency if possible.

@kevinrizza kevinrizza mentioned this pull request Mar 25, 2020
Closed
@estroz
Copy link
Member Author

estroz commented Mar 25, 2020

@kevinrizza the intent wasn't to supersede your PR, but now that its closed I'll split your commit along with some changes unrelated to bundle add into a separate PR. Thanks for working on the version bump btw.

@estroz estroz force-pushed the feature/bundle-index-cli branch 3 times, most recently from a2468da to 18f41b5 Compare March 25, 2020 17:40
@estroz
bundle: 'add' shells out to 'opm index add' to build index images
9eef07b 
doc,website: regenerate cli docs

CHANGELOG.md: 'bundle add' addition
@estroz estroz force-pushed the feature/bundle-index-cli branch from 18f41b5 to 9eef07b Compare March 25, 2020 17:43
@joelanford
Copy link
Member

joelanford commented Mar 25, 2020

/hold

There's some recent debate (as of today) about how the UX for this should work in SDK. We don't necessarily want to have commands in SDK that duplicate opm commands.

The thought is that we want to enable running the operator via OLM by deploying it via an index image containing the operator bundle AND the operator dependencies. But we might want to hide the details of the catalog under a simpler SDK workflow.

@openshift-ci-robot openshift-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Mar 25, 2020
@openshift-ci-robot
Copy link

openshift-ci-robot commented Mar 27, 2020

@estroz: PR needs rebase.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@estroz
Copy link
Member Author

estroz commented Apr 17, 2020

Closing until we have a better idea of how we want to structure a bundle index feature.

@estroz estroz closed this Apr 17, 2020
@estroz estroz deleted the feature/bundle-index-cli branch August 5, 2020 16:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kevinrizza kevinrizza kevinrizza left review comments

@jmrodri jmrodri Awaiting requested review from jmrodri

@joelanford joelanford Awaiting requested review from joelanford

@jmccormick2001 jmccormick2001 Awaiting requested review from jmccormick2001

@dinhxuanvu dinhxuanvu Awaiting requested review from dinhxuanvu

@fabianvf fabianvf Awaiting requested review from fabianvf

@theishshah theishshah Awaiting requested review from theishshah

Assignees

No one assigned

Labels

do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. kind/feature Categorizes issue or PR as related to a new feature. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. olm-integration Issue relates to the OLM integration

Projects

None yet

Milestone

v1.0.0

Development

Successfully merging this pull request may close these issues.

4 participants

@estroz @joelanford @openshift-ci-robot @kevinrizza