Bug 1982781 - "opm index rm" doesn't remove deprecated bundles
Summary: "opm index rm" doesn't remove deprecated bundles
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: OpenShift Container Platform
Classification: Red Hat
Component: OLM
Version: 4.7
Hardware: Unspecified
OS: Unspecified
urgent
urgent
Target Milestone: ---
: 4.9.0
Assignee: Nick Hale
QA Contact: Tom Buskey
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2021-07-15 17:12 UTC by Parthey Khanderia
Modified: 2021-10-18 17:40 UTC (History)
9 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Cause: Deprecated bundle info wasn't properly garbage collected. Consequence: Adding a previously deprecated bundle to a package after that package was removed resulted in the bundle being re-deprecated. Fix: Better garbage collect deprecated bundle info. Result: After a package is removed previously deprecated bundles can be re-added.
Clone Of:
Environment:
Last Closed: 2021-10-18 17:39:54 UTC
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
Github openshift operator-framework-olm pull 162 0 None None None 2021-08-11 19:34:05 UTC
Red Hat Product Errata RHSA-2021:3759 0 None None None 2021-10-18 17:40:08 UTC

Description Parthey Khanderia 2021-07-15 17:12:11 UTC
Description of problem:

We recently faced an issue trying to undeprecate a bundle that was accidentally deprecated from v4.7 index.

It seems as "opm index rm" command doesn't remove already deprecated bundles of the package


Version-Release number of selected component (if applicable):


How reproducible:
First instance of this happening but expecting it to happen every time

Steps to Reproduce:
1. Remove amq-streams operator from registry-proxy.engineering.redhat.com/rh-osbs/iib-pub-pending:v4.7
2. Check deprecated table in index-db 
3. amqstreams.v1.6.3 is still present


Actual results:
The bundle version is still present in the deprecated table for the given operator 

Expected results:
The bundle version should be removed from the deprecated table for the given operator

Additional info:
iib build to remove the package:
https://iib.engineering.redhat.com/api/v1/builds/87483/logs

Comment 1 Simon Woodman 2021-08-05 17:48:15 UTC
This is ultimately blocking the AMQ Streams 1.8.0 release - please can someone advise on a fix time or a work around. The problem is that it is blocking the un-deprecating of AMQ Streams 1.6.3 and customers cannot currently upgrade as the update graph has multiple heads. 

cc  @kliberti @bhills

Comment 2 Yashvardhan Nanavati 2021-08-05 18:34:25 UTC
Raising the priority to Urgent since this is blocking product teams.

/cc @@Ben Parees @nhale

Comment 3 Nick Hale 2021-08-10 02:57:42 UTC
I have an upstream PR that's ready for review (see https://github.com/operator-framework/operator-registry/pull/746).

It adds some logic to `opm index|registry rm` that effectively erases _some_ history for the given package. I'm using the term "some" here because I want to be clear that _only the latest deprecation in that package will be forgotten_. This is because the table that persists deprecation history doesn't include any info about the package a deprecated bundle belonged to, and once a bundle becomes truncated -- i.e. belongs to the "tail" of a deprecated bundle -- all records outside of the deprecation history are deleted.

From a pipeline perspective, I think this makes sense -- Re-adding all of the previously truncated bundles to a given package after it is removed _should_ result in the package reverting to its relative state before the last deprecatetruncate on it; we can think of it as "popping the stack" (deprecation history). We can clear the history of truncated bundles by "popping the stack" until the index is in the desired state.

From an opm user's perspective, I would expect `opm index|registry rm` to remove _all_ info related to a particular package -- the fact that I need to "pop the stack" is a rough edge.

From an opm maintainer perspective, I expect declarative config to obviate these commands entirely -- this behavior covers the "oops, I accidentally deprecated my last bundle" 90% use case with a small code diff.

Alternative approaches:

- add package info to deprecation history (pro: can clear all deprecation history for a package atomically, cons: existing truncated bundles in deprecation history still require "pop and re-add")
- add a new flag or command to "un-deprecate" bundles by path (pros: package removal remains side-effect free, surgical precision, cons: command sprawl, new automation in pipelines needed to use)

Now that a PR is up with a prospective fix, I think we should stop and review the alternatives before merging.

I'll be double posting this to the downstream thread on the issue (gchat).

Comment 4 Nick Hale 2021-08-10 13:16:29 UTC
Spoke with Ben Parees about this and ended up adding some logic that removes all truncated bundles when deprecatetruncate is called (as well as an initial cleanup migration).

This lets us avoid related problems outlined in https://bugzilla.redhat.com/show_bug.cgi?id=1982781#c3 -- TL;DR, removing a package now removes all related deprecations, avoiding side-effects when adding previously truncated bundles afterwards.

I'm going to try to get this merged upstream and will open a downstream PR ASAP.

Comment 5 Nick Hale 2021-08-11 19:25:54 UTC
upstream PR merged, downstream PR open

Comment 7 Tom Buskey 2021-08-16 20:49:50 UTC
opm index prune -f registry-proxy.engineering.redhat.com/rh-osbs/iib-pub-pending:v4.7 -p amq-streams -t quay.io/tbuskey/amq
podman push quay.io/tbuskey/amq:latest
opm index rm -f quay.io/tbuskey/amq:latest  --operators amq-streams -t quay.io/tbuskey/amq:new
podman push quay.io/tbuskey/amq:new
oc image extract quay.io/tbuskey/amq:new --file=/database/index.db


$ sqlite3 index.db 
SQLite version 3.34.1 2021-01-20 14:10:07
Enter ".help" for usage hints.
sqlite> select * FROM deprecated;
kubevirt-hyperconverged-operator.v2.3.0
sqlite>

Comment 9 Simone Tiraboschi 2021-08-24 11:56:42 UTC
I'm reopening this because the fix for the original bug introduced unexpected side effects.

Comment 10 Nick Hale 2021-08-25 04:28:29 UTC
Simone,

It looks like the kubevirt issue may be orthogonal to the patch for this bug. I'm closing this out and opening a new BZ that better characterizes the problem encountered by kubevirt: https://bugzilla.redhat.com/show_bug.cgi?id=1997362

Thanks

Comment 13 errata-xmlrpc 2021-10-18 17:39:54 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory (Moderate: OpenShift Container Platform 4.9.0 bug fix and security update), and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHSA-2021:3759


Note You need to log in before you can comment on or make changes to this bug.