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

SRE-2452 rpms: Script to build RPMs from scratch #15419

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

ryon-jensen
Copy link
Contributor

Script to build all RPMs, including dependencies.

Before requesting gatekeeper:

  • Two review approvals and any prior change requests have been resolved.
  • Testing is complete and all tests passed or there is a reason documented in the PR why it should be force landed and forced-landing tag is set.
  • Features: (or Test-tag*) commit pragma was used or there is a reason documented that there are no appropriate tags for this PR.
  • Commit messages follows the guidelines outlined here.
  • Any tests skipped by the ticket being addressed have been run and passed in the PR.

Gatekeeper:

  • You are the appropriate gatekeeper to be landing the patch.
  • The PR has 2 reviews by people familiar with the code, including appropriate owners.
  • Githooks were used. If not, request that user install them and check copyright dates.
  • Checkpatch issues are resolved. Pay particular attention to ones that will show up on future PRs.
  • All builds have passed. Check non-required builds for any new compiler warnings.
  • Sufficient testing is done. Check feature pragmas and test tags and that tests skipped for the ticket are run and now pass with the changes.
  • If applicable, the PR has addressed any potential version compatibility issues.
  • Check the target branch. If it is master branch, should the PR go to a feature branch? If it is a release branch, does it have merge approval in the JIRA ticket.
  • Extra checks if forced landing is requested
    • Review comments are sufficiently resolved, particularly by prior reviewers that requested changes.
    • No new NLT or valgrind warnings. Check the classic view.
    • Quick-build or Quick-functional is not used.
  • Fix the commit message upon landing. Check the standard here. Edit it to create a single commit. If necessary, ask submitter for a new summary.

Copy link

Errors are Unable to load ticket data
https://daosio.atlassian.net/browse/SRE-2452

@daosbuild1
Copy link
Collaborator

Test stage Unit Test on EL 8.8 completed with status FAILURE. https://build.hpdd.intel.com/job/daos-stack/job/daos/job/PR-15419/5/display/redirect

@daosbuild1
Copy link
Collaborator

Test stage NLT on EL 8.8 completed with status FAILURE. https://build.hpdd.intel.com/job/daos-stack/job/daos/job/PR-15419/5/display/redirect

@daosbuild1
Copy link
Collaborator

Test stage NLT on EL 8.8 completed with status FAILURE. https://build.hpdd.intel.com/job/daos-stack/job/daos/job/PR-15419/9/display/redirect

@daosbuild1
Copy link
Collaborator

Test stage Unit Test bdev on EL 8.8 completed with status FAILURE. https://build.hpdd.intel.com/job/daos-stack/job/daos/job/PR-15419/20/display/redirect

@daosbuild1
Copy link
Collaborator

Test stage NLT on EL 8.8 completed with status FAILURE. https://build.hpdd.intel.com/job/daos-stack/job/daos/job/PR-15419/20/display/redirect

@daosbuild1
Copy link
Collaborator

Test stage Unit Test with memcheck on EL 8.8 completed with status FAILURE. https://build.hpdd.intel.com/job/daos-stack/job/daos/job/PR-15419/20/display/redirect

@daosbuild1
Copy link
Collaborator

Test stage Unit Test on EL 8.8 completed with status FAILURE. https://build.hpdd.intel.com/job/daos-stack/job/daos/job/PR-15419/20/display/redirect

@daosbuild1
Copy link
Collaborator

Test stage Unit Test bdev with memcheck on EL 8.8 completed with status FAILURE. https://build.hpdd.intel.com/job/daos-stack/job/daos/job/PR-15419/20/display/redirect

@daosbuild1
Copy link
Collaborator

Test stage Unit Test bdev on EL 8.8 completed with status FAILURE. https://build.hpdd.intel.com/job/daos-stack/job/daos/job/PR-15419/23/display/redirect

@daosbuild1
Copy link
Collaborator

Test stage Unit Test bdev on EL 8.8 completed with status FAILURE. https://build.hpdd.intel.com/job/daos-stack/job/daos/job/PR-15419/24/display/redirect

@daosbuild1
Copy link
Collaborator

Test stage NLT on EL 8.8 completed with status FAILURE. https://build.hpdd.intel.com/job/daos-stack/job/daos/job/PR-15419/31/display/redirect

@daosbuild1
Copy link
Collaborator

Test stage Unit Test with memcheck on EL 8.8 completed with status FAILURE. https://build.hpdd.intel.com/job/daos-stack/job/daos/job/PR-15419/31/display/redirect

@daosbuild1
Copy link
Collaborator

Test stage Unit Test on EL 8.8 completed with status FAILURE. https://build.hpdd.intel.com/job/daos-stack/job/daos/job/PR-15419/31/display/redirect

@daosbuild1
Copy link
Collaborator

Test stage Unit Test bdev with memcheck on EL 8.8 completed with status FAILURE. https://build.hpdd.intel.com/job/daos-stack/job/daos/job/PR-15419/31/display/redirect

@daosbuild1
Copy link
Collaborator

Test stage Unit Test bdev on EL 8.8 completed with status FAILURE. https://build.hpdd.intel.com/job/daos-stack/job/daos/job/PR-15419/31/display/redirect

@ryon-jensen ryon-jensen marked this pull request as ready for review October 30, 2024 12:25
Script to build all RPMs, including dependencies on
RockyLinux 8 and 9.

Introduces a new GHA workflow file for running the script.

Signed-off-by: Ryon Jensen <ryon.jensen@intel.com>

Doc-only: true
utils/build_rpms_from_scratch.sh Outdated Show resolved Hide resolved
utils/build_rpms_from_scratch.sh Outdated Show resolved Hide resolved
utils/build_rpms_from_scratch.sh Outdated Show resolved Hide resolved
utils/build_rpms_from_scratch.sh Outdated Show resolved Hide resolved
Signed-off-by: Ryon Jensen <ryon.jensen@intel.com>

Doc-only: true
Signed-off-by: Ryon Jensen <ryon.jensen@intel.com>

Doc-only: true
Signed-off-by: Ryon Jensen <ryon.jensen@intel.com>
@daosbuild1
Copy link
Collaborator

Test stage Functional Hardware Medium Verbs Provider completed with status FAILURE. https://build.hpdd.intel.com/job/daos-stack/job/daos/job/PR-15419/43/display/redirect

@daosbuild1
Copy link
Collaborator

Test stage Functional Hardware Medium completed with status FAILURE. https://build.hpdd.intel.com/job/daos-stack/job/daos/job/PR-15419/43/display/redirect

utils/build_rpms_from_scratch.sh Outdated Show resolved Hide resolved
utils/build_rpms_from_scratch.sh Outdated Show resolved Hide resolved
utils/build_rpms_from_scratch.sh Outdated Show resolved Hide resolved
rpmdevtools \
createrepo_c \
make \
python3-Sphinx
Copy link
Contributor

Choose a reason for hiding this comment

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

Any idea what are requiring this? If it's failing rpmbuilds that's a packaging bug.

Copy link
Contributor Author

@ryon-jensen ryon-jensen Nov 4, 2024

Choose a reason for hiding this comment

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

There's a comment for python3-Sphinx:

  # python3-Sphinx is needed by dpdk. The file /usr/bin/sphinx-build is listed as a dependency in the spec file,
  #   however, zypper isn't able to resolve a filename to a package and the scripting to handle that case is pretty
  #   messy, especially because "zypper what-provides" actually returns three options for sphinx-build. So, forgoing
  #   automation here and simply hardcoding the dependency.

I recognize it's not ideal, but weighing the effort and complexity (at least for me) for managing the dependency dynamically compared to hardcoding it and dealing with the impact on maintainability, I chose this approach. Definitely open to suggestions.

Copy link
Contributor

Choose a reason for hiding this comment

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

So that dpdk needs it means that the BuildRequires: in dpdk.spec should specify it. Did you get a failure to build dpdk without it? If you had a link to the failure that'd be awesome.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

BuildRequires: specifies /usr/bin/sphinx-build, however, zypper doesn't resolve filenames to packages to install.

Copy link
Contributor

Choose a reason for hiding this comment

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

The other approach is that even if you want to continue to use zypper for the bulk of this script, recognize that zypper is inferior (or we are just ignorant) in regards to installing build-dependencies and install and use dnf for that. Perhaps if we are more obvious about this short-coming (or our ignorance) in zypper, somebody with more knowledge of zypper that is using and looking at this script can find/suggest a better way.

But I think by simply adding python3-Sphinx as an exception to the package install list here we are making the issue much (much!) more subtle than if we were to use dnf with a nice big comment about why we are using it.

