-
Notifications
You must be signed in to change notification settings - Fork 149
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
Migrate index generation process to File-Based Catalog #2054
Migrate index generation process to File-Based Catalog #2054
Conversation
Pull Request Test Coverage Report for Build 2897762102
💛 - Coveralls |
hco-e2e-upgrade-prev-index-aws lane succeeded. |
@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-upgrade-prev-index-azure 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. |
hco-e2e-image-index-azure, hco-e2e-image-index-aws lanes succeeded. |
@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-image-index-gcp 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. |
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.
Looks very good.
Added some comments.
hack/build-index-image.sh
Outdated
fi | ||
|
||
${OPM} render ${BUNDLE_IMAGE_NAME} > bundle.json | ||
CSV_NAME=$(jq .name bundle.json | tr -d '"') |
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.
you can use the -r
flag instead of tr -d
:
CSV_NAME=$(jq .name bundle.json | tr -d '"') | |
CSV_NAME=$(jq -r .name bundle.json) |
hack/build-index-image.sh
Outdated
${OPM} index add --bundles "${BUNDLE_IMAGE_NAME}" ${INDEX_IMAGE_PARAM} --tag "${INDEX_IMAGE_NAME}" -u podman --mode semver | ||
|
||
# File-Based Catalog handling | ||
${OPM} migrate ${INDEX_IMAGE_NAME} fbc-catalog || true |
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.
I think we should avoid writing to the root directory of the project. What do you think about output to _out/fbc-catalog
?
If you do that, please add an env var for this path, in order to use it later in the script.
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.
I think it would be better to change the working directory in this part to ${PROJECT_ROOT}/_out
to avoid adding the _out/
prefix for every file reference.
oc image extract ${INDEX_IMAGE_NAME} --file /configs/catalog.json | ||
else | ||
# The migration took place | ||
mv fbc-catalog/community-kubevirt-hyperconverged/catalog.json catalog.json |
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.
Please move to _out/catalog.json
hack/build-index-image.sh
Outdated
mv fbc-catalog/community-kubevirt-hyperconverged/catalog.json catalog.json | ||
fi | ||
|
||
${OPM} render ${BUNDLE_IMAGE_NAME} > bundle.json |
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.
please write to _out/bundle.json
hack/build-index-image.sh
Outdated
${OPM} index add --bundles "${BUNDLE_IMAGE_NAME}" ${INDEX_IMAGE_PARAM} --tag "${INDEX_IMAGE_NAME}" -u podman --mode semver | ||
|
||
# File-Based Catalog handling | ||
${OPM} migrate ${INDEX_IMAGE_NAME} fbc-catalog || true |
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.
Please wrap with double quotes.
${OPM} migrate ${INDEX_IMAGE_NAME} fbc-catalog || true | |
${OPM} migrate "${INDEX_IMAGE_NAME}" _out/fbc-catalog || true |
hack/build-index-image.sh
Outdated
if [ ! -d fbc-catalog ] | ||
then | ||
# The index image is already in file-based format. Extracting its catalog file | ||
oc image extract ${INDEX_IMAGE_NAME} --file /configs/catalog.json |
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.
Please wrap with double quotes.
oc image extract ${INDEX_IMAGE_NAME} --file /configs/catalog.json | |
oc image extract "${INDEX_IMAGE_NAME}" --file /configs/catalog.json |
hack/build-index-image.sh
Outdated
mv fbc-catalog/community-kubevirt-hyperconverged/catalog.json catalog.json | ||
fi | ||
|
||
${OPM} render ${BUNDLE_IMAGE_NAME} > bundle.json |
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.
${OPM} render ${BUNDLE_IMAGE_NAME} > bundle.json | |
${OPM} render "${BUNDLE_IMAGE_NAME}" > _out/bundle.json |
select(.schema=="olm.bundle" and (.name | contains($CHANNEL))))' \ | ||
updated_fbc.json.tmp > updated_fbc.json | ||
|
||
mkdir -p fbc-catalog |
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.
maybe _out/fbc-catalog
?
hack/build-index-image.sh
Outdated
remove_tmp_files | ||
} | ||
|
||
function remove_tmp_files() { |
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.
not needed if everything in _out
3c10dee
to
f06e5c8
Compare
hco-e2e-upgrade-index-azure lane succeeded. |
@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-upgrade-index-aws 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. |
hco-e2e-image-index-azure, hco-e2e-image-index-aws lanes succeeded. |
@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-image-index-gcp, ci/prow/hco-e2e-upgrade-prev-index-azure 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. |
hco-e2e-kv-smoke-gcp lane succeeded. |
@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-kv-smoke-azure 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. |
hack/build-index-image.sh
Outdated
${OPM} index add --bundles "${BUNDLE_IMAGE_NAME}" ${INDEX_IMAGE_PARAM} --tag "${INDEX_IMAGE_NAME}" -u podman --mode semver | ||
|
||
# File-Based Catalog handling | ||
cd "${OUT_DIR}" |
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.
This will work. But I'm not sure I'm happy with cd
and cd
back. What do you think about placing all of this in a new function, then call it like this?
(cd "${OUT_DIR}" && my_new_function)
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.
valid point.
updated, thanks.
SQLite-Based catlog format, which we were using so far, is being deprecated in favour of the new format of file-based catalog. With this change, the 'opm index add --bundles' command no longer works, and we need to change the workflow to work with json files inside the catalog image. References: - https://olm.operatorframework.io/docs/reference/file-based-catalogs/ - k8s-operatorhub/community-operators#505 Signed-off-by: Oren Cohen <ocohen@redhat.com>
f06e5c8
to
05c1fb7
Compare
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
/lgtm |
hco-e2e-upgrade-prev-index-aws lane succeeded. |
@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-upgrade-prev-index-azure 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. |
hco-e2e-image-index-gcp, hco-e2e-image-index-aws lanes succeeded. |
@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-image-index-azure 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. |
@orenc1: The following tests failed, say
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. |
hco-e2e-upgrade-prev-index-sno-aws lane succeeded. |
@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-upgrade-prev-index-sno-azure 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. |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: nunnatsa 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 |
SQLite-Based catlog format, which we were using so far, is being deprecated in favour of the new format of file-based catalog.
With this change, the 'opm index add --bundles' command no longer works, and we need to change the workflow to work with json files inside the catalog image.
References:
Signed-off-by: Oren Cohen ocohen@redhat.com
Reviewer Checklist
Release note: