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 <[email protected]>
zeeke · Nov 8, 2023 · a1027f8 · a1027f8
1 parent edbcb7a
commit a1027f8
Show file tree

Hide file tree

Showing 3 changed files with 90 additions and 2 deletions.
diff --git 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
@@ -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 := util.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
@@ -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("..")