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

fix: merge existing and extra fbc while rendering the index image #6512

Merged
merged 1 commit into from
Jul 25, 2023

Conversation

iamniting
Copy link
Contributor

@iamniting iamniting commented Jul 25, 2023

Description of the change:
During the creation of a new catalog image using the operator-sdk, a critical bug was identified where existing FBC (File-Based Configuration) files were not being copied to the new location of the catalog image. Consequently, the generated catalog image was missing essential FBC files, leading to incorrect behavior and potential issues for end-users.

This bug fix addresses the root cause of the problem by ensuring that both the old and new FBC files are correctly included in the catalog image creation process. With this fix, the resulting catalog image will contain all the necessary FBC files, thereby resolving the issue of missing configurations and ensuring the operator functions as expected. Users can now rely on the updated catalog image to access and utilize the complete set of FBC files, enhancing the overall stability and reliability of the operator-sdk.

Motivation for the change:
Fix: #6505

Checklist

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

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.

Thanks for taking the time to make this contribution @iamniting ! These changes look good to me!

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 25, 2023
@iamniting iamniting temporarily deployed to deploy July 25, 2023 13:30 — with GitHub Actions Inactive
@iamniting iamniting temporarily deployed to deploy July 25, 2023 13:30 — with GitHub Actions Inactive
@iamniting iamniting temporarily deployed to deploy July 25, 2023 13:30 — with GitHub Actions Inactive
@iamniting iamniting temporarily deployed to deploy July 25, 2023 13:30 — with GitHub Actions Inactive
@iamniting iamniting temporarily deployed to deploy July 25, 2023 13:30 — with GitHub Actions Inactive
@iamniting iamniting temporarily deployed to deploy July 25, 2023 13:31 — with GitHub Actions Inactive
Copy link
Member

@rashmigottipati rashmigottipati left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution @iamniting, changes look good to me.

/lgtm

Copy link
Member

@rashmigottipati rashmigottipati left a comment

Choose a reason for hiding this comment

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

Could you please add a changelog fragment for this PR? We can merge the PR after that's added.

see changelog/fragments/00-template.yaml

copy the FBC of index-image as well if --index-image is passed to the
operator-sdk run bundle command

Fixes operator-framework#6505

Signed-off-by: Nitin Goyal <nigoyal@redhat.com>
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Jul 25, 2023
@iamniting
Copy link
Contributor Author

Could you please add a changelog fragment for this PR? We can merge the PR after that's added.

see changelog/fragments/00-template.yaml

I added it, Can you pls take a look and see if it is good enough or shall I make it more descriptive?

@iamniting iamniting temporarily deployed to deploy July 25, 2023 15:52 — with GitHub Actions Inactive
@iamniting iamniting temporarily deployed to deploy July 25, 2023 15:52 — with GitHub Actions Inactive
@iamniting iamniting temporarily deployed to deploy July 25, 2023 15:52 — with GitHub Actions Inactive
@iamniting iamniting temporarily deployed to deploy July 25, 2023 15:52 — with GitHub Actions Inactive
@iamniting iamniting temporarily deployed to deploy July 25, 2023 15:52 — with GitHub Actions Inactive
@iamniting iamniting temporarily deployed to deploy July 25, 2023 15:52 — with GitHub Actions Inactive
Copy link
Member

@rashmigottipati rashmigottipati 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 Jul 25, 2023
@rashmigottipati rashmigottipati merged commit bd0ccd2 into operator-framework:master Jul 25, 2023
@iamniting iamniting deleted the test branch August 14, 2024 07:45
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.

operator-sdk does not copy the configs from the given index image
3 participants