Skip to content
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

Bugfix: Generate just the extra FBC for run bundle-upgrade #5891

Conversation

rashmigottipati
Copy link
Member

@rashmigottipati rashmigottipati commented Jun 21, 2022

Description of the change:
Partial fix to #5870

Motivation for the change:
For run bundle, when an index image is being passed, we are now generating just the extra FBC needed in the ConfigMap and mounting that as a volume in the registry pod. So run bundle-upgrade needs to be updated to generate just the extra FBC instead of creating the entire declarative config and appending bundle contents to it which will cause duplicate package/channel/bundle errors and fail.

Checklist

If the pull request includes user-facing changes, extra documentation is required:

@openshift-ci openshift-ci bot requested review from asmacdo and marc-obrien June 21, 2022 16:10
@rashmigottipati
Copy link
Member Author

/cc @everettraven @jmrodri

@openshift-ci openshift-ci bot requested review from everettraven and jmrodri June 21, 2022 16:10
@rashmigottipati rashmigottipati requested review from VenkatRamaraju and removed request for marc-obrien June 21, 2022 16:11
@@ -0,0 +1,25 @@
# entries is a list of entries to include in
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For run bundle, when an index image is being passed, we are now generating just the extra FBC needed in the ConfigMap and mounting that as a volume in the registry pod. Sorun bundle-upgrade` needs to be updated to generate just the extra FBC instead of creating the entire declarative config and appending bundle contents to it which will cause duplicate package/channel/bundle errors and fail.

So, we create another FBC index to add the upgraded bundle. Is that the solution?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@camilamacedo86 We don't create another FBC index to add the bundle. We add the just the extra bundle contents to a ConfigMap under the directory (which is /configs) that hosts the index already, so we don't have to append to index's contents again.

# release notes and/or the migration guide
entries:
- description: >
For run bundle-upgrade, generate just the extra FBC of the bundle instead of rendering the entire index and appending bundle contents to it.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How can we test this one to check that the PR fix?
What is the image/data used to do the test?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are the images I used for testing:
run bundle with bundle quay.io/olmqe/upgradeindex-bundle:v0.1 and index image quay.io/olmqe/nginxolm-operator-index:v1.

And run bundle-upgrade with quay.io/olmqe/upgradeindex-bundle:v0.2

Copy link
Contributor

@everettraven everettraven left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jun 21, 2022
@rashmigottipati rashmigottipati merged commit 73ab493 into operator-framework:master Jun 21, 2022
@everettraven
Copy link
Contributor

/cherry-pick v1.22.x

@openshift-cherrypick-robot

@everettraven: new pull request created: #5917

In response to this:

/cherry-pick v1.22.x

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants