internal/olm: refactor package internal/operator and run packagemanifests logic#3764
Conversation
estroz
force-pushed
the
refactor/run-olm
branch
4 times, most recently
from
d4b7e11 to
e24c75b
Compare
| } | ||
| } | ||
|
|
||
| func (c ConfigMapCatalogCreator) CreateCatalog(ctx context.Context, name string) (*v1alpha1.CatalogSource, error) { |
There was a problem hiding this comment.
This was refactored from internal/olm/operator/packagemanifests_manager.go.
estroz
force-pushed
the
refactor/run-olm
branch
from
e24c75b to
524cb3a
Compare
set of types under internal/olm/operator, and move the OLM installer into its own package
| @@ -0,0 +1,110 @@ | |||
| // Copyright 2019 The Operator-SDK Authors | |||
There was a problem hiding this comment.
This was refactored from internal/olm/operator/olm.go.
| // createOperatorGroup creates an OperatorGroup using package name if an OperatorGroup does not exist. | ||
| // If one exists in the desired namespace and it's target namespaces do not match the desired set, | ||
| // createOperatorGroup will return an error. | ||
| func (o OperatorInstaller) createOperatorGroup(ctx context.Context) error { |
There was a problem hiding this comment.
This was moved here from internal/olm/operator/tenancy.go.
|
/cc @joelanford |
jmrodri
left a comment
jmrodri
left a comment
There was a problem hiding this comment.
/lgtm
The merge commit made it harder to review, but I was able to reconcile the commits from this PR and the ones from master.
I was able to trace the code from the run bundle all the way through into the OperatorInstaller. It makes sense. I didn't see anything that stood out. I was also able to trace through run packagemanifests as well.
rashmigottipati
left a comment
rashmigottipati
left a comment
There was a problem hiding this comment.
Made a few observations regarding BundleImage not being passed or set in IndexImageCatalogCreator, but that's not really the scope of this PR, so totally fine.
Everything looks great. Thanks a lot for your effort in refactoring this, as it eliminates additional duplicate code in the internal/operator package.
/lgtm
4 participants
Description of the change:
internal/operatorinto a coherent set of types underinternal/olm/operator, and move the OLM installer into its own packageMotivation for the change: both
run bundleandrun packagemanifestsshare a lot of code, so should live under the same package such that types/functions can be shared as much as possible.Closes #2938
/cc @rashmigottipati @jmrodri
Checklist
If the pull request includes user-facing changes, extra documentation is required:
changelog/fragments(seechangelog/fragments/00-template.yaml)website/content/en/docs