From 051e5cca5cb8240ed923e1fc784d0801d5c13997 Mon Sep 17 00:00:00 2001 From: Matej Kralik Date: Fri, 18 Oct 2024 10:35:38 +0200 Subject: [PATCH] [OSSM-8259] Add test for CNI annotation + create function to getting all names of a particular resource (#755) --- pkg/tests/non-dependant/olm_webhooks_test.go | 2 +- pkg/tests/ossm/deploy_on_infra_test.go | 4 +-- pkg/tests/ossm/ior_test.go | 6 +---- pkg/tests/ossm/smcp_annotation_test.go | 21 +++------------ pkg/tests/ossm/smcp_must_gather_test.go | 2 +- pkg/tests/ossm/smoke_test.go | 28 ++++++++++++++++++-- pkg/util/oc/oc.go | 18 +++++++++++-- pkg/util/oc/oc_struct.go | 15 ++++++----- pkg/util/oc/pod_commands.go | 20 ++++++++++++++ pkg/util/pod/pods.go | 15 ++++++----- 10 files changed, 87 insertions(+), 44 deletions(-) diff --git a/pkg/tests/non-dependant/olm_webhooks_test.go b/pkg/tests/non-dependant/olm_webhooks_test.go index 48b5b856..64d729bb 100644 --- a/pkg/tests/non-dependant/olm_webhooks_test.go +++ b/pkg/tests/non-dependant/olm_webhooks_test.go @@ -125,6 +125,6 @@ func checkSmcpWebhooksDoesNotExist(t TestHelper, kind string, label string) { } func deleteGlobalWebhook(t TestHelper, kind string, label string) { - name := oc.GetResouceNameByLabel(t, "", kind, label) + name := oc.GetAllResoucesNamesByLabel(t, "", kind, label)[0] oc.DeleteResource(t, "", kind, name) } diff --git a/pkg/tests/ossm/deploy_on_infra_test.go b/pkg/tests/ossm/deploy_on_infra_test.go index 36133595..19371061 100644 --- a/pkg/tests/ossm/deploy_on_infra_test.go +++ b/pkg/tests/ossm/deploy_on_infra_test.go @@ -200,9 +200,9 @@ func assertPodScheduledToNode(t test.TestHelper, pLabel string) { } func pickWorkerNode(t test.TestHelper) string { - workerNodes := shell.Execute(t, "oc get nodes -l node-role.kubernetes.io/worker= -o jsonpath='{.items[*].metadata.name}'") + workerNodes := oc.GetAllResoucesNamesByLabel(t, "", "nodes", "node-role.kubernetes.io/worker=") operatorNode := shell.Execute(t, "oc get pods -n openshift-operators -l name=istio-operator -o jsonpath='{.items[0].spec.nodeName}'") - for _, node := range strings.Split(workerNodes, " ") { + for _, node := range workerNodes { node = strings.TrimSpace(node) if node != operatorNode { return node diff --git a/pkg/tests/ossm/ior_test.go b/pkg/tests/ossm/ior_test.go index f456a9d0..897be75d 100644 --- a/pkg/tests/ossm/ior_test.go +++ b/pkg/tests/ossm/ior_test.go @@ -394,11 +394,7 @@ func getRoutes(t test.TestHelper, ns string) []Route { } func getRouteNames(t test.TestHelper, ns string) []string { - return strings.Split( - shell.Executef(t, - "oc -n %s get --selector 'maistra.io/generated-by=ior' --output 'jsonpath={.items[*].metadata.name}' route", - ns), - " ") + return oc.GetAllResoucesNamesByLabel(t, ns, "route", "maistra.io/generated-by=ior") } func buildManagedRouteYamlDocument(t test.TestHelper, ns string) string { diff --git a/pkg/tests/ossm/smcp_annotation_test.go b/pkg/tests/ossm/smcp_annotation_test.go index 6c12ca20..54fac597 100644 --- a/pkg/tests/ossm/smcp_annotation_test.go +++ b/pkg/tests/ossm/smcp_annotation_test.go @@ -18,8 +18,6 @@ import ( _ "embed" "testing" - "gopkg.in/yaml.v2" - "github.com/maistra/maistra-test-tool/pkg/util/ns" "github.com/maistra/maistra-test-tool/pkg/util/oc" "github.com/maistra/maistra-test-tool/pkg/util/pod" @@ -30,6 +28,7 @@ import ( func TestSMCPAnnotations(t *testing.T) { test.NewTest(t).Id("T29").Groups(test.Full, test.ARM).Run(func(t test.TestHelper) { t.Log("Test annotations: verify deployment with sidecar.maistra.io/proxyEnv annotations and Enable automatic injection in SMCP to propagate the annotations to the sidecar") + t.Log("See https://issues.redhat.com/browse/OSSM-1074") DeployControlPlane(t) // TODO: move this to individual subtests and integrate patch if one exists @@ -78,26 +77,12 @@ func TestSMCPAnnotations(t *testing.T) { } func VerifyAndGetPodAnnotation(t test.TestHelper, podLocator oc.PodLocatorFunc) map[string]string { - var data struct { - Metadata struct { - Annotations map[string]string `yaml:"annotations"` - } `yaml:"metadata"` - } - - po := podLocator(t, oc.DefaultOC) - yamlString := oc.GetYaml(t, po.Namespace, "pod", po.Name) - err := yaml.Unmarshal([]byte(yamlString), &data) - if err != nil { - t.Fatalf("Failed to unmarshal YAML: %s", err) - } - - annotations := data.Metadata.Annotations + annotations := oc.GetPodAnnotations(t, podLocator) if len(annotations) == 0 { oc.DeletePod(t, podLocator) oc.WaitPodReady(t, podLocator) - t.Fatalf("Failed to get annotations from pod %s", po.Name) + t.Fatalf("Failed to get annotations from pod. The pod has 0 annotations") } - return annotations } diff --git a/pkg/tests/ossm/smcp_must_gather_test.go b/pkg/tests/ossm/smcp_must_gather_test.go index 841ec094..d3bc8121 100644 --- a/pkg/tests/ossm/smcp_must_gather_test.go +++ b/pkg/tests/ossm/smcp_must_gather_test.go @@ -136,7 +136,7 @@ func TestMustGather(t *testing.T) { } for webhook, kind := range webhookMap { - name := oc.GetResouceNameByLabel(t, "", kind, fmt.Sprintf("olm.webhook-description-generate-name=%s", webhook)) + name := oc.GetAllResoucesNamesByLabel(t, "", kind, fmt.Sprintf("olm.webhook-description-generate-name=%s", webhook))[0] filename := fmt.Sprintf("%s.yaml", name) assertFilesExist(t, dir, diff --git a/pkg/tests/ossm/smoke_test.go b/pkg/tests/ossm/smoke_test.go index 6dbeae06..71cde090 100644 --- a/pkg/tests/ossm/smoke_test.go +++ b/pkg/tests/ossm/smoke_test.go @@ -27,7 +27,9 @@ import ( "github.com/maistra/maistra-test-tool/pkg/util/env" "github.com/maistra/maistra-test-tool/pkg/util/ns" "github.com/maistra/maistra-test-tool/pkg/util/oc" + "github.com/maistra/maistra-test-tool/pkg/util/pod" "github.com/maistra/maistra-test-tool/pkg/util/retry" + "github.com/maistra/maistra-test-tool/pkg/util/shell" "github.com/maistra/maistra-test-tool/pkg/util/version" . "github.com/maistra/maistra-test-tool/pkg/util/test" @@ -148,6 +150,28 @@ func checkSMCP(t TestHelper, ns string) { t.LogStep("verify proxy startup time. Expected to be less than 10 seconds") t.Log("Jira related: https://issues.redhat.com/browse/OSSM-3586") assertProxiesReadyInLessThan10Seconds(t, ns) + + t.LogStep("Check that CNI pods for each nodes have been created") + nodesNames := oc.GetAllResoucesNames(t, "", "nodes") + cniNames := oc.GetAllResoucesNamesByLabel(t, "openshift-operators", "pods", fmt.Sprintf("k8s-app=istio-cni-node-v%d-%d", env.GetSMCPVersion().Major, env.GetSMCPVersion().Minor)) + if len(nodesNames) != len(cniNames) { + t.Errorf("Number of nodes and number of CNI is not the same! Nodes: %s CNI pods: %s", nodesNames, cniNames) + } + t.LogStep("Check that CNI pods have the correct priority class name and don't have unsupported annotation") + t.Log("Jira related: https://issues.redhat.com/browse/OSSM-1162") + for _, cniPodName := range cniNames { + annotations := oc.GetPodAnnotations(t, pod.MatchingName("openshift-operators", cniPodName)) + _, exists := annotations["scheduler.alpha.kubernetes.io"] + if exists { + t.Errorf("The CNI pod %s contains unsupported annotation scheduler.alpha.kubernetes.io!") + } + shell.Execute(t, + fmt.Sprintf(`oc get pod -n openshift-operators %s -o jsonpath='{.spec.priorityClassName}'`, cniPodName), + assert.OutputContains( + "system-node-critical", + fmt.Sprintf("CNI pod %s contains spec.priorityClassName: system-node-critical", cniPodName), + fmt.Sprintf("CNI pod %s doesn't contain spec.priorityClassName: system-node-critical", cniPodName))) + } } func assertJaegerAndTracingSettings(t TestHelper) { @@ -202,9 +226,9 @@ func assertTrafficFlowsThroughProxy(t TestHelper, ns string) { func assertProxiesReadyInLessThan10Seconds(t TestHelper, ns string) { t.Log("Extracting proxy startup time and last transition time for all the pods in the namespace") - podsList := oc.GetJson(t, ns, "pods", "", `{.items[*].metadata.name}`) + podsNamesList := oc.GetAllResoucesNamesByLabel(t, ns, "pods", "") - for _, podName := range strings.Split(podsList, " ") { + for _, podName := range podsNamesList { // skip sleep pod because it doesn't have a proxy if strings.Contains(podName, "sleep") { continue diff --git a/pkg/util/oc/oc.go b/pkg/util/oc/oc.go index f138ccb8..c0c71ed8 100644 --- a/pkg/util/oc/oc.go +++ b/pkg/util/oc/oc.go @@ -89,9 +89,18 @@ func DeleteResource(t test.TestHelper, ns string, kind string, name ...string) { DefaultOC.DeleteResource(t, ns, kind, name...) } -func GetResouceNameByLabel(t test.TestHelper, ns string, kind string, label string) string { +// Function returns names of all resources (kind input) in the namespace (ns input) that match a particular label (label input). +// When you are looking for a global scoped resource (e.g. nodes), ns can be empty +func GetAllResoucesNamesByLabel(t test.TestHelper, ns string, kind string, label string) []string { t.T().Helper() - return DefaultOC.GetResouceNameByLabel(t, ns, kind, label) + return DefaultOC.GetAllResoucesNames(t, ns, kind, label) +} + +// Function returns names of all resources (kind input) in the namespace (ns input). +// When you are looking for a global scoped resource (e.g. nodes), ns can be empty +func GetAllResoucesNames(t test.TestHelper, ns string, kind string) []string { + t.T().Helper() + return DefaultOC.GetAllResoucesNames(t, ns, kind, "") } func ResourceByLabelExists(t test.TestHelper, ns string, kind string, label string) bool { @@ -300,3 +309,8 @@ func WaitUntilResourceExist(t test.TestHelper, ns string, kind string, name stri t.T().Helper() DefaultOC.WaitUntilResourceExist(t, ns, kind, name) } + +func GetPodAnnotations(t test.TestHelper, podLocator PodLocatorFunc) map[string]string { + t.T().Helper() + return DefaultOC.GetPodAnnotations(t, podLocator) +} diff --git a/pkg/util/oc/oc_struct.go b/pkg/util/oc/oc_struct.go index 5bdb9a5e..267b7729 100644 --- a/pkg/util/oc/oc_struct.go +++ b/pkg/util/oc/oc_struct.go @@ -488,18 +488,21 @@ func (o OC) ResourceExists(t test.TestHelper, ns, kind, name string) bool { return exists } -func (o OC) GetResouceNameByLabel(t test.TestHelper, ns, kind, label string) string { +// Function returns names of all resources (kind input) in the namespace (ns input) that match a particular label (label input). +// Label input can be an empty string, and then all resources in the namespace are returned +// When you are looking for a global scoped resource (e.g. nodes), ns can be empty +func (o OC) GetAllResoucesNames(t test.TestHelper, ns, kind, label string) []string { t.T().Helper() - var value string + var values []string o.withKubeconfig(t, func() { t.T().Helper() - value = shell.Execute(t, fmt.Sprintf("oc %s get %s -l %s -o custom-columns=NAME:.metadata.name --no-headers || true", nsFlag(ns), kind, label)) - value = strings.TrimSpace(value) - if value == "" { + output := shell.Execute(t, fmt.Sprintf("oc %s get %s -l '%s' -o jsonpath='{.items[*].metadata.name}' || true", nsFlag(ns), kind, label)) + if output == "" { t.Fatalf("Could not find resource %s with label %s in namespace %s", kind, label, ns) } + values = strings.Split(output, " ") }) - return value + return values } func (o OC) ResourceByLabelExists(t test.TestHelper, ns, kind, label string) bool { diff --git a/pkg/util/oc/pod_commands.go b/pkg/util/oc/pod_commands.go index f4b3a32e..d7157476 100644 --- a/pkg/util/oc/pod_commands.go +++ b/pkg/util/oc/pod_commands.go @@ -20,6 +20,8 @@ import ( "strings" "time" + "gopkg.in/yaml.v2" + "github.com/maistra/maistra-test-tool/pkg/util/check/assert" "github.com/maistra/maistra-test-tool/pkg/util/check/common" "github.com/maistra/maistra-test-tool/pkg/util/check/require" @@ -246,3 +248,21 @@ func (o OC) WaitUntilResourceExist(t test.TestHelper, ns string, kind string, na }) }) } + +func (o OC) GetPodAnnotations(t test.TestHelper, podLocator PodLocatorFunc) map[string]string { + var data struct { + Metadata struct { + Annotations map[string]string `yaml:"annotations"` + } `yaml:"metadata"` + } + + po := podLocator(t, &o) + yamlString := o.GetYaml(t, po.Namespace, "pod", po.Name) + err := yaml.Unmarshal([]byte(yamlString), &data) + if err != nil { + t.Fatalf("Failed to unmarshal YAML: %s", err) + } + + annotations := data.Metadata.Annotations + return annotations +} diff --git a/pkg/util/pod/pods.go b/pkg/util/pod/pods.go index 7b684941..45b3cdc5 100644 --- a/pkg/util/pod/pods.go +++ b/pkg/util/pod/pods.go @@ -15,19 +15,13 @@ package pod import ( - "strings" - ocpackage "github.com/maistra/maistra-test-tool/pkg/util/oc" "github.com/maistra/maistra-test-tool/pkg/util/test" ) func getPods(t test.TestHelper, oc *ocpackage.OC, selector string, ns string) []ocpackage.NamespacedName { t.T().Helper() - output := oc.Invokef(t, "kubectl -n %s get pods -l %q -o jsonpath='{.items[*].metadata.name}'", ns, selector) - if output == "" { - t.Fatalf("no pods found using selector %s in namespace %s", selector, ns) - } - pods := strings.Split(output, " ") + pods := oc.GetAllResoucesNames(t, ns, "pods", selector) var namespacedNames []ocpackage.NamespacedName for _, pod := range pods { namespacedNames = append(namespacedNames, ocpackage.NewNamespacedName(ns, pod)) @@ -56,3 +50,10 @@ func MatchingSelectorFirst(selector string, ns string) ocpackage.PodLocatorFunc return pods[0] } } + +// "placeholder", when we know the name of the pod and the namespace but we want to use a different function which takes PodLocatorFunc as an input parameter +func MatchingName(ns string, name string) ocpackage.PodLocatorFunc { + return func(t test.TestHelper, oc *ocpackage.OC) ocpackage.NamespacedName { + return ocpackage.NewNamespacedName(ns, name) + } +}