Skip to content

Commit

Permalink
[ovn-controller] Don't create ovn-controller ds if no NicMappings set
Browse files Browse the repository at this point in the history 
Resolves: OSPRH-7463
  • Loading branch information
@averdagu
averdagu committed Aug 7, 2024
1 parent 65f8da3 commit 356465e
Show file tree
Hide file tree
Showing 3 changed files with 167 additions and 8 deletions.
114 changes: 114 additions & 0 deletions controllers/ovncontroller_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -339,6 +398,38 @@ 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
}
Log.Info(fmt.Sprintf("DEBUG ARNAU: NicMappings not empty: %v, continuing normal", instance.Spec.NicMappings))

// ConfigMap
configMapVars := make(map[string]env.Setter)

Expand Down Expand Up @@ -676,6 +767,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,
Expand Down
42 changes: 39 additions & 3 deletions tests/functional/ovncontroller_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -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)),
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand All @@ -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() {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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

Expand All @@ -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()}
Expand Down
19 changes: 14 additions & 5 deletions tests/kuttl/common/assert_sample_deployment.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -315,12 +315,21 @@ commands:
num_pods_ovn=$(oc get pods -n $NAMESPACE -l service=ovn-controller -o name --field-selector=status.phase=Running | wc -l)
num_pods_ovs=$(oc get pods -n $NAMESPACE -l service=ovn-controller-ovs -o name --field-selector=status.phase=Running | wc -l)
num_pods=$(( num_pods_ovn + num_pods_ovs))
# for each nodes, two pods are spawned - ovn-controller and ovn-controller-ovs
if [ "$((num_nodes * 2))" -ne "$num_pods" ]; then
echo "Cluster has $num_nodes nodes but OVNController spawned $num_pods pods, it should have $num_nodes * 2"
exit 1
nic_mappings_set=$(oc get ovncontroller ovncontroller-sample -o json | jq '.spec | has("nicMappings")')
# if Spec.NicMappings is empty number should be 0, else should be equal to num_nodes.
if [ $nic_mappings_set == "true" ]; then
# for each nodes, two pods are spawned - ovn-controller and ovn-controller-ovs
if [ "$((num_nodes * 2))" -ne "$num_pods" ]; then
echo "Cluster has $num_nodes nodes but OVNController spawned $num_pods pods, it should have $num_nodes * 2"
exit 1
fi
else
# nicMappings is not set, it should be zero regardless of num_nodes
if [ "0" -ne "$num_pods" ]; then
echo "Cluster has $num_nodes nodes but OVNController spawned $num_pods pods, it should have $num_nodes * 2"
exit 1
fi
fi
tupleTemplate='{{ range (index .spec.template.spec.containers 1).env }}{{ .name }}{{ "#" }}{{ .value}}{{"\n"}}{{ end }}'
imageTuples=$(oc get -n openstack-operators deployment ovn-operator-controller-manager -o go-template="$tupleTemplate")
for ITEM in $(echo $imageTuples); do
Expand Down

0 comments on commit 356465e

Please sign in to comment.