Add bundle library to build bundle manifest image by dinhxuanvu · Pull Request #97 · operator-framework/operator-registry

dinhxuanvu · 2019-10-11T04:24:02Z

Add top level CLI command opm
Add bundle command to generate annotations.yaml and Dockerfile
Add bundle command to build Bundle image

The build command will generate annotations.yaml and Dockerfile
as well

Signed-off-by: Vu Dinh vdinh@redhat.com

Description of the change:

Motivation for the change:

Reviewer Checklist

Implementation matches the proposed design, or proposal is updated to match implementation
Sufficient unit test coverage
Sufficient end-to-end test coverage
Docs updated or added to /docs
Commit messages sensible and descriptive

kevinrizza · 2019-10-11T18:39:42Z

@dinhxuanvu won't this break the makefile because you are nesting an additional package into the cmd folder that doesn't compile directly? I think make build relies on go list to get its list to compile.

dinhxuanvu · 2019-10-11T19:11:49Z

@kevinrizza This is just a cherry-pick PR from original PR in OLM repo. I'm working on separating the functional code into an actual package under pkg and cli/cobra code remaining in /cmd for internal use. Stay tuned.

dinhxuanvu · 2019-10-18T04:30:37Z

/test unit

ecordell

This looks great! Tests and docs are great as well.

I have one comment on the labels for future-proofing, but once you make those changes we should merge 🎉

ecordell · 2019-10-18T12:25:53Z

Ah, one issue to resolve for the tests:

pkg/lib/bundle/build_test.go:55:27: cmd.String undefined (type *exec.Cmd has no field or method String)

ecordell · 2019-10-18T15:52:10Z

/retest

dinhxuanvu · 2019-10-22T08:47:26Z

/retest

kevinrizza

Nice job on this! I just had a few small comments.

kevinrizza · 2019-10-23T12:19:36Z

+
+	bundleBuildCmd.Flags().StringVarP(&imageBuilderArgs, "image-builder", "b", "docker", "Tool to build container images. One of: [docker, podman, buildah]")
+
+	bundleBuildCmd.Flags().StringVarP(&channelDefaultArgs, "default", "e", "", "The default channel for the bundle image")


maybe defaultChannel is a better name here?

The defaultChannel is a fine name but the reason I use default here is to keep the command flag to be as short as possible. Long name only makes the command even longer and difficult to read. I try to keep all flags to be single word with the exception of image-builder because SDK has the a flag with that name with the same functionality. So I decided to keep it the same as SDK to keep the consistency given this command will be added to SDK.

kevinrizza · 2019-10-23T12:26:35Z

Sorry one more request @dinhxuanvu , can you squash your commits? There's a lot of old history that isn't really relevant anymore.

1. Add top level CLI command `operator-cli` 2. Add bundle command to generate annotations.yaml and Dockerfile 3. Add bundle command to build Bundle image * The build command will generate annotations.yaml and Dockerfile as well 4. Add unit test cases for operator bundle commands 5. Add operator bundle documentation Signed-off-by: Vu Dinh <vdinh@redhat.com>

1. Move bundle funcs to /pkg as a bundle package 2. ix some minor errors in unit test. 3. Modify bundle doc to reflect recent changes + operator-sdk information + Folder structure 4. Add bundle command to opm binary as hidden commands + The bundle commands are only available for internal development use as a part of opm binary. 5. Move bundle doc under /docs/design. Signed-off-by: Vu Dinh <vdinh@redhat.com>

1. Package name, channels, default channel info is now added to annotations.yaml 2. `bundle generate` will retain overwritten ability while `bundle build` will only overwrite Dockerfile. 3. Validate annotations.yaml if existed. 4. `--overwrite/o` flag is available to overwrite annotations.yaml during build command. Signed-off-by: Vu Dinh <vdinh@redhat.com>

Signed-off-by: Vu Dinh <vdinh@redhat.com>

dinhxuanvu · 2019-10-24T02:45:43Z

Ready for another round of review.

kevinrizza · 2019-10-24T11:37:09Z

lgtm

1. Clarify a few helper texts in cli code 2. Improve librar document to specific input information 3. Move `bundle` comands under hidden `alpha` command Signed-off-by: Vu Dinh <vdinh@redhat.com>

gallettilance · 2019-10-28T14:38:11Z

lgtm

Signed-off-by: Vu Dinh <vdinh@redhat.com>

jpeeler · 2019-10-28T17:44:30Z

/lgtm

openshift-ci-robot · 2019-10-28T17:44:34Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dinhxuanvu, jpeeler

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [dinhxuanvu,jpeeler]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci-robot requested review from alecmerdler and ecordell October 11, 2019 04:24

openshift-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Oct 11, 2019

alecmerdler removed their request for review October 11, 2019 05:19

dinhxuanvu changed the title ~~Add bundle library to build bundle manifest image~~ [WIP] Add bundle library to build bundle manifest image Oct 11, 2019

openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 11, 2019

dinhxuanvu changed the title ~~[WIP] Add bundle library to build bundle manifest image~~ Add bundle library to build bundle manifest image Oct 17, 2019

openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 17, 2019

dinhxuanvu force-pushed the bundle-lib branch 3 times, most recently from 93c3351 to 9b53535 Compare October 17, 2019 07:07

dinhxuanvu mentioned this pull request Oct 17, 2019

Add CLI tool to build Operator Bundle image operator-framework/operator-lifecycle-manager#1053

Closed

5 tasks

dinhxuanvu force-pushed the bundle-lib branch from 9b53535 to 1fda468 Compare October 18, 2019 04:14

dinhxuanvu force-pushed the bundle-lib branch from 090f87b to 52b73ff Compare October 18, 2019 06:29

ecordell reviewed Oct 18, 2019

View reviewed changes

Comment thread docs/design/operator-bundle.md Outdated

ecordell reviewed Oct 18, 2019

View reviewed changes

ecordell mentioned this pull request Oct 18, 2019

Expose bundle data from bundle image #94

Merged

openshift-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Oct 22, 2019

dinhxuanvu force-pushed the bundle-lib branch 2 times, most recently from 693e669 to 24a26d3 Compare October 22, 2019 07:45

kevinrizza requested changes Oct 23, 2019

View reviewed changes

dinhxuanvu force-pushed the bundle-lib branch 2 times, most recently from 89f5204 to 6c117a3 Compare October 23, 2019 18:04

Vu Dinh added 5 commits October 23, 2019 14:06

Fix bundle library unit tests to cover new code

ff44b59

Signed-off-by: Vu Dinh <vdinh@redhat.com>

Fix bundle library doc to reflect new changes/features

9762fd5

Signed-off-by: Vu Dinh <vdinh@redhat.com>

dinhxuanvu force-pushed the bundle-lib branch 2 times, most recently from f033b42 to 0bb8f40 Compare October 24, 2019 02:06

dinhxuanvu force-pushed the bundle-lib branch 2 times, most recently from 1ceb026 to 8037b61 Compare October 24, 2019 03:41

gallettilance reviewed Oct 25, 2019

View reviewed changes

Comment thread cmd/opm/alpha/bundle/build.go Outdated

Comment thread docs/design/operator-bundle.md

Comment thread docs/design/operator-bundle.md

Clean up minor issues with command-line code and documentation

ee575b1

1. Clarify a few helper texts in cli code 2. Improve librar document to specific input information 3. Move `bundle` comands under hidden `alpha` command Signed-off-by: Vu Dinh <vdinh@redhat.com>

dinhxuanvu force-pushed the bundle-lib branch from 8037b61 to ee575b1 Compare October 27, 2019 22:07

jpeeler reviewed Oct 28, 2019

View reviewed changes

Comment thread pkg/lib/bundle/generate.go Outdated

Change const to upper case so it can be used

f5dc03e

Signed-off-by: Vu Dinh <vdinh@redhat.com>

openshift-ci-robot assigned jpeeler Oct 28, 2019

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 28, 2019

openshift-merge-robot merged commit cdfd65b into operator-framework:master Oct 28, 2019


		bundleBuildCmd.Flags().StringVarP(&imageBuilderArgs, "image-builder", "b", "docker", "Tool to build container images. One of: [docker, podman, buildah]")

		bundleBuildCmd.Flags().StringVarP(&channelDefaultArgs, "default", "e", "", "The default channel for the bundle image")

Conversation

dinhxuanvu commented Oct 11, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kevinrizza commented Oct 11, 2019

Uh oh!

dinhxuanvu commented Oct 11, 2019

Uh oh!

dinhxuanvu commented Oct 18, 2019

Uh oh!

Uh oh!

ecordell left a comment

Choose a reason for hiding this comment

Uh oh!

ecordell commented Oct 18, 2019

Uh oh!

ecordell commented Oct 18, 2019

Uh oh!

dinhxuanvu commented Oct 22, 2019

Uh oh!

kevinrizza left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

kevinrizza Oct 23, 2019

Choose a reason for hiding this comment

Uh oh!

dinhxuanvu Oct 24, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

kevinrizza commented Oct 23, 2019

Uh oh!

dinhxuanvu commented Oct 24, 2019

Uh oh!

kevinrizza commented Oct 24, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gallettilance commented Oct 28, 2019

Uh oh!

Uh oh!

jpeeler commented Oct 28, 2019

Uh oh!

openshift-ci-robot commented Oct 28, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

dinhxuanvu commented Oct 11, 2019 •

edited

Loading