operator-registry icon indicating copy to clipboard operation
operator-registry copied to clipboard

Surface the manifest filenames for higher level logic

Open fgiloux opened this issue 4 years ago • 9 comments

Signed-off-by: Frederic Giloux [email protected]

Description of the change:

This pull request surfaces the manifest filenames. There is currently no metadata, only the content of a manifest file carried over by bundle.Object. By returning the names of the manifest files it is possible to attach properties that can influence OLM behavior in regards to a specific manifest. This can then be leveraged for making manifests optional

Motivation for the change:

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

fgiloux avatar Jan 06 '22 13:01 fgiloux

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: fgiloux To complete the pull request process, please assign benluddy after the PR has been reviewed. You can assign the PR to them by writing /assign @benluddy in a comment when ready.

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

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[bot] avatar Jan 06 '22 13:01 openshift-ci[bot]

Hi @fgiloux. Thanks for your PR.

I'm waiting for a operator-framework member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

openshift-ci[bot] avatar Jan 06 '22 13:01 openshift-ci[bot]

/ok-to-test

timflannagan avatar Jan 06 '22 14:01 timflannagan

Codecov Report

Merging #893 (1511eec) into master (9999f79) will increase coverage by 0.01%. The diff coverage is 60.00%.

:exclamation: Current head 1511eec differs from pull request most recent head 0b64b11. Consider uploading reports for the commit 0b64b11 to get more accurate results

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #893      +/-   ##
==========================================
+ Coverage   52.21%   52.22%   +0.01%     
==========================================
  Files         103      103              
  Lines        9117     9119       +2     
==========================================
+ Hits         4760     4762       +2     
  Misses       3451     3451              
  Partials      906      906              
Impacted Files Coverage Δ
pkg/configmap/configmap.go 76.47% <60.00%> (+0.96%) :arrow_up:
alpha/declcfg/declcfg.go 100.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 9999f79...0b64b11. Read the comment docs.

codecov[bot] avatar Jan 06 '22 14:01 codecov[bot]

Needed to downgrade grpc version due to: https://github.com/operator-framework/operator-lifecycle-manager/pull/2423#pullrequestreview-819616924

fgiloux avatar Feb 03 '22 16:02 fgiloux

I think I'd like to see us add some testing around the filenames that get returned from the Load(...) method to ensure we don't regress on the registry side-of-things down the line.

Fair enough. I have added some.

fgiloux avatar Feb 04 '22 11:02 fgiloux

I see a couple of pkg/lib/indexer/index.Dockerfile* changes that are present in this PR. Any idea what they're needed for, or were they generated locally, and we're not correctly filtering them out in the .gitignore properly?

I have done some cleansing. I believe that they are generated by the unit tests for mirror. When opening files it would be advisable to use: TemFile without specifying a dir. It would then use the OS default tmp dir, /tmp for Linux.

fgiloux avatar Feb 04 '22 16:02 fgiloux

/hold

fgiloux avatar Mar 30 '22 17:03 fgiloux

@fgiloux: 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.

openshift-ci[bot] avatar May 31 '22 04:05 openshift-ci[bot]

closing PR as stale. Please re-open if it's still important.

perdasilva avatar Feb 19 '24 13:02 perdasilva