From 9e3ad0a8b32cc3243e549d073c48bffb65a24ad5 Mon Sep 17 00:00:00 2001 From: Andrea Panattoni Date: Mon, 21 Aug 2023 12:33:55 +0200 Subject: [PATCH 1/2] SriovNetwork: react on namespace creation 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 --- controllers/sriovnetwork_controller.go | 34 +++++++++++++- controllers/sriovnetwork_controller_test.go | 50 +++++++++++++++++++++ controllers/suite_test.go | 12 ++++- main.go | 4 ++ 4 files changed, 98 insertions(+), 2 deletions(-) diff --git a/controllers/sriovnetwork_controller.go b/controllers/sriovnetwork_controller.go index 588b8b5f3..72fd3077c 100644 --- a/controllers/sriovnetwork_controller.go +++ b/controllers/sriovnetwork_controller.go @@ -21,13 +21,15 @@ import ( "reflect" netattdefv1 "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" - + "k8s.io/client-go/util/workqueue" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/event" "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/reconcile" @@ -140,6 +142,13 @@ func (r *SriovNetworkReconciler) Reconcile(ctx context.Context, req ctrl.Request err = r.Get(ctx, types.NamespacedName{Name: netAttDef.Name, Namespace: netAttDef.Namespace}, found) if err != nil { if errors.IsNotFound(err) { + targetNamespace := &corev1.Namespace{} + err = r.Get(ctx, types.NamespacedName{Name: netAttDef.Namespace}, targetNamespace) + if errors.IsNotFound(err) { + reqLogger.Info("Target namespace doesn't exist, NetworkAttachmentDefinition will be created when namespace is available", "Namespace", netAttDef.Namespace, "Name", netAttDef.Name) + return reconcile.Result{}, nil + } + reqLogger.Info("NetworkAttachmentDefinition CR not exist, creating") err = r.Create(ctx, netAttDef) if err != nil { @@ -173,8 +182,31 @@ func (r *SriovNetworkReconciler) Reconcile(ctx context.Context, req ctrl.Request // SetupWithManager sets up the controller with the Manager. func (r *SriovNetworkReconciler) SetupWithManager(mgr ctrl.Manager) error { + // Reconcile when the target namespace is created after the SriovNetwork object. + namespaceHandler := handler.Funcs{ + CreateFunc: func(ctx context.Context, e event.CreateEvent, q workqueue.RateLimitingInterface) { + networkList := sriovnetworkv1.SriovNetworkList{} + err := r.List(ctx, + &networkList, + client.MatchingFields{"spec.networkNamespace": e.Object.GetName()}, + ) + if err != nil { + log.Log.WithName("SriovNetworkReconciler"). + Info("Can't list SriovNetworks for namespace", "resource", e.Object.GetName(), "error", err) + } + + for _, network := range networkList.Items { + q.Add(reconcile.Request{NamespacedName: types.NamespacedName{ + Namespace: network.Namespace, + Name: network.Name, + }}) + } + }, + } + return ctrl.NewControllerManagedBy(mgr). For(&sriovnetworkv1.SriovNetwork{}). Watches(&netattdefv1.NetworkAttachmentDefinition{}, &handler.EnqueueRequestForObject{}). + Watches(&corev1.Namespace{}, &namespaceHandler). Complete(r) } diff --git a/controllers/sriovnetwork_controller_test.go b/controllers/sriovnetwork_controller_test.go index 7c2aab258..e4123135e 100644 --- a/controllers/sriovnetwork_controller_test.go +++ b/controllers/sriovnetwork_controller_test.go @@ -8,7 +8,9 @@ import ( "time" netattdefv1 "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/util/retry" dynclient "sigs.k8s.io/controller-runtime/pkg/client" @@ -214,6 +216,54 @@ var _ = Describe("SriovNetwork Controller", func() { Expect(err).NotTo(HaveOccurred()) }) }) + + Context("When the target NetworkNamespace doesn't exists", func() { + It("should create the NetAttachDef when the namespace is created", func() { + cr := sriovnetworkv1.SriovNetwork{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-missing-namespace", + Namespace: testNamespace, + }, + Spec: sriovnetworkv1.SriovNetworkSpec{ + NetworkNamespace: "ns-xxx", + ResourceName: "resource_missing_namespace", + IPAM: `{"type":"dhcp"}`, + Vlan: 200, + }, + } + var err error + expect := generateExpectedNetConfig(&cr) + + err = k8sClient.Create(goctx.TODO(), &cr) + Expect(err).NotTo(HaveOccurred()) + + DeferCleanup(func() { + err = k8sClient.Delete(goctx.TODO(), &cr) + Expect(err).NotTo(HaveOccurred()) + }) + + // Sleep 3 seconds to be sure the Reconcile loop has been invoked. This can be improved by exposing some information (e.g. the error) + // in the SriovNetwork.Status field. + time.Sleep(3 * time.Second) + + netAttDef := &netattdefv1.NetworkAttachmentDefinition{} + err = k8sClient.Get(ctx, types.NamespacedName{Name: cr.GetName(), Namespace: "ns-xxx"}, netAttDef) + Expect(err).To(HaveOccurred()) + + err = k8sClient.Create(goctx.TODO(), &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{Name: "ns-xxx"}, + }) + Expect(err).NotTo(HaveOccurred()) + + err = util.WaitForNamespacedObject(netAttDef, k8sClient, "ns-xxx", cr.GetName(), util.RetryInterval, util.Timeout) + Expect(err).NotTo(HaveOccurred()) + + anno := netAttDef.GetAnnotations() + Expect(anno["k8s.v1.cni.cncf.io/resourceName"]).To(Equal("openshift.io/" + cr.Spec.ResourceName)) + Expect(strings.TrimSpace(netAttDef.Spec.Config)).To(Equal(expect)) + }) + }) + }) }) diff --git a/controllers/suite_test.go b/controllers/suite_test.go index 3d534695f..74b38a2e9 100644 --- a/controllers/suite_test.go +++ b/controllers/suite_test.go @@ -28,6 +28,7 @@ import ( . "github.com/onsi/gomega" openshiftconfigv1 "github.com/openshift/api/config/v1" mcfgv1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1" + "go.uber.org/zap/zapcore" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -64,7 +65,12 @@ const ( ) var _ = BeforeSuite(func(done Done) { - logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true))) + logf.SetLogger(zap.New( + zap.WriteTo(GinkgoWriter), + zap.UseDevMode(true), + func(o *zap.Options) { + o.TimeEncoder = zapcore.RFC3339NanoTimeEncoder + })) // Go to project root directory os.Chdir("..") @@ -103,6 +109,10 @@ var _ = BeforeSuite(func(done Done) { }) Expect(err).ToNot(HaveOccurred()) + k8sManager.GetCache().IndexField(context.Background(), &sriovnetworkv1.SriovNetwork{}, "spec.networkNamespace", func(o client.Object) []string { + return []string{o.(*sriovnetworkv1.SriovNetwork).Spec.NetworkNamespace} + }) + err = (&SriovNetworkReconciler{ Client: k8sManager.GetClient(), Scheme: k8sManager.GetScheme(), diff --git a/main.go b/main.go index fed5ef862..41d3a2bb1 100644 --- a/main.go +++ b/main.go @@ -126,6 +126,10 @@ func main() { os.Exit(1) } + mgrGlobal.GetCache().IndexField(context.Background(), &sriovnetworkv1.SriovNetwork{}, "spec.networkNamespace", func(o client.Object) []string { + return []string{o.(*sriovnetworkv1.SriovNetwork).Spec.NetworkNamespace} + }) + if err := initNicIDMap(); err != nil { setupLog.Error(err, "unable to init NicIdMap") os.Exit(1) From 66a50a16d29becad955c7659e862eec1cc3fb8a5 Mon Sep 17 00:00:00 2001 From: Andrea Panattoni Date: Thu, 24 Aug 2023 12:04:58 +0200 Subject: [PATCH 2/2] SriovIBetwork: react on namespace creation Apply the same namespace reaction as the SriovNetwork Signed-off-by: Andrea Panattoni --- controllers/sriovibnetwork_controller.go | 33 ++++++++++++- controllers/sriovibnetwork_controller_test.go | 47 +++++++++++++++++++ controllers/suite_test.go | 4 ++ main.go | 4 ++ 4 files changed, 87 insertions(+), 1 deletion(-) diff --git a/controllers/sriovibnetwork_controller.go b/controllers/sriovibnetwork_controller.go index b67bdec35..86caf92fe 100644 --- a/controllers/sriovibnetwork_controller.go +++ b/controllers/sriovibnetwork_controller.go @@ -21,12 +21,15 @@ import ( "reflect" netattdefv1 "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/util/workqueue" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/event" "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/reconcile" @@ -136,10 +139,16 @@ func (r *SriovIBNetworkReconciler) Reconcile(ctx context.Context, req ctrl.Reque } // Check if this NetworkAttachmentDefinition already exists found := &netattdefv1.NetworkAttachmentDefinition{} - err = r.Get(ctx, types.NamespacedName{Name: netAttDef.Name, Namespace: netAttDef.Namespace}, found) if err != nil { if errors.IsNotFound(err) { + targetNamespace := &corev1.Namespace{} + err = r.Get(ctx, types.NamespacedName{Name: netAttDef.Namespace}, targetNamespace) + if errors.IsNotFound(err) { + reqLogger.Info("Target namespace doesn't exist, NetworkAttachmentDefinition will be created when namespace is available", "Namespace", netAttDef.Namespace, "Name", netAttDef.Name) + return reconcile.Result{}, nil + } + reqLogger.Info("NetworkAttachmentDefinition CR not exist, creating") err = r.Create(ctx, netAttDef) if err != nil { @@ -172,8 +181,30 @@ func (r *SriovIBNetworkReconciler) Reconcile(ctx context.Context, req ctrl.Reque // SetupWithManager sets up the controller with the Manager. func (r *SriovIBNetworkReconciler) SetupWithManager(mgr ctrl.Manager) error { + // Reconcile when the target namespace is created after the SriovNetwork object. + namespaceHandler := handler.Funcs{ + CreateFunc: func(ctx context.Context, e event.CreateEvent, q workqueue.RateLimitingInterface) { + ibNetworkList := sriovnetworkv1.SriovIBNetworkList{} + err := r.List(ctx, + &ibNetworkList, + client.MatchingFields{"spec.networkNamespace": e.Object.GetName()}, + ) + if err != nil { + log.Log.WithName("SriovIBNetworkReconciler"). + Info("Can't list SriovIBNetworkReconciler for namespace", "resource", e.Object.GetName(), "error", err) + } + + for _, network := range ibNetworkList.Items { + q.Add(reconcile.Request{NamespacedName: types.NamespacedName{ + Namespace: network.Namespace, + Name: network.Name, + }}) + } + }, + } return ctrl.NewControllerManagedBy(mgr). For(&sriovnetworkv1.SriovIBNetwork{}). Watches(&netattdefv1.NetworkAttachmentDefinition{}, &handler.EnqueueRequestForObject{}). + Watches(&corev1.Namespace{}, &namespaceHandler). Complete(r) } diff --git a/controllers/sriovibnetwork_controller_test.go b/controllers/sriovibnetwork_controller_test.go index 5ee52c856..fa676c369 100644 --- a/controllers/sriovibnetwork_controller_test.go +++ b/controllers/sriovibnetwork_controller_test.go @@ -8,6 +8,7 @@ import ( "time" netattdefv1 "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/util/retry" @@ -189,6 +190,52 @@ var _ = Describe("SriovIBNetwork Controller", func() { Expect(err).NotTo(HaveOccurred()) }) }) + + Context("When the target NetworkNamespace doesn't exists", func() { + It("should create the NetAttachDef when the namespace is created", func() { + cr := sriovnetworkv1.SriovIBNetwork{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-missing-namespace", + Namespace: testNamespace, + }, + Spec: sriovnetworkv1.SriovIBNetworkSpec{ + NetworkNamespace: "ib-ns-xxx", + ResourceName: "resource_missing_namespace", + IPAM: `{"type":"dhcp"}`, + }, + } + var err error + expect := generateExpectedIBNetConfig(&cr) + + err = k8sClient.Create(goctx.TODO(), &cr) + Expect(err).NotTo(HaveOccurred()) + + DeferCleanup(func() { + err = k8sClient.Delete(goctx.TODO(), &cr) + Expect(err).NotTo(HaveOccurred()) + }) + + // Sleep 3 seconds to be sure the Reconcile loop has been invoked. This can be improved by exposing some information (e.g. the error) + // in the SriovIBNetwork.Status field. + time.Sleep(3 * time.Second) + + netAttDef := &netattdefv1.NetworkAttachmentDefinition{} + err = k8sClient.Get(ctx, types.NamespacedName{Name: cr.GetName(), Namespace: "ib-ns-xxx"}, netAttDef) + Expect(err).To(HaveOccurred()) + + err = k8sClient.Create(goctx.TODO(), &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{Name: "ib-ns-xxx"}, + }) + Expect(err).NotTo(HaveOccurred()) + + err = util.WaitForNamespacedObject(netAttDef, k8sClient, "ib-ns-xxx", cr.GetName(), util.RetryInterval, util.Timeout) + Expect(err).NotTo(HaveOccurred()) + + anno := netAttDef.GetAnnotations() + Expect(anno["k8s.v1.cni.cncf.io/resourceName"]).To(Equal("openshift.io/" + cr.Spec.ResourceName)) + Expect(strings.TrimSpace(netAttDef.Spec.Config)).To(Equal(expect)) + }) + }) }) func generateExpectedIBNetConfig(cr *sriovnetworkv1.SriovIBNetwork) string { diff --git a/controllers/suite_test.go b/controllers/suite_test.go index 74b38a2e9..c3eda1a90 100644 --- a/controllers/suite_test.go +++ b/controllers/suite_test.go @@ -113,6 +113,10 @@ var _ = BeforeSuite(func(done Done) { return []string{o.(*sriovnetworkv1.SriovNetwork).Spec.NetworkNamespace} }) + k8sManager.GetCache().IndexField(context.Background(), &sriovnetworkv1.SriovIBNetwork{}, "spec.networkNamespace", func(o client.Object) []string { + return []string{o.(*sriovnetworkv1.SriovIBNetwork).Spec.NetworkNamespace} + }) + err = (&SriovNetworkReconciler{ Client: k8sManager.GetClient(), Scheme: k8sManager.GetScheme(), diff --git a/main.go b/main.go index 41d3a2bb1..c8225b54f 100644 --- a/main.go +++ b/main.go @@ -130,6 +130,10 @@ func main() { return []string{o.(*sriovnetworkv1.SriovNetwork).Spec.NetworkNamespace} }) + mgrGlobal.GetCache().IndexField(context.Background(), &sriovnetworkv1.SriovIBNetwork{}, "spec.networkNamespace", func(o client.Object) []string { + return []string{o.(*sriovnetworkv1.SriovIBNetwork).Spec.NetworkNamespace} + }) + if err := initNicIDMap(); err != nil { setupLog.Error(err, "unable to init NicIdMap") os.Exit(1)