plugin: write Makefile recipes for generate bundle subcommand

[estroz]

Description of the change:
Changes:

• operator-sdk init writes config/bundle/kustomization.yaml and adds bundle and bundle-build recipes that generate a bundle on disk and build a bundle image, respectively.
• operator-sdk create api manages config/samples/kustomization.yaml so kustomize build config/bundle retrieves custom resource samples.

Motivation for the change:

Example workflow:

$ operator-sdk init
$ operator-sdk create api --group ship --version v1beta1 --kind Frigate

Then invoke:

$ make bundle 
/home/estroz/go/bin/controller-gen "crd:trivialVersions=true" rbac:roleName=manager-role webhook paths="./..." output:crd:artifacts:config=config/crd/bases
operator-sdk generate bundle -q --kustomize
kustomize build config/bundle | operator-sdk generate bundle -q --manifests --metadata --overwrite --version 0.0.1
$ make bundle-build
docker build -f bundle.Dockerfile -t controller-manager-bundle:latest .

TODO:

• comment everything.
• [x] better testdata for current and new project layouts.
• [x] rewrite tests (currently broken) in BDD style using gomega/ginkgo.
• break out into several PR's: internal/generate: refactoring for new generate commands #3018, generate csv: move manifest collector to its own package #3064, generate: add bundle subcommand for new project layouts #3065

Reference PR: #2956

/kind feature

[joelanford]

Overall looks like a great start! A few comments:

• I'm not a huge fan of introducing a new update subcommand for this. Is there a way that operator-sdk update bundle can be folded into operator-sdk generate bundle?

  I think one question we need to answer is this: Do we or users care about persisting the result of operator-sdk update bundle -q? If not, then can operator-sdk generate bundle do the marker parsing and CSV building in memory, so that only the base UI metadata needs to be persisted?

• IIRC, the bundle manifests directory has to have a file in it with a clusterserviceversion.yaml suffix. Do we need to split out memcached-operator.bundle.yaml into its constituent parts?

[estroz]

@joelanford Perhaps it would be better to fold update bundle into generate bundle; when in-binary CSV generation is removed in favor of kustomize build, the UI metadata generation component in generate bundle can stick around.

As for CSV file suffix: do you mean that is a requirement in the SDK (generate csv) or elsewhere?

[joelanford]

@joelanford Perhaps it would be better to fold update bundle into generate bundle; when in-binary CSV generation is removed in favor of kustomize build, the UI metadata generation component in generate bundle can stick around.

I think the make target in this PR is pretty close to what I imagined. I'm just wondering if the entrypoint can be operator-sdk bundle generate and we use a flag or another argument to run the kustomize generation vs the bundle directory creation. For example:

• operator-sdk bundle generate manifests
• operator-sdk bundle generate directory

kustomize build config/default | operator-sdk bundle generate manifests:

• creates ./config/bundle/base/clusterserviceversion.yaml if not present (this is the base containing only the non-derivable metadata). This is populated by prompting the user for input.
• creates ./config/bundle/patches/* which are the CSV patches to add the CRD sections, the roles, deployment, alm-examples, etc. These are all derived from the Go APIs and from kustomize output from other ./config kustomizations. We maybe could shell out to kustomize if nothing is present on STDIN?
• creates ./config/bundle/kustomization.yaml (if not present) that ties together the above files and includes ./config/crd so that the CRDs end up in the kustomize output for config/bundle.

kustomize build config/bundle | operator-sdk bundle generate directory:

• gets all of the bundle resources from STDIN (or maybe shells out to kustomize)
• deletes and then re-creates <bundle-dir>/manifests/ with each of the bundle manifest YAMLs
• creates <bundle-dir>/metadata/annotations.yaml if not present (anything here we need to or can update automatically?)
• creates <bundle-dir>/Dockerfile if not present

So the make target could be:

bundle: manifests
	kustomize build config/default | operator-sdk generate bundle manifests
	kustomize build config/bundle  | operator-sdk generate bundle directory

As for CSV file suffix: do you mean that is a requirement in the SDK (generate csv) or elsewhere?

I swear I read something about OLM or the operator-registry knowing what a bundle directory is by searching for a file with a clusterserviceversion.yaml suffix. But now I can't find that. 🤷‍♂️

[estroz]

I think we're on the right track. I'd still like to initially separate the timing of metadata generation from actually creating the bundle, even if generate bundle has the ability to do both, while allowing an interactive prompt at later stages in case a kubebuilder user is migrating to operator-sdk. More importantly though I want to provide both a simple and deeper OLM/bundle workflow. I've modified an example workflow from my comment to illustrate a simplified bundle experience:

Normal kubebuilder workflow

The following few commands are no different than the normal kubebuilder workflow; their behavior differs slightly:

Interactive prompt for UI metadata on init:

$ operator-sdk init
Interactive CSV metadata prompt. Please fill in the following UI fields:
DisplayName: Memcached Operator
...
Icon file path: path/to/icon.png

Create APIs:

$ operator-sdk create api --group cache --version v1 --kind Memcached

Create or update all CRD bases after Go types are created or modified, and update the CSV base:

$ make manifests
controller-gen "crd:trivialVersions=true" rbac:roleName=manager-role webhook paths="./..." output:crd:artifacts:config=config/crd/bases
operator-sdk generate bundle --update-bases --interactive=false

Additional bundle workflow

Here's where we add to the normal kubebuilder workflow, after we've added APIs. We can run the following SDK-specific make recipes to work with bundles. Each recipe except run-olm gives an interactive prompt if a bundle kustomize base does not exist:

Generate a bundle on disk:

$ make manifests-olm
## Calls the 'manifests' recipe
controller-gen "crd:trivialVersions=true" rbac:roleName=manager-role webhook paths="./..." output:crd:artifacts:config=config/crd/bases
operator-sdk generate bundle --update-bases
## Generate the bundle and metadata.
kustomize build config/olm-catalog | operator-sdk generate bundle --output-dir config/olm-catalog

Create a bundle image:

$ make bundle-build IMG=quay.io/example/memcached-operator:v0.0.1
## Calls the 'manifests' recipe
controller-gen "crd:trivialVersions=true" rbac:roleName=manager-role webhook paths="./..." output:crd:artifacts:config=config/crd/bases
operator-sdk generate bundle --update-bases
## Creates a bundle in-memory then builds an image.
kustomize build config/olm-catalog | operator-sdk generate bundle --stdout | operator-sdk bundle create $(IMG)

Run directly with OLM:

$ make run-olm
## Calls the 'manifests' recipe
controller-gen "crd:trivialVersions=true" rbac:roleName=manager-role webhook paths="./..." output:crd:artifacts:config=config/crd/bases
operator-sdk generate bundle --update-bases --interactive=false
## Creates a bundle in-memory then installs it with OLM.
kustomize build config/olm-catalog | operator-sdk generate bundle --stdout | operator-sdk run --olm

---

generate bundle and bundle create exist as separate commands in my example to allow fine-grained control over each step of the process if you want it, ex. in a pipeline with pre-generated bundles/metadata. If you want things to "just work", then use any of those three make recipes. Once we move to kustomize build to generate the entire CSV from a base generate bundle --stdout/--output-dir will no longer be needed and that functionality can be removed; generate bundle --update-bases will still exist as the default behavior.

One drawback is a potentially confusing CLI if you run operator-sdk --help. I think we can mitigate confusion by adding a message like Most users will only need 'make <recipe>'. Use this command directly only if you know what you are doing.

[estroz]

/cc @dmesser @shawn-hurley

Thoughts on the above two CLI proposals?

[estroz]

@hasbro17 can you take another look at this? Changes:

• Added command help text and examples.
• init now scaffolds config/bundle/kustomization.yaml instead of generate bundle --kustomize, which makes a little more sense to me because then users won't be dependent on running the kustomize base generator before running kustomize build config/bundle | operator-sdk generate bundle
  • Neither way works without running make manifests first, but that's fine because neither does kustomize build config/default.

[hasbro17]

init now scaffolds config/bundle/kustomization.yaml instead of generate bundle --kustomize

@estroz Don't you mean "init" now scaffolds config/bundle/kustomization.yaml instead of "create api"?
From my understanding generate bundle --kustomize only generated config/bundle/bases/... and not config/bundle/kustomization.yaml.

[hasbro17]

LGTM to scaffolding config/bundle/kustomize on init since it's not API specific.

[camilamacedo86]

Hi @estroz and @hasbro17,

The current behaviour to follow the Quick Start should not be affected

I am as a user would like to create the project and not a bundle. However, the basic steps are asking the bundle/CSV info when I run make install. It is happening because the make manifest will call the bundle. However, should not mandatory generate a bundle for all projects. Many users do not care about OLM and are not looking to integrate the project with. Is it make sense?

IMO we can simplify here by just adding the new makefile targets but we should not change the existents ones.

The operator-sdk generate bundle -q --kustomize should be called just when we call make bundle or make build-bundle.

See:

See here all steps

camilamacedo@Camilas-MacBook-Pro ~/go/src $ mkdir pr-bundle
camilamacedo@Camilas-MacBook-Pro ~/go/src $ cd pr-bundle/
camilamacedo@Camilas-MacBook-Pro ~/go/src/pr-bundle $ operator-sdk init --domain my.domain
Writing scaffold for you to edit...
Get controller runtime:
$ go get sigs.k8s.io/controller-runtime@v0.6.0
Update go.mod:
$ go mod tidy
Running make:
$ make
/Users/camilamacedo/go/bin/controller-gen object:headerFile="hack/boilerplate.go.txt" paths="./..."
go fmt ./...
go vet ./...
go build -o bin/manager main.go
Next: define a resource with:
$ operator-sdk create api
camilamacedo@Camilas-MacBook-Pro ~/go/src/pr-bundle $ operator-sdk create api --group webapp --version v1 --kind Guestbook
Create Resource [y/n]
y
Create Controller [y/n]
y
Writing scaffold for you to edit...
api/v1/guestbook_types.go
controllers/guestbook_controller.go
Running make:
$ make
/Users/camilamacedo/go/bin/controller-gen object:headerFile="hack/boilerplate.go.txt" paths="./..."
go fmt ./...
go vet ./...
go build -o bin/manager main.go
camilamacedo@Camilas-MacBook-Pro ~/go/src/pr-bundle $ make install
/Users/camilamacedo/go/bin/controller-gen "crd:trivialVersions=true" rbac:roleName=manager-role webhook paths="./..." output:crd:artifacts:config=config/crd/bases
operator-sdk generate bundle -q --kustomize

Display name for the operator (required): 
> 

The make bundle is not working

it is fixed now 👍

[camilamacedo86]

Is required to add the tree? What value does it bring?

[estroz]

It isn't strictly necessary, although other commands show trees.

[camilamacedo86]

Is required to add its output?

[estroz]

Nope, just helps the user know what expected output should be. I can see this drifting from actual output though.

[camilamacedo86]

Is required to add the output?

[camilamacedo86]

Could we add a note for we try to improve its structure and have something that will follow up the KB layout?

E.g pkg/scaffold/internal/templates/bundle/makefile.go where would have the new bundle targets.

[estroz]

These should live here now, then can be moved if we change anything else about the Makefile.

[estroz]

@camilamacedo86 lets have UX discussions in operator-framework/enhancements#16.

[hasbro17]

@estroz Actually I just tested what @camilamacedo86 pointed out and I'm not sure if this was always the case but after scaffolding a basic project without intending to write CSVs or bundles I'm still forced to input the metadata for the CSV bases on make manifest or make install:

$ operator-sdk init --repo=github.com/hasbro17/memcached-operator --domain=example.com
...

$ operator-sdk create api --group cache --version v1alpha1 --kind Memcached
...

$ make manifests
/Users/haseeb/work/go-space/bin//controller-gen "crd:trivialVersions=true" rbac:roleName=manager-role webhook paths="./..." output:crd:artifacts:config=config/crd/bases
operator-sdk generate bundle -q --kustomize

Display name for the operator (required):
...

This kind of forces CSVs and bundles into the workflow by default rather than making it opt-in like we had with generate csv or generate bundle in the old layout.

[hasbro17]

@estroz Nvm just saw the comment for UX discussions. Ignore my previous comment.

[camilamacedo86]

Just to clarifies:

• I think we cold just centralized the code. https://github.com/operator-framework/operator-sdk/pull/2860/files#r431067951.
• I do not think that we should change the makefile targets which are done/used by Kubebuilder and in this way, break the compatibility with and force the user to provide the CSV info when is just looking for creating a new project (following the quick-start). https://github.com/operator-framework/enhancements/pull/16/files#r431432999 which I understand that @estroz also agreed to change as well

IMO it is the broker to move forward with this PR. Any other required change/improvements could indeed be done in a follow-up.

c/c @estroz @hasbro17 @joelanford

[camilamacedo86]

I executed the quick start steps and the steps described https://github.com/operator-framework/enhancements/pull/16/files and I could not face issues with. So, it shows OK for me.

/lgtm
/approve