Skip to content

Commit

Permalink
Merge pull request #645 from booxter/ovn-test-case
Browse files Browse the repository at this point in the history 
ovn: fix ovn-controller cleanup on ovn disabled
  • Loading branch information
@openshift-merge-bot
openshift-merge-bot[bot] authored Feb 7, 2024
2 parents d266057 + f7fe693 commit 00c81bc
Show file tree
Hide file tree
Showing 4 changed files with 181 additions and 41 deletions.
90 changes: 61 additions & 29 deletions pkg/openstack/ovn.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,44 @@ import (
// ReconcileOVN -
func ReconcileOVN(ctx context.Context, instance *corev1beta1.OpenStackControlPlane, helper *helper.Helper) (ctrl.Result, error) {

setOVNReadyError := func(instance *corev1beta1.OpenStackControlPlane, err error) {
instance.Status.Conditions.Set(condition.FalseCondition(
corev1beta1.OpenStackControlPlaneOVNReadyCondition,
condition.ErrorReason,
condition.SeverityWarning,
corev1beta1.OpenStackControlPlaneOVNReadyErrorMessage,
err.Error()))
}

OVNDBClustersReady, err := ReconcileOVNDbClusters(ctx, instance, helper)
if err != nil {
setOVNReadyError(instance, err)
}

OVNNorthdReady, err := ReconcileOVNNorthd(ctx, instance, helper)
if err != nil {
setOVNReadyError(instance, err)
}

OVNControllerReady, err := ReconcileOVNController(ctx, instance, helper)
if err != nil {
setOVNReadyError(instance, err)
}

// Expect all services (dbclusters, northd, ovn-controller) ready
if OVNDBClustersReady && OVNNorthdReady && OVNControllerReady {
instance.Status.Conditions.MarkTrue(corev1beta1.OpenStackControlPlaneOVNReadyCondition, corev1beta1.OpenStackControlPlaneOVNReadyMessage)
} else {
instance.Status.Conditions.Set(condition.FalseCondition(
corev1beta1.OpenStackControlPlaneOVNReadyCondition,
condition.RequestedReason,
condition.SeverityInfo,
corev1beta1.OpenStackControlPlaneOVNReadyRunningMessage))
}
return ctrl.Result{}, nil
}

func ReconcileOVNDbClusters(ctx context.Context, instance *corev1beta1.OpenStackControlPlane, helper *helper.Helper) (bool, error) {
Log := GetLogger(ctx)

OVNDBClustersReady := len(instance.Spec.Ovn.Template.OVNDBCluster) != 0
Expand All @@ -30,8 +68,8 @@ func ReconcileOVN(ctx context.Context, instance *corev1beta1.OpenStackControlPla
}

if !instance.Spec.Ovn.Enabled {
if res, err := EnsureDeleted(ctx, helper, OVNDBCluster); err != nil {
return res, err
if _, err := EnsureDeleted(ctx, helper, OVNDBCluster); err != nil {
return false, err
}
instance.Status.Conditions.Remove(corev1beta1.OpenStackControlPlaneOVNReadyCondition)
continue
Expand All @@ -57,19 +95,18 @@ func ReconcileOVN(ctx context.Context, instance *corev1beta1.OpenStackControlPla
})

if err != nil {
instance.Status.Conditions.Set(condition.FalseCondition(
corev1beta1.OpenStackControlPlaneOVNReadyCondition,
condition.ErrorReason,
condition.SeverityWarning,
corev1beta1.OpenStackControlPlaneOVNReadyErrorMessage,
err.Error()))
return ctrl.Result{}, err
return false, err
}
if op != controllerutil.OperationResultNone {
Log.Info(fmt.Sprintf("OVNDBCluster %s - %s", OVNDBCluster.Name, op))
}
OVNDBClustersReady = OVNDBClustersReady && OVNDBCluster.IsReady()
}
return OVNDBClustersReady, nil
}

func ReconcileOVNNorthd(ctx context.Context, instance *corev1beta1.OpenStackControlPlane, helper *helper.Helper) (bool, error) {
Log := GetLogger(ctx)

OVNNorthd := &ovnv1.OVNNorthd{
ObjectMeta: metav1.ObjectMeta{
Expand All @@ -79,11 +116,11 @@ func ReconcileOVN(ctx context.Context, instance *corev1beta1.OpenStackControlPla
}

if !instance.Spec.Ovn.Enabled {
if res, err := EnsureDeleted(ctx, helper, OVNNorthd); err != nil {
return res, err
if _, err := EnsureDeleted(ctx, helper, OVNNorthd); err != nil {
return false, err
}
instance.Status.Conditions.Remove(corev1beta1.OpenStackControlPlaneOVNReadyCondition)
return ctrl.Result{}, nil
return false, nil
}

Log.Info("Reconciling OVNNorthd", "OVNNorthd.Namespace", instance.Namespace, "OVNNorthd.Name", "ovnnorthd")
Expand All @@ -109,11 +146,16 @@ func ReconcileOVN(ctx context.Context, instance *corev1beta1.OpenStackControlPla
condition.SeverityWarning,
corev1beta1.OpenStackControlPlaneOVNReadyErrorMessage,
err.Error()))
return ctrl.Result{}, err
return false, err
}
if op != controllerutil.OperationResultNone {
Log.Info(fmt.Sprintf("OVNNorthd %s - %s", OVNNorthd.Name, op))
}
return OVNNorthd.IsReady(), nil
}

func ReconcileOVNController(ctx context.Context, instance *corev1beta1.OpenStackControlPlane, helper *helper.Helper) (bool, error) {
Log := GetLogger(ctx)

OVNController := &ovnv1.OVNController{
ObjectMeta: metav1.ObjectMeta{
Expand All @@ -123,15 +165,15 @@ func ReconcileOVN(ctx context.Context, instance *corev1beta1.OpenStackControlPla
}

if !instance.Spec.Ovn.Enabled {
if res, err := EnsureDeleted(ctx, helper, OVNController); err != nil {
return res, err
if _, err := EnsureDeleted(ctx, helper, OVNController); err != nil {
return false, err
}
instance.Status.Conditions.Remove(corev1beta1.OpenStackControlPlaneOVNReadyCondition)
return ctrl.Result{}, nil
return false, nil
}

Log.Info("Reconciling OVNController", "OVNController.Namespace", instance.Namespace, "OVNController.Name", "ovncontroller")
op, err = controllerutil.CreateOrPatch(ctx, helper.GetClient(), OVNController, func() error {
op, err := controllerutil.CreateOrPatch(ctx, helper.GetClient(), OVNController, func() error {

instance.Spec.Ovn.Template.OVNController.DeepCopyInto(&OVNController.Spec)

Expand All @@ -153,21 +195,11 @@ func ReconcileOVN(ctx context.Context, instance *corev1beta1.OpenStackControlPla
condition.SeverityWarning,
corev1beta1.OpenStackControlPlaneOVNReadyErrorMessage,
err.Error()))
return ctrl.Result{}, err
return false, err
}
if op != controllerutil.OperationResultNone {
Log.Info(fmt.Sprintf("OVNController %s - %s", OVNController.Name, op))
}

// Expect all services (dbclusters, northd, ovn-controller) ready
if OVNDBClustersReady && OVNNorthd.IsReady() && OVNController.IsReady() {
instance.Status.Conditions.MarkTrue(corev1beta1.OpenStackControlPlaneOVNReadyCondition, corev1beta1.OpenStackControlPlaneOVNReadyMessage)
} else {
instance.Status.Conditions.Set(condition.FalseCondition(
corev1beta1.OpenStackControlPlaneOVNReadyCondition,
condition.RequestedReason,
condition.SeverityInfo,
corev1beta1.OpenStackControlPlaneOVNReadyRunningMessage))
}
return ctrl.Result{}, nil
return OVNController.IsReady(), nil
}
20 changes: 20 additions & 0 deletions tests/functional/base_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,10 @@ type Names struct {
SelfSignedIssuerName types.NamespacedName
CABundleName types.NamespacedName
OpenStackClientName types.NamespacedName
OVNNorthdName types.NamespacedName
OVNControllerName types.NamespacedName
OVNDbServerNBName types.NamespacedName
OVNDbServerSBName types.NamespacedName
}

func CreateNames(openstackControlplaneName types.NamespacedName) Names {
Expand Down Expand Up @@ -99,6 +103,22 @@ func CreateNames(openstackControlplaneName types.NamespacedName) Names {
Namespace: openstackControlplaneName.Namespace,
Name: "openstackclient",
},
OVNNorthdName: types.NamespacedName{
Namespace: openstackControlplaneName.Namespace,
Name: "ovnnorthd",
},
OVNDbServerNBName: types.NamespacedName{
Namespace: openstackControlplaneName.Namespace,
Name: "ovndbcluster-nb",
},
OVNDbServerSBName: types.NamespacedName{
Namespace: openstackControlplaneName.Namespace,
Name: "ovndbcluster-sb",
},
OVNControllerName: types.NamespacedName{
Namespace: openstackControlplaneName.Namespace,
Name: "ovncontroller",
},
}
}

Expand Down
108 changes: 96 additions & 12 deletions tests/functional/openstackoperator_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (
. "github.com/openstack-k8s-operators/lib-common/modules/common/test/helpers"
"github.com/openstack-k8s-operators/lib-common/modules/common/tls"
clientv1 "github.com/openstack-k8s-operators/openstack-operator/apis/client/v1beta1"
ovnv1 "github.com/openstack-k8s-operators/ovn-operator/api/v1beta1"
)

var _ = Describe("OpenStackOperator controller", func() {
Expand All @@ -38,22 +39,23 @@ var _ = Describe("OpenStackOperator controller", func() {
// will fail to generate the ConfigMap as it does not find common.sh
err := os.Setenv("OPERATOR_TEMPLATES", "../../templates")
Expect(err).NotTo(HaveOccurred())

// (mschuppert) create root CA secret as there is no certmanager running.
// it is not used, just to make sure reconcile proceeds and creates the ca-bundle.
Eventually(func(g Gomega) {
th.CreateSecret(
names.RootCAPublicName,
map[string][]byte{
"ca.crt": []byte("test"),
"tls.crt": []byte("test"),
"tls.key": []byte("test"),
})
}, timeout, interval).Should(Succeed())

})

When("A default OpenStackControlplane instance is created", func() {
BeforeEach(func() {
// (mschuppert) create root CA secret as there is no certmanager running.
// it is not used, just to make sure reconcile proceeds and creates the ca-bundle.
Eventually(func(g Gomega) {
th.CreateSecret(
names.RootCAPublicName,
map[string][]byte{
"ca.crt": []byte("test"),
"tls.crt": []byte("test"),
"tls.key": []byte("test"),
})
}, timeout, interval).Should(Succeed())

DeferCleanup(
th.DeleteInstance,
CreateOpenStackControlPlane(names.OpenStackControlplaneName, GetDefaultOpenStackControlPlaneSpec()),
Expand Down Expand Up @@ -179,4 +181,86 @@ var _ = Describe("OpenStackOperator controller", func() {
}, timeout, interval).Should(Succeed())
})
})
When("A OVN OpenStackControlplane instance is created", func() {
BeforeEach(func() {
spec := GetDefaultOpenStackControlPlaneSpec()
spec["ovn"] = map[string]interface{}{
"enabled": true,
"template": map[string]interface{}{
"ovnDBCluster": map[string]interface{}{
"ovndbcluster-nb": map[string]interface{}{
"dbType": "NB",
},
"ovndbcluster-sb": map[string]interface{}{
"dbType": "SB",
},
},
},
}
DeferCleanup(
th.DeleteInstance,
CreateOpenStackControlPlane(names.OpenStackControlplaneName, spec),
)
})

It("should have OVN enabled", func() {
OSCtlplane := GetOpenStackControlPlane(names.OpenStackControlplaneName)
Expect(OSCtlplane.Spec.Ovn.Enabled).Should(BeTrue())

// ovn services exist
Eventually(func(g Gomega) {
ovnNorthd := ovn.GetOVNNorthd(names.OVNNorthdName)
g.Expect(ovnNorthd).Should(Not(BeNil()))
}, timeout, interval).Should(Succeed())

Eventually(func(g Gomega) {
ovnController := ovn.GetOVNController(names.OVNControllerName)
g.Expect(ovnController).Should(Not(BeNil()))
}, timeout, interval).Should(Succeed())

Eventually(func(g Gomega) {
ovnDbServerNB := ovn.GetOVNDBCluster(names.OVNDbServerNBName)
g.Expect(ovnDbServerNB).Should(Not(BeNil()))
}, timeout, interval).Should(Succeed())

Eventually(func(g Gomega) {
ovnDbServerSB := ovn.GetOVNDBCluster(names.OVNDbServerSBName)
g.Expect(ovnDbServerSB).Should(Not(BeNil()))
}, timeout, interval).Should(Succeed())
})

It("should remove OVN resources on disable", func() {
Eventually(func(g Gomega) {
OSCtlplane := GetOpenStackControlPlane(names.OpenStackControlplaneName)
OSCtlplane.Spec.Ovn.Enabled = false
g.Expect(k8sClient.Update(ctx, OSCtlplane)).Should(Succeed())
}, timeout, interval).Should(Succeed())

Eventually(func(g Gomega) {
OSCtlplane := GetOpenStackControlPlane(names.OpenStackControlplaneName)
g.Expect(OSCtlplane.Spec.Ovn.Enabled).Should(BeFalse())
}, timeout, interval).Should(Succeed())

// ovn services don't exist
Eventually(func(g Gomega) {
instance := &ovnv1.OVNNorthd{}
g.Expect(th.K8sClient.Get(th.Ctx, names.OVNNorthdName, instance)).Should(Not(Succeed()))
}, timeout, interval).Should(Succeed())

Eventually(func(g Gomega) {
instance := &ovnv1.OVNDBCluster{}
g.Expect(th.K8sClient.Get(th.Ctx, names.OVNDbServerNBName, instance)).Should(Not(Succeed()))
}, timeout, interval).Should(Succeed())

Eventually(func(g Gomega) {
instance := &ovnv1.OVNDBCluster{}
g.Expect(th.K8sClient.Get(th.Ctx, names.OVNDbServerSBName, instance)).Should(Not(Succeed()))
}, timeout, interval).Should(Succeed())

Eventually(func(g Gomega) {
instance := &ovnv1.OVNController{}
g.Expect(th.K8sClient.Get(th.Ctx, names.OVNControllerName, instance)).Should(Not(Succeed()))
}, timeout, interval).Should(Succeed())
})
})
})
4 changes: 4 additions & 0 deletions tests/functional/suite_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ import (
common_test "github.com/openstack-k8s-operators/lib-common/modules/common/test/helpers"
test "github.com/openstack-k8s-operators/lib-common/modules/test"
mariadb_test "github.com/openstack-k8s-operators/mariadb-operator/api/test/helpers"
ovn_test "github.com/openstack-k8s-operators/ovn-operator/api/test/helpers"
//+kubebuilder:scaffold:imports
)

Expand All @@ -80,6 +81,7 @@ var (
mariadb *mariadb_test.TestHelper
infra *infra_test.TestHelper
crtmgr *certmanager_test.TestHelper
ovn *ovn_test.TestHelper
namespace string
names Names
)
Expand Down Expand Up @@ -277,6 +279,8 @@ var _ = BeforeSuite(func() {
Expect(infra).NotTo(BeNil())
crtmgr = certmanager_test.NewTestHelper(ctx, k8sClient, timeout, interval, logger)
Expect(crtmgr).NotTo(BeNil())
ovn = ovn_test.NewTestHelper(ctx, k8sClient, timeout, interval, logger)
Expect(ovn).NotTo(BeNil())

// Start the controller-manager if goroutine
webhookInstallOptions := &testEnv.WebhookInstallOptions
Expand Down

0 comments on commit 00c81bc

Please sign in to comment.