New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CFE-825: As a oc-mirror user, I would like mirrored operator catalogs to have valid caches #651
Conversation
@sherine-k: This pull request references CFE-825 which is a valid jira issue. In response to this:
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. |
@sherine-k: This pull request references CFE-825 which is a valid jira issue. In response to this:
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. |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: sherine-k The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
@sherine-k: This pull request references CFE-825 which is a valid jira issue. In response to this:
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. |
@sherine-k: This pull request references CFE-825 which is a valid jira issue. In response to this:
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. |
59ec3d0
to
f101e83
Compare
@sherine-k: This pull request references CFE-825 which is a valid jira issue. In response to this:
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. |
0633d31
to
c211924
Compare
@sherine-k: This pull request references CFE-825 which is a valid jira issue. In response to this:
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. |
a811186
to
4de9da1
Compare
@sherine-k: This pull request references CFE-825 which is a valid jira issue. In response to this:
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. |
@sherine-k: This pull request references CFE-825 which is a valid jira issue. In response to this:
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. |
31892d4
to
c99376f
Compare
@sherine-k - Once we get feedback from QE lets get this PR merged. You have addressed most of John's suggestions. |
@sherine-k I added a new comment in #651 (comment) but the comment thread was marked as resolved and the buttons for marking it as unresolved are not present. I wanted to make sure you saw this reply. |
Fix code regarding finding the location of the opm binary
Fix E2E test catalog_full
/retest |
return fmt.Errorf("unable to determine location of cache for image %s. Cache generation failed: %v", ctlgRef, err) | ||
} | ||
cacheLocation := string(dat) | ||
cacheLocationElmts := strings.Split(strings.TrimPrefix(cacheLocation, string(os.PathSeparator)), string(os.PathSeparator)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: I've not tested this, but doesn't filepath.SplitList()
handle this?
pkg/cli/mirror/catalog_images.go
Outdated
if err != nil { | ||
return err | ||
} | ||
cfl.Close() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: isn't this handled at line 400 in the defer?
@@ -87,6 +87,12 @@ const ( | |||
// IndexDir is the location of the | |||
// file-based catalog json file. | |||
IndexDir = "index" | |||
// TmpDir is the location of the | |||
// catalog cache directory. | |||
TmpDir = "tmp/cache" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Might be good to update the workspace tree diagram with this new directory
…resence of argument `--cache-dir` in the original image container CMD
fd4efc5
to
7511209
Compare
@sherine-k: all tests passed! Full PR test history. Your PR dashboard. 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. I understand the commands that are listed here. |
/unhold |
@sherine-k - great work , lets get this PR merged |
/lgtm |
… to have valid caches (openshift#651) * Regenerate opm cache when building catalog images * Recreate the cache of local OCI catalogs * Incomplete - Include opm cache integrity verification to E2E Fix code regarding finding the location of the opm binary * TEMPORARY: use a different repository for source catalogs Fix E2E test catalog_full * Make cache regeneration during catalog rebuild conditional based on presence of argument `--cache-dir` in the original image container CMD * Fix review comments
…o deploy because of invalid caches (#691) * CFE-825: As a oc-mirror user, I would like mirrored operator catalogs to have valid caches (#651) * Regenerate opm cache when building catalog images * Recreate the cache of local OCI catalogs * Incomplete - Include opm cache integrity verification to E2E Fix code regarding finding the location of the opm binary * TEMPORARY: use a different repository for source catalogs Fix E2E test catalog_full * Make cache regeneration during catalog rebuild conditional based on presence of argument `--cache-dir` in the original image container CMD * Fix review comments * OCPBUGS-17545: Improve extracting opm binary from catalogs (#676) * Improve extracting opm binary from catalogs * Fix OCPBUGS-17545 - Force pull catalog by platform architecture * Rewrite extractOPMBinary in a non-recursive way * Attempt 2 - Rewrite extractOPMBinary in a non-recursive way This new implementation extracts the whole catalog to a local folder in order to make it easier to grab the opm binary. * Account for cases where catalog image's entrypoint might be empty * Add comments to opm binary extraction logic * Fix E2E tests after conflict resolution
Description
This PR implements the changes described in CFE-825.
This is a Work In Progress.
List of Done:
List of ToDo:
Fixes CFE-825.
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Checklist: