Implementation for new systemd configuration method #365

SchSeba · 2022-09-29T14:16:31Z

Add a new configuration method called systemd mode.

With this new mode all the VFs are created before kubelet even starts this will allow the operator to have a much better recovery time after reboot.

it will also fix issues like kubernetes/kubernetes#109595

The downside of this solution and the reason why for now it will not be the "default" configuration mode is that every configuration change in sriov will require a node reboot.

After this PR is merged we will need to work on
#225
#226

to improve the configuration/reconfiguration/update time on large clusters

github-actions · 2022-09-29T14:16:44Z

Thanks for your PR,
To run vendors CIs use one of:

/test-all: To run all tests for all vendors.
/test-e2e-all: To run all E2E tests for all vendors.
/test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

/skip-all: To skip all tests for all vendors.
/skip-e2e-all: To skip all E2E tests for all vendors.
/skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
Best regards.

github-actions · 2022-09-29T14:23:45Z

Thanks for your PR,
To run vendors CIs use one of:

/test-all: To run all tests for all vendors.
/test-e2e-all: To run all E2E tests for all vendors.
/test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

/skip-all: To skip all tests for all vendors.
/skip-e2e-all: To skip all E2E tests for all vendors.
/skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
Best regards.

github-actions · 2022-09-29T14:32:18Z

Thanks for your PR,
To run vendors CIs use one of:

/test-all: To run all tests for all vendors.
/test-e2e-all: To run all E2E tests for all vendors.
/test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

/skip-all: To skip all tests for all vendors.
/skip-e2e-all: To skip all E2E tests for all vendors.
/skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
Best regards.

github-actions · 2022-09-29T14:32:57Z

Thanks for your PR,
To run vendors CIs use one of:

/test-all: To run all tests for all vendors.
/test-e2e-all: To run all E2E tests for all vendors.
/test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

/skip-all: To skip all tests for all vendors.
/skip-e2e-all: To skip all E2E tests for all vendors.
/skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
Best regards.

github-actions · 2022-09-29T14:38:59Z

Thanks for your PR,
To run vendors CIs use one of:

/test-all: To run all tests for all vendors.
/test-e2e-all: To run all E2E tests for all vendors.
/test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

/skip-all: To skip all tests for all vendors.
/skip-e2e-all: To skip all E2E tests for all vendors.
/skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
Best regards.

github-actions · 2022-09-29T14:39:58Z

Thanks for your PR,
To run vendors CIs use one of:

/test-all: To run all tests for all vendors.
/test-e2e-all: To run all E2E tests for all vendors.
/test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

/skip-all: To skip all tests for all vendors.
/skip-e2e-all: To skip all E2E tests for all vendors.
/skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
Best regards.

jjuele · 2022-10-28T14:16:16Z

Can one of the admins verify this patch?

github-actions · 2022-11-07T10:09:29Z

Thanks for your PR,
To run vendors CIs use one of:

/test-all: To run all tests for all vendors.
/test-e2e-all: To run all E2E tests for all vendors.
/test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

/skip-all: To skip all tests for all vendors.
/skip-e2e-all: To skip all E2E tests for all vendors.
/skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
Best regards.

github-actions · 2022-11-07T18:47:34Z

Thanks for your PR,
To run vendors CIs use one of:

/test-all: To run all tests for all vendors.
/test-e2e-all: To run all E2E tests for all vendors.
/test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

/skip-all: To skip all tests for all vendors.
/skip-e2e-all: To skip all E2E tests for all vendors.
/skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
Best regards.

cmd/sriov-network-config-daemon/service.go

pkg/plugins/k8s/k8s_plugin.go

controllers/sriovoperatorconfig_controller.go

cmd/sriov-network-config-daemon/start.go

bindata/manifests/siov-config-service/units/sriov-config-service.yaml

api/v1/sriovoperatorconfig_types.go

github-actions · 2022-12-01T14:50:17Z

Thanks for your PR,
To run vendors CIs use one of:

/test-all: To run all tests for all vendors.
/test-e2e-all: To run all E2E tests for all vendors.
/test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

/skip-all: To skip all tests for all vendors.
/skip-e2e-all: To skip all E2E tests for all vendors.
/skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
Best regards.

pkg/host/host.go

controllers/sriovoperatorconfig_controller.go

github-actions · 2022-12-01T15:50:56Z

Thanks for your PR,
To run vendors CIs use one of:

/test-all: To run all tests for all vendors.
/test-e2e-all: To run all E2E tests for all vendors.
/test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

/skip-all: To skip all tests for all vendors.
/skip-e2e-all: To skip all E2E tests for all vendors.
/skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
Best regards.

github-actions · 2022-12-01T16:40:55Z

Thanks for your PR,
To run vendors CIs use one of:

/test-all: To run all tests for all vendors.
/test-e2e-all: To run all E2E tests for all vendors.
/test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

/skip-all: To skip all tests for all vendors.
/skip-e2e-all: To skip all E2E tests for all vendors.
/skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
Best regards.

coveralls · 2022-12-01T17:46:51Z

Pull Request Test Coverage Report for Build 5322664195

138 of 997 (13.84%) changed or added relevant lines in 18 files are covered.
3 unchanged lines in 2 files lost coverage.
Overall coverage decreased (-1.5%) to 24.285%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
pkg/service/utils.go	0	1	0.0%
pkg/webhook/webhook.go	0	1	0.0%
pkg/daemon/writer.go	0	2	0.0%
api/v1/helper.go	1	4	25.0%
pkg/daemon/plugin.go	0	4	0.0%
pkg/plugins/generic/generic_plugin.go	3	11	27.27%
pkg/utils/utils.go	0	9	0.0%
pkg/plugins/virtual/virtual_plugin.go	0	10	0.0%
pkg/utils/utils_virtual.go	0	19	0.0%
pkg/utils/mock/mock_command.go	0	24	0.0%

Files with Coverage Reduction	New Missed Lines	%
pkg/plugins/k8s/k8s_plugin.go	1	0%
pkg/utils/utils.go	2	6.44%

Totals
Change from base Build 5268039487:	-1.5%
Covered Lines:	2056
Relevant Lines:	8466

💛 - Coveralls

SchSeba · 2023-02-19T14:59:05Z

waiting for #409

github-actions · 2023-02-22T12:52:35Z

Thanks for your PR,
To run vendors CIs use one of:

/test-all: To run all tests for all vendors.
/test-e2e-all: To run all E2E tests for all vendors.
/test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

/skip-all: To skip all tests for all vendors.
/skip-e2e-all: To skip all E2E tests for all vendors.
/skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
Best regards.

e0ne · 2023-03-10T14:03:40Z

api/v1/helper.go

