Skip to content

(feat) Load operator registry bundle from ConfigMap#92

Merged
openshift-merge-robot merged 3 commits intooperator-framework:masterfrom
tkashem:configmap-bundle
Oct 29, 2019
Merged

(feat) Load operator registry bundle from ConfigMap#92
openshift-merge-robot merged 3 commits intooperator-framework:masterfrom
tkashem:configmap-bundle

Conversation

@tkashem
Copy link
Copy Markdown
Contributor

@tkashem tkashem commented Oct 1, 2019

  • Iterate through the Data section of a ConfigMap, process each
    resource and construct a registry.Bundle object for use.
  • The ConfigMap may have a resource that contains a PackageManifest
    resource. Process this resource if present and deserialize.

@openshift-ci-robot openshift-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Oct 1, 2019
@tkashem
Copy link
Copy Markdown
Contributor Author

tkashem commented Oct 1, 2019

(Putting a hold on it since it could change as we integrate the pieces)
/hold

@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 Oct 1, 2019
@openshift-ci-robot openshift-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Oct 14, 2019
@jpeeler
Copy link
Copy Markdown

jpeeler commented Oct 14, 2019

/hold cancel
Removing the hold as the added commit made by me verifies that this code will work correctly and no further changes are known at present. (Though it might be nice if #94 is merged first just in case.)

@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 Oct 14, 2019
@ecordell
Copy link
Copy Markdown
Member

ecordell commented Oct 18, 2019

this will need to change slightly when this merges: openshift/enhancements#70

@jpeeler
Copy link
Copy Markdown

jpeeler commented Oct 28, 2019

/test unit

@jpeeler
Copy link
Copy Markdown

jpeeler commented Oct 28, 2019

I forgot this depends on constants in #94.

tkashem and others added 3 commits October 28, 2019 14:49
@tkashem
(feat) Load operator registry bundle from ConfigMap
f5c95c0 
- Iterate through the Data section of a ConfigMap, process each
  resource and construct a registry.Bundle object for use.
- The ConfigMap may have a resource that contains a `PackageManifest`
  resource. Process this resource if present and deserialize.
test(unit): add test case from generated configmap
7804c8c 
This verifies that the configmap is being properly read from the
extracted data of a bundle image (from operator-framework#94).
chore(configmap): load packagemanifest data from annotations
914eee2 
Previously, the package manifest data was going to be included as a file
within the bundle. The planning changed and this updates the
implementation to match.

The configmap annotations for manifest data has been made as a
requirement.
@jpeeler
Copy link
Copy Markdown

jpeeler commented Oct 28, 2019

Removed the dependency on #94. Also, decided to keep the latest commit separate since these commits evolved over time with different authors instead of squashing it all together.

ecordell
ecordell approved these changes Oct 29, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@ecordell ecordell left a comment

Choose a reason for hiding this comment

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

/lgtm

As discussed offline, the PackageManifest component is unneeded. But since that shouldn't affect the use of this elsewhere and it blocks, lgtm and we can remove the packagemanifest parsing later.

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 29, 2019
@openshift-ci-robot
Copy link
Copy Markdown

openshift-ci-robot commented Oct 29, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ecordell, tkashem

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 29, 2019
@openshift-merge-robot openshift-merge-robot merged commit 916b3b7 into operator-framework:master Oct 29, 2019
@jpeeler jpeeler mentioned this pull request Dec 18, 2019
Merged
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ecordell ecordell ecordell approved these changes

@dinhxuanvu dinhxuanvu Awaiting requested review from dinhxuanvu

@njhale njhale Awaiting requested review from njhale

Assignees

@ecordell ecordell

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@tkashem @jpeeler @ecordell @openshift-ci-robot @openshift-merge-robot