OCPBUGS-6741: refactor FBC caching (#1051) #435

grokspawn · 2023-01-27T22:03:00Z

This commit improves maintainability and extensibility of the FBC caching used to serve the GRPC API.

It:

introduces a new cache package and interface, which defines primitives for checking cache integrity, building the cache, and loading the cache.
moves the existing registry.Querier to an implementation of the cache interface, called cache.JSON

This refactor also resolves two outstanding issues:

The current code contains a bug that causes panics when certain failures occur loading the cache.
The current code is hardcoded to always rebuild the cache if it is unreadable or its digest doesn't contain the expected value.

This commit:

Refactors and removes the cases the led to the panic situation
Adds a new flag, --cache-enforce-integrity, that causes opm serve to exit with a failure if the cache is unreadable or its digest doesn't contain the expected value. This is useful in production contexts when one always expects the cache to be pre-populated correctly.

Signed-off-by: Joe Lanford [email protected]
Upstream-repository: operator-registry
Upstream-commit: 562cf9c9bc649f1df2bbae26f8b543529499b60f

openshift-ci-robot · 2023-01-27T22:03:05Z

@grokspawn: This pull request references Jira Issue OCPBUGS-6741, which is invalid:

expected the bug to target the "4.13.0" version, but it targets "4.13.z" instead

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

The bug has been updated to refer to the pull request using the external bug tracker.

In response to this:

This commit improves maintainability and extensibility of the FBC caching used to serve the GRPC API.

It:

introduces a new cache package and interface, which defines primitives for checking cache integrity, building the cache, and loading the cache.

moves the existing registry.Querier to an implementation of the cache interface, called cache.JSON

This refactor also resolves two outstanding issues:

The current code contains a bug that causes panics when certain failures occur loading the cache.

The current code is hardcoded to always rebuild the cache if it is unreadable or its digest doesn't contain the expected value.

This commit:

Refactors and removes the cases the led to the panic situation

Adds a new flag, --cache-enforce-integrity, that causes opm serve to exit with a failure if the cache is unreadable or its digest doesn't contain the expected value. This is useful in production contexts when one always expects the cache to be pre-populated correctly.

Signed-off-by: Joe Lanford [email protected]
Upstream-repository: operator-registry
Upstream-commit: 562cf9c9bc649f1df2bbae26f8b543529499b60f

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.

grokspawn · 2023-01-27T22:03:36Z

/jira refresh

openshift-ci-robot · 2023-01-27T22:03:43Z

@grokspawn: This pull request references Jira Issue OCPBUGS-6741, which is valid. The bug has been moved to the POST state.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target version (4.13.0) matches configured target version for branch (4.13.0)
bug is in the state New, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @jianzhangbjz

In response to this:

/jira refresh

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 · 2023-01-28T03:22:13Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: grokspawn

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:

~~OWNERS~~ [grokspawn]

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

grokspawn · 2023-01-28T15:20:14Z

/retest-required

grokspawn · 2023-01-29T04:30:02Z

/retest-required

Xia-Zhao-rh · 2023-01-29T05:02:07Z

/label qe-approved

grokspawn · 2023-01-30T13:45:24Z

/retest-required

grokspawn · 2023-01-30T16:29:45Z

/retest-required

grokspawn · 2023-01-30T20:39:02Z

/test e2e-gcp-ovn

grokspawn · 2023-01-31T14:00:36Z

/retest-required

grokspawn · 2023-01-31T16:16:05Z

/retest-required

oceanc80 · 2023-01-31T19:05:30Z

/lgtm

grokspawn · 2023-01-31T19:10:45Z

/cherry-pick release-4.12

openshift-cherrypick-robot · 2023-01-31T19:10:47Z

@grokspawn: once the present PR merges, I will cherry-pick it on top of release-4.12 in a new PR and assign it to you.

In response to this:

/cherry-pick release-4.12

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-robot · 2023-01-31T23:12:16Z

/retest-required

Remaining retests: 0 against base HEAD c62913d and 2 for PR HEAD 5af8f3a in total

grokspawn · 2023-02-01T04:39:13Z

/retest-required

This commit improves maintainability and extensibility of the FBC caching used to serve the GRPC API. It: - introduces a new cache package and interface, which defines primitives for checking cache integrity, building the cache, and loading the cache. - moves the existing registry.Querier to an implementation of the cache interface, called cache.JSON This refactor also resolves two outstanding issues: 1. The current code contains a bug that causes panics when certain failures occur loading the cache. 2. The current code is hardcoded to always rebuild the cache if it is unreadable or its digest doesn't contain the expected value. This commit: 1. Refactors and removes the cases the led to the panic situation 2. Adds a new flag, --cache-enforce-integrity, that causes `opm serve` to exit with a failure if the cache is unreadable or its digest doesn't contain the expected value. This is useful in production contexts when one always expects the cache to be pre-populated correctly. Signed-off-by: Joe Lanford <[email protected]> Upstream-repository: operator-registry Upstream-commit: 562cf9c9bc649f1df2bbae26f8b543529499b60f

grokspawn · 2023-02-01T13:18:35Z

/retest-required

grokspawn · 2023-02-01T17:37:50Z

/retest-required

oceanc80 · 2023-02-01T20:04:27Z

/lgtm

openshift-ci-robot · 2023-02-01T20:08:28Z

/retest-required

Remaining retests: 0 against base HEAD c62913d and 2 for PR HEAD bf5fac6 in total

Xia-Zhao-rh · 2023-02-02T06:30:37Z

/retest

grokspawn · 2023-02-02T14:12:49Z

/retest

openshift-ci · 2023-02-02T16:36:55Z

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

openshift-ci-robot · 2023-02-02T16:41:13Z

@grokspawn: All pull requests linked via external trackers have merged:

openshift/operator-framework-olm#435

Jira Issue OCPBUGS-6741 has been moved to the MODIFIED state.

In response to this:

This commit improves maintainability and extensibility of the FBC caching used to serve the GRPC API.

It:

introduces a new cache package and interface, which defines primitives for checking cache integrity, building the cache, and loading the cache.

moves the existing registry.Querier to an implementation of the cache interface, called cache.JSON

This refactor also resolves two outstanding issues:

The current code contains a bug that causes panics when certain failures occur loading the cache.

The current code is hardcoded to always rebuild the cache if it is unreadable or its digest doesn't contain the expected value.

This commit:

Refactors and removes the cases the led to the panic situation

Adds a new flag, --cache-enforce-integrity, that causes opm serve to exit with a failure if the cache is unreadable or its digest doesn't contain the expected value. This is useful in production contexts when one always expects the cache to be pre-populated correctly.

Signed-off-by: Joe Lanford [email protected]
Upstream-repository: operator-registry
Upstream-commit: 562cf9c9bc649f1df2bbae26f8b543529499b60f

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-cherrypick-robot · 2023-02-02T16:42:03Z

@grokspawn: #435 failed to apply on top of branch "release-4.12":

Applying: refactor FBC caching (#1051)
Using index info to reconstruct a base tree...
M	staging/operator-registry/go.mod
M	staging/operator-registry/go.sum
M	vendor/modules.txt
Falling back to patching base and 3-way merge...
Auto-merging vendor/modules.txt
Removing vendor/github.com/operator-framework/operator-registry/pkg/registry/query.go
Removing staging/operator-registry/pkg/registry/query.go
Auto-merging staging/operator-registry/go.sum
CONFLICT (content): Merge conflict in staging/operator-registry/go.sum
Auto-merging staging/operator-registry/go.mod
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 refactor FBC caching (#1051)
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

In response to this:

/cherry-pick release-4.12

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-robot added jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. labels Jan 27, 2023

openshift-ci bot requested review from jianzhangbjz, anik120 and kevinrizza January 27, 2023 22:03

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 27, 2023

grokspawn force-pushed the master branch from 2e81380 to 14d9ccf Compare January 28, 2023 03:21

grokspawn force-pushed the master branch from 14d9ccf to 5af8f3a Compare January 28, 2023 04:00

openshift-ci bot added the qe-approved Signifies that QE has signed off on this PR label Jan 29, 2023

openshift-ci bot assigned oceanc80 Jan 31, 2023

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jan 31, 2023

grokspawn force-pushed the master branch from 5af8f3a to 0dbe95c Compare January 31, 2023 23:10

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Jan 31, 2023

grokspawn force-pushed the master branch from 0dbe95c to 87ee862 Compare January 31, 2023 23:16

grokspawn force-pushed the master branch from 87ee862 to bf5fac6 Compare February 1, 2023 04:58

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Feb 1, 2023

openshift-merge-robot merged commit 0c03a71 into openshift:master Feb 2, 2023

This was referenced Apr 21, 2023

[release-4.12] OCPBUGS-12155: work around OCPBUGS-6741 by explicitly setting --cache-dir on opm in catalog images openshift/oc-mirror#612

Closed

OCPBUGS-12236: Revert adding '--cache-dir /tmp/cache' to catalog images openshift/oc-mirror#613

Merged

aguidirh mentioned this pull request Jun 25, 2025

cs-redhat-operator-index-v4-18 generated by oc-mirror CrashLoopBackoff openshift/oc-mirror#1099

Closed

OCPBUGS-6741: refactor FBC caching (#1051) #435

OCPBUGS-6741: refactor FBC caching (#1051) #435

Uh oh!

Conversation

grokspawn commented Jan 27, 2023

Uh oh!

openshift-ci-robot commented Jan 27, 2023

Uh oh!

grokspawn commented Jan 27, 2023

Uh oh!

openshift-ci-robot commented Jan 27, 2023

Uh oh!

openshift-ci bot commented Jan 28, 2023

Uh oh!

grokspawn commented Jan 28, 2023

Uh oh!

grokspawn commented Jan 29, 2023

Uh oh!

Xia-Zhao-rh commented Jan 29, 2023

Uh oh!

grokspawn commented Jan 30, 2023

Uh oh!

grokspawn commented Jan 30, 2023

Uh oh!

grokspawn commented Jan 30, 2023

Uh oh!

grokspawn commented Jan 31, 2023

Uh oh!

grokspawn commented Jan 31, 2023

Uh oh!

oceanc80 commented Jan 31, 2023

Uh oh!

grokspawn commented Jan 31, 2023

Uh oh!

openshift-cherrypick-robot commented Jan 31, 2023

Uh oh!

openshift-ci-robot commented Jan 31, 2023

Uh oh!

grokspawn commented Feb 1, 2023

Uh oh!

grokspawn commented Feb 1, 2023

Uh oh!

grokspawn commented Feb 1, 2023

Uh oh!

oceanc80 commented Feb 1, 2023

Uh oh!

openshift-ci-robot commented Feb 1, 2023

Uh oh!

Xia-Zhao-rh commented Feb 2, 2023

Uh oh!

grokspawn commented Feb 2, 2023

Uh oh!

openshift-ci bot commented Feb 2, 2023

Uh oh!

openshift-ci-robot commented Feb 2, 2023

Uh oh!

openshift-cherrypick-robot commented Feb 2, 2023

Uh oh!

Uh oh!