From 699c2b2113188723d16b50eda7a1f8fd45bf5a01 Mon Sep 17 00:00:00 2001 From: Balazs Gibizer Date: Fri, 17 Feb 2023 12:09:45 +0100 Subject: [PATCH 1/6] [envtest]Allow using NetworkAttachmentDefinition To be able to use the NetworkAttachmentDefinition in EnvTest this patch loads the CRD and applyies the schema during the env setup. Fortunately the client package contains the CRD definition so we can load it easily. --- test/functional/suite_test.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/test/functional/suite_test.go b/test/functional/suite_test.go index c3753951b..53a40a8ec 100644 --- a/test/functional/suite_test.go +++ b/test/functional/suite_test.go @@ -27,6 +27,7 @@ import ( . "github.com/onsi/gomega" "go.uber.org/zap/zapcore" + networkv1 "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1" routev1 "github.com/openshift/api/route/v1" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" @@ -91,6 +92,11 @@ var _ = BeforeSuite(func() { Expect(err).ShouldNot(HaveOccurred()) routev1CRDs, err := test.GetOpenShiftCRDDir("route/v1", gomod) Expect(err).ShouldNot(HaveOccurred()) + // NOTE(gibi): there are packages where the CRD directory has other + // yamls files as well, then we need to specify the extac file to load + networkv1CRD, err := test.GetCRDDirFromModule( + "github.com/k8snetworkplumbingwg/network-attachment-definition-client", gomod, "artifacts/networks-crd.yaml") + Expect(err).ShouldNot(HaveOccurred()) By("bootstrapping test environment") testEnv = &envtest.Environment{ @@ -102,6 +108,11 @@ var _ = BeforeSuite(func() { rabbitCRDs, routev1CRDs, }, + CRDInstallOptions: envtest.CRDInstallOptions{ + Paths: []string{ + networkv1CRD, + }, + }, ErrorIfCRDPathMissing: true, } @@ -128,6 +139,8 @@ var _ = BeforeSuite(func() { Expect(err).NotTo(HaveOccurred()) err = rabbitmqv1.AddToScheme(scheme.Scheme) Expect(err).NotTo(HaveOccurred()) + err = networkv1.AddToScheme(scheme.Scheme) + Expect(err).NotTo(HaveOccurred()) //+kubebuilder:scaffold:scheme From c1e26ed5dffd8b70fbe73a051239f41a3dc1b007 Mon Sep 17 00:00:00 2001 From: Balazs Gibizer Date: Fri, 17 Feb 2023 12:11:27 +0100 Subject: [PATCH 2/6] [envtest]Coverage for NovaAPI.NetworkAttachments --- go.mod | 2 +- go.sum | 4 +- test/functional/base_test.go | 71 ++++++++++ test/functional/novaapi_controller_test.go | 145 +++++++++++++++++++++ 4 files changed, 219 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index db25bb41c..918e61f14 100644 --- a/go.mod +++ b/go.mod @@ -9,7 +9,7 @@ require ( github.com/onsi/ginkgo/v2 v2.8.3 github.com/onsi/gomega v1.27.1 github.com/openshift/api v3.9.0+incompatible - github.com/openstack-k8s-operators/infra-operator/apis v0.0.0-20230206011116-de79e1982b80 + github.com/openstack-k8s-operators/infra-operator/apis v0.0.0-20230210143210-6e3aad14c3aa github.com/openstack-k8s-operators/keystone-operator/api v0.0.0-20230208150008-87df8c2f32cb github.com/openstack-k8s-operators/lib-common/modules/common v0.0.0-20230220103808-bd9bda2ad709 github.com/openstack-k8s-operators/lib-common/modules/database v0.0.0-20230215134634-d31141e5bbba diff --git a/go.sum b/go.sum index 6dbd1e9a9..661edab27 100644 --- a/go.sum +++ b/go.sum @@ -233,8 +233,8 @@ github.com/onsi/gomega v1.27.1 h1:rfztXRbg6nv/5f+Raen9RcGoSecHIFgBBLQK3Wdj754= github.com/onsi/gomega v1.27.1/go.mod h1:aHX5xOykVYzWOV4WqQy0sy8BQptgukenXpCXfadcIAw= github.com/openshift/api v3.9.0+incompatible h1:fJ/KsefYuZAjmrr3+5U9yZIZbTOpVkDDLDLFresAeYs= github.com/openshift/api v3.9.0+incompatible/go.mod h1:dh9o4Fs58gpFXGSYfnVxGR9PnV53I8TW84pQaJDdGiY= -github.com/openstack-k8s-operators/infra-operator/apis v0.0.0-20230206011116-de79e1982b80 h1:uIZERZo3P3wzFTvb+8eVKltXlkVDcr6LwQKTovhr3PM= -github.com/openstack-k8s-operators/infra-operator/apis v0.0.0-20230206011116-de79e1982b80/go.mod h1:1eXDVJYo3+NPrAyvrDo9BTcc2W/v+b6Jw/cL7O0aAno= +github.com/openstack-k8s-operators/infra-operator/apis v0.0.0-20230210143210-6e3aad14c3aa h1:HJypldaUFUol3iBea4P6UDRuCvpAVSOtnBqOSKSnW50= +github.com/openstack-k8s-operators/infra-operator/apis v0.0.0-20230210143210-6e3aad14c3aa/go.mod h1:5kG0Ct412tO3fNkZ5b3/BwwSsV7LkSNfOB/apUlbMJI= github.com/openstack-k8s-operators/keystone-operator/api v0.0.0-20230208150008-87df8c2f32cb h1:2liBXr5C9LaAM1e/CQWNY7PRwDVnPp3df9Q8lvVv5BI= github.com/openstack-k8s-operators/keystone-operator/api v0.0.0-20230208150008-87df8c2f32cb/go.mod h1:gr1gxe+nRHt1UNzc7D5jTXN6auvufTzhqdVsubQsLDc= github.com/openstack-k8s-operators/lib-common/modules/common v0.0.0-20230220103808-bd9bda2ad709 h1:g3sO7qyvAz3ltQG+I81bsM3SwrI8ILYPv+s6bWT3tLU= diff --git a/test/functional/base_test.go b/test/functional/base_test.go index 31a0457c9..5642deca8 100644 --- a/test/functional/base_test.go +++ b/test/functional/base_test.go @@ -16,11 +16,13 @@ limitations under the License. package functional_test import ( + "encoding/json" "time" "github.com/google/uuid" . "github.com/onsi/gomega" corev1 "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" k8s_errors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -28,6 +30,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + networkv1 "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1" routev1 "github.com/openshift/api/route/v1" rabbitmqv1 "github.com/openstack-k8s-operators/infra-operator/apis/rabbitmq/v1beta1" keystonev1 "github.com/openstack-k8s-operators/keystone-operator/api/v1beta1" @@ -669,3 +672,71 @@ func NovaSchedulerNotExists(name types.NamespacedName) { g.Expect(k8s_errors.IsNotFound(err)).To(BeTrue()) }, consistencyTimeout, interval).Should(Succeed()) } + +func CreateNetworkAttachmentDefinition(name types.NamespacedName) { + instance := &networkv1.NetworkAttachmentDefinition{ + ObjectMeta: metav1.ObjectMeta{ + Name: name.Name, + Namespace: name.Namespace, + }, + Spec: networkv1.NetworkAttachmentDefinitionSpec{ + Config: "", + }, + } + Expect(k8sClient.Create(ctx, instance)).Should(Succeed()) +} + +func DeleteNetworkAttachmentDefinition(name types.NamespacedName) { + Eventually(func(g Gomega) { + instance := &networkv1.NetworkAttachmentDefinition{} + err := k8sClient.Get(ctx, name, instance) + // if it is already gone that is OK + if k8s_errors.IsNotFound(err) { + return + } + g.Expect(err).Should(BeNil()) + + g.Expect(k8sClient.Delete(ctx, instance)).Should(Succeed()) + + err = k8sClient.Get(ctx, name, instance) + g.Expect(k8s_errors.IsNotFound(err)).To(BeTrue()) + }, timeout, interval).Should(Succeed()) +} + +func SimulateStatefulSetReplicaReadyWithPods(name types.NamespacedName, networkIPs map[string][]string) { + ss := th.GetStatefulSet(name) + for i := 0; i < int(*ss.Spec.Replicas); i++ { + pod := &v1.Pod{ + ObjectMeta: ss.Spec.Template.ObjectMeta, + Spec: ss.Spec.Template.Spec, + } + pod.ObjectMeta.Namespace = name.Namespace + pod.ObjectMeta.GenerateName = name.Name + + var netStatus []networkv1.NetworkStatus + for network, IPs := range networkIPs { + netStatus = append( + netStatus, + networkv1.NetworkStatus{ + Name: network, + IPs: IPs, + }, + ) + } + netStatusAnnotation, err := json.Marshal(netStatus) + Expect(err).NotTo(HaveOccurred()) + pod.Annotations[networkv1.NetworkStatusAnnot] = string(netStatusAnnotation) + + Expect(k8sClient.Create(ctx, pod)).Should(Succeed()) + } + + Eventually(func(g Gomega) { + ss := th.GetStatefulSet(name) + ss.Status.Replicas = 1 + ss.Status.ReadyReplicas = 1 + g.Expect(k8sClient.Status().Update(ctx, ss)).To(Succeed()) + + }, timeout, interval).Should(Succeed()) + + logger.Info("Simulated statefulset success", "on", name) +} diff --git a/test/functional/novaapi_controller_test.go b/test/functional/novaapi_controller_test.go index b6641c962..d2b1de8ef 100644 --- a/test/functional/novaapi_controller_test.go +++ b/test/functional/novaapi_controller_test.go @@ -16,6 +16,7 @@ limitations under the License. package functional_test import ( + "encoding/json" "fmt" "os" @@ -24,6 +25,7 @@ import ( . "github.com/onsi/gomega" . "github.com/openstack-k8s-operators/lib-common/modules/test/helpers" + networkv1 "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" @@ -397,4 +399,147 @@ var _ = Describe("NovaAPI controller", func() { Expect(endpoint.Finalizers).NotTo(ContainElement("NovaAPI")) }) }) + When("NovaAPI is created with networkAttachments", func() { + BeforeEach(func() { + DeferCleanup( + k8sClient.Delete, ctx, CreateNovaAPISecret(namespace, SecretName)) + DeferCleanup( + k8sClient.Delete, ctx, CreateNovaMessageBusSecret(namespace, MessageBusSecretName)) + + spec := GetDefaultNovaAPISpec() + spec["networkAttachments"] = []string{"internalapi"} + novaAPIName = CreateNovaAPI(namespace, spec) + DeferCleanup(DeleteNovaAPI, novaAPIName) + }) + + It("reports that the definition is missing", func() { + th.ExpectConditionWithDetails( + novaAPIName, + ConditionGetterFunc(NovaAPIConditionGetter), + condition.NetworkAttachmentsReadyCondition, + corev1.ConditionFalse, + condition.RequestedReason, + "NetworkAttachment resources missing: internalapi", + ) + th.ExpectCondition( + novaAPIName, + ConditionGetterFunc(NovaAPIConditionGetter), + condition.ReadyCondition, + corev1.ConditionUnknown, + ) + }) + It("reports that network attachment is missing", func() { + internalAPINADName := types.NamespacedName{Namespace: namespace, Name: "internalapi"} + CreateNetworkAttachmentDefinition(internalAPINADName) + DeferCleanup(DeleteNetworkAttachmentDefinition, internalAPINADName) + + statefulSetName := types.NamespacedName{ + Namespace: namespace, + Name: novaAPIName.Name, + } + ss := th.GetStatefulSet(statefulSetName) + + expectedAnnotation, err := json.Marshal( + []networkv1.NetworkSelectionElement{ + { + Name: "internalapi", + Namespace: namespace, + }}) + Expect(err).ShouldNot(HaveOccurred()) + Expect(ss.Spec.Template.ObjectMeta.Annotations).To( + HaveKeyWithValue("k8s.v1.cni.cncf.io/networks", string(expectedAnnotation)), + ) + + // We don't add network attachment status annotations to the Pods + // to simulate that the network attachments are missing. + SimulateStatefulSetReplicaReadyWithPods(statefulSetName, map[string][]string{}) + + th.ExpectConditionWithDetails( + novaAPIName, + ConditionGetterFunc(NovaAPIConditionGetter), + condition.NetworkAttachmentsReadyCondition, + corev1.ConditionFalse, + condition.ErrorReason, + "NetworkAttachments error occured "+ + "not all pods have interfaces with ips as configured in NetworkAttachments: [internalapi]", + ) + }) + It("reports that an IP is missing", func() { + internalAPINADName := types.NamespacedName{Namespace: namespace, Name: "internalapi"} + CreateNetworkAttachmentDefinition(internalAPINADName) + DeferCleanup(DeleteNetworkAttachmentDefinition, internalAPINADName) + + statefulSetName := types.NamespacedName{ + Namespace: namespace, + Name: novaAPIName.Name, + } + ss := th.GetStatefulSet(statefulSetName) + + expectedAnnotation, err := json.Marshal( + []networkv1.NetworkSelectionElement{ + { + Name: "internalapi", + Namespace: namespace, + }}) + Expect(err).ShouldNot(HaveOccurred()) + Expect(ss.Spec.Template.ObjectMeta.Annotations).To( + HaveKeyWithValue("k8s.v1.cni.cncf.io/networks", string(expectedAnnotation)), + ) + + // We simulat that there is no IP associated with the internalapi + // network attachment + SimulateStatefulSetReplicaReadyWithPods( + statefulSetName, + map[string][]string{namespace + "/internalapi": {}}, + ) + + th.ExpectConditionWithDetails( + novaAPIName, + ConditionGetterFunc(NovaAPIConditionGetter), + condition.NetworkAttachmentsReadyCondition, + corev1.ConditionFalse, + condition.ErrorReason, + "NetworkAttachments error occured "+ + "not all pods have interfaces with ips as configured in NetworkAttachments: [internalapi]", + ) + }) + It("reports NetworkAttachmentsReady if the Pods got the proper annotiations", func() { + internalAPINADName := types.NamespacedName{Namespace: namespace, Name: "internalapi"} + CreateNetworkAttachmentDefinition(internalAPINADName) + DeferCleanup(DeleteNetworkAttachmentDefinition, internalAPINADName) + + statefulSetName := types.NamespacedName{ + Namespace: namespace, + Name: novaAPIName.Name, + } + SimulateStatefulSetReplicaReadyWithPods( + statefulSetName, + map[string][]string{namespace + "/internalapi": {"10.0.0.1"}}, + ) + + th.ExpectCondition( + novaAPIName, + ConditionGetterFunc(NovaAPIConditionGetter), + condition.NetworkAttachmentsReadyCondition, + corev1.ConditionTrue, + ) + + Eventually(func(g Gomega) { + novaAPI := GetNovaAPI(novaAPIName) + g.Expect(novaAPI.Status.NetworkAttachments).To( + Equal(map[string][]string{namespace + "/internalapi": {"10.0.0.1"}})) + + }, timeout, interval).Should(Succeed()) + + keystoneEndpointName := types.NamespacedName{Namespace: namespace, Name: "nova"} + SimulateKeystoneEndpointReady(keystoneEndpointName) + + th.ExpectCondition( + novaAPIName, + ConditionGetterFunc(NovaAPIConditionGetter), + condition.ReadyCondition, + corev1.ConditionTrue, + ) + }) + }) }) From 934cd84608b771acbee0b3b6a3e3fade3760c798 Mon Sep 17 00:00:00 2001 From: Balazs Gibizer Date: Fri, 17 Feb 2023 18:16:01 +0100 Subject: [PATCH 3/6] [envtest]Coverage for NovaAPI.ExternalEndpoints --- test/functional/base_test.go | 11 +++- test/functional/novaapi_controller_test.go | 67 +++++++++++++++++++++- 2 files changed, 75 insertions(+), 3 deletions(-) diff --git a/test/functional/base_test.go b/test/functional/base_test.go index 5642deca8..bd4a4c903 100644 --- a/test/functional/base_test.go +++ b/test/functional/base_test.go @@ -559,7 +559,7 @@ func SimulateKeystoneServiceReady(name types.NamespacedName) { logger.Info("Simulated KeystoneService ready", "on", name) } -func AssertServiceExists(name types.NamespacedName) *corev1.Service { +func GetService(name types.NamespacedName) *corev1.Service { instance := &corev1.Service{} Eventually(func(g Gomega) { g.Expect(k8sClient.Get(ctx, name, instance)).Should(Succeed()) @@ -575,6 +575,15 @@ func AssertRouteExists(name types.NamespacedName) *routev1.Route { return instance } +func AssertRouteNotExists(name types.NamespacedName) *routev1.Route { + instance := &routev1.Route{} + Consistently(func(g Gomega) { + err := k8sClient.Get(ctx, name, instance) + g.Expect(k8s_errors.IsNotFound(err)).To(BeTrue()) + }, consistencyTimeout, interval).Should(Succeed()) + return instance +} + func GetKeystoneEndpoint(name types.NamespacedName) *keystonev1.KeystoneEndpoint { instance := &keystonev1.KeystoneEndpoint{} Eventually(func(g Gomega) { diff --git a/test/functional/novaapi_controller_test.go b/test/functional/novaapi_controller_test.go index d2b1de8ef..af8039098 100644 --- a/test/functional/novaapi_controller_test.go +++ b/test/functional/novaapi_controller_test.go @@ -332,8 +332,8 @@ var _ = Describe("NovaAPI controller", func() { condition.ExposeServiceReadyCondition, corev1.ConditionTrue, ) - AssertServiceExists(types.NamespacedName{Namespace: namespace, Name: "nova-public"}) - AssertServiceExists(types.NamespacedName{Namespace: namespace, Name: "nova-internal"}) + GetService(types.NamespacedName{Namespace: namespace, Name: "nova-public"}) + GetService(types.NamespacedName{Namespace: namespace, Name: "nova-internal"}) AssertRouteExists(types.NamespacedName{Namespace: namespace, Name: "nova-public"}) }) @@ -542,4 +542,67 @@ var _ = Describe("NovaAPI controller", func() { ) }) }) + + When("NovaAPI is created with externalEndpoints", func() { + BeforeEach(func() { + DeferCleanup( + k8sClient.Delete, ctx, CreateNovaAPISecret(namespace, SecretName)) + DeferCleanup( + k8sClient.Delete, ctx, CreateNovaMessageBusSecret(namespace, MessageBusSecretName)) + + spec := GetDefaultNovaAPISpec() + // NOTE(gibi): We need to create the data as raw list of maps + // to allow defaulting to happen according to the kubebuilder + // definitions + var externalEndpoints []interface{} + externalEndpoints = append( + externalEndpoints, map[string]interface{}{ + "endpoint": "internal", + "ipAddressPool": "osp-internalapi", + "loadBalancerIPs": []string{"internal-lb-ip-1", "internal-lb-ip-2"}, + }, + ) + spec["externalEndpoints"] = externalEndpoints + + novaAPIName = CreateNovaAPI(namespace, spec) + DeferCleanup(DeleteNovaAPI, novaAPIName) + }) + + It("creates MetalLB service", func() { + statefulSetName := types.NamespacedName{ + Namespace: namespace, + Name: novaAPIName.Name, + } + th.SimulateStatefulSetReplicaReady(statefulSetName) + + keystoneEndpointName := types.NamespacedName{Namespace: namespace, Name: "nova"} + SimulateKeystoneEndpointReady(keystoneEndpointName) + + // As the internal enpoint is configured in ExternalEndpoints it does not + // get a Route but a Service with MetalLB annotations instead + service := GetService(types.NamespacedName{Namespace: namespace, Name: "nova-internal"}) + Expect(service.Annotations).To( + HaveKeyWithValue("metallb.universe.tf/address-pool", "osp-internalapi")) + Expect(service.Annotations).To( + HaveKeyWithValue("metallb.universe.tf/allow-shared-ip", "osp-internalapi")) + Expect(service.Annotations).To( + HaveKeyWithValue("metallb.universe.tf/loadBalancerIPs", "internal-lb-ip-1,internal-lb-ip-2")) + AssertRouteNotExists(types.NamespacedName{Namespace: namespace, Name: "nova-internal"}) + + // As the public endpoint is not mentioned in the ExternalEndpoints a generic Service and + // a Route is created + service = GetService(types.NamespacedName{Namespace: namespace, Name: "nova-public"}) + Expect(service.Annotations).NotTo(HaveKey("metallb.universe.tf/address-pool")) + Expect(service.Annotations).NotTo(HaveKey("metallb.universe.tf/allow-shared-ip")) + Expect(service.Annotations).NotTo(HaveKey("metallb.universe.tf/loadBalancerIPs")) + AssertRouteExists(types.NamespacedName{Namespace: namespace, Name: "nova-public"}) + + th.ExpectCondition( + novaAPIName, + ConditionGetterFunc(NovaAPIConditionGetter), + condition.ReadyCondition, + corev1.ConditionTrue, + ) + }) + }) }) From 3ba67278bbaafaf891bc798cf5d284a8d5f008c3 Mon Sep 17 00:00:00 2001 From: Balazs Gibizer Date: Mon, 20 Feb 2023 12:30:02 +0100 Subject: [PATCH 4/6] [envtest]Coverage for NovaConductor.NetworkAttachments --- .../novaconductor_controller_test.go | 151 ++++++++++++++++++ 1 file changed, 151 insertions(+) diff --git a/test/functional/novaconductor_controller_test.go b/test/functional/novaconductor_controller_test.go index 9297fe793..33eda8edd 100644 --- a/test/functional/novaconductor_controller_test.go +++ b/test/functional/novaconductor_controller_test.go @@ -14,10 +14,12 @@ limitations under the License. package functional_test import ( + "encoding/json" "fmt" "os" "github.com/google/uuid" + networkv1 "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" . "github.com/openstack-k8s-operators/lib-common/modules/test/helpers" @@ -515,4 +517,153 @@ var _ = Describe("NovaConductor controller", func() { }, timeout, interval).Should(Succeed()) }) }) + + When("NovaConductor is created with networkAttachments", func() { + var jobName types.NamespacedName + BeforeEach(func() { + DeferCleanup( + k8sClient.Delete, ctx, CreateNovaConductorSecret(namespace, SecretName)) + DeferCleanup( + k8sClient.Delete, ctx, CreateNovaMessageBusSecret(namespace, MessageBusSecretName)) + + spec := GetDefaultNovaConductorSpec() + spec["networkAttachments"] = []string{"internalapi"} + novaConductorName = CreateNovaConductor(namespace, spec) + DeferCleanup(DeleteNovaConductor, novaConductorName) + jobName = types.NamespacedName{ + Namespace: namespace, + Name: novaConductorName.Name + "-db-sync", + } + + }) + + It("reports that the definition is missing", func() { + th.ExpectConditionWithDetails( + novaConductorName, + ConditionGetterFunc(NovaConductorConditionGetter), + condition.NetworkAttachmentsReadyCondition, + corev1.ConditionFalse, + condition.RequestedReason, + "NetworkAttachment resources missing: internalapi", + ) + th.ExpectCondition( + novaConductorName, + ConditionGetterFunc(NovaConductorConditionGetter), + condition.ReadyCondition, + corev1.ConditionUnknown, + ) + }) + It("reports that network attachment is missing", func() { + internalAPINADName := types.NamespacedName{Namespace: namespace, Name: "internalapi"} + CreateNetworkAttachmentDefinition(internalAPINADName) + DeferCleanup(DeleteNetworkAttachmentDefinition, internalAPINADName) + th.SimulateJobSuccess(jobName) + + statefulSetName := types.NamespacedName{ + Namespace: namespace, + Name: novaConductorName.Name, + } + ss := th.GetStatefulSet(statefulSetName) + + expectedAnnotation, err := json.Marshal( + []networkv1.NetworkSelectionElement{ + { + Name: "internalapi", + Namespace: namespace, + }}) + Expect(err).ShouldNot(HaveOccurred()) + Expect(ss.Spec.Template.ObjectMeta.Annotations).To( + HaveKeyWithValue("k8s.v1.cni.cncf.io/networks", string(expectedAnnotation)), + ) + + // We don't add network attachment status annotations to the Pods + // to simulate that the network attachments are missing. + SimulateStatefulSetReplicaReadyWithPods(statefulSetName, map[string][]string{}) + + th.ExpectConditionWithDetails( + novaConductorName, + ConditionGetterFunc(NovaConductorConditionGetter), + condition.NetworkAttachmentsReadyCondition, + corev1.ConditionFalse, + condition.ErrorReason, + "NetworkAttachments error occured "+ + "not all pods have interfaces with ips as configured in NetworkAttachments: [internalapi]", + ) + }) + It("reports that an IP is missing", func() { + internalAPINADName := types.NamespacedName{Namespace: namespace, Name: "internalapi"} + CreateNetworkAttachmentDefinition(internalAPINADName) + DeferCleanup(DeleteNetworkAttachmentDefinition, internalAPINADName) + th.SimulateJobSuccess(jobName) + + statefulSetName := types.NamespacedName{ + Namespace: namespace, + Name: novaConductorName.Name, + } + ss := th.GetStatefulSet(statefulSetName) + + expectedAnnotation, err := json.Marshal( + []networkv1.NetworkSelectionElement{ + { + Name: "internalapi", + Namespace: namespace, + }}) + Expect(err).ShouldNot(HaveOccurred()) + Expect(ss.Spec.Template.ObjectMeta.Annotations).To( + HaveKeyWithValue("k8s.v1.cni.cncf.io/networks", string(expectedAnnotation)), + ) + + // We simulat that there is no IP associated with the internalapi + // network attachment + SimulateStatefulSetReplicaReadyWithPods( + statefulSetName, + map[string][]string{namespace + "/internalapi": {}}, + ) + + th.ExpectConditionWithDetails( + novaConductorName, + ConditionGetterFunc(NovaConductorConditionGetter), + condition.NetworkAttachmentsReadyCondition, + corev1.ConditionFalse, + condition.ErrorReason, + "NetworkAttachments error occured "+ + "not all pods have interfaces with ips as configured in NetworkAttachments: [internalapi]", + ) + }) + It("reports NetworkAttachmentsReady if the Pods got the proper annotiations", func() { + internalAPINADName := types.NamespacedName{Namespace: namespace, Name: "internalapi"} + CreateNetworkAttachmentDefinition(internalAPINADName) + DeferCleanup(DeleteNetworkAttachmentDefinition, internalAPINADName) + th.SimulateJobSuccess(jobName) + + statefulSetName := types.NamespacedName{ + Namespace: namespace, + Name: novaConductorName.Name, + } + SimulateStatefulSetReplicaReadyWithPods( + statefulSetName, + map[string][]string{namespace + "/internalapi": {"10.0.0.1"}}, + ) + + th.ExpectCondition( + novaConductorName, + ConditionGetterFunc(NovaConductorConditionGetter), + condition.NetworkAttachmentsReadyCondition, + corev1.ConditionTrue, + ) + + Eventually(func(g Gomega) { + novaConductor := GetNovaConductor(novaConductorName) + g.Expect(novaConductor.Status.NetworkAttachments).To( + Equal(map[string][]string{namespace + "/internalapi": {"10.0.0.1"}})) + }, timeout, interval).Should(Succeed()) + + th.ExpectCondition( + novaConductorName, + ConditionGetterFunc(NovaConductorConditionGetter), + condition.ReadyCondition, + corev1.ConditionTrue, + ) + }) + }) }) From 0238b57cba934170eda833d81c96043ad9debbfa Mon Sep 17 00:00:00 2001 From: Balazs Gibizer Date: Mon, 20 Feb 2023 14:09:57 +0100 Subject: [PATCH 5/6] [envtest]Coverage for NovaScheduler.NetworkAttachments --- test/functional/nova_scheduler_test.go | 139 +++++++++++++++++++++++++ 1 file changed, 139 insertions(+) diff --git a/test/functional/nova_scheduler_test.go b/test/functional/nova_scheduler_test.go index 6b25bd823..8e6528589 100644 --- a/test/functional/nova_scheduler_test.go +++ b/test/functional/nova_scheduler_test.go @@ -14,10 +14,12 @@ limitations under the License. package functional_test import ( + "encoding/json" "fmt" "os" "github.com/google/uuid" + networkv1 "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" condition "github.com/openstack-k8s-operators/lib-common/modules/common/condition" @@ -300,4 +302,141 @@ var _ = Describe("NovaScheduler controller", func() { ) }) }) + When("NovaScheduler is created with networkAttachments", func() { + BeforeEach(func() { + DeferCleanup( + k8sClient.Delete, ctx, CreateNovaAPISecret(namespace, SecretName)) + + spec := GetDefaultNovaSchedulerSpec() + spec["networkAttachments"] = []string{"internalapi"} + novaSchedulerName = CreateNovaScheduler(namespace, spec) + DeferCleanup(DeleteNovaScheduler, novaSchedulerName) + }) + + It("reports that the definition is missing", func() { + th.ExpectConditionWithDetails( + novaSchedulerName, + ConditionGetterFunc(NovaSchedulerConditionGetter), + condition.NetworkAttachmentsReadyCondition, + corev1.ConditionFalse, + condition.RequestedReason, + "NetworkAttachment resources missing: internalapi", + ) + th.ExpectCondition( + novaSchedulerName, + ConditionGetterFunc(NovaSchedulerConditionGetter), + condition.ReadyCondition, + corev1.ConditionUnknown, + ) + }) + It("reports that network attachment is missing", func() { + internalAPINADName := types.NamespacedName{Namespace: namespace, Name: "internalapi"} + CreateNetworkAttachmentDefinition(internalAPINADName) + DeferCleanup(DeleteNetworkAttachmentDefinition, internalAPINADName) + + statefulSetName := types.NamespacedName{ + Namespace: namespace, + Name: novaSchedulerName.Name, + } + ss := th.GetStatefulSet(statefulSetName) + + expectedAnnotation, err := json.Marshal( + []networkv1.NetworkSelectionElement{ + { + Name: "internalapi", + Namespace: namespace, + }}) + Expect(err).ShouldNot(HaveOccurred()) + Expect(ss.Spec.Template.ObjectMeta.Annotations).To( + HaveKeyWithValue("k8s.v1.cni.cncf.io/networks", string(expectedAnnotation)), + ) + + // We don't add network attachment status annotations to the Pods + // to simulate that the network attachments are missing. + SimulateStatefulSetReplicaReadyWithPods(statefulSetName, map[string][]string{}) + + th.ExpectConditionWithDetails( + novaSchedulerName, + ConditionGetterFunc(NovaSchedulerConditionGetter), + condition.NetworkAttachmentsReadyCondition, + corev1.ConditionFalse, + condition.ErrorReason, + "NetworkAttachments error occured "+ + "not all pods have interfaces with ips as configured in NetworkAttachments: [internalapi]", + ) + }) + It("reports that an IP is missing", func() { + internalAPINADName := types.NamespacedName{Namespace: namespace, Name: "internalapi"} + CreateNetworkAttachmentDefinition(internalAPINADName) + DeferCleanup(DeleteNetworkAttachmentDefinition, internalAPINADName) + + statefulSetName := types.NamespacedName{ + Namespace: namespace, + Name: novaSchedulerName.Name, + } + ss := th.GetStatefulSet(statefulSetName) + + expectedAnnotation, err := json.Marshal( + []networkv1.NetworkSelectionElement{ + { + Name: "internalapi", + Namespace: namespace, + }}) + Expect(err).ShouldNot(HaveOccurred()) + Expect(ss.Spec.Template.ObjectMeta.Annotations).To( + HaveKeyWithValue("k8s.v1.cni.cncf.io/networks", string(expectedAnnotation)), + ) + + // We simulat that there is no IP associated with the internalapi + // network attachment + SimulateStatefulSetReplicaReadyWithPods( + statefulSetName, + map[string][]string{namespace + "/internalapi": {}}, + ) + + th.ExpectConditionWithDetails( + novaSchedulerName, + ConditionGetterFunc(NovaSchedulerConditionGetter), + condition.NetworkAttachmentsReadyCondition, + corev1.ConditionFalse, + condition.ErrorReason, + "NetworkAttachments error occured "+ + "not all pods have interfaces with ips as configured in NetworkAttachments: [internalapi]", + ) + }) + It("reports NetworkAttachmentsReady if the Pods got the proper annotiations", func() { + internalAPINADName := types.NamespacedName{Namespace: namespace, Name: "internalapi"} + CreateNetworkAttachmentDefinition(internalAPINADName) + DeferCleanup(DeleteNetworkAttachmentDefinition, internalAPINADName) + + statefulSetName := types.NamespacedName{ + Namespace: namespace, + Name: novaSchedulerName.Name, + } + SimulateStatefulSetReplicaReadyWithPods( + statefulSetName, + map[string][]string{namespace + "/internalapi": {"10.0.0.1"}}, + ) + + th.ExpectCondition( + novaSchedulerName, + ConditionGetterFunc(NovaSchedulerConditionGetter), + condition.NetworkAttachmentsReadyCondition, + corev1.ConditionTrue, + ) + + Eventually(func(g Gomega) { + novaScheduler := GetNovaScheduler(novaSchedulerName) + g.Expect(novaScheduler.Status.NetworkAttachments).To( + Equal(map[string][]string{namespace + "/internalapi": {"10.0.0.1"}})) + }, timeout, interval).Should(Succeed()) + + th.ExpectCondition( + novaSchedulerName, + ConditionGetterFunc(NovaSchedulerConditionGetter), + condition.ReadyCondition, + corev1.ConditionTrue, + ) + }) + }) }) From 0203e54fb009c937de0181602849041a877c532c Mon Sep 17 00:00:00 2001 From: Balazs Gibizer Date: Thu, 23 Feb 2023 11:54:18 +0100 Subject: [PATCH 6/6] [envtest]Ensure network related config is passed down Adds test coverage to show that NetworkAttachments and ExternalEndpoints defined on the Nova CR level are passed down to the sub CRs (NovaAPI, NovaConductor, NovaScheduler) --- test/functional/nova_controller_test.go | 98 +++++++++++++++++++++++++ 1 file changed, 98 insertions(+) diff --git a/test/functional/nova_controller_test.go b/test/functional/nova_controller_test.go index c034f1129..e4d1a9970 100644 --- a/test/functional/nova_controller_test.go +++ b/test/functional/nova_controller_test.go @@ -622,4 +622,102 @@ var _ = Describe("Nova controller", func() { Expect(cell0DB.Finalizers).NotTo(ContainElement("Nova")) }) }) + When("Nova CR instance is created with NetworkAttachment and ExternalEndpoints", func() { + BeforeEach(func() { + DeferCleanup( + k8sClient.Delete, ctx, CreateNovaSecret(namespace, SecretName)) + DeferCleanup( + k8sClient.Delete, + ctx, + CreateNovaMessageBusSecret(namespace, MessageBusSecretName), + ) + DeferCleanup( + DeleteDBService, + CreateDBService( + namespace, + "openstack", + corev1.ServiceSpec{ + Ports: []corev1.ServicePort{{Port: 3306}}, + }, + ), + ) + DeferCleanup(DeleteKeystoneAPI, CreateKeystoneAPI(namespace)) + + internalAPINADName := types.NamespacedName{Namespace: namespace, Name: "internalapi"} + CreateNetworkAttachmentDefinition(internalAPINADName) + DeferCleanup(DeleteNetworkAttachmentDefinition, internalAPINADName) + + var externalEndpoints []interface{} + externalEndpoints = append( + externalEndpoints, map[string]interface{}{ + "endpoint": "internal", + "ipAddressPool": "osp-internalapi", + "loadBalancerIPs": []string{"10.1.0.1", "10.1.0.2"}, + }, + ) + rawSpec := map[string]interface{}{ + "secret": SecretName, + "cellTemplates": map[string]interface{}{ + "cell0": map[string]interface{}{ + "cellDatabaseUser": "nova_cell0", + "hasAPIAccess": true, + "conductorServiceTemplate": map[string]interface{}{ + "networkAttachments": []string{"internalapi"}, + }, + }, + }, + "apiServiceTemplate": map[string]interface{}{ + "networkAttachments": []string{"internalapi"}, + "externalEndpoints": externalEndpoints, + }, + "schedulerServiceTemplate": map[string]interface{}{ + "networkAttachments": []string{"internalapi"}, + }, + } + CreateNova(novaName, rawSpec) + DeferCleanup(DeleteNova, novaName) + + SimulateKeystoneServiceReady(novaKeystoneServiceName) + SimulateMariaDBDatabaseCompleted(mariaDBDatabaseNameForAPI) + SimulateMariaDBDatabaseCompleted(mariaDBDatabaseNameForCell0) + SimulateTransportURLReady(apiTransportURLName) + th.SimulateJobSuccess(cell0DBSyncJobName) + }) + + It("creates all the sub CRs and passes down the network parameters", func() { + SimulateStatefulSetReplicaReadyWithPods( + novaCell0ConductorStatefulSetName, + map[string][]string{namespace + "/internalapi": {"10.0.0.1"}}, + ) + SimulateStatefulSetReplicaReadyWithPods( + novaSchedulerStatefulSetName, + map[string][]string{namespace + "/internalapi": {"10.0.0.1"}}, + ) + SimulateStatefulSetReplicaReadyWithPods( + novaAPIdeploymentName, + map[string][]string{namespace + "/internalapi": {"10.0.0.1"}}, + ) + + th.ExpectCondition( + novaName, + ConditionGetterFunc(NovaConditionGetter), + condition.ReadyCondition, + corev1.ConditionTrue, + ) + + nova := GetNova(novaName) + + conductor := GetNovaConductor(cell0ConductorName) + Expect(conductor.Spec.NetworkAttachments).To( + Equal(nova.Spec.CellTemplates["cell0"].ConductorServiceTemplate.NetworkAttachments)) + + api := GetNovaAPI(novaAPIName) + Expect(api.Spec.NetworkAttachments).To(Equal(nova.Spec.APIServiceTemplate.NetworkAttachments)) + Expect(api.Spec.ExternalEndpoints).To(Equal(nova.Spec.APIServiceTemplate.ExternalEndpoints)) + + scheduler := GetNovaScheduler(novaSchedulerName) + Expect(scheduler.Spec.NetworkAttachments).To(Equal(nova.Spec.APIServiceTemplate.NetworkAttachments)) + }) + + }) })