+type ConfigurationModeType string
+
+const (
+	DaemonConfigurationMode  = "daemon"


Maybe it's better to create types consts here:

DaemonConfigurationMode ConfigurationModeType = "daemon" SystemdConfigurationMode ConfigurationModeType = "systemd"

e0ne · 2023-03-10T14:07:57Z

controllers/sriovoperatorconfig_controller.go

@@ -299,7 +319,7 @@ func (r *SriovOperatorConfigReconciler) deleteK8sResource(ctx context.Context, i

 func (r *SriovOperatorConfigReconciler) syncK8sResource(ctx context.Context, cr *sriovnetworkv1.SriovOperatorConfig, in *uns.Unstructured) error {
 	switch in.GetKind() {
-	case "ClusterRole", "ClusterRoleBinding", "MutatingWebhookConfiguration", "ValidatingWebhookConfiguration":
+	case "ClusterRole", "ClusterRoleBinding", "MutatingWebhookConfiguration", "ValidatingWebhookConfiguration", machineConfigCRDName:


nit: let's move all CRDs name into constant

e0ne · 2023-03-10T14:12:09Z

bindata/manifests/daemon/daemonset.yaml

+          command:
+            - /bin/bash
+            - -c
+            - mkdir -p /host/var/lib/sriov/ && cp /usr/bin/sriov-network-config-daemon /host/var/lib/sriov/sriov-network-config-daemon && chcon -t bin_t /host/var/lib/sriov/sriov-network-config-daemon | true # Allow systemd to run the file, use pipe true to not failed if the system doesn't have selinux enabled


selinux => selinux or apparmor

e0ne · 2023-03-10T14:18:16Z

pkg/host/host.go

+	}
+
+	if mlx4Err != nil && mlx5Err != nil {
+		glog.Errorf("tryEnableRdma(): no RDMA capable devices")


What about Intel devices? will this check work for them?

e0ne · 2023-03-10T14:21:47Z

pkg/host/host.go

+		return h.enableRDMAOnUbuntuMachine()
+	}
+
+	osName, err := h.getOSPrettyName()


line 167 returns result if isRhelSystem
link 176 returns result if isUbuntuSystem

what other OSs are supported to get into this code?

@SchSeba ^^

e0ne · 2023-03-10T14:25:19Z

pkg/utils/command.go

@@ -0,0 +1,41 @@
+/*
+Copyright 2021.


e0ne · 2023-03-10T14:27:21Z

pkg/service/utils.go

@@ -5,6 +5,7 @@ import (
 	"strings"


nit: it would be good to have more detailed commit message

github-actions · 2023-05-02T09:26:17Z

Thanks for your PR,
To run vendors CIs use one of:

/test-all: To run all tests for all vendors.
/test-e2e-all: To run all E2E tests for all vendors.
/test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

/skip-all: To skip all tests for all vendors.
/skip-e2e-all: To skip all E2E tests for all vendors.
/skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
Best regards.

main.go

tariq1890 · 2023-05-30T15:58:59Z

bindata/manifests/sriov-config-service/kubernetes/sriov-config-service.yaml

+  [Install]
+  WantedBy=multi-user.target
+enabled: true
+name: sriov-config.service


Just thinking out loud, would we want to add templatisation for dependent services? Is it possible that some users may want to add services in the After= field based on their host OS setup?

That is a great Idea the question is how we will expose that template to the user?

we definately need to have this configurable. Let's think about thi in a follow-up PR

github-actions · 2023-06-12T08:28:24Z

Thanks for your PR,
To run vendors CIs use one of:

/test-all: To run all tests for all vendors.
/test-e2e-all: To run all E2E tests for all vendors.
/test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

/skip-all: To skip all tests for all vendors.
/skip-e2e-all: To skip all E2E tests for all vendors.
/skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
Best regards.

github-actions · 2023-06-13T13:08:05Z

Thanks for your PR,
To run vendors CIs use one of:

/test-all: To run all tests for all vendors.
/test-e2e-all: To run all E2E tests for all vendors.
/test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

/skip-all: To skip all tests for all vendors.
/skip-e2e-all: To skip all E2E tests for all vendors.
/skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
Best regards.

Signed-off-by: Sebastian Sch <sebassch@gmail.com>

This is needed so we can static link everything in the binary Signed-off-by: Sebastian Sch <sebassch@gmail.com>

Signed-off-by: Sebastian Sch <sebassch@gmail.com>

github-actions · 2023-06-20T12:41:46Z

Thanks for your PR,
To run vendors CIs use one of:

/test-all: To run all tests for all vendors.
/test-e2e-all: To run all E2E tests for all vendors.
/test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

/skip-all: To skip all tests for all vendors.
/skip-e2e-all: To skip all E2E tests for all vendors.
/skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
Best regards.

e0ne

Added few comments and nits which could be addressed in a new revision of this PR or in a follow up PR.

I tested these changes on the vanilla k8s only

e0ne · 2023-07-04T08:43:14Z

main.go

@@ -69,6 +69,7 @@ func main() {
 	var metricsAddr string
 	var enableLeaderElection bool
 	var probeAddr string
+


nit: this change is not needed

e0ne · 2023-07-04T08:43:18Z

main.go

@@ -82,6 +83,7 @@ func main() {
 	ctrl.SetLogger(zap.New(zap.UseFlagOptions(&opts)))

 	restConfig := ctrl.GetConfigOrDie()
+


nit: this change is not needed

e0ne · 2023-07-04T08:43:40Z

test/conformance/tests/test_sriov_operator.go

@@ -36,7 +36,7 @@ import (
 	"github.com/k8snetworkplumbingwg/sriov-network-operator/test/util/pod"
 )

-var waitingTime time.Duration = 20 * time.Minute
+var waitingTime = 20 * time.Minute


nit: this change is not needed

e0ne · 2023-07-04T08:48:01Z

bindata/manifests/sriov-config-service/kubernetes/sriov-config-service.yaml

+  [Install]
+  WantedBy=multi-user.target
+enabled: true
+name: sriov-config.service


we definately need to have this configurable. Let's think about thi in a follow-up PR

e0ne · 2023-07-04T08:52:54Z

pkg/utils/sriov.go

+			return i, nil
+		}
+	}
+	return sriovnetworkv1.Interface{}, fmt.Errorf("unable to find interface: %v", name)


why don't you return 'nil, err' here?

e0ne · 2023-07-04T08:54:25Z

pkg/host/host.go

+		return h.enableRDMAOnUbuntuMachine()
+	}
+
+	osName, err := h.getOSPrettyName()


@SchSeba ^^

SchSeba force-pushed the systemd branch from 328532c to dd7cb7d Compare September 29, 2022 14:23

SchSeba force-pushed the systemd branch from dd7cb7d to 67439ab Compare September 29, 2022 14:32

SchSeba force-pushed the systemd branch from 67439ab to 90f23a8 Compare September 29, 2022 14:32

SchSeba force-pushed the systemd branch from 82ee5f0 to b280fb8 Compare September 29, 2022 14:39

SchSeba force-pushed the systemd branch from 695525f to bf81bdc Compare November 7, 2022 18:47

bn222 reviewed Nov 21, 2022

View reviewed changes

cmd/sriov-network-config-daemon/service.go Outdated Show resolved Hide resolved

bn222 reviewed Nov 21, 2022

View reviewed changes

pkg/plugins/k8s/k8s_plugin.go Show resolved Hide resolved

bn222 reviewed Nov 21, 2022

View reviewed changes

controllers/sriovoperatorconfig_controller.go Show resolved Hide resolved

bn222 reviewed Nov 21, 2022

View reviewed changes

cmd/sriov-network-config-daemon/start.go Outdated Show resolved Hide resolved

lmilleri reviewed Nov 23, 2022

View reviewed changes

bindata/manifests/siov-config-service/units/sriov-config-service.yaml Outdated Show resolved Hide resolved

lmilleri reviewed Nov 23, 2022

View reviewed changes

api/v1/sriovoperatorconfig_types.go Outdated Show resolved Hide resolved

SchSeba force-pushed the systemd branch from bf81bdc to 9abedfa Compare December 1, 2022 14:50

SchSeba changed the title ~~WIP: support new systemd method~~ Implementation for new systemd configuration method Dec 1, 2022

bn222 reviewed Dec 1, 2022

View reviewed changes

pkg/host/host.go Outdated Show resolved Hide resolved

bn222 reviewed Dec 1, 2022

View reviewed changes

pkg/host/host.go Show resolved Hide resolved

bn222 reviewed Dec 1, 2022

View reviewed changes

controllers/sriovoperatorconfig_controller.go Show resolved Hide resolved

SchSeba force-pushed the systemd branch from d308398 to 376ff8b Compare December 1, 2022 16:40

SchSeba force-pushed the systemd branch from 63490b2 to 0755d24 Compare February 22, 2023 12:52

e0ne reviewed Mar 10, 2023

View reviewed changes

SchSeba force-pushed the systemd branch from 0755d24 to 8e7676c Compare May 2, 2023 09:26

github-advanced-security bot found potential problems May 2, 2023

View reviewed changes

main.go Fixed Show fixed Hide fixed

tariq1890 reviewed May 30, 2023

View reviewed changes

adrianchiris mentioned this pull request May 31, 2023

Release SRIOV Network Operator V1.3.0 #450

Closed

6 tasks

SchSeba force-pushed the systemd branch from 8e7676c to 221046b Compare June 12, 2023 08:28

SchSeba force-pushed the systemd branch from 221046b to 9ff8d34 Compare June 13, 2023 13:07

SchSeba added 8 commits June 20, 2023 15:34

Add gomock binary download and makefile entry to generate mocks

a519556

Signed-off-by: Sebastian Sch <sebassch@gmail.com>

enable CGO

e78cae4

This is needed so we can static link everything in the binary Signed-off-by: Sebastian Sch <sebassch@gmail.com>

Create the new api for ConfigurationMode and update the helper functions

e440c7a

Signed-off-by: Sebastian Sch <sebassch@gmail.com>

Create the systemd changes for the bindata yaml folder

f72f2a0

Signed-off-by: Sebastian Sch <sebassch@gmail.com>

Update controllers

727022c

Signed-off-by: Sebastian Sch <sebassch@gmail.com>

Update utils and create interface for better unit tests implementation

2adccca

Signed-off-by: Sebastian Sch <sebassch@gmail.com>

switch the bash files implementation with golang configuration

cfb0e3d

Signed-off-by: Sebastian Sch <sebassch@gmail.com>

Create the new systemd cmd and implementation changes in the daemon pkg

4e0b661

Signed-off-by: Sebastian Sch <sebassch@gmail.com>

SchSeba force-pushed the systemd branch from 9ff8d34 to 4e0b661 Compare June 20, 2023 12:41

e0ne approved these changes Jul 4, 2023

View reviewed changes

e0ne merged commit 680654f into k8snetworkplumbingwg:master Jul 5, 2023

e0ne mentioned this pull request Jul 5, 2023

SR-IOV systemd configuration improvements #465

Closed

4 tasks

lmilleri mentioned this pull request Jul 12, 2023

SRIOV/vDPA configuration no longer works #471

Closed

zeeke mentioned this pull request Oct 31, 2023

OCPBUGS-22709: [release-4.13] virtual: get real PCI address for each device found openshift/sriov-network-operator#852

Merged

		@@ -82,6 +83,7 @@ func main() {
		ctrl.SetLogger(zap.New(zap.UseFlagOptions(&opts)))

		restConfig := ctrl.GetConfigOrDie()

Implementation for new systemd configuration method #365

Implementation for new systemd configuration method #365

Conversation

SchSeba commented Sep 29, 2022 • edited Loading

github-actions bot commented Sep 29, 2022

github-actions bot commented Sep 29, 2022

github-actions bot commented Sep 29, 2022

github-actions bot commented Sep 29, 2022

github-actions bot commented Sep 29, 2022

github-actions bot commented Sep 29, 2022

jjuele commented Oct 28, 2022

github-actions bot commented Nov 7, 2022

github-actions bot commented Nov 7, 2022

github-actions bot commented Dec 1, 2022

github-actions bot commented Dec 1, 2022

github-actions bot commented Dec 1, 2022

coveralls commented Dec 1, 2022 • edited Loading

Pull Request Test Coverage Report for Build 5322664195

💛 - Coveralls

SchSeba commented Feb 19, 2023

github-actions bot commented Feb 22, 2023

e0ne Mar 10, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented May 2, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jun 12, 2023

github-actions bot commented Jun 13, 2023

github-actions bot commented Jun 20, 2023

e0ne left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SchSeba commented Sep 29, 2022 •

edited

Loading

coveralls commented Dec 1, 2022 •

edited

Loading

e0ne Mar 10, 2023 •

edited

Loading