Skip to content

Commit

Permalink
Merge pull request #153 from karelyatin/fix-ovn-configuration
Browse files Browse the repository at this point in the history 
Create External CM only when DBClusters have NetworkAttachment
  • Loading branch information
@openshift-merge-bot
openshift-merge-bot[bot] authored Nov 17, 2023
2 parents e5276dc + b2f3970 commit 48dbc8d
Show file tree
Hide file tree
Showing 5 changed files with 118 additions and 80 deletions.
2 changes: 1 addition & 1 deletion api/v1beta1/ovndbcluster_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ func (instance OVNDBCluster) GetInternalEndpoint() (string, error) {
}

func (instance OVNDBCluster) GetExternalEndpoint() (string, error) {
if instance.Status.DBAddress == "" {
if instance.Spec.NetworkAttachment != "" && instance.Status.DBAddress == "" {
return "", fmt.Errorf("external DBEndpoint not ready yet for %s", instance.Spec.DBType)
}
return instance.Status.DBAddress, nil
Expand Down
18 changes: 10 additions & 8 deletions controllers/ovncontroller_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -455,8 +455,8 @@ func (r *OVNControllerReconciler) reconcileNormal(ctx context.Context, instance
return ctrl.Result{}, nil
}

_, err = sbCluster.GetExternalEndpoint()
if err != nil {
ep, err := sbCluster.GetExternalEndpoint()
if err != nil || ep == "" {
Log.Info("No external endpoint defined for SB OVNDBCluster, deleting external ConfigMap")
cleanupConfigMapErr := r.deleteExternalConfigMaps(ctx, helper, instance)
if cleanupConfigMapErr != nil {
Expand All @@ -466,12 +466,14 @@ func (r *OVNControllerReconciler) reconcileNormal(ctx context.Context, instance
return ctrl.Result{}, nil
}

// Create ConfigMap for external dataplane consumption
// TODO(ihar) - is there any hashing mechanism for EDP config? do we trigger deploy somehow?
err = r.generateExternalConfigMaps(ctx, helper, instance, sbCluster, &configMapVars)
if err != nil {
Log.Error(err, "Failed to generate external ConfigMap")
return ctrl.Result{}, err
if sbCluster.Spec.NetworkAttachment != "" {
// Create ConfigMap for external dataplane consumption
// TODO(ihar) - is there any hashing mechanism for EDP config? do we trigger deploy somehow?
err = r.generateExternalConfigMaps(ctx, helper, instance, sbCluster, &configMapVars)
if err != nil {
Log.Error(err, "Failed to generate external ConfigMap")
return ctrl.Result{}, err
}
}

// create OVN Config Job - start
Expand Down
8 changes: 6 additions & 2 deletions tests/functional/base_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,12 +100,14 @@ func OVNDBClusterConditionGetter(name types.NamespacedName) condition.Conditions
}

// CreateOVNDBClusters Creates NB and SB OVNDBClusters
func CreateOVNDBClusters(namespace string) []types.NamespacedName {
func CreateOVNDBClusters(namespace string, nad string) []types.NamespacedName {
dbs := []types.NamespacedName{}
for _, db := range []string{v1beta1.NBDBType, v1beta1.SBDBType} {
name := fmt.Sprintf("ovn-%s", uuid.New().String())
spec := GetDefaultOVNDBClusterSpec()
spec["dbType"] = db
spec["networkAttachment"] = nad

instance := CreateOVNDBCluster(namespace, name, spec)

instance_name := types.NamespacedName{Name: instance.GetName(), Namespace: instance.GetNamespace()}
Expand All @@ -114,11 +116,13 @@ func CreateOVNDBClusters(namespace string) []types.NamespacedName {
if db == v1beta1.SBDBType {
dbaddr = "tcp:10.1.1.1:6642"
}

// the Status field needs to be written via a separate client
Eventually(func(g Gomega) {
ovndbcluster := GetOVNDBCluster(instance_name)
ovndbcluster.Status.InternalDBAddress = dbaddr
if nad != "" {
ovndbcluster.Status.DBAddress = dbaddr
}
g.Expect(k8sClient.Status().Update(ctx, ovndbcluster)).Should(Succeed())
}, timeout, interval).Should(Succeed())

Expand Down
164 changes: 98 additions & 66 deletions tests/functional/ovncontroller_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,11 +84,11 @@ var _ = Describe("OVNController controller", func() {
)
})

When("OVNDBCluster instances are available", func() {
When("OVNDBCluster instances are available without networkAttachments", func() {
var scriptsCM types.NamespacedName
var dbs []types.NamespacedName
BeforeEach(func() {
dbs = CreateOVNDBClusters(namespace)
dbs = CreateOVNDBClusters(namespace, "")
DeferCleanup(DeleteOVNDBClusters, dbs)
daemonSetName := types.NamespacedName{
Namespace: namespace,
Expand All @@ -101,23 +101,6 @@ var _ = Describe("OVNController controller", func() {
}
})

It("should create a ConfigMap for init.sh with the ovn remote config option set based on the OVNDBCluster", func() {
Eventually(func() corev1.ConfigMap {
return *th.GetConfigMap(scriptsCM)
}, timeout, interval).ShouldNot(BeNil())
for _, db := range dbs {
ovndb := GetOVNDBCluster(db)
Expect(th.GetConfigMap(scriptsCM).Data["functions"]).Should(
ContainSubstring("ovn-remote=%s", ovndb.Status.DBAddress))
}

th.ExpectCondition(
OVNControllerName,
ConditionGetterFunc(OVNControllerConditionGetter),
condition.ServiceConfigReadyCondition,
corev1.ConditionTrue,
)
})
It("should create a ConfigMap for net_setup.sh with eth0 as Interface Name", func() {
Eventually(func() corev1.ConfigMap {
return *th.GetConfigMap(scriptsCM)
Expand All @@ -138,12 +121,53 @@ var _ = Describe("OVNController controller", func() {
Eventually(func() []corev1.ConfigMap {
return th.ListConfigMaps(fmt.Sprintf("%s-%s", OVNControllerName.Name, "config")).Items
}, timeout, interval).Should(BeEmpty())

})
})

When("OVNDBCluster instances with networkAttachments are available", func() {
var configCM types.NamespacedName
var dbs []types.NamespacedName
BeforeEach(func() {
internalAPINADName := types.NamespacedName{Namespace: namespace, Name: "internalapi"}
nad := th.CreateNetworkAttachmentDefinition(internalAPINADName)
DeferCleanup(th.DeleteInstance, nad)
dbs = CreateOVNDBClusters(namespace, "internalapi")
DeferCleanup(DeleteOVNDBClusters, dbs)
daemonSetName := types.NamespacedName{
Namespace: namespace,
Name: "ovn-controller",
}
SimulateDaemonsetNumberReady(daemonSetName)
configCM = types.NamespacedName{
Namespace: OVNControllerName.Namespace,
Name: fmt.Sprintf("%s-%s", OVNControllerName.Name, "config"),
}
})

It("should create an external config map", func() {
Eventually(func() corev1.ConfigMap {
return *th.GetConfigMap(configCM)
}, timeout, interval).ShouldNot(BeNil())
})

It("should delete the external config map when networkAttachment is detached from SB DB", func() {
Eventually(func() corev1.ConfigMap {
return *th.GetConfigMap(configCM)
}, timeout, interval).ShouldNot(BeNil())
Eventually(func(g Gomega) {
ovndbcluster := GetOVNDBCluster(dbs[1])
ovndbcluster.Spec.NetworkAttachment = ""
g.Expect(k8sClient.Update(ctx, ovndbcluster)).Should(Succeed())
}, timeout, interval).Should(Succeed())
SetExternalEndpoint(dbs[1], "")
th.AssertConfigMapDoesNotExist(configCM)
})
})

When("OVNController CR is deleted", func() {
It("removes the Config MAP", func() {
DeferCleanup(DeleteOVNDBClusters, CreateOVNDBClusters(namespace))
DeferCleanup(DeleteOVNDBClusters, CreateOVNDBClusters(namespace, ""))
scriptsCM := types.NamespacedName{
Namespace: OVNControllerName.Namespace,
Name: fmt.Sprintf("%s-%s", OVNControllerName.Name, "scripts"),
Expand All @@ -165,7 +189,7 @@ var _ = Describe("OVNController controller", func() {

When("A OVNController instance is created with debug on", func() {
BeforeEach(func() {
dbs := CreateOVNDBClusters(namespace)
dbs := CreateOVNDBClusters(namespace, "")
DeferCleanup(DeleteOVNDBClusters, dbs)
name := fmt.Sprintf("ovn-controller-%s", uuid.New().String())
spec := GetDefaultOVNControllerSpec()
Expand Down Expand Up @@ -201,12 +225,15 @@ var _ = Describe("OVNController controller", func() {
})
})

When("OVNController is created with networkAttachments", func() {
When("OVNController and OVNDBClusters are created with networkAttachments", func() {
var OVNControllerName types.NamespacedName
var dbs []types.NamespacedName

BeforeEach(func() {
dbs = CreateOVNDBClusters(namespace)
internalAPINADName := types.NamespacedName{Namespace: namespace, Name: "internalapi"}
nad := th.CreateNetworkAttachmentDefinition(internalAPINADName)
DeferCleanup(th.DeleteInstance, nad)
dbs = CreateOVNDBClusters(namespace, "internalapi")
for _, db := range dbs {
DeferCleanup(th.DeleteInstance, GetOVNDBCluster(db))
}
Expand All @@ -218,20 +245,7 @@ var _ = Describe("OVNController controller", func() {
DeferCleanup(th.DeleteInstance, instance)
})

It("reports that the definition is missing", func() {
th.ExpectConditionWithDetails(
OVNControllerName,
ConditionGetterFunc(OVNControllerConditionGetter),
condition.NetworkAttachmentsReadyCondition,
corev1.ConditionFalse,
condition.RequestedReason,
"NetworkAttachment resources missing: internalapi",
)
})
It("reports that network attachment is missing", func() {
internalAPINADName := types.NamespacedName{Namespace: namespace, Name: "internalapi"}
nad := th.CreateNetworkAttachmentDefinition(internalAPINADName)
DeferCleanup(th.DeleteInstance, nad)

daemonSetName := types.NamespacedName{
Namespace: namespace,
Expand Down Expand Up @@ -267,9 +281,6 @@ var _ = Describe("OVNController controller", func() {
)
})
It("reports that an IP is missing", func() {
internalAPINADName := types.NamespacedName{Namespace: namespace, Name: "internalapi"}
nad := th.CreateNetworkAttachmentDefinition(internalAPINADName)
DeferCleanup(th.DeleteInstance, nad)

daemonSetName := types.NamespacedName{
Namespace: namespace,
Expand Down Expand Up @@ -308,9 +319,6 @@ var _ = Describe("OVNController controller", func() {
)
})
It("reports NetworkAttachmentsReady if the Pods got the proper annotations", func() {
internalAPINADName := types.NamespacedName{Namespace: namespace, Name: "internalapi"}
nad := th.CreateNetworkAttachmentDefinition(internalAPINADName)
DeferCleanup(th.DeleteInstance, nad)

daemonSetName := types.NamespacedName{
Namespace: namespace,
Expand All @@ -336,9 +344,7 @@ var _ = Describe("OVNController controller", func() {
}, timeout, interval).Should(Succeed())
})
It("should create a ConfigMap for net_setup.sh with nic name as Network Attachment", func() {
internalAPINADName := types.NamespacedName{Namespace: namespace, Name: "internalapi"}
nad := th.CreateNetworkAttachmentDefinition(internalAPINADName)
DeferCleanup(th.DeleteInstance, nad)

scriptsCM := types.NamespacedName{
Namespace: OVNControllerName.Namespace,
Name: fmt.Sprintf("%s-%s", OVNControllerName.Name, "scripts"),
Expand All @@ -353,9 +359,7 @@ var _ = Describe("OVNController controller", func() {
ContainSubstring("addr show dev %s", ovncontroller.Spec.NetworkAttachment))
})
It("should create an external ConfigMap with expected key-value pairs", func() {
internalAPINADName := types.NamespacedName{Namespace: namespace, Name: "internalapi"}
nad := th.CreateNetworkAttachmentDefinition(internalAPINADName)
DeferCleanup(th.DeleteInstance, nad)

externalCM := types.NamespacedName{
Namespace: OVNControllerName.Namespace,
Name: fmt.Sprintf("%s-%s", OVNControllerName.Name, "config"),
Expand All @@ -376,16 +380,18 @@ var _ = Describe("OVNController controller", func() {
return *th.GetConfigMap(externalCM)
}, timeout, interval).ShouldNot(BeNil())

Expect(th.GetConfigMap(externalCM).Data["ovsdb-config"]).Should(
ContainSubstring("ovn-remote: %s", externalSBEndpoint))
Expect(th.GetConfigMap(externalCM).Data["ovsdb-config"]).Should(
ContainSubstring("ovn-encap-type: %s", "geneve"))
Eventually(func(g Gomega) {
g.Expect(th.GetConfigMap(externalCM).Data["ovsdb-config"]).Should(
ContainSubstring("ovn-remote: %s", externalSBEndpoint))
}, timeout, interval).Should(Succeed())
Eventually(func(g Gomega) {
g.Expect(th.GetConfigMap(externalCM).Data["ovsdb-config"]).Should(
ContainSubstring("ovn-encap-type: %s", "geneve"))
}, timeout, interval).Should(Succeed())
})

It("should delete an external ConfigMap once SB DBCluster is deleted", func() {
internalAPINADName := types.NamespacedName{Namespace: namespace, Name: "internalapi"}
nad := th.CreateNetworkAttachmentDefinition(internalAPINADName)
DeferCleanup(th.DeleteInstance, nad)

externalCM := types.NamespacedName{
Namespace: OVNControllerName.Namespace,
Name: fmt.Sprintf("%s-%s", OVNControllerName.Name, "config"),
Expand Down Expand Up @@ -413,9 +419,7 @@ var _ = Describe("OVNController controller", func() {
})

It("should delete an external ConfigMap once SB DBCluster is detached from NAD", func() {
internalAPINADName := types.NamespacedName{Namespace: namespace, Name: "internalapi"}
nad := th.CreateNetworkAttachmentDefinition(internalAPINADName)
DeferCleanup(th.DeleteInstance, nad)

externalCM := types.NamespacedName{
Namespace: OVNControllerName.Namespace,
Name: fmt.Sprintf("%s-%s", OVNControllerName.Name, "config"),
Expand Down Expand Up @@ -443,9 +447,7 @@ var _ = Describe("OVNController controller", func() {
})

It("should update the external ConfigMap once SB DBCluster is updated", func() {
internalAPINADName := types.NamespacedName{Namespace: namespace, Name: "internalapi"}
nad := th.CreateNetworkAttachmentDefinition(internalAPINADName)
DeferCleanup(th.DeleteInstance, nad)

externalCM := types.NamespacedName{
Namespace: OVNControllerName.Namespace,
Name: fmt.Sprintf("%s-%s", OVNControllerName.Name, "config"),
Expand All @@ -466,8 +468,10 @@ var _ = Describe("OVNController controller", func() {
return *th.GetConfigMap(externalCM)
}, timeout, interval).ShouldNot(BeNil())

Expect(th.GetConfigMap(externalCM).Data["ovsdb-config"]).Should(
ContainSubstring("ovn-remote: %s", externalSBEndpoint))
Eventually(func(g Gomega) {
g.Expect(th.GetConfigMap(externalCM).Data["ovsdb-config"]).Should(
ContainSubstring("ovn-remote: %s", externalSBEndpoint))
}, timeout, interval).Should(Succeed())

newExternalSBEndpoint := "10.0.0.250"
SetExternalEndpoint(dbs[1], newExternalSBEndpoint)
Expand All @@ -478,11 +482,39 @@ var _ = Describe("OVNController controller", func() {
}, timeout, interval).Should(Succeed())
})
})
When("OVNController is created with missing networkAttachment", func() {
var OVNControllerName types.NamespacedName
var dbs []types.NamespacedName

BeforeEach(func() {
dbs = CreateOVNDBClusters(namespace, "")
for _, db := range dbs {
DeferCleanup(th.DeleteInstance, GetOVNDBCluster(db))
}
name := fmt.Sprintf("ovn-controller-%s", uuid.New().String())
spec := GetDefaultOVNControllerSpec()
spec["networkAttachment"] = "internalapi"
instance := CreateOVNController(namespace, name, spec)
OVNControllerName = types.NamespacedName{Name: instance.GetName(), Namespace: instance.GetNamespace()}
DeferCleanup(th.DeleteInstance, instance)
})

It("reports that the definition is missing", func() {
th.ExpectConditionWithDetails(
OVNControllerName,
ConditionGetterFunc(OVNControllerConditionGetter),
condition.NetworkAttachmentsReadyCondition,
corev1.ConditionFalse,
condition.RequestedReason,
"NetworkAttachment resources missing: internalapi",
)
})
})

When("OVNController is created with nic configs", func() {
var OVNControllerName types.NamespacedName
BeforeEach(func() {
dbs := CreateOVNDBClusters(namespace)
dbs := CreateOVNDBClusters(namespace, "")
DeferCleanup(DeleteOVNDBClusters, dbs)
name := fmt.Sprintf("ovn-controller-%s", uuid.New().String())
spec := GetDefaultOVNControllerSpec()
Expand Down Expand Up @@ -550,7 +582,7 @@ var _ = Describe("OVNController controller", func() {

When("OVNController is created with networkAttachment and nic configs", func() {
BeforeEach(func() {
dbs := CreateOVNDBClusters(namespace)
dbs := CreateOVNDBClusters(namespace, "")
DeferCleanup(DeleteOVNDBClusters, dbs)
name := fmt.Sprintf("ovn-controller-%s", uuid.New().String())
spec := GetDefaultOVNControllerSpec()
Expand Down Expand Up @@ -597,7 +629,7 @@ var _ = Describe("OVNController controller", func() {
var ovnControllerName types.NamespacedName

BeforeEach(func() {
dbs := CreateOVNDBClusters(namespace)
dbs := CreateOVNDBClusters(namespace, "")
DeferCleanup(DeleteOVNDBClusters, dbs)
name := fmt.Sprintf("ovn-controller-%s", uuid.New().String())
ovnControllerName = types.NamespacedName{Namespace: namespace, Name: name}
Expand Down
Loading

0 comments on commit 48dbc8d

Please sign in to comment.