From 08d5f65b4c681c1ea166bfa0d5b015adb3fe3ba9 Mon Sep 17 00:00:00 2001 From: Soule BA Date: Tue, 16 Jul 2024 17:08:14 +0200 Subject: [PATCH] chore: Add unit test for nv-ipam-cni objects rendering and sync Signed-off-by: Soule BA --- pkg/state/common_test.go | 113 ++++++++++-- pkg/state/state_nv_ipam_cni.go | 2 +- pkg/state/state_nv_ipam_cni_test.go | 265 ++++++++++++++++++++++++++-- 3 files changed, 345 insertions(+), 35 deletions(-) diff --git a/pkg/state/common_test.go b/pkg/state/common_test.go index 7d0eee624..4c07e6174 100644 --- a/pkg/state/common_test.go +++ b/pkg/state/common_test.go @@ -25,18 +25,28 @@ import ( mellanoxv1alpha1 "github.com/Mellanox/network-operator/api/v1alpha1" clustertype_mocks "github.com/Mellanox/network-operator/pkg/clustertype/mocks" + "github.com/Mellanox/network-operator/pkg/consts" "github.com/Mellanox/network-operator/pkg/state" "github.com/Mellanox/network-operator/pkg/staticconfig" staticconfig_mocks "github.com/Mellanox/network-operator/pkg/staticconfig/mocks" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" + k8serrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/resource" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/log" ) +const ( + defaultTestRepository = "myRepo" + defaultTestImage = "myImage" + defaultTestVersion = "myVersion" +) + func getTestCatalog() state.InfoCatalog { catalog := state.NewInfoCatalog() clusterTypeProvider := clustertype_mocks.Provider{} @@ -88,42 +98,53 @@ func assertCommonPodTemplateFields(template *corev1.PodTemplateSpec, image *mell // Container Resources Expect(template.Spec.Containers[0].Resources.Limits).To(Equal(image.ContainerResources[0].Limits)) Expect(template.Spec.Containers[0].Resources.Requests).To(Equal(image.ContainerResources[0].Requests)) + + Expect(template.Spec.Tolerations).To(ContainElements( + corev1.Toleration{ + Key: "nvidia.com/gpu", + Operator: "Exists", + Value: "", + Effect: "NoSchedule", + TolerationSeconds: nil, + }, + )) } -func assertCommonDeploymentFields(u *unstructured.Unstructured, image *mellanoxv1alpha1.ImageSpec) { +func assertCommonDeploymentFieldsFromUnstructured(u *unstructured.Unstructured, image *mellanoxv1alpha1.ImageSpec) { d := &appsv1.Deployment{} err := runtime.DefaultUnstructuredConverter.FromUnstructured(u.UnstructuredContent(), d) Expect(err).ToNot(HaveOccurred()) + assertCommonDeploymentFields(d, image) +} + +func assertCommonDeploymentFields(d *appsv1.Deployment, image *mellanoxv1alpha1.ImageSpec) { assertCommonPodTemplateFields(&d.Spec.Template, image) } -func assertCommonDaemonSetFields(u *unstructured.Unstructured, +func assertCommonDaemonSetFieldsFromUnstructured(u *unstructured.Unstructured, image *mellanoxv1alpha1.ImageSpec, policy *mellanoxv1alpha1.NicClusterPolicy) { ds := &appsv1.DaemonSet{} err := runtime.DefaultUnstructuredConverter.FromUnstructured(u.UnstructuredContent(), ds) Expect(err).ToNot(HaveOccurred()) + assertCommonDaemonSetFields(ds, image, policy) +} + +func assertCommonDaemonSetFields(ds *appsv1.DaemonSet, image *mellanoxv1alpha1.ImageSpec, + policy *mellanoxv1alpha1.NicClusterPolicy) { assertCommonPodTemplateFields(&ds.Spec.Template, image) - // Tolerations + Expect(ds.Spec.Template.Spec.Tolerations).To(ContainElements( corev1.Toleration{Key: "first-taint"}, - corev1.Toleration{ - Key: "nvidia.com/gpu", - Operator: "Exists", - Value: "", - Effect: "NoSchedule", - TolerationSeconds: nil, - }, )) - // NodeAffinity Expect(ds.Spec.Template.Spec.Affinity.NodeAffinity).To(Equal(policy.Spec.NodeAffinity)) } func getTestImageSpec() *mellanoxv1alpha1.ImageSpec { return &mellanoxv1alpha1.ImageSpec{ - Image: "image-one", - Repository: "repository", - Version: "five", + Image: defaultTestImage, + Repository: defaultTestRepository, + Version: defaultTestVersion, ImagePullSecrets: []string{"secret-one", "secret-two"}, } } @@ -148,10 +169,14 @@ func isNamespaced(obj *unstructured.Unstructured) bool { obj.GetKind() != "ValidatingWebhookConfiguration" } -func assertCNIBinDirForDS(u *unstructured.Unstructured) { +func assertCNIBinDirForDSFromUnstructured(u *unstructured.Unstructured) { ds := &appsv1.DaemonSet{} err := runtime.DefaultUnstructuredConverter.FromUnstructured(u.UnstructuredContent(), ds) Expect(err).ToNot(HaveOccurred()) + assertCNIBinDirForDS(ds) +} + +func assertCNIBinDirForDS(ds *appsv1.DaemonSet) { for i := range ds.Spec.Template.Spec.Volumes { vol := ds.Spec.Template.Spec.Volumes[i] if vol.Name == "cnibin" { @@ -171,10 +196,10 @@ func GetManifestObjectsTest(ctx context.Context, cr *mellanoxv1alpha1.NicCluster } switch got[i].GetKind() { case "DaemonSet": - assertCommonDaemonSetFields(got[i], imageSpec, cr) - assertCNIBinDirForDS(got[i]) + assertCommonDaemonSetFieldsFromUnstructured(got[i], imageSpec, cr) + assertCNIBinDirForDSFromUnstructured(got[i]) case "Deployment": - assertCommonDeploymentFields(got[i], imageSpec) + assertCommonDeploymentFieldsFromUnstructured(got[i], imageSpec) } } } @@ -201,3 +226,55 @@ func getTestClusterPolicyWithBaseFields() *mellanoxv1alpha1.NicClusterPolicy { }, } } + +func getKindState(ctx context.Context, c client.Client, objs []*unstructured.Unstructured, + targetKind string) (state.SyncState, error) { + reqLogger := log.FromContext(ctx) + reqLogger.V(consts.LogLevelInfo).Info("Checking related object states") + for _, obj := range objs { + if obj.GetKind() != targetKind { + continue + } + found := obj.DeepCopy() + err := c.Get( + ctx, types.NamespacedName{Name: found.GetName(), Namespace: found.GetNamespace()}, found) + if err != nil { + if k8serrors.IsNotFound(err) { + return state.SyncStateNotReady, nil + } + return state.SyncStateNotReady, fmt.Errorf("failed to get object: %w", err) + } + + buf, err := found.MarshalJSON() + if err != nil { + return state.SyncStateNotReady, fmt.Errorf("failed to marshall unstructured daemonset object: %w", err) + } + + switch obj.GetKind() { + case "DaemonSet": + ds := &appsv1.DaemonSet{} + if err = json.Unmarshal(buf, ds); err != nil { + return state.SyncStateNotReady, fmt.Errorf("failed to unmarshall to daemonset object: %w", err) + } + if ds.Status.DesiredNumberScheduled != 0 && ds.Status.DesiredNumberScheduled == ds.Status.NumberAvailable && + ds.Status.UpdatedNumberScheduled == ds.Status.NumberAvailable { + return state.SyncStateReady, nil + } + return state.SyncStateNotReady, nil + case "Deployment": + d := &appsv1.Deployment{} + if err = json.Unmarshal(buf, d); err != nil { + return state.SyncStateNotReady, fmt.Errorf("failed to unmarshall to deployment object: %w", err) + } + + if d.Status.ObservedGeneration > 0 && d.Status.Replicas == d.Status.ReadyReplicas && + d.Status.UpdatedReplicas == d.Status.AvailableReplicas { + return state.SyncStateReady, nil + } + return state.SyncStateNotReady, nil + default: + return state.SyncStateNotReady, fmt.Errorf("unsupported target kind") + } + } + return state.SyncStateNotReady, fmt.Errorf("objects list does not contain the specified target kind") +} diff --git a/pkg/state/state_nv_ipam_cni.go b/pkg/state/state_nv_ipam_cni.go index 99c7f6db8..7b8d2a82d 100644 --- a/pkg/state/state_nv_ipam_cni.go +++ b/pkg/state/state_nv_ipam_cni.go @@ -71,7 +71,7 @@ type NVIPAMManifestRenderData struct { // //nolint:dupl func (s *stateNVIPAMCNI) Sync( - ctx context.Context, customResource interface{}, infoCatalog InfoCatalog) (SyncState, error) { + ctx context.Context, customResource any, infoCatalog InfoCatalog) (SyncState, error) { reqLogger := log.FromContext(ctx) cr := customResource.(*mellanoxv1alpha1.NicClusterPolicy) reqLogger.V(consts.LogLevelInfo).Info( diff --git a/pkg/state/state_nv_ipam_cni_test.go b/pkg/state/state_nv_ipam_cni_test.go index 82feead88..448adad02 100644 --- a/pkg/state/state_nv_ipam_cni_test.go +++ b/pkg/state/state_nv_ipam_cni_test.go @@ -21,33 +21,266 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + admv1 "k8s.io/api/admissionregistration/v1" + appsv1 "k8s.io/api/apps/v1" + v1 "k8s.io/api/core/v1" + rbacv1 "k8s.io/api/rbac/v1" + "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + "k8s.io/utils/ptr" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/log" + "github.com/Mellanox/network-operator/pkg/config" "github.com/Mellanox/network-operator/pkg/state" + staticconfig_mocks "github.com/Mellanox/network-operator/pkg/staticconfig/mocks" mellanoxv1alpha1 "github.com/Mellanox/network-operator/api/v1alpha1" "github.com/Mellanox/network-operator/pkg/staticconfig" ) var _ = Describe("NVIPAM Controller", func() { - ctx := context.Background() + var ( + nvIpamState state.State + catalog state.InfoCatalog + client client.Client + namespace string + renderer state.ManifestRenderer + ) - imageSpec := getTestImageSpec() - imageSpec = addContainerResources(imageSpec, "nv-ipam-node", "5", "3") - imageSpec = addContainerResources(imageSpec, "nv-ipam-controller", "5", "3") + BeforeEach(func() { + scheme := runtime.NewScheme() + Expect(mellanoxv1alpha1.AddToScheme(scheme)).NotTo(HaveOccurred()) + Expect(v1.AddToScheme(scheme)).NotTo(HaveOccurred()) + Expect(appsv1.AddToScheme(scheme)).NotTo(HaveOccurred()) + Expect(admv1.AddToScheme(scheme)).NotTo(HaveOccurred()) + Expect(rbacv1.AddToScheme(scheme)).NotTo(HaveOccurred()) + client = fake.NewClientBuilder().WithScheme(scheme).Build() + manifestDir := "../../manifests/state-nv-ipam-cni" + s, r, err := state.NewStateNVIPAMCNI(client, manifestDir) + Expect(err).NotTo(HaveOccurred()) + nvIpamState = s + renderer = r + catalog = getTestCatalog() + catalog.Add(state.InfoTypeStaticConfig, + staticconfig.NewProvider(staticconfig.StaticConfig{CniBinDirectory: "custom-cni-bin-directory"})) + namespace = config.FromEnv().State.NetworkOperatorResourceNamespace + }) + + Context("Verify objects rendering", func() { + It("should create Daemonset - minimal spec", func() { + By("Sync") + cr := getMinimalNicClusterPolicyWithNVIpam("nv-ipam-node") + status, err := nvIpamState.Sync(context.Background(), cr, catalog) + Expect(err).NotTo(HaveOccurred()) + Expect(status).To(BeEquivalentTo(state.SyncStateNotReady)) + By("Verify DaemonSet") + ds := &appsv1.DaemonSet{} + err = client.Get(context.Background(), types.NamespacedName{Namespace: namespace, Name: "nv-ipam-node"}, ds) + Expect(err).NotTo(HaveOccurred()) + assertCommonDaemonSetFields(ds, &cr.Spec.NvIpam.ImageSpec, cr) + assertCNIBinDirForDS(ds) + }) + + It("should create Deployment - minimal spec", func() { + By("Sync") + cr := getMinimalNicClusterPolicyWithNVIpam("nv-ipam-controller") + status, err := nvIpamState.Sync(context.Background(), cr, catalog) + Expect(err).NotTo(HaveOccurred()) + Expect(status).To(BeEquivalentTo(state.SyncStateNotReady)) + By("Verify Deployment") + d := &appsv1.Deployment{} + err = client.Get(context.Background(), types.NamespacedName{Namespace: namespace, Name: "nv-ipam-controller"}, d) + Expect(err).NotTo(HaveOccurred()) + assertCommonDeploymentFields(d, &cr.Spec.NvIpam.ImageSpec) + }) + + It("should create Deployment with Webhook when specified in CR", func() { + By("Sync") + cr := getMinimalNicClusterPolicyWithNVIpam("nv-ipam-controller") + cr.Spec.NvIpam.EnableWebhook = true + status, err := nvIpamState.Sync(context.Background(), cr, catalog) + Expect(err).NotTo(HaveOccurred()) + Expect(status).To(BeEquivalentTo(state.SyncStateNotReady)) + By("Verify Deployment") + d := &appsv1.Deployment{} + err = client.Get(context.Background(), types.NamespacedName{Namespace: namespace, Name: "nv-ipam-controller"}, d) + Expect(err).NotTo(HaveOccurred()) + assertCommonDeploymentFields(d, &cr.Spec.NvIpam.ImageSpec) + By("Verify Webhook") + assertPodTemplateWebhookFields(&d.Spec.Template) + }) + + It("should create ValidatingWebhookConfiguration when specified in CR", func() { + By("Sync") + cr := getMinimalNicClusterPolicyWithNVIpam("nv-ipam-controller") + cr.Spec.NvIpam.EnableWebhook = true + status, err := nvIpamState.Sync(context.Background(), cr, catalog) + Expect(err).NotTo(HaveOccurred()) + Expect(status).To(BeEquivalentTo(state.SyncStateNotReady)) + By("Verify ValidatingWebhookConfiguration") + adm := &admv1.ValidatingWebhookConfiguration{} + err = client.Get(context.Background(), types.NamespacedName{Name: "nv-ipam-validating-webhook-configuration"}, adm) + Expect(err).NotTo(HaveOccurred()) + assertValidatingWebhookFields(adm) + By("Verify ValidatingWebhookConfiguration Service") + sv := &v1.Service{} + err = client.Get(context.Background(), + types.NamespacedName{Namespace: namespace, Name: "nv-ipam-webhook-service"}, sv) + Expect(err).NotTo(HaveOccurred()) + Expect(sv.Spec.Selector).Should(ContainElement("nv-ipam-controller")) + }) + }) + Context("Verify Sync flows", func() { + It("should create Daemonset, update state to Ready", func() { + By("Sync") + cr := getMinimalNicClusterPolicyWithNVIpam("nv-ipam-node") + status, err := nvIpamState.Sync(context.Background(), cr, catalog) + Expect(err).NotTo(HaveOccurred()) + Expect(status).To(BeEquivalentTo(state.SyncStateNotReady)) + By("Verify DaemonSet") + ds := &appsv1.DaemonSet{} + err = client.Get(context.Background(), types.NamespacedName{Namespace: namespace, Name: "nv-ipam-node"}, ds) + Expect(err).NotTo(HaveOccurred()) + assertCommonDaemonSetFields(ds, &cr.Spec.NvIpam.ImageSpec, cr) + assertCNIBinDirForDS(ds) + ds.Status = appsv1.DaemonSetStatus{ + DesiredNumberScheduled: 1, + NumberAvailable: 1, + UpdatedNumberScheduled: 1, + } + By("Update DaemonSet Status, and re-run Sync") + err = client.Status().Update(context.Background(), ds) + Expect(err).NotTo(HaveOccurred()) + By("Verify State is ready") + ctx := context.Background() + objs, err := renderer.GetManifestObjects(ctx, cr, catalog, log.FromContext(ctx)) + Expect(err).NotTo(HaveOccurred()) + status, err = getKindState(ctx, client, objs, "DaemonSet") + Expect(err).NotTo(HaveOccurred()) + Expect(status).To(BeEquivalentTo(state.SyncStateReady)) + }) + + It("should create Daemonset and delete if Spec is nil", func() { + By("Sync") + cr := getMinimalNicClusterPolicyWithNVIpam("nv-ipam-node") + status, err := nvIpamState.Sync(context.Background(), cr, catalog) + Expect(err).NotTo(HaveOccurred()) + Expect(status).To(BeEquivalentTo(state.SyncStateNotReady)) + By("Verify DaemonSet") + ds := &appsv1.DaemonSet{} + err = client.Get(context.Background(), types.NamespacedName{Namespace: namespace, Name: "nv-ipam-node"}, ds) + Expect(err).NotTo(HaveOccurred()) + assertCommonDaemonSetFields(ds, &cr.Spec.NvIpam.ImageSpec, cr) + assertCNIBinDirForDS(ds) + By("Set spec to nil and Sync") + cr.Spec.NvIpam = nil + status, err = nvIpamState.Sync(context.Background(), cr, catalog) + Expect(err).NotTo(HaveOccurred()) + Expect(status).To(BeEquivalentTo(state.SyncStateNotReady)) + By("Verify DaemonSet is deleted") + ds = &appsv1.DaemonSet{} + err = client.Get(context.Background(), types.NamespacedName{Namespace: namespace, Name: "nv-ipam-node"}, ds) + Expect(errors.IsNotFound(err)).To(BeTrue()) + }) + + It("should create Deployment, update state to Ready", func() { + By("Sync") + cr := getMinimalNicClusterPolicyWithNVIpam("nv-ipam-controller") + status, err := nvIpamState.Sync(context.Background(), cr, catalog) + Expect(err).NotTo(HaveOccurred()) + Expect(status).To(BeEquivalentTo(state.SyncStateNotReady)) + By("Verify Deployment") + d := &appsv1.Deployment{} + err = client.Get(context.Background(), types.NamespacedName{Namespace: namespace, Name: "nv-ipam-controller"}, d) + Expect(err).NotTo(HaveOccurred()) + assertCommonDeploymentFields(d, &cr.Spec.NvIpam.ImageSpec) + d.Status = appsv1.DeploymentStatus{ + ObservedGeneration: 1, + Replicas: 1, + UpdatedReplicas: 1, + AvailableReplicas: 1, + ReadyReplicas: 1, + } + By("Update Deployment Status, and re-run Sync") + err = client.Status().Update(context.Background(), d) + Expect(err).NotTo(HaveOccurred()) + By("Verify State is ready") + ctx := context.Background() + objs, err := renderer.GetManifestObjects(ctx, cr, catalog, log.FromContext(ctx)) + Expect(err).NotTo(HaveOccurred()) + status, err = getKindState(ctx, client, objs, "Deployment") + Expect(err).NotTo(HaveOccurred()) + Expect(status).To(BeEquivalentTo(state.SyncStateReady)) + }) + + It("should fail if static config provider not set in catalog", func() { + By("Sync") + catalog := state.NewInfoCatalog() + cr := getMinimalNicClusterPolicyWithNVIpam("nv-ipam-node") + status, err := nvIpamState.Sync(context.Background(), cr, catalog) + Expect(err).To(HaveOccurred()) + Expect(status).To(BeEquivalentTo(state.SyncStateError)) + }) + + It("should fail if clustertype provider not set in catalog", func() { + By("Sync") + catalog := state.NewInfoCatalog() + staticConfigProvider := staticconfig_mocks.Provider{} + staticConfigProvider.On("GetStaticConfig").Return(staticconfig.StaticConfig{CniBinDirectory: ""}) + catalog.Add(state.InfoTypeStaticConfig, &staticConfigProvider) + cr := getMinimalNicClusterPolicyWithNVIpam("nv-ipam-node") + status, err := nvIpamState.Sync(context.Background(), cr, catalog) + Expect(err).To(HaveOccurred()) + Expect(status).To(BeEquivalentTo(state.SyncStateError)) + }) + }) +}) + +func assertValidatingWebhookFields(adm *admv1.ValidatingWebhookConfiguration) { + Expect(adm.Webhooks[0].Name).To(Equal("validate-ippool.nv-ipam.nvidia.com")) + Expect(adm.Webhooks[1].Name).To(Equal("validate-cidrpool.nv-ipam.nvidia.com")) +} + +func assertPodTemplateWebhookFields(template *v1.PodTemplateSpec) { + Expect(template.Spec.Containers[0].Args).Should(ContainElement(ContainSubstring("--webhook=true"))) + Expect(template.Spec.Volumes).To(Equal( + []v1.Volume{ + { + Name: "cert", + VolumeSource: v1.VolumeSource{ + Secret: &v1.SecretVolumeSource{ + SecretName: "nv-ipam-webhook-server-cert", + DefaultMode: ptr.To(int32(420)), + }, + }, + }, + }, + )) + + Expect(template.Spec.Containers[0].VolumeMounts).To(Equal( + []v1.VolumeMount{ + { + Name: "cert", + MountPath: "/tmp/k8s-webhook-server/serving-certs", + ReadOnly: true, + }, + }, + )) +} + +func getMinimalNicClusterPolicyWithNVIpam(name string) *mellanoxv1alpha1.NicClusterPolicy { cr := getTestClusterPolicyWithBaseFields() - cr.Spec.NvIpam = &mellanoxv1alpha1.NVIPAMSpec{ - // TODO(killianmuldoon): Test with the webhook enabled. + cr.Name = "nic-cluster-policy" + + // add an arbitrary resource, this prevent adding defaut cpu,mem limits + imageSpec := addContainerResources(getTestImageSpec(), name, "5", "3") + nvIpamSpec := &mellanoxv1alpha1.NVIPAMSpec{ EnableWebhook: false, ImageSpec: *imageSpec, } - catalog := getTestCatalog() - catalog.Add(state.InfoTypeStaticConfig, - staticconfig.NewProvider(staticconfig.StaticConfig{CniBinDirectory: "custom-cni-bin-directory"})) - - _, s, err := state.NewStateNVIPAMCNI(fake.NewClientBuilder().Build(), "../../manifests/state-nv-ipam-cni") - Expect(err).NotTo(HaveOccurred()) - It("should test that manifests are rendered and fields are set correctly", func() { - GetManifestObjectsTest(ctx, cr, catalog, imageSpec, s) - }) -}) + cr.Spec.NvIpam = nvIpamSpec + return cr +}