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

composer: don't create RepoRegistry using reporegistry.New() #4378

Merged
merged 2 commits into from
Sep 23, 2024

Conversation

thozza
Copy link
Member

@thozza thozza commented Sep 23, 2024

The reporegistry.New() has been enhanced to return an error, in case there were no repositories loaded. This was to fix the situation in many unit tests, which were previously not loading any repositories and silently not running any tests.

This however broke our SaaS deployment, where we actually do not configure any repositories on the filesystem. As a result, osbuild-composer started to fail on it.

Workaround this situation in osbuild-composer by reverting to the old behavior by loading the repo configs separately and then using the loaded repos (which could be empty map) to initialize the RepoRegistry.

Also update the tools/provision.sh to delete all repositories in the Service scenario.

This pull request includes:

  • adequate testing for the new functionality or fixed issue
  • adequate documentation informing people about the change such as
    • submit a PR for the READMEs listed here
    • submit a PR for the osbuild.org website repository if this PR changed any behavior not covered by the automatically updated READMEs

The `reporegistry.New()` has been enhanced to return an error, in case
there were no repositories loaded. This was to fix the situation in many
unit tests, which were previously not loading any repositories and
silently not running any tests.

This however broke our SaaS deployment, where we actually do not
configure any repositories on the filesystem. As a result,
osbuild-composer started to fail on it.

Workaround this situation in osbuild-composer by reverting to the old
behavior by loading the repo configs separately and then using the
loaded repos (which could be empty map) to initialize the RepoRegistry.

Signed-off-by: Tomáš Hozza <thozza@redhat.com>
@thozza thozza marked this pull request as draft September 23, 2024 13:38
In the Service scenario, we should be testing that osbuild-composer
works fine, without any repository configurations being present on the
filesystem.

Signed-off-by: Tomáš Hozza <thozza@redhat.com>
@thozza
Copy link
Member Author

thozza commented Sep 23, 2024

FTR, I'm happy to fix this in a different way in images, just wanted to fix composer ASAP 😇

Copy link
Member

@achilleas-k achilleas-k left a comment

Choose a reason for hiding this comment

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

Good to merge if it fixes prod. For the real fix, maybe we could return a special error from New() of type reporegistry.NoReposError or something and then callers can check for that.

@thozza thozza marked this pull request as ready for review September 23, 2024 16:51
@thozza thozza merged commit 3607783 into osbuild:main Sep 23, 2024
48 of 50 checks passed
@thozza thozza deleted the fix-no-repos-situation branch September 23, 2024 16:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants