Adds SRIOV srpm/s and container image/s to suse-edge/Factory (EDGE-1535) #282

Merged
antaloala merged 16 commits from antaloala/Factory:edge-1535 into main 2025-10-27 08:50:43 +01:00
Owner

This PR is to include several commits, one per each of the following SRIOV packaging artifacts:
(deeper indentation used in some lines below tries to show which images are created from which srpm:s)

  • [srpm] mstflint (required in the sriov-network-operator-config-daemon image, not provided/available by/from BCL)

  • [srpm] sriov-network-operator -------------------> [suse-edge/Factory folder:] sriov-network-operator/

    • [image] sriov-network-manager --------------> [suse-edge/Factory folder:] sriov-network-operator-manager-image/
    • [image] sriov-network-config-daemon -------> [suse-edge/Factory folder:] sriov-network-operator-config-daemon-image/
    • [image] sriov-network-webhook --------------> [suse-edge/Factory folder:] sriov-network-operator-webhook-image/
  • [srpm] sriov-cni -----------------------------------> [suse-edge/Factory folder:] sriov-cni/

    • [image] sriov-cni -----------------------------> [suse-edge/Factory folder:] sriov-cni-image/
  • [srpm] ib-sriov-cni -------------------------------> [suse-edge/Factory folder:] ib-sriov-cni/

    • [image] ib-sriov-cni -------------------------> [suse-edge/Factory folder:] ib-sriov-cni-image/
  • [srpm] sriov-network-device-plugin -------------> [suse-edge/Factory folder:] sriov-network-device-plugin/

    • [image] sriov-network-device-plugin -------> [suse-edge/Factory folder:] sriov-network-device-plugin-image
  • [srpm] network-resources-injector --------------> [suse-edge/Factory folder:] network-resources-injector/

    • [image] network-resources-injector --------> [suse-edge/Factory folder:] network-resources-injector-image
  • [srpm] node-features-discovery --------------> [suse-edge/Factory folder:] node-features-discovery/

    • [image] node-features-discovery --------> [suse-edge/Factory folder:] node-features-discovery-image

A final commit to update the default values in the (already existing in suse-edge/Factory) sriov-network-operator Helm chart (to point to the new images); this commit updating also the default values in the sriov-nfd subchart.

Commits are being pushed to this ongoing PR once having assured the involved artifact is properly built (OBS server builds through a parallel OBS home project's subproject created, holding all these artifacts/packages: https://build.opensuse.org/project/show/home:antaloala:sriov-test)

This PR is to include several commits, one per each of the following SRIOV packaging artifacts: (deeper indentation used in some lines below tries to show which images are created from which srpm:s) - [srpm] `mstflint` (required in the `sriov-network-operator-config-daemon` image, not provided/available by/from BCL) - [srpm] `sriov-network-operator` -------------------> [suse-edge/Factory folder:] `sriov-network-operator/` - - [image] `sriov-network-manager` --------------> [suse-edge/Factory folder:] `sriov-network-operator-manager-image/` - - [image] `sriov-network-config-daemon` -------> [suse-edge/Factory folder:] `sriov-network-operator-config-daemon-image/` - - [image] `sriov-network-webhook` --------------> [suse-edge/Factory folder:] `sriov-network-operator-webhook-image/` - [srpm] `sriov-cni` -----------------------------------> [suse-edge/Factory folder:] `sriov-cni/` - - [image] `sriov-cni` -----------------------------> [suse-edge/Factory folder:] `sriov-cni-image/` - [srpm] `ib-sriov-cni` -------------------------------> [suse-edge/Factory folder:] `ib-sriov-cni/` - - [image] `ib-sriov-cni` -------------------------> [suse-edge/Factory folder:] `ib-sriov-cni-image/` - [srpm] `sriov-network-device-plugin` -------------> [suse-edge/Factory folder:] `sriov-network-device-plugin/` - - [image] `sriov-network-device-plugin` -------> [suse-edge/Factory folder:] `sriov-network-device-plugin-image` - [srpm] `network-resources-injector` --------------> [suse-edge/Factory folder:] `network-resources-injector/` - - [image] `network-resources-injector` --------> [suse-edge/Factory folder:] `network-resources-injector-image` - [srpm] `node-features-discovery` --------------> [suse-edge/Factory folder:] `node-features-discovery/` - - [image] `node-features-discovery` --------> [suse-edge/Factory folder:] `node-features-discovery-image` A final commit to update the default values in the (already existing in suse-edge/Factory) `sriov-network-operator` Helm chart (to point to the new images); this commit updating also the default values in the `sriov-nfd` subchart. Commits are being pushed to this ongoing PR once having assured the involved artifact is properly built (OBS server builds through a parallel OBS home project's subproject created, holding all these artifacts/packages: https://build.opensuse.org/project/show/home:antaloala:sriov-test)
antaloala requested review from amorgante 2025-10-03 12:48:01 +02:00
antaloala requested review from nbelouin 2025-10-03 12:48:01 +02:00
antaloala requested review from steven.hardy 2025-10-03 12:48:01 +02:00
nbelouin requested changes 2025-10-20 14:18:26 +02:00
Dismissed
nbelouin left a comment
Owner

Some of the comments are applying to all the specfiles.

Also don't forget to bump the chart version to reflect the fact we made a change.

Some of the comments are applying to all the specfiles. Also don't forget to bump the chart version to reflect the fact we made a change.
@@ -0,0 +42,4 @@
%build
%define cgoenabled "0"
%define gotags "no_openssl"
Owner

Can you add a comment that these two are set like this upstream. It feels weird otherwise to define them like this

Can you add a comment that these two are set like this upstream. It feels weird otherwise to define them like this
nbelouin marked this conversation as resolved
@@ -0,0 +50,4 @@
%install
install -D -m0755 ib-sriov %{buildroot}%{_bindir}/ib-sriov
install -D -m0755 images/entrypoint.sh %{buildroot}/entrypoint.sh
Owner

%{_libexecdir}/ib-sriov-cni/entrypoint.sh would make more sense imho, and then in Dockerfile use /usr/libexec/... as entrypoint.

`%{_libexecdir}/ib-sriov-cni/entrypoint.sh` would make more sense imho, and then in Dockerfile use `/usr/libexec/...` as entrypoint.
Owner

Discussed offline, would deviate too much from upstream Dockerfiles and make maintenance more difficult

Discussed offline, would deviate too much from upstream Dockerfiles and make maintenance more difficult
nbelouin marked this conversation as resolved
@@ -0,0 +1,91 @@
#
Owner

If this is verbatim from openSUSE Factory or better from SLFO, then we usually do a git submodule to link to it.

If this is verbatim from openSUSE Factory or better from SLFO, then we usually do a git submodule to link to it.
Author
Owner

When deciding (with Steven) how to handle this missing "mstflint" rpm (rancher/ecm colleagues take it -during the image generation- from a centOS repo) we finally agreed to better create our own rpm (in suse-edge/Factory) and so avoiding any dependency with BCL ... under the assumption this mstflint was not raising other dependencies to take care of (which, fortunately, is not the case.
Note the mstflint package from openSUSE:Factory looks to be some kind of "unmaintained" (it is based on upstream version 1.25.00-1 which is 2 years older than latest 1.33.00-1 I am now moving to as part of EDGE-1637)

When deciding (with Steven) how to handle this missing "mstflint" rpm (rancher/ecm colleagues take it -during the image generation- from a centOS repo) we finally agreed to better create our own rpm (in suse-edge/Factory) and so avoiding any dependency with BCL ... under the assumption this mstflint was not raising other dependencies to take care of (which, fortunately, is not the case. Note the mstflint package from openSUSE:Factory looks to be some kind of "unmaintained" (it is based on upstream version 1.25.00-1 which is 2 years older than latest 1.33.00-1 I am now moving to as part of EDGE-1637)
Owner

It looks like it is actually included in SLES https://build.opensuse.org/package/show/SUSE:SLE-15-SP6:GA/mstflint - I don't see a version for SP7, but can we use that instead?

It looks like it is actually included in SLES https://build.opensuse.org/package/show/SUSE:SLE-15-SP6:GA/mstflint - I don't see a version for SP7, but can we use that instead?
Owner

Looking at SCC we can see that version has been inherited by SP7 https://scc.suse.com/packages/37997463

Looking at SCC we can see that version has been inherited by SP7 https://scc.suse.com/packages/37997463
Author
Owner

Then, if we all agree, we can (re)use that existing mstflint rpm and so avoiding to have to build (and keep) our own one.
I am then removing the mstflint package (folder) from this PR.

Then, if we all agree, we can (re)use that existing mstflint rpm and so avoiding to have to build (and keep) our own one. I am then removing the mstflint package (folder) from this PR.
Owner

+1 on re-using it, if it looks like it's unmaintained and we want it updated we can have a discussion with BCL or do it ourself, but let's postpone that to when we need it

+1 on re-using it, if it looks like it's unmaintained and we want it updated we can have a discussion with BCL or do it ourself, but let's postpone that to when we need it
@@ -0,0 +6,4 @@
FROM registry.suse.com/bci/bci-base:$SLE_VERSION AS base
COPY --from=micro / /installroot/
RUN if [ "$(uname -m)" != "s390x" ]; then \
Owner

I'd keep this one simple and just don't care about s390x as we don't support that architecture

I'd keep this one simple and just don't care about s390x as we don't support that architecture
nbelouin marked this conversation as resolved
@@ -0,0 +94,4 @@
%files common
%license LICENSE
%doc README.md
/bindata
Owner

A pattern that can be used for this use-case is to use %{datadir}/sriov-network-operator and have a WORKDIR statement in the Dockerfile pointing to that directory.

A pattern that can be used for this use-case is to use `%{datadir}/sriov-network-operator` and have a `WORKDIR` statement in the Dockerfile pointing to that directory.
Owner

As the other one, would deviate too much from upstream Dockerfile then, and make it harder to maintain.

As the other one, would deviate too much from upstream Dockerfile then, and make it harder to maintain.
nbelouin marked this conversation as resolved
antaloala requested review from nbelouin 2025-10-21 19:24:13 +02:00
antaloala changed title from WIP: Adds SRIOV srpm/s and container image/s to suse-edge/Factory (EDGE-1535) to Adds SRIOV srpm/s and container image/s to suse-edge/Factory (EDGE-1535) 2025-10-24 10:51:58 +02:00
nbelouin approved these changes 2025-10-24 11:00:11 +02:00
nbelouin left a comment
Owner

LGTM, just a nit about commits, I'm happy with the 1 commit per package thing, but can you squash those additional fix commits into their respective package commit, to make it easier to read the commit history.
If you want to add details about the choices we made in the commits please do it in the commits body (the titles are already long enough 😅)

LGTM, just a nit about commits, I'm happy with the 1 commit per package thing, but can you squash those additional fix commits into their respective package commit, to make it easier to read the commit history. If you want to add details about the choices we made in the commits please do it in the commits body (the titles are already long enough 😅)
antaloala added 1 commit 2025-10-26 20:45:52 +01:00
Updates release_images.yaml to point to the new sriov-related images and release_manifest.yaml to point to the bumped (in the semver patch field) sriov-network-operator-chart
All checks were successful
Check Release Manifest Local Charts Versions / Check Release Manifest Local Charts Versions (pull_request) Successful in 8s
Build PR in OBS / Build PR in OBS (pull_request_target) Successful in 57s
c4aefbf455
antaloala merged commit ba703821b1 into main 2025-10-27 08:50:43 +01:00
antaloala deleted branch edge-1535 2025-10-27 08:50:43 +01:00
Sign in to join this conversation.
No Label
3 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: suse-edge/Factory#282