SriovNetwork: react on namespace creation #493

zeeke · 2023-08-21T11:10:32Z

If a user creates an SriovNetwork with a network namespace that doesn't exist, retrying reconciling with an exponential backoff is not efficient, as the routing will fail until the namespace is created.

This patch makes the controller watch Namespace resource creation and reconcile the related SriovNetworks if needed.

Before this PR, the SriovNetwork reconcile function returned an error, triggering the exponential back-off retry system:

2023-08-11T16:14:18.839626394Z  ERROR   Couldn't create NetworkAttachmentDefinition CR  {"controller": "sriovnetwork", "controllerGroup": "sriovnetwork.openshift.io", "controllerKind": "SriovNetwork", "SriovNetwork": {"name":"test-missing-ns","namespace":"openshift-sriov-network-operator"}, "namespace": "openshift-sriov-network-operator", "name": "test-missing-ns", "reconcileID": "9783b6b4-b193-4d78-9371-be4f3487d68b", "sriovnetwork": "openshift-sriov-network-operator/test-missing-ns", "Namespace": "ns-xxx", "Name": "test-missing-ns", "error": "namespaces \"ns-xxx\" not found"}
2023-08-11T16:14:18.905244077Z  ERROR   Couldn't create NetworkAttachmentDefinition CR  {"controller": "sriovnetwork", "controllerGroup": "sriovnetwork.openshift.io", "controllerKind": "SriovNetwork", "SriovNetwork": {"name":"test-missing-ns","namespace":"openshift-sriov-network-operator"}, "namespace": "openshift-sriov-network-operator", "name": "test-missing-ns", "reconcileID": "fccf9f74-f963-47f4-b672-d5099eb40484", "sriovnetwork": "openshift-sriov-network-operator/test-missing-ns", "Namespace": "ns-xxx", "Name": "test-missing-ns", "error": "namespaces \"ns-xxx\" not found"}
2023-08-11T16:14:18.970988294Z  ERROR   Couldn't create NetworkAttachmentDefinition CR  {"controller": "sriovnetwork", "controllerGroup": "sriovnetwork.openshift.io", "controllerKind": "SriovNetwork", "SriovNetwork": {"name":"test-missing-ns","namespace":"openshift-sriov-network-operator"}, "namespace": "openshift-sriov-network-operator", "name": "test-missing-ns", "reconcileID": "5c790fc5-ca13-4283-b0a8-22753d0ccb89", "sriovnetwork": "openshift-sriov-network-operator/test-missing-ns", "Namespace": "ns-xxx", "Name": "test-missing-ns", "error": "namespaces \"ns-xxx\" not found"}
2023-08-11T16:14:19.047349515Z  ERROR   Couldn't create NetworkAttachmentDefinition CR  {"controller": "sriovnetwork", "controllerGroup": "sriovnetwork.openshift.io", "controllerKind": "SriovNetwork", "SriovNetwork": {"name":"test-missing-ns","namespace":"openshift-sriov-network-operator"}, "namespace": "openshift-sriov-network-operator", "name": "test-missing-ns", "reconcileID": "388e00a0-dfe3-4b01-afef-b2ec0b62a675", "sriovnetwork": "openshift-sriov-network-operator/test-missing-ns", "Namespace": "ns-xxx", "Name": "test-missing-ns", "error": "namespaces \"ns-xxx\" not found"}
2023-08-11T16:14:19.142652361Z  ERROR   Couldn't create NetworkAttachmentDefinition CR  {"controller": "sriovnetwork", "controllerGroup": "sriovnetwork.openshift.io", "controllerKind": "SriovNetwork", "SriovNetwork": {"name":"test-missing-ns","namespace":"openshift-sriov-network-operator"}, "namespace": "openshift-sriov-network-operator", "name": "test-missing-ns", "reconcileID": "dd8a06d4-08d8-479f-b5c4-b29e2d07a6b9", "sriovnetwork": "openshift-sriov-network-operator/test-missing-ns", "Namespace": "ns-xxx", "Name": "test-missing-ns", "error": "namespaces \"ns-xxx\" not found"}
2023-08-11T16:14:19.278729554Z  ERROR   Couldn't create NetworkAttachmentDefinition CR  {"controller": "sriovnetwork", "controllerGroup": "sriovnetwork.openshift.io", "controllerKind": "SriovNetwork", "SriovNetwork": {"name":"test-missing-ns","namespace":"openshift-sriov-network-operator"}, "namespace": "openshift-sriov-network-operator", "name": "test-missing-ns", "reconcileID": "8267549b-ec6c-4370-a162-c4b88987c2aa", "sriovnetwork": "openshift-sriov-network-operator/test-missing-ns", "Namespace": "ns-xxx", "Name": "test-missing-ns", "error": "namespaces \"ns-xxx\" not found"}
2023-08-11T16:14:19.496675162Z  ERROR   Couldn't create NetworkAttachmentDefinition CR  {"controller": "sriovnetwork", "controllerGroup": "sriovnetwork.openshift.io", "controllerKind": "SriovNetwork", "SriovNetwork": {"name":"test-missing-ns","namespace":"openshift-sriov-network-operator"}, "namespace": "openshift-sriov-network-operator", "name": "test-missing-ns", "reconcileID": "7764a8ea-fd2e-4704-82b1-902f066e2cda", "sriovnetwork": "openshift-sriov-network-operator/test-missing-ns", "Namespace": "ns-xxx", "Name": "test-missing-ns", "error": "namespaces \"ns-xxx\" not found"}
2023-08-11T16:14:19.87228528Z   ERROR   Couldn't create NetworkAttachmentDefinition CR  {"controller": "sriovnetwork", "controllerGroup": "sriovnetwork.openshift.io", "controllerKind": "SriovNetwork", "SriovNetwork": {"name":"test-missing-ns","namespace":"openshift-sriov-network-operator"}, "namespace": "openshift-sriov-network-operator", "name": "test-missing-ns", "reconcileID": "5c111316-1313-470c-9e95-3637f5386bd1", "sriovnetwork": "openshift-sriov-network-operator/test-missing-ns", "Namespace": "ns-xxx", "Name": "test-missing-ns", "error": "namespaces \"ns-xxx\" not found"}
2023-08-11T16:14:20.569108499Z  ERROR   Couldn't create NetworkAttachmentDefinition CR  {"controller": "sriovnetwork", "controllerGroup": "sriovnetwork.openshift.io", "controllerKind": "SriovNetwork", "SriovNetwork": {"name":"test-missing-ns","namespace":"openshift-sriov-network-operator"}, "namespace": "openshift-sriov-network-operator", "name": "test-missing-ns", "reconcileID": "ac4bfd17-af0e-4421-8ec7-b583ce7c2831", "sriovnetwork": "openshift-sriov-network-operator/test-missing-ns", "Namespace": "ns-xxx", "Name": "test-missing-ns", "error": "namespaces \"ns-xxx\" not found"}
2023-08-11T16:14:21.905604308Z  ERROR   Couldn't create NetworkAttachmentDefinition CR  {"controller": "sriovnetwork", "controllerGroup": "sriovnetwork.openshift.io", "controllerKind": "SriovNetwork", "SriovNetwork": {"name":"test-missing-ns","namespace":"openshift-sriov-network-operator"}, "namespace": "openshift-sriov-network-operator", "name": "test-missing-ns", "reconcileID": "c63c700a-537d-4fc4-b64d-6b4dba8f7a4e", "sriovnetwork": "openshift-sriov-network-operator/test-missing-ns", "Namespace": "ns-xxx", "Name": "test-missing-ns", "error": "namespaces \"ns-xxx\" not found"}