utils/build_rpms_from_scratch.sh Show resolved Hide resolved
utils/build_rpms_from_scratch.sh Show resolved Hide resolved
utils/build_rpms_from_scratch.sh Outdated Show resolved Hide resolved
utils/build_rpms_from_scratch.sh Outdated Show resolved Hide resolved
git clone https://github.com/daos-stack/"$repo_name".git
cd "$repo_name"
install_dependencies "$repo_name".spec
make rpms RPM_BUILD_OPTIONS="--nocheck" # --nocheck = don't run tests during rpm build process
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
make rpms RPM_BUILD_OPTIONS="--nocheck" # --nocheck = don't run tests during rpm build process
MAKEARGS=()
if [ -f Makefile-rpm.mk ]; then
MAKEARGS=(-f Makefile-rpm.mk)
fi
if [ -d utils/rpms ]; then
MAKEARGS+=(-C utils/rpms)
fi
make "${MAKEARGS[@]}" rpms RPM_BUILD_OPTIONS="--nocheck" # --nocheck = don't run tests during rpm build process

will let you use this for raft and daos also and be future-proof against any other project that utilizes these exceptions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's a little more complicated than this. build_and_copy_rpm also clones the repo and because raft is a submodule of daos, the daos repo must be cloned first, then raft rpms built, then daos rpms can be built. So a bit of a chicken and egg issue with the way I wrote build_and_copy_rpms. I appreciate the help trying to remove the redundancy, though. I've probably already spent too much time trying to figure out how to remove the duplicate code without making things too complicated.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's a little more complicated than this.

I don't think it is.

build_and_copy_rpm also clones the repo and because raft is a submodule of daos, the daos repo must be cloned first, then raft rpms built, then daos rpms can be built.

You can clone and build the raft repo independently of daos just like you do the other dozen repos. There is no need to clone daos and build raft out of the daos clone.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My concern is getting the correct raft commit that's actually used for daos. The 'fun' of it being a submodule is it doesn't have to be the tip of master. Right now it looks like the submodule daos uses is 1 commit behind the tip of master.

Doc-only: true

Signed-off-by: Ryon Jensen <ryon.jensen@intel.com>
Comment on lines 12 to 19
os: ["rockylinux:8.6",
"rockylinux:8.8",
"rockylinux:8.10",
"rockylinux:9.2",
"rockylinux:9.3",
"opensuse/leap:15.4",
"opensuse/leap:15.5",
"opensuse/leap:15.6"]
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if using our self-hosted runner and therefore our own docker hub/cache, you will find more of these older versions. @JohnMalmberg Would we have these old point releases in our docker runners?

Copy link
Contributor

Choose a reason for hiding this comment

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

On the docker runners we are only keeping rockylinux8 and 9 up to date, no dot releases.
We have opensuse/leap:15.[2-5] and opensuse/leap-dnf:15.[4-5]

Just updated so that soon they will have leap and leap-dnf 15.6 and Fedora:41

Copy link
Contributor

Choose a reason for hiding this comment

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

On the docker runners we are only keeping rockylinux8 and 9 up to date, no dot releases.

This is a good case why it would be useful if we were keeping the EL point releases in our docker hub.

Doc-only: true
Signed-off-by: Ryon Jensen <ryon.jensen@intel.com>
- DRY-ed some stuff
- Fixed use of wrong variable
- Fixed miss of directory change

Doc-only: true
Signed-off-by: Ryon Jensen <ryon.jensen@intel.com>
Doc-only: true
Signed-off-by: Ryon Jensen <ryon.jensen@intel.com>
- matching of el major version for Power Tools repo was wrong
- Added fail-fast: false (at least for now)

Doc-only: true
Signed-off-by: Ryon Jensen <ryon.jensen@intel.com>
- Not sure why I was using actions/checkout@v3 - bumped to v4
- Removed leap 15.6 for now. Having issues with it building
- A little reorganization
- Removed accidental add of file

Doc-only: true
Signed-off-by: Ryon Jensen <ryon.jensen@intel.com>

steps:
- name: Checkout code
uses: actions/checkout@v4

Check warning

Code scanning / Scorecard

Pinned-Dependencies Medium test

score is 1: GitHub-owned GitHubAction not pinned by hash
Click Remediation section below to solve this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants