Skip to content

documentation for new OLM command workflows#16

Merged
estroz merged 3 commits intooperator-framework:masterfrom
varshaprasad96:olm-cmd-workflow
Jun 9, 2020
Merged

documentation for new OLM command workflows#16
estroz merged 3 commits intooperator-framework:masterfrom
varshaprasad96:olm-cmd-workflow

Conversation

@varshaprasad96
Copy link
Copy Markdown
Member

@varshaprasad96 varshaprasad96 commented Apr 14, 2020

This PR provides documentation explaining the workflows for
proposed OLM commands with Operator SDK and a README describing
the deployment of operator by following the suggested workflows.

@varshaprasad96
Copy link
Copy Markdown
Member Author

varshaprasad96 commented Apr 14, 2020 •
edited
Loading

@jmrodri @estroz @joelanford @dmesser - I have moved the PR (operator-framework/operator-sdk#2834) from Operator SDK to this repo. Please provide your inputs.

@varshaprasad96 varshaprasad96 force-pushed the olm-cmd-workflow branch 2 times, most recently from 276a96b to 30757cf Compare April 16, 2020 22:42
@varshaprasad96 varshaprasad96 mentioned this pull request Apr 16, 2020
Closed
@varshaprasad96
Copy link
Copy Markdown
Member Author

varshaprasad96 commented Apr 16, 2020

Updated the PR with man page for commands.

dmesser
dmesser previously approved these changes Apr 17, 2020
View reviewed changes
@camilamacedo86
Copy link
Copy Markdown
Contributor

camilamacedo86 commented Apr 17, 2020

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Apr 17, 2020
@camilamacedo86
Copy link
Copy Markdown
Contributor

camilamacedo86 commented Apr 17, 2020

/approve

camilamacedo86
camilamacedo86 previously approved these changes Apr 17, 2020
View reviewed changes
joelanford
joelanford reviewed Apr 17, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@joelanford joelanford left a comment

Choose a reason for hiding this comment

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

Looks great! Just want to clarify a few details.

Comment thread enhancements/olm-cmd-workflows.md Outdated
Comment thread enhancements/olm-cmd-workflows.md Outdated
Comment thread enhancements/olm-cmd-workflows.md Outdated
@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Apr 17, 2020
@varshaprasad96
Copy link
Copy Markdown
Member Author

varshaprasad96 commented Apr 17, 2020

Updated the doc based on reviews.

@varshaprasad96
documentation for olm command workflows
e555f67 
This PR provides documentation explaining the workflows for
proposed OLM commands with Operator SDK and a README describing
the deployment of operator by following the suggested workflows.
estroz
estroz reviewed Apr 17, 2020
View reviewed changes
Comment thread enhancements/olm-cmd-workflows.md Outdated

OLM Workflow:

1. Use `operator-sdk generate bundle` to generate kustomize base templates with CSV metadata and Dockerfile to build the operator.
Copy link
Copy Markdown
Member

@estroz estroz Apr 17, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

@varshaprasad96 @dmesser a few thoughts on the base project -> OLM-ready workflow:

Eventually we'll want to move bundle (manifest) generation to kustomize to align with kubebuilder-style deployment. This will be especially relevant when CSV-less bundles come around. Therefore I don't thing we should write generate bundle so that it both ingests manifests from disk/stdin AND generates UI metadata. There should be a step, perhaps in operator-sdk init, that scaffolds UI metadata in a base. The user then updates it manually or via an interactive prompt, after which they can run kustomize build config/olm-catalog to generate their bundle to disk or as input to bundle create or run --olm.

The workflow then becomes:

$ operator-sdk init
Interactive metadata prompt. Please fill in the following UI fields:
...
$ operator-sdk create api --group cache --version v1 --kind Memcached
...

Then:

$ make manifests-olm
kustomize build config/olm-catalog > config/olm-catalog/manifests/memcached-operator.bundle.yaml

or

$ make bundle IMG=quay.io/example/memcached-operator:v0.0.1
kustomize build config/olm-catalog | operator-sdk create bundle quay.io/example/memcached-operator:v0.0.1

or

$ make run-olm
kustomize build config/olm-catalog | operator-sdk run --olm

The make recipes above are just suggestions to illustrate how moving to kustomize will be much more flexible. I have something a little different set up in operator-framework/operator-sdk#2860 because we still need to regenerate CSV definitions from API types. Still tinkering around with that PR. Thoughts?

Copy link
Copy Markdown
Member

@estroz estroz Apr 17, 2020

Choose a reason for hiding this comment

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

Regardless on-disk manifests in a kubebuilder project are mostly just bases, so some part of bundle generation needs to be run through kustomize. We have two options then, which are inverses:

  1. Run kustomize | operator-sdk, which is how kubebuilder does things.
  2. Invoke kustomize within operator-sdk.

The former is a lot more appealing to me.

@estroz estroz mentioned this pull request Apr 22, 2020
Merged
2 tasks
@estroz estroz force-pushed the olm-cmd-workflow branch from 3019a5e to 0a5a8ea Compare May 19, 2020 21:52
@estroz
Copy link
Copy Markdown
Member

estroz commented May 19, 2020

This PR has been updated to a full EP.

/cc @varshaprasad96 @jmrodri

@openshift-ci-robot
Copy link
Copy Markdown

openshift-ci-robot commented May 19, 2020

@estroz: GitHub didn't allow me to request PR reviews from the following users: varshaprasad96.

Note that only operator-framework members and repo collaborators can review this PR, and authors cannot review their own PRs.

Details

In response to this:

This PR has been updated to a full EP.

/cc @varshaprasad96 @jmrodri

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 estroz force-pushed the olm-cmd-workflow branch 4 times, most recently from c5906d5 to 839dc5c Compare May 20, 2020 06:02
@estroz
Copy link
Copy Markdown
Member

estroz commented Jun 2, 2020

@camilamacedo86 @shawn-hurley @joelanford @hasbro17 I removed the manifests recipe modification and changed packagemanifests -> package.

jmccormick2001
jmccormick2001 reviewed Jun 2, 2020
View reviewed changes
Comment thread enhancements/sdk-generate-commands.md Outdated
@jmccormick2001
Copy link
Copy Markdown

jmccormick2001 commented Jun 2, 2020

is the convention normally "operator-sdk or the other way ? that might be made clearer or more consistent if possible.

@estroz estroz force-pushed the olm-cmd-workflow branch from 9da2026 to 403e3f2 Compare June 2, 2020 18:24
@estroz
Copy link
Copy Markdown
Member

estroz commented Jun 2, 2020

@jmccormick2001 when talking about the binary, I try to use operator-sdk. When talking about the general Operator SDK project I try to use SDK or Operator SDK. Is that the discrepancy you're referring to?

@estroz estroz force-pushed the olm-cmd-workflow branch from 403e3f2 to d87c999 Compare June 2, 2020 18:26
camilamacedo86
camilamacedo86 reviewed Jun 2, 2020
View reviewed changes
Comment thread enhancements/sdk-generate-commands.md Outdated
```console
$ make manifests
controller-gen "crd:trivialVersions=true" rbac:roleName=manager-role webhook paths="./..." output:crd:artifacts:config=config/crd/bases
$ kustomize build config/packages | operator-sdk generate packagemanifests -q --channel beta --version 0.0.1
Copy link
Copy Markdown
Contributor

@camilamacedo86 camilamacedo86 Jun 2, 2020

Choose a reason for hiding this comment

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

Should we run $ kustomize build config/packages | operator-sdk generate packagemanifests -q --channel beta --version 0.0.1 ? Could we not be a new target make bundle-manifestfor example?

Copy link
Copy Markdown
Member

@estroz estroz Jun 2, 2020

Choose a reason for hiding this comment

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

We're going to transition away from package manifests at some point, although we don't have a concrete timeline. Regardless I don't think it's a good idea to add this target by default. There will be docs explaining how to add this to the Makefile at the very least.

camilamacedo86
camilamacedo86 reviewed Jun 2, 2020
View reviewed changes
Comment thread enhancements/sdk-generate-commands.md Outdated
camilamacedo86
camilamacedo86 reviewed Jun 2, 2020
View reviewed changes
Comment thread enhancements/sdk-generate-commands.md Outdated
camilamacedo86
camilamacedo86 reviewed Jun 2, 2020
View reviewed changes
Comment thread enhancements/sdk-generate-commands.md Outdated
camilamacedo86
camilamacedo86 reviewed Jun 2, 2020
View reviewed changes
Comment thread enhancements/sdk-generate-commands.md Outdated
camilamacedo86
camilamacedo86 reviewed Jun 2, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@camilamacedo86 camilamacedo86 left a comment

Choose a reason for hiding this comment

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

Just few nits now.
After addressing them it would be for me:

/lgtm
/approve

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jun 2, 2020
@estroz estroz force-pushed the olm-cmd-workflow branch from d87c999 to c66ca2f Compare June 2, 2020 19:31
@openshift-ci-robot
Copy link
Copy Markdown

openshift-ci-robot commented Jun 2, 2020

New changes are detected. LGTM label has been removed.

@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Jun 2, 2020
jmrodri
jmrodri requested changes Jun 2, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@jmrodri jmrodri left a comment

Choose a reason for hiding this comment

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

A few nits regarding the title and template usage. Also, I wanted clarification on the open question. All in all really good.

Comment thread enhancements/sdk-generate-commands.md Outdated
Comment thread enhancements/sdk-generate-commands.md Outdated
Comment thread enhancements/sdk-generate-commands.md Outdated
Comment thread enhancements/sdk-generate-commands.md Outdated
Comment thread enhancements/sdk-generate-commands.md Outdated
Comment thread enhancements/sdk-generate-commands.md Outdated
Comment thread enhancements/sdk-generate-commands.md Outdated
@estroz estroz force-pushed the olm-cmd-workflow branch from c66ca2f to e45b429 Compare June 2, 2020 20:59
@estroz estroz requested a review from jmrodri June 2, 2020 21:37
@estroz estroz force-pushed the olm-cmd-workflow branch from e45b429 to 1965c3e Compare June 3, 2020 17:10
@estroz estroz merged commit 43131bf into operator-framework:master Jun 9, 2020
@estroz estroz deleted the olm-cmd-workflow branch June 9, 2020 16:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jmrodri jmrodri jmrodri approved these changes

@camilamacedo86 camilamacedo86 camilamacedo86 approved these changes

@joelanford joelanford Awaiting requested review from joelanford

@dmesser dmesser Awaiting requested review from dmesser

@fabianvf fabianvf Awaiting requested review from fabianvf

@hasbro17 hasbro17 Awaiting requested review from hasbro17

@shawn-hurley shawn-hurley Awaiting requested review from shawn-hurley

@estroz estroz Awaiting requested review from estroz

@jmccormick2001 jmccormick2001 Awaiting requested review from jmccormick2001

Assignees

@camilamacedo86 camilamacedo86

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.