Skip to content

Conversation

@exdx
Copy link
Contributor

@exdx exdx commented Sep 30, 2021

Description of the change:
Builds off #775

Pruning an index via opm index prune can fail when permissions on the filesystem in the container are read-only. Since the command copies all files out from the container it is prone to failure. Fora example, the red hat operators catalog is based on the UBI image, which contains some directories with elevated permissions (/root/* for example). Extract the entire contents of this container filesystem via docker cp will fail. This PR copies out the content of the file by copying the container contents to stdout as a tar stream and then writing the untarred content to disk.

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

exdx and others added 4 commits September 10, 2021 11:10
entire filesystem via a tar stream.

Signed-off-by: Daniel Sover <dsover@redhat.com>
Signed-off-by: Daniel Sover <dsover@redhat.com>
- simplify concurrent docker|podman cp + untar logic
- rename exporter -> untarer to match use
- rename exporter.Export to exporter.Untar
- make exporter.Untar more "pure" by adding parameters for the path and
  tar reader

Signed-off-by: Nick Hale <njohnhale@gmail.com>
Signed-off-by: Daniel Sover <dsover@redhat.com>
@exdx exdx requested a review from njhale September 30, 2021 13:51
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Sep 30, 2021

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

The full list of commands accepted by this bot can be found 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

@codecov
Copy link

codecov bot commented Sep 30, 2021

Codecov Report

Merging #800 (9fb3dec) into master (a3253eb) will decrease coverage by 1.13%.
The diff coverage is 53.29%.

❗ Current head 9fb3dec differs from pull request most recent head e9f1811. Consider uploading reports for the commit e9f1811 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master     #800      +/-   ##
==========================================
- Coverage   50.90%   49.77%   -1.14%     
==========================================
  Files         102      108       +6     
  Lines        8753     9342     +589     
==========================================
+ Hits         4456     4650     +194     
- Misses       3458     3807     +349     
- Partials      839      885      +46     
Impacted Files Coverage Δ
pkg/containertools/runner.go 0.00% <0.00%> (ø)
pkg/lib/bundle/exporter.go 56.66% <0.00%> (ø)
pkg/lib/bundle/supported_resources.go 100.00% <ø> (ø)
pkg/lib/bundle/validate.go 60.69% <0.00%> (ø)
pkg/lib/indexer/indexer.go 10.67% <0.00%> (-0.11%) ⬇️
pkg/registry/parse.go 73.68% <ø> (ø)
pkg/sqlite/query.go 64.53% <0.00%> (-0.08%) ⬇️
alpha/declcfg/diff_include.go 28.30% <2.66%> (-62.33%) ⬇️
pkg/image/containerdregistry/registry.go 4.38% <45.45%> (ø)
pkg/lib/registry/registry.go 19.50% <48.00%> (-4.11%) ⬇️
... and 15 more

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 a3253eb...e9f1811. Read the comment docs.

@exdx
Copy link
Contributor Author

exdx commented Sep 30, 2021

Seeing a new interesting failure

 • Failure [33.719 seconds]
opm
/home/runner/work/operator-registry/operator-registry/test/e2e/opm_test.go:219
  using docker
  /home/runner/work/operator-registry/operator-registry/test/e2e/opm_test.go:489
    builds and manipulates bundle and index images [It]
    /home/runner/work/operator-registry/operator-registry/test/e2e/opm_test.go:260

    Unexpected error:
        <sqlite3.Error>: {
            Code: 1,
            ExtendedCode: 1,
            err: "no such table: operatorbundle",
        }
        no such table: operatorbundle
    occurred

    /home/runner/work/operator-registry/operator-registry/test/e2e/opm_test.go:318

Signed-off-by: Daniel Sover <dsover@redhat.com>
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jan 21, 2022

@exdx: PR needs rebase.

Details

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 openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 21, 2022
@exdx exdx closed this Sep 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants