Skip to content

internal/operator-sdk/run|cleanup: initial boilerplate#3636

Merged
joelanford merged 9 commits intooperator-framework:masterfrom
joelanford:boilerplate-run-bundle
Aug 13, 2020
Merged

internal/operator-sdk/run|cleanup: initial boilerplate#3636
joelanford merged 9 commits intooperator-framework:masterfrom
joelanford:boilerplate-run-bundle

Conversation

@joelanford
Copy link
Copy Markdown
Member

@joelanford joelanford commented Aug 4, 2020

Description of the change:
Adds commented out run bundle command with flag and bundle parsing boilerplate.

Motivation for the change:
To initialize a new package to be use for the run bundle command and enable progress on implementation to be made in parallel.

Checklist

If the pull request includes user-facing changes, extra documentation is required:

@joelanford
Copy link
Copy Markdown
Member Author

/hold

We might want to put this on a feature branch at least until 1.0 is released. WDYT?

@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 Aug 4, 2020
Comment thread internal/operator/config.go Outdated
Comment thread internal/operator/internal/index_image.go
Comment thread internal/operator/bundle/install.go
Comment thread internal/operator/bundle/install.go Outdated
@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 9, 2020
@joelanford joelanford force-pushed the boilerplate-run-bundle branch from bc2ed5f to 92b998d Compare August 12, 2020 13:20
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 12, 2020
@joelanford joelanford force-pushed the boilerplate-run-bundle branch 2 times, most recently from 1d600a2 to d8d5990 Compare August 12, 2020 15:22
Comment thread internal/operator/bundle/install.go Outdated
Comment thread internal/cmd/operator-sdk/run/cmd.go
Comment thread internal/registry/logger.go Outdated
@joelanford joelanford force-pushed the boilerplate-run-bundle branch 2 times, most recently from d8147cc to f8972eb Compare August 13, 2020 00:16
@joelanford joelanford force-pushed the boilerplate-run-bundle branch from 579aa0b to b560e68 Compare August 13, 2020 01:06
Copy link
Copy Markdown
Member

@rashmigottipati rashmigottipati left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Aug 13, 2020
@joelanford
Copy link
Copy Markdown
Member Author

/hold cancel

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 13, 2020
return fmt.Errorf("load bundle: %v", err)
}

i.OperatorInstaller.PackageName = labels["operators.operatorframework.io.bundle.package.v1"]
Copy link
Copy Markdown
Member

@estroz estroz Aug 13, 2020

Choose a reason for hiding this comment

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

Related: I have a PR open that makes getting bundle metadata very straightforward, which we should start using once merged (no changes required now)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Sweet! Look forward to that.

cfg *operator.Configuration
}

func NewIndexImageCatalogCreator(cfg *operator.Configuration) *IndexImageCatalogCreator {
Copy link
Copy Markdown
Member

@estroz estroz Aug 13, 2020

Choose a reason for hiding this comment

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

Any reason IndexImageCatalogCreator is exported and this signature isn't

Suggested change
func NewIndexImageCatalogCreator(cfg *operator.Configuration) *IndexImageCatalogCreator {
func NewIndexImageCatalogCreator(cfg *operator.Configuration, indexImage string, injectBundles []string, injectBundleMode string) CatalogCreator {

?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'll take a look at this for a follow-up.

Copy link
Copy Markdown
Member Author

@joelanford joelanford Aug 13, 2020

Choose a reason for hiding this comment

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

Actually, one reason is that we could make NewIndexImageCatalogCreator work with just an index image. In that case, it could skip the entire registry pod setup and just create the CatalogSource with the spec.index field set.

cc @rashmigottipati

Copy link
Copy Markdown
Member

@estroz estroz left a comment

Choose a reason for hiding this comment

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

/lgtm

@joelanford joelanford merged commit 3aca113 into operator-framework:master Aug 13, 2020
@joelanford joelanford deleted the boilerplate-run-bundle branch August 13, 2020 17:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants