From 6da2f62db65956fc39bd6d15cdabeef52c38d607 Mon Sep 17 00:00:00 2001 From: Andrea Panattoni Date: Mon, 21 Aug 2023 12:33:55 +0200 Subject: [PATCH] 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 9f28d4d78..cc2275cdd 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(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 { @@ -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 613b15cc9..9d1093dd4 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" @@ -211,5 +213,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 := 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 c052b3ada..83d2b6970 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("..")