feat(opm): Create multiple files during migration
Description of the change:
In this PR, the opm migrate command is enhanced to create multiple
plaintext files, one each for each package, bundles and channels
intead of a single file for each package.
Motivation for the change:
When plaintext files are loaded onto memory, they are able to be loaded even if the metadata is spread across multiple files (instead of just one file). Splitting the metadata logically in a way this PR does makes the files more human consumable.
Reviewer Checklist
- [ ] Implementation matches the proposed design, or proposal is updated to match implementation
- [ ] Sufficient unit test coverage
- [ ] Sufficient end-to-end test coverage
- [ ] Docs updated or added to
/docs - [ ] Commit messages sensible and descriptive
[APPROVALNOTIFIER] This PR is NOT APPROVED
This pull-request has been approved by: anik120
To complete the pull request process, please assign dinhxuanvu after the PR has been reviewed.
You can assign the PR to them by writing /assign @dinhxuanvu in a comment when ready.
The full list of commands accepted by this bot can be found here.
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
Codecov Report
Merging #815 (512ecc3) into master (7af20e0) will decrease coverage by
0.02%. The diff coverage is44.00%.
@@ Coverage Diff @@
## master #815 +/- ##
==========================================
- Coverage 51.04% 51.03% -0.02%
==========================================
Files 103 103
Lines 9050 9071 +21
==========================================
+ Hits 4620 4629 +9
- Misses 3553 3559 +6
- Partials 877 883 +6
| Impacted Files | Coverage Δ | |
|---|---|---|
| alpha/action/migrate.go | 55.17% <44.00%> (-6.99%) |
:arrow_down: |
It feels like this is something we should probably have a veneer for, opm render also has this problem with not splitting out object types for readability.
@ankitathomas while approaching this PR, I'd like to not conflate all of the issues you bring up here. This change is independent of any other effort, for the sake of simplicity, all this PR approaches is, "it'll be useful to have the output of opm migrate organized a bit more". If there are any reasons why having that convenience of organized output will be detrimental to users, I'm happy to hear those and start another discussion.
I'm inclined to put a /hold on this PR, mainly because I think changing migrate right now is premature.
I don't want us adding more complexity to it (e.g. a new flag) that has the potential to dictate the direction we take migrate in the future unless we're absolutely positive that the feature (and the added complexity) is actually being requested by the pipeline or a group of representative operator authors.
unless we're absolutely positive that the feature (and the added complexity) is actually being requested by the pipeline or a group of representative operator authors.
@joelanford AFAIK no pipeline/group of representative operator authors requested migrate to begin with. The migrate that's in there now was cooked up by us to begin with, and this PR is just enhancing the soup a little bit.
I don't want us adding more complexity to it (e.g. a new flag) that has the potential to dictate the direction we take migrate in the future
If anything the flag is just to appease both set of requests, "I want the output organised" and "I want the output as single files". Bottom line, I being the only user of the command at this point (as a result of prototyping migrating a catalog), I'd rather have it only as organized output than giant single files.
@anik120: PR needs rebase.
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.
We wrote a simple script to expand the catalog.json after a migrate into schema only json files but would be great to have this built into opm migrate and/or opm render. Example catalog.json expanded output for aimanager-operator package with two channels and one bundle in each:
olm.bundle-aimanager-operator.v1.0.0.json
olm.bundle-aimanager-operator.v2.0.0.json
olm.channel-v1.0.json
olm.channel-v2.0.json
olm.package-aimanager-operator.json
Here's a one-liner:
cat catalog.json | jq -sr '.[] | @sh "cat >\(.schema)-\(.name).json <<\\END",.,"END"' | sh
closing PR as stale. Please re-open if it's still important.