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

[WIP] bundle: fix bundle create, bump operator-registry v1.6.0 #2715

Closed

Conversation

estroz
Copy link
Member

@estroz estroz commented Mar 25, 2020

Description of the change: some cleanup of bundle code, bump operator-registry to v1.6.0, add --output-dir (from #2662), various other changes described in the motivation section.

  • doc,website: update generated CLI docs
  • CHANGELOG.md: --output-dir addition and breaking change for bundle.Dockerfile location
  • doc/migration: add breaking change to migration guide

Motivation for the change: From #2662:

Description of the change:

*Update operator-registry dependency to 1.6.0
*Adds fix for generated dockerfiles
*Adds optional output-directory parameter

Motivation for the change:

Include fixes for operator-sdk bundle create

This PR fixes an issue where manifests/ is expected by the operator-registry image builder but not generated by the SDK. In case deploy/olm-catalog/<operator-name>/manifests doesn't exist, create one and delete it after the image is done building. If --generate-only is set, do not remove manifests/. --version lets a user specify which operator version they want to build an image for, and --latest uses the highest semver'd bundle.

This change partially allows the SDK to support the new manifests format for bundles while being backwards-compatible with versioned bundle directories. Still some work to do on the CSV generation side of things, but at least bundle create now works with current projects.

TODO:

  • discuss breaking changes in behavior
  • CHANGELOG.md updates
  • test flow from bundle create [--generate-only] -> bundle validate
  • update CLI and usage docs

/area dependency

@estroz estroz added the olm-integration Issue relates to the OLM integration label Mar 25, 2020
@openshift-ci-robot openshift-ci-robot added the area/dependency Issues or PRs related to dependency changes label Mar 25, 2020
@estroz estroz force-pushed the refactor/update-bundle-cmd branch from 88abadb to d0b6474 Compare March 25, 2020 15:48

Note that --watch-namespace can be used to set the namespace(s) which the operator will watch for changes. It will set the environment variable WATCH_NAMESPACE. Use explicitly an empty string to watch all namespaces or inform a List of namespaces such as "ns1,ns2" when the operator is cluster-scoped. If you use a List, then it needs contains the namespace where the operator is "deployed" in since the default metrics implementation will manage resources in the Operator's namespace. By default, it will be the Operator Namespace.

Then, use the flag --operator-namespace to inform the namespace where the Operator will be "deployed" in and then, it will set the environment variable OPERATOR_NAMESPACE. If this value is not set, then it will be the namespace defined as default in the Kubeconfig.

NOTE: For more information check the PRs which are responsible for the above changes [#2617](https://github.com/operator-framework/operator-sdk/pull/2617).
- If you've run `operator-sdk bundle create --generate-only`, move your bundle Dockerfile at `<project-root>/deploy/olm-catalog/<operator-name>/Dockerfile` to `<project-root>/bundle.Dockerfile` and update the first `COPY` from `COPY /*.yaml manifests/` to `COPY deploy/olm-catalog/<operator-name>/<bundle-dir> manifests/`. [#2715](https://github.com/operator-framework/operator-sdk/pull/2715)
Copy link
Member Author

Choose a reason for hiding this comment

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

@kevinrizza the v1.6 update breaks --generate-only behavior, since the argument to --directory, which is for a named bundle dir ex deploy/olm-catalog/my-operator/0.0.1, is placed in a top-level bundle.Dockerfile. Running operator-sdk bundle create --directory deploy/olm-catalog/my-operator/0.0.2 after generating a metadata/Dockerfile for 0.0.1 results in a bundle for 0.0.1.

How does opm work with a directory structure like this?

Copy link
Member

Choose a reason for hiding this comment

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

we had a weird implementation where the metadata/ folder was getting inserted inside the manifest folder and then the dockerfile had to extract it out. we modified it so that the scaffold matched what actually appeared on the directory structure in the container image.

that was one of the big purposes of this update. You shouldn't be creating versioned folders, instead you should have a single manifests/ folder and a matching metadata/ folder. the registry api update pushes the metadata/ folder to the same directory level as the manifest/ folder

Copy link
Member Author

Choose a reason for hiding this comment

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

So users are expected to use a VCS to manage bundle versions going forward, and everything in manifests/ is always latest?

@joelanford @jmccormick2001 @dmesser we're going to have to make a serious breaking change to the CSV generator/dir structure unless we can come up with a middle ground.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, this is what i was trying to say for a while. The CI pipelines are going to be built around having just one bundle in your repository and when you want to push a change you just update that bundle -- this is for a few reasons, but the big one is because the operator bundles are just single versions, and if we didn't do it this way we would have to come up with a bunch of complex CI that would need to know that a new folder was generated in a specific subdirectory and know how to aggregate it. Also, the annotations.yaml file is aggregated at the specific bundle version and not at the package layer.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @kevinrizza,

move your bundle Dockerfile at <project-root>/deploy/olm-catalog/<operator-name>/Dockerfile to <project-root>/bundle.Dockerfile

Note that what you are asking for the user does will break the commands for SDK cli.

if we didn't do it this way we would have to come up with a bunch of complex CI that would need to know that a new folder was generated in a specific subdirectory and know how to aggregate it.

Would not be possible to add flags there to pass the current location and the CI job looking for the files in the path informed and/or copy them for anyplace according to this needs?

Copy link
Member Author

Choose a reason for hiding this comment

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

@kevinrizza #2746 helps users migrate to a manifests/ format while supporting versioned bundle dirs. Next is for generate csv to do the same.

@estroz
Copy link
Member Author

estroz commented Mar 25, 2020

/hold

Waiting to resolve #2715 (comment)

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 25, 2020
@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 27, 2020
*Update operator-registry dependency to 1.6.0
*Adds fix for generated dockerfiles
*Adds optional output-directory parameter
*Update cleanup funcs to handle new structure
@estroz estroz force-pushed the refactor/update-bundle-cmd branch from d0b6474 to b0d9d75 Compare March 28, 2020 02:41
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 28, 2020
@estroz estroz changed the title bundle: clean up bundle code, bump operator-registry v1.6.0 [WIP] bundle: clean up bundle code, bump operator-registry v1.6.0 Mar 28, 2020
@openshift-ci-robot 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 Mar 28, 2020
@estroz estroz changed the title [WIP] bundle: clean up bundle code, bump operator-registry v1.6.0 [WIP] bundle: fix bundle create, bump operator-registry v1.6.0 Mar 28, 2020
@estroz
Copy link
Member Author

estroz commented Mar 28, 2020

/kind bug

@openshift-ci-robot openshift-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label Mar 28, 2020
@estroz estroz force-pushed the refactor/update-bundle-cmd branch 2 times, most recently from 09c5515 to 1810754 Compare March 28, 2020 19:24
operator-registry image builder but not generated by the SDK.
In case deploy/olm-catalog/<operator-name>/manifests doesn't
exist, create one and delete it after the image is done
building. If --generate-only is set, do not remove manifests/.
--version lets a user specify which operator version they want
to build an image for, and --latest uses the highest semver'd
operator in the bundle parent dir.

hack/tests: add 'bundle create' test cases

doc,website: update generated CLI docs

CHANGELOG.md: --output-dir addition and breaking change for bundle.Dockerfile location

doc/migration: add breaking change to migration guide
@estroz
Copy link
Member Author

estroz commented Mar 29, 2020

Now blocked by operator-framework/operator-registry#237 since we'll have to bump versions once a new one is released with this fix.

@estroz estroz force-pushed the refactor/update-bundle-cmd branch from 1810754 to accf2bf Compare March 30, 2020 20:43
@estroz estroz mentioned this pull request Mar 31, 2020
@estroz
Copy link
Member Author

estroz commented Mar 31, 2020

Closing in favor of #2746

@estroz estroz closed this Mar 31, 2020
@estroz estroz deleted the refactor/update-bundle-cmd branch April 1, 2020 22:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/dependency Issues or PRs related to dependency changes 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/bug Categorizes issue or PR as related to a bug. olm-integration Issue relates to the OLM integration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants