From 654c29506f162544048700a578024aeb831e3d87 Mon Sep 17 00:00:00 2001 From: Arnau Verdaguer Date: Tue, 30 Jul 2024 18:06:11 +0200 Subject: [PATCH] [ovn-controller] Don't create ovn-controller ds if no NicMappings set Resolves: OSPRH-7463 --- controllers/ovncontroller_controller.go | 115 ++++++++++++++++++ .../ovncontroller_controller_test.go | 42 ++++++- 2 files changed, 154 insertions(+), 3 deletions(-) diff --git a/controllers/ovncontroller_controller.go b/controllers/ovncontroller_controller.go index 2a5d1035..d44950e7 100644 --- a/controllers/ovncontroller_controller.go +++ b/controllers/ovncontroller_controller.go @@ -313,6 +313,65 @@ func (r *OVNControllerReconciler) reconcileUpgrade(ctx context.Context) (ctrl.Re return ctrl.Result{}, nil } +func getDaemonSetWithName( + ctx context.Context, + h *helper.Helper, + name string, + namespace string, +) (*appsv1.DaemonSet, error) { + + dset := &appsv1.DaemonSet{} + err := h.GetClient().Get(ctx, types.NamespacedName{Name: name, Namespace: namespace}, dset) + if err != nil { + return dset, err + } + + return dset, nil +} + +func (r *OVNControllerReconciler) ensureCMDeleted( + ctx context.Context, + h *helper.Helper, + instance *ovnv1.OVNController, +) error { + // Delete, if they exist, ovn and ovs ds + cms := []string{"ovncontroller-config", "ovncontroller-scripts"} + for _, cmName := range cms { + err := r.deleteConfigMaps(ctx, h, cmName, instance.Namespace) + if err != nil && !k8s_errors.IsNotFound(err) { + return fmt.Errorf("could not delete config map %v: %w", cmName, err) + } + + } + return nil +} + +func ensureDSDeleted( + ctx context.Context, + h *helper.Helper, + instance *ovnv1.OVNController, +) error { + // Delete, if they exist, ovn and ovs ds + daemonsets := []string{"ovn-controller", "ovn-controller-ovs"} + for _, dsName := range daemonsets { + ds, err := getDaemonSetWithName(ctx, h, dsName, instance.Namespace) + if err != nil && !k8s_errors.IsNotFound(err) { + return fmt.Errorf("error getting %s daemonset: %w", dsName, err) + } else if err != nil && k8s_errors.IsNotFound(err) { + // Already deleted + continue + } + + // Delete ds + err = h.GetClient().Delete(ctx, ds) + if err != nil && !k8s_errors.IsNotFound(err) { + return fmt.Errorf("error deleting daemonset %s: %w", ds.Name, err) + } + } + + return nil +} + func (r *OVNControllerReconciler) reconcileNormal(ctx context.Context, instance *ovnv1.OVNController, helper *helper.Helper) (ctrl.Result, error) { Log := r.GetLogger(ctx) @@ -339,6 +398,39 @@ func (r *OVNControllerReconciler) reconcileNormal(ctx context.Context, instance return rbacResult, nil } + // if ovn-controller has no nicMappings, it's useless create daemonset on + // OC nodes, since it won't do anything. + if len(instance.Spec.NicMappings) == 0 { + // Check if DS are created, if so delete the resources associated with it + // Resources associated with ovn-controller: + // - ovncontroller-scripts [cm] + // - ovncontroller-config [cm] + // - ovn-controller [ds] + // - ovn-controller-ovs [ds] + // - job [it gets deleted once it's done, skip] + err := ensureDSDeleted(ctx, helper, instance) + if err != nil { + return ctrl.Result{}, err + } + + err = r.ensureCMDeleted(ctx, helper, instance) + if err != nil { + return ctrl.Result{}, err + } + + // Set conditions to true as no more work is needed. + Log.Info("DEBUG: No nicMappings provided, skipping creation of DS") + instance.Status.Conditions.MarkTrue(condition.NetworkAttachmentsReadyCondition, condition.NetworkAttachmentsReadyMessage) + instance.Status.Conditions.MarkTrue(condition.InputReadyCondition, condition.InputReadyMessage) + instance.Status.Conditions.MarkTrue(condition.ServiceConfigReadyCondition, condition.ServiceConfigReadyMessage) + instance.Status.Conditions.MarkTrue(condition.NetworkAttachmentsReadyCondition, condition.NetworkAttachmentsReadyMessage) + instance.Status.Conditions.MarkTrue(condition.DeploymentReadyCondition, condition.DeploymentReadyMessage) + instance.Status.Conditions.MarkTrue(condition.TLSInputReadyCondition, condition.InputReadyMessage) + return ctrl.Result{}, nil + } else { + Log.Info(fmt.Sprintf("DEBUG ARNAU: NicMappings not empty: %v, continuing normal", instance.Spec.NicMappings)) + } + // ConfigMap configMapVars := make(map[string]env.Setter) @@ -676,6 +768,29 @@ func (r *OVNControllerReconciler) reconcileNormal(ctx context.Context, instance return ctrl.Result{}, nil } +// generateServiceConfigMaps - create configmaps which hold scripts and service configuration +func (r *OVNControllerReconciler) deleteConfigMaps( + ctx context.Context, + h *helper.Helper, + name string, + namespace string, +) error { + serviceCM := &corev1.ConfigMap{} + err := h.GetClient().Get(ctx, types.NamespacedName{Name: name, Namespace: namespace}, serviceCM) + if err != nil && !k8s_errors.IsNotFound(err) { + return err + } else if k8s_errors.IsNotFound(err) { + return nil + } + + err = h.GetClient().Delete(ctx, serviceCM) + if err != nil && !k8s_errors.IsNotFound(err) { + return fmt.Errorf("error deleting config map %s: %w", serviceCM.Name, err) + } + + return nil +} + // generateServiceConfigMaps - create configmaps which hold scripts and service configuration func (r *OVNControllerReconciler) generateServiceConfigMaps( ctx context.Context, diff --git a/tests/functional/ovncontroller_controller_test.go b/tests/functional/ovncontroller_controller_test.go index ba321325..cb175608 100644 --- a/tests/functional/ovncontroller_controller_test.go +++ b/tests/functional/ovncontroller_controller_test.go @@ -35,12 +35,19 @@ import ( "k8s.io/apimachinery/pkg/types" ) +// GINKGO_ARGS="--focus 'OVNController controller'" make test var _ = Describe("OVNController controller", func() { + // GINKGO_ARGS="--focus 'OVNController controller when A OVNController instance is created'" make test When("A OVNController instance is created", func() { var OVNControllerName types.NamespacedName BeforeEach(func() { - instance := CreateOVNController(namespace, GetDefaultOVNControllerSpec()) + specWithNicMappings := GetDefaultOVNControllerSpec() + + nicMappings := make(map[string]string) + nicMappings["examplebr"] = "examplebr" + specWithNicMappings.OVNControllerSpecCore.NicMappings = nicMappings + instance := CreateOVNController(namespace, specWithNicMappings) OVNControllerName = types.NamespacedName{Name: instance.GetName(), Namespace: instance.GetNamespace()} DeferCleanup(th.DeleteInstance, instance) }) @@ -125,6 +132,7 @@ var _ = Describe("OVNController controller", func() { ) }) + // GINKGO_ARGS="--focus 'OVNController controller when A OVNController instance is created when OVNDBCluster instances are available without networkAttachments'" make test When("OVNDBCluster instances are available without networkAttachments", func() { var scriptsCM types.NamespacedName var dbs []types.NamespacedName @@ -204,6 +212,7 @@ var _ = Describe("OVNController controller", func() { }) }) + // GINKGO_ARGS="--focus 'OVNController controller when A OVNController instance is created when OVNDBCluster instances with networkAttachments are available'" make test When("OVNDBCluster instances with networkAttachments are available", func() { var configCM types.NamespacedName var daemonSetName types.NamespacedName @@ -274,6 +283,7 @@ var _ = Describe("OVNController controller", func() { }) }) + // GINKGO_ARGS="--focus 'OVNController controller when OVNController and OVNDBClusters are created with networkAttachments'" make test When("OVNController and OVNDBClusters are created with networkAttachments", func() { var OVNControllerName types.NamespacedName var dbs []types.NamespacedName @@ -288,6 +298,9 @@ var _ = Describe("OVNController controller", func() { } spec := GetDefaultOVNControllerSpec() spec.NetworkAttachment = "internalapi" + nicMappings := make(map[string]string) + nicMappings["foo-phynet"] = "foo-bridge" + spec.NicMappings = nicMappings instance := CreateOVNController(namespace, spec) OVNControllerName = types.NamespacedName{Name: instance.GetName(), Namespace: instance.GetNamespace()} DeferCleanup(th.DeleteInstance, instance) @@ -341,12 +354,18 @@ var _ = Describe("OVNController controller", func() { expectedAnnotation, err := json.Marshal( []networkv1.NetworkSelectionElement{ + { + Name: "foo-phynet", + Namespace: namespace, + InterfaceRequest: "foo-phynet", + }, { Name: "internalapi", Namespace: namespace, InterfaceRequest: "internalapi", }, - }) + }, + ) Expect(err).ShouldNot(HaveOccurred()) Expect(ds.Spec.Template.ObjectMeta.Annotations).To( HaveKeyWithValue("k8s.v1.cni.cncf.io/networks", string(expectedAnnotation)), @@ -384,6 +403,11 @@ var _ = Describe("OVNController controller", func() { expectedAnnotation, err := json.Marshal( []networkv1.NetworkSelectionElement{ + { + Name: "foo-phynet", + Namespace: namespace, + InterfaceRequest: "foo-phynet", + }, { Name: "internalapi", Namespace: namespace, @@ -578,6 +602,7 @@ var _ = Describe("OVNController controller", func() { }) }) + // GINKGO_ARGS="--focus 'OVNController controller when OVNController is created with missing networkAttachment'" make test When("OVNController is created with missing networkAttachment", func() { var OVNControllerName types.NamespacedName var dbs []types.NamespacedName @@ -589,6 +614,9 @@ var _ = Describe("OVNController controller", func() { } spec := GetDefaultOVNControllerSpec() spec.NetworkAttachment = "internalapi" + spec.NicMappings = map[string]string{ + "physnet1": "enp2s0.100", + } instance := CreateOVNController(namespace, spec) OVNControllerName = types.NamespacedName{Name: instance.GetName(), Namespace: instance.GetNamespace()} DeferCleanup(th.DeleteInstance, instance) @@ -606,6 +634,7 @@ var _ = Describe("OVNController controller", func() { }) }) + // GINKGO_ARGS="--focus 'OVNController controller when OVNController is created with nic configs'" make test When("OVNController is created with nic configs", func() { var OVNControllerName types.NamespacedName BeforeEach(func() { @@ -762,6 +791,7 @@ var _ = Describe("OVNController controller", func() { }) }) + // GINKGO_ARGS="--focus 'OVNController controller when OVNController is created with networkAttachment and nic configs'" make test When("OVNController is created with networkAttachment and nic configs", func() { BeforeEach(func() { dbs := CreateOVNDBClusters(namespace, map[string][]string{}, 1) @@ -813,6 +843,7 @@ var _ = Describe("OVNController controller", func() { }) }) + // GINKGO_ARGS="--focus 'OVNController controller when OVNController is created with empty spec'" make test When("OVNController is created with empty spec", func() { var ovnControllerName types.NamespacedName @@ -834,13 +865,18 @@ var _ = Describe("OVNController controller", func() { }) }) + // GINKGO_ARGS="--focus 'OVNController controller when OVNController is created with TLS'" make test When("OVNController is created with TLS", func() { var ovnControllerName types.NamespacedName BeforeEach(func() { dbs := CreateOVNDBClusters(namespace, map[string][]string{}, 1) DeferCleanup(DeleteOVNDBClusters, dbs) - instance := CreateOVNController(namespace, GetTLSOVNControllerSpec()) + specWithNicMappings := GetTLSOVNControllerSpec() + nicMappings := make(map[string]string) + nicMappings["examplebr"] = "examplebr" + specWithNicMappings.OVNControllerSpecCore.NicMappings = nicMappings + instance := CreateOVNController(namespace, specWithNicMappings) DeferCleanup(th.DeleteInstance, instance) ovnControllerName = types.NamespacedName{Name: instance.GetName(), Namespace: instance.GetNamespace()}