Skip to content

Conversation

@sanjay20m
Copy link
Contributor

Summary
This patch fixes an issue in the entrypoint.sh scripts for both CentOS 8 and Fedora 35 where the local YUM repository was not being used by default during test runs.

Issue
Even though the script creates a local YUM repository file (local.repo), it was configured with enabled=0, which means it is disabled by default. While yum-config-manager --enable local-repository was called, this is not always reliable in automated or minimal environments (e.g. CI containers) and may fail silently or be skipped.

As a result, any locally built RPM packages are not actually used during testing, making the local-repository setup ineffective.

Fix
Changed enabled=0 to enabled=1 directly in the .repo file to ensure it's active by default.
Set the correct baseurl using the ${LOCAL_REPO_DIRECTORY} environment variable.
Removed the redundant call to yum-config-manager --enable local-repository.
Impact
Ensures local RPMs are tested properly in CI.
Fixes misleading behavior where the script pretended to use the local repo but didn’t.
Helps catch packaging or integration issues earlier in the CI pipeline.

IMP NOTE: Previous PR (#1196) had issues during squash/sign-off process due to local setup constraints. This PR is the corrected version and replaces the old one. Thank you for your understanding and patience!

@copy-pr-bot
Copy link

copy-pr-bot bot commented Jul 23, 2025

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

…ints

- Changed enabled=0 to enabled=1 in local.repo
- Updated baseurl to use ${LOCAL_REPO_DIRECTORY}
- Removed redundant yum-config-manager --enable command
- Ensures local RPMs are properly tested in CI for both CentOS8 and Fedora35

Signed-off-by: Sanjay Jangid <136222049+sanjay20m@users.noreply.github.com>
@sanjay20m
Copy link
Contributor Author

Hi @ArangoGutierrez @elezar

I've rebased the branch, squashed the commits into a single logical change, and ensured the Signed-off-by is correct

Please review the updated PR. Thank you for your guidance and patience!

@ArangoGutierrez ArangoGutierrez requested a review from Copilot July 23, 2025 13:01
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes a critical issue where local YUM repositories were not being used by default in CentOS 8 and Fedora 35 test environments, despite being configured. The fix ensures that locally built RPM packages are properly tested in CI by enabling the repository directly in the configuration file.

  • Changed repository configuration from enabled=0 to enabled=1 to activate local repos by default
  • Updated hardcoded paths to use the LOCAL_REPO_DIRECTORY environment variable for consistency
  • Removed unreliable yum-config-manager --enable command that could fail silently in minimal environments

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
tests/release/docker/fedora35/entrypoint.sh Updates local repo setup to enable by default and use variable paths
tests/release/docker/centos8/entrypoint.sh Identical fixes to fedora35 plus removal of redundant yum-config-manager call
Comments suppressed due to low confidence (1)

Copy link
Collaborator

@ArangoGutierrez ArangoGutierrez left a comment

Choose a reason for hiding this comment

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

lgtm - Thoughts @elezar ?

@ArangoGutierrez ArangoGutierrez self-assigned this Jul 23, 2025
@sanjay20m
Copy link
Contributor Author

Hi @elezar I noticed this PR hasn’t been reviewed yet. Just wanted to check if there are any issues on my side or anything else I should address. Please let me know.

Copy link
Member

@elezar elezar left a comment

Choose a reason for hiding this comment

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

Thanks @sanjay20m. These are containers that test basic package functionality, but the change are appreciated.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 16469767223

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 35.023%

Totals Coverage Status
Change from base Build 16421142187: 0.0%
Covered Lines: 4463
Relevant Lines: 12743

💛 - Coveralls

@sanjay20m
Copy link
Contributor Author

Thank you very much for the review and feedback! Please let me know if there’s anything else I should address. I truly appreciate your time and consideration, and I’m looking forward to the merge whenever it’s convenient.

@elezar elezar merged commit 9fde20d into NVIDIA:main Aug 8, 2025
6 checks passed
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.

4 participants