github-actions · 2023-08-21T11:10: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 · 2023-08-21T11:14:36Z

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 · 2023-08-21T11:23:25Z

Pull Request Test Coverage Report for Build 5962355735

45 of 51 (88.24%) changed or added relevant lines in 2 files are covered.
2 unchanged lines in 1 file lost coverage.
Overall coverage increased (+0.4%) to 25.078%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
controllers/sriovibnetwork_controller.go	23	26	88.46%
controllers/sriovnetwork_controller.go	22	25	88.0%

Files with Coverage Reduction	New Missed Lines	%
controllers/sriovibnetwork_controller.go	2	70.23%

Totals
Change from base Build 5949253686:	0.4%
Covered Lines:	2158
Relevant Lines:	8605

💛 - Coveralls

e0ne

LGTM

adrianchiris

overall LGTM !

few small nits

@zeeke you mind adding the same logic for sriovIbNetwork controller ? i believe we have the same problem there as well.

if you dont have BW for it, can you create an issue to track ?

controllers/sriovnetwork_controller.go

adrianchiris · 2023-08-22T13:36:05Z

controllers/sriovnetwork_controller.go

 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+


nit: could you remove spaces here and below ?

controllers/sriovnetwork_controller.go

github-actions · 2023-08-24T10:09:27Z

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.

zeeke · 2023-08-24T10:11:37Z

@zeeke you mind adding the same logic for sriovIbNetwork controller ? i believe we have the same problem there as well.

Done, please take a look

github-actions · 2023-08-24T10:20:04Z

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.

SchSeba

LGTM

nice work!

I learn about the index one it's nice!

adrianchiris

LGTM, just need to rebase this one and we are good 2 go 🚗

adrianchiris · 2023-09-06T07:36:23Z

@zeeke you mind adding the same logic for sriovIbNetwork controller ? i believe we have the same problem there as well.

Done, please take a look

Awesome Thanks !

If a user creates an SriovNetwork with a network namespace that doesn't exist, retrying reconciling with an exponential backoff is not efficient, as the routing will fail until the namespace is created. This patch makes the controller watch Namespace resource creation and reconcile the related SriovNetworks if needed. Signed-off-by: Andrea Panattoni <apanatto@redhat.com>

Apply the same namespace reaction as the SriovNetwork Signed-off-by: Andrea Panattoni <apanatto@redhat.com>

github-actions · 2023-09-06T08:38: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.

controllers/sriovnetwork_controller.go

zeeke marked this pull request as ready for review August 21, 2023 11:10

zeeke force-pushed the ocpbugs-8266 branch from 35c2cbf to e8ea99e Compare August 21, 2023 11:14

e0ne approved these changes Aug 22, 2023

View reviewed changes

adrianchiris reviewed Aug 22, 2023

View reviewed changes

zeeke force-pushed the ocpbugs-8266 branch from e8ea99e to 7cf4195 Compare August 24, 2023 10:09

zeeke force-pushed the ocpbugs-8266 branch from 7cf4195 to 026e539 Compare August 24, 2023 10:19

SchSeba approved these changes Aug 27, 2023

View reviewed changes

adrianchiris approved these changes Sep 6, 2023

View reviewed changes

zeeke added 2 commits September 6, 2023 10:36

SriovIBetwork: react on namespace creation

66a50a1

Apply the same namespace reaction as the SriovNetwork Signed-off-by: Andrea Panattoni <apanatto@redhat.com>

zeeke force-pushed the ocpbugs-8266 branch from 026e539 to 66a50a1 Compare September 6, 2023 08:38

SchSeba reviewed Sep 13, 2023

View reviewed changes

controllers/sriovnetwork_controller.go Show resolved Hide resolved

SchSeba merged commit 08badf6 into k8snetworkplumbingwg:master Sep 14, 2023
9 checks passed

zeeke mentioned this pull request Sep 20, 2023

OCPBUGS-19440: [release-4.14]: SriovNetwork, SriovIBNetwork, react on namespace creation openshift/sriov-network-operator#830

Merged

zeeke mentioned this pull request Nov 8, 2023

OCPBUGS-23059: [release-4.13]: SriovNetwork, SriovIBNetwork, react on namespace creation openshift/sriov-network-operator#860

Merged

zeeke mentioned this pull request Dec 22, 2023

OCPBUGS-25854: [release-4.12]: SriovNetwork, SriovIBNetwork, react on namespace creation openshift/sriov-network-operator#877

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SriovNetwork: react on namespace creation #493

SriovNetwork: react on namespace creation #493

zeeke commented Aug 21, 2023

github-actions bot commented Aug 21, 2023

github-actions bot commented Aug 21, 2023

coveralls commented Aug 21, 2023 •

edited

Loading

e0ne left a comment

adrianchiris left a comment

adrianchiris Aug 22, 2023

github-actions bot commented Aug 24, 2023

zeeke commented Aug 24, 2023

github-actions bot commented Aug 24, 2023

SchSeba left a comment

adrianchiris left a comment

adrianchiris commented Sep 6, 2023

github-actions bot commented Sep 6, 2023

		"k8s.io/apimachinery/pkg/api/errors"
		metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

SriovNetwork: react on namespace creation #493

SriovNetwork: react on namespace creation #493

Conversation

zeeke commented Aug 21, 2023

github-actions bot commented Aug 21, 2023

github-actions bot commented Aug 21, 2023

coveralls commented Aug 21, 2023 • edited Loading

Pull Request Test Coverage Report for Build 5962355735

💛 - Coveralls

e0ne left a comment

Choose a reason for hiding this comment

adrianchiris left a comment

Choose a reason for hiding this comment

adrianchiris Aug 22, 2023

Choose a reason for hiding this comment

github-actions bot commented Aug 24, 2023

zeeke commented Aug 24, 2023

github-actions bot commented Aug 24, 2023

SchSeba left a comment

Choose a reason for hiding this comment

adrianchiris left a comment

Choose a reason for hiding this comment

adrianchiris commented Sep 6, 2023

github-actions bot commented Sep 6, 2023

coveralls commented Aug 21, 2023 •

edited

Loading