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

cmd/operator-sdk/olmcatalog: scaffold package.yaml for a set of… #1364

Merged
merged 19 commits into from
Jun 28, 2019

Conversation

estroz
Copy link
Member

@estroz estroz commented Apr 30, 2019

Description of the change:

  • cmd/operator-sdk/olmcatalog/gen-csv.go: Added --csv-channel to add the CSV name with version supplied to --csv-version to a package manifest deploy/olm-catalog/operator-name/{operator_name}.package.yaml, and --default-channel to specify that the supplied channel is the default package manifest channel.
  • internal/pkg/scaffold/olm-catalog/package_manifest*.go: Added a PackageManifest scaffold to create and update a package manifest file at the file location mentioned above.

Motivation for the change: automatically creating and updating a package manifest for OLM users during CSV generation more easily onboards them.

and the optional --csv-channel flag to specify in which channel an
operator's CSV name should reside in their package manifest

internal/pkg/scaffold/olm-catalog: PackageManifest scaffold adds and
updates an <operatorname>.package.yaml file in
deploy/olm-catalo/<operator-name>, a file used by OLM to manage
operator CSV's
specify that c in --csv-channel=c is the package manifests' default channel

internal/pkg/scaffold/olm-catalog/package_manifest*.go: add ChannelIsDefault
field to PackageManifest scaffold to hold --default-channel value
@estroz estroz added kind/feature Categorizes issue or PR as related to a new feature. olm-integration Issue relates to the OLM integration labels Apr 30, 2019
@openshift-ci-robot openshift-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Apr 30, 2019
lilic
lilic previously requested changes May 7, 2019
Copy link
Member

@lilic lilic left a comment

Choose a reason for hiding this comment

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

Few comments but lgtm overall.

internal/pkg/scaffold/olm-catalog/config.go Show resolved Hide resolved
internal/pkg/scaffold/olm-catalog/package_manifest.go Outdated Show resolved Hide resolved
internal/pkg/scaffold/olm-catalog/package_manifest_test.go Outdated Show resolved Hide resolved
cmd/operator-sdk/olmcatalog/gen-csv.go Outdated Show resolved Hide resolved
doc/sdk-cli-reference.md Outdated Show resolved Hide resolved
jcrossley3 added a commit to openshift-knative/knative-eventing-operator that referenced this pull request May 7, 2019
Current gen-csv doesn't generate a valid package bundle because it
doesn't create a package manifest.

This should be fixed by operator-framework/operator-sdk#1364
@estroz estroz requested a review from lilic May 9, 2019 21:46
@estroz estroz requested a review from joelanford May 31, 2019 19:17
Copy link
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.

LGTM

@estroz
Copy link
Member Author

estroz commented Jun 3, 2019

@lilic @hasbro17 @AlexNPavel PTAL

@hasbro17
Copy link
Contributor

Update the CHANGELOG for the new flags --csv-channel and --default-channel.

Copy link
Contributor

@AlexNPavel AlexNPavel left a comment

Choose a reason for hiding this comment

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

LGTM

internal/pkg/scaffold/olm-catalog/package_manifest.go Outdated Show resolved Hide resolved
doc/sdk-cli-reference.md Outdated Show resolved Hide resolved
Eric Stroczynski and others added 2 commits June 27, 2019 12:29
Co-Authored-By: Haseeb Tariq <hasbro17@gmail.com>
Co-Authored-By: Haseeb Tariq <hasbro17@gmail.com>
Copy link
Contributor

@hasbro17 hasbro17 left a comment

Choose a reason for hiding this comment

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

LGTM overall.

Your call on reformatting the unit tests now or as a TODO in a separate PR.

@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 28, 2019
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 28, 2019
@estroz estroz changed the title cmd/operator-sdk/olmcatalog: scaffold package.yaml for a set of operator manifests cmd/operator-sdk/olmcatalog: scaffold package.yaml for a set of… Jun 28, 2019
@estroz estroz merged commit 0fc5746 into operator-framework:master Jun 28, 2019
@estroz estroz deleted the olm-package-yaml branch June 28, 2019 18:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. olm-integration Issue relates to the OLM integration size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants