From b39233d554eb07a507b39d7adb60b8c3b994b53c Mon Sep 17 00:00:00 2001 From: Andrea Panattoni Date: Thu, 24 Aug 2023 12:04:58 +0200 Subject: [PATCH 1/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 | 8 ++++ main.go | 8 ++++ 4 files changed, 95 insertions(+), 1 deletion(-) diff --git a/controllers/sriovibnetwork_controller.go b/controllers/sriovibnetwork_controller.go index 8fee236e1..f301090f6 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" @@ -131,10 +134,16 @@ func (r *SriovIBNetworkReconciler) Reconcile(ctx context.Context, req ctrl.Reque } // Check if this NetworkAttachmentDefinition already exists found := &netattdefv1.NetworkAttachmentDefinition{} - err = r.Get(context.TODO(), 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(context.TODO(), netAttDef) if err != nil { @@ -167,8 +176,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(e event.CreateEvent, q workqueue.RateLimitingInterface) { + ibNetworkList := sriovnetworkv1.SriovIBNetworkList{} + err := r.List(context.Background(), + &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(&source.Kind{Type: &netattdefv1.NetworkAttachmentDefinition{}}, &handler.EnqueueRequestForObject{}). + Watches(&source.Kind{Type: &corev1.Namespace{}}, &namespaceHandler). Complete(r) } diff --git a/controllers/sriovibnetwork_controller_test.go b/controllers/sriovibnetwork_controller_test.go index 4351124d8..9ef5c7a8a 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" @@ -190,4 +191,50 @@ 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 := util.GenerateExpectedIBNetConfig(&cr) + + err = k8sClient.Create(goctx.TODO(), &cr) + Expect(err).NotTo(HaveOccurred()) + + defer 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)) + }) + }) }) diff --git a/controllers/suite_test.go b/controllers/suite_test.go index 33f13eba6..aec7532bd 100644 --- a/controllers/suite_test.go +++ b/controllers/suite_test.go @@ -103,6 +103,14 @@ 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} + }) + + 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 b1e73501f..f33b3f37a 100644 --- a/main.go +++ b/main.go @@ -124,6 +124,14 @@ 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} + }) + + 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) From aaaab92fd54978d269eb0dd044b8c9e4d9406927 Mon Sep 17 00:00:00 2001 From: Andrea Panattoni Date: Mon, 21 Aug 2023 12:33:55 +0200 Subject: [PATCH 2/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 | 8 +++- 3 files changed, 90 insertions(+), 2 deletions(-) diff --git a/controllers/sriovnetwork_controller.go b/controllers/sriovnetwork_controller.go index 9d3d25d6f..726a4d74a 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" @@ -135,6 +137,13 @@ func (r *SriovNetworkReconciler) Reconcile(ctx context.Context, req ctrl.Request err = r.Get(context.TODO(), 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(context.TODO(), netAttDef) if err != nil { @@ -168,8 +177,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(e event.CreateEvent, q workqueue.RateLimitingInterface) { + networkList := sriovnetworkv1.SriovNetworkList{} + err := r.List(context.Background(), + &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(&source.Kind{Type: &netattdefv1.NetworkAttachmentDefinition{}}, &handler.EnqueueRequestForObject{}). + Watches(&source.Kind{Type: &corev1.Namespace{}}, &namespaceHandler). Complete(r) } diff --git a/controllers/sriovnetwork_controller_test.go b/controllers/sriovnetwork_controller_test.go index 81b57ca00..2b5067510 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" @@ -212,5 +214,53 @@ 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 := util.GenerateExpectedNetConfig(&cr) + + err = k8sClient.Create(goctx.TODO(), &cr) + Expect(err).NotTo(HaveOccurred()) + + defer 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 aec7532bd..eb37b7fbb 100644 --- a/controllers/suite_test.go +++ b/controllers/suite_test.go @@ -29,6 +29,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" @@ -66,7 +67,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("..")