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

build: remove iproute build dependency on centos repo #14299

Merged

Conversation

BlaineEXE
Copy link
Member

@BlaineEXE BlaineEXE commented Jun 3, 2024

Multus requires that the 'ip' tool be installed in the Rook image. Today, it is present in the upstream Ceph base image that is used, but that is expected to change in the future. The Ceph project intends to switch to CentOS 9 minimal images, and the 'ip' tool will likely be removed from the base image at that point.

However, Rook CI has occasionally had issues with the current 'dnf install' command when CentOS repos go down, or when there is otherwise some problem. Because the package is already installed today, there is no need to hamstring Rook builds when CentOS is having problems. But we do want to make sure that Rook builds don't silently succeed in the eventual future when 'ip' tool is removed from the Ceph image.

For now, replace the 'dnf install' with a check to verify that 'ip' tool is installed, and add a shorter form of this note as a comment above it to help Rook maintainers know how to resolve future 'ip' tool removal.

I am proposing this in lieu of #14296, but thank you @sp98 for finding the root cause !

Checklist:

  • Commit Message Formatting: Commit titles and messages follow guidelines in the developer guide.
  • Reviewed the developer guide on Submitting a Pull Request
  • Pending release notes updated with breaking and/or notable changes for the next minor release.
  • Documentation has been updated, if necessary.
  • Unit tests have been added, if necessary.
  • Integration tests have been added, if necessary.

@BlaineEXE BlaineEXE requested review from sp98 and travisn June 3, 2024 20:02
# have other package build errors. In order to make sure Rook CI catches the eventual removal and
# also limit Rook CI breakage due to centos breakage, simply check that 'ip' is present.
# Eventually: dnf install -y --repo baseos --setopt=install_weak_deps=False iproute && dnf clean all
RUN which ip
Copy link
Member

Choose a reason for hiding this comment

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

This fails the build if not found?

Copy link
Member Author

Choose a reason for hiding this comment

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

Correct. In the eventual future, when ip is removed from the image, CI will start failing, and we will need to add the dnf install (or equivalent) line back to the build. But until then, this should succeed without adding a hard dependency on CentOS repos.

Multus requires that the 'ip' tool be installed in the Rook image.
Today, it is present in the upstream Ceph base image that is used, but
that is expected to change in the future. The Ceph project intends to
switch to CentOS 9 minimal images, and the 'ip' tool will likely be
removed from the base image at that point.

However, Rook CI has occasionally had issues with the current 'dnf
install' command when CentOS repos go down, or when there is otherwise
some problem. Because the package is already installed today, there is
no need to hamstring Rook builds when CentOS is having problems. But we
do want to make sure that Rook builds don't silently succeed in the
eventual future when 'ip' tool is removed from the Ceph image.

For now, replace the 'dnf install' with a check to verify that 'ip' tool
is installed, and add a shorter form of this note as a comment above it
to help Rook maintainers know how to resolve future 'ip' tool removal.

Signed-off-by: Blaine Gardner <blaine.gardner@ibm.com>
@BlaineEXE BlaineEXE force-pushed the remove-centos-baseos-repo-dependency branch from 87bc8d3 to c4e99c1 Compare June 3, 2024 20:38
@BlaineEXE
Copy link
Member Author

DCO check is stuck in "Waiting for status to be reported" state as noted here: https://github.com/orgs/community/discussions/26698#discussioncomment-9654155

Will manually merge when other CI passes successfully.

@BlaineEXE BlaineEXE merged commit ff448e1 into rook:master Jun 3, 2024
52 checks passed
@BlaineEXE BlaineEXE deleted the remove-centos-baseos-repo-dependency branch June 3, 2024 21:16
BlaineEXE added a commit that referenced this pull request Jun 3, 2024
build: remove iproute build dependency on centos repo (backport #14299)
BlaineEXE added a commit that referenced this pull request Jun 26, 2024
build: remove iproute build dependency on centos repo (backport #14299)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants