From 013b65f2ca046ffc84b67156becf4a6e643f8c57 Mon Sep 17 00:00:00 2001 From: Tatsinnit Date: Sun, 3 Oct 2021 19:14:34 +1300 Subject: [PATCH 1/2] Add GetName() tests. (#121) --- pkg/collector/dns_collector_test.go | 19 ++++++++++++------ pkg/collector/helm_collector_test.go | 19 ++++++++++++------ pkg/collector/kubeobjects_collector_test.go | 19 ++++++++++++------ .../networkoutbound_collector_test.go | 19 ++++++++++++------ pkg/collector/nodelogs_collector_test.go | 19 ++++++++++++------ .../pods_containerlogs_collector_test.go | 20 +++++++++++++------ 6 files changed, 79 insertions(+), 36 deletions(-) diff --git a/pkg/collector/dns_collector_test.go b/pkg/collector/dns_collector_test.go index 7117ee73..ec2423c6 100644 --- a/pkg/collector/dns_collector_test.go +++ b/pkg/collector/dns_collector_test.go @@ -6,14 +6,16 @@ import ( func TestNewDNSCollector(t *testing.T) { tests := []struct { - name string - want int - wantErr bool + name string + want int + wantErr bool + collectorName string }{ { - name: "get dns logs", - want: 1, - wantErr: false, + name: "get dns logs", + want: 1, + wantErr: false, + collectorName: "dns", }, } @@ -29,6 +31,11 @@ func TestNewDNSCollector(t *testing.T) { if len(raw) < tt.want { t.Errorf("len(GetData()) = %v, want %v", len(raw), tt.want) } + + name := c.GetName() + if name != tt.collectorName { + t.Errorf("GetName()) = %v, want %v", name, tt.collectorName) + } }) } } diff --git a/pkg/collector/helm_collector_test.go b/pkg/collector/helm_collector_test.go index 0881aa7e..cde54a64 100644 --- a/pkg/collector/helm_collector_test.go +++ b/pkg/collector/helm_collector_test.go @@ -11,14 +11,16 @@ import ( func TestHelmCollector(t *testing.T) { tests := []struct { - name string - want int - wantErr bool + name string + want int + wantErr bool + collectorName string }{ { - name: "get release history", - want: 1, - wantErr: false, + name: "get release history", + want: 1, + wantErr: false, + collectorName: "helm", }, } @@ -53,6 +55,11 @@ func TestHelmCollector(t *testing.T) { if len(releases) < tt.want { t.Errorf("len(GetData()) = %v, want %v", len(releases), tt.want) } + + name := c.GetName() + if name != tt.collectorName { + t.Errorf("GetName()) = %v, want %v", name, tt.collectorName) + } }) } } diff --git a/pkg/collector/kubeobjects_collector_test.go b/pkg/collector/kubeobjects_collector_test.go index 46fa7f65..4bedb87d 100644 --- a/pkg/collector/kubeobjects_collector_test.go +++ b/pkg/collector/kubeobjects_collector_test.go @@ -10,14 +10,16 @@ import ( func TestNewKubeObjectsCollector(t *testing.T) { tests := []struct { - name string - want int - wantErr bool + name string + want int + wantErr bool + collectorName string }{ { - name: "get kube objects logs", - want: 1, - wantErr: false, + name: "get kube objects logs", + want: 1, + wantErr: false, + collectorName: "kubeobjects", }, } @@ -51,6 +53,11 @@ func TestNewKubeObjectsCollector(t *testing.T) { if len(raw) < tt.want { t.Errorf("len(GetData()) = %v, want %v", len(raw), tt.want) } + + name := c.GetName() + if name != tt.collectorName { + t.Errorf("GetName()) = %v, want %v", name, tt.collectorName) + } }) } } diff --git a/pkg/collector/networkoutbound_collector_test.go b/pkg/collector/networkoutbound_collector_test.go index 4d4a4dad..5d6b14b8 100644 --- a/pkg/collector/networkoutbound_collector_test.go +++ b/pkg/collector/networkoutbound_collector_test.go @@ -6,14 +6,16 @@ import ( func TestNewNetworkOutboundCollector(t *testing.T) { tests := []struct { - name string - want int - wantErr bool + name string + want int + wantErr bool + collectorName string }{ { - name: "get networkbound logs", - want: 1, - wantErr: false, + name: "get networkbound logs", + want: 1, + wantErr: false, + collectorName: "networkoutbound", }, } @@ -31,6 +33,11 @@ func TestNewNetworkOutboundCollector(t *testing.T) { if len(raw) < tt.want { t.Errorf("len(GetData()) = %v, want %v", len(raw), tt.want) } + + name := c.GetName() + if name != tt.collectorName { + t.Errorf("GetName()) = %v, want %v", name, tt.collectorName) + } }) } } diff --git a/pkg/collector/nodelogs_collector_test.go b/pkg/collector/nodelogs_collector_test.go index 65a0f070..74539d79 100644 --- a/pkg/collector/nodelogs_collector_test.go +++ b/pkg/collector/nodelogs_collector_test.go @@ -7,14 +7,16 @@ import ( func TestNodeLogsCollector(t *testing.T) { tests := []struct { - name string - want int - wantErr bool + name string + want int + wantErr bool + collectorName string }{ { - name: "get node logs", - want: 1, - wantErr: false, + name: "get node logs", + want: 1, + wantErr: false, + collectorName: "nodelogs", }, } @@ -36,6 +38,11 @@ func TestNodeLogsCollector(t *testing.T) { if len(raw) < tt.want { t.Errorf("len(GetData()) = %v, want %v", len(raw), tt.want) } + + name := c.GetName() + if name != tt.collectorName { + t.Errorf("GetName()) = %v, want %v", name, tt.collectorName) + } }) } } diff --git a/pkg/collector/pods_containerlogs_collector_test.go b/pkg/collector/pods_containerlogs_collector_test.go index d2cd1644..86b02791 100644 --- a/pkg/collector/pods_containerlogs_collector_test.go +++ b/pkg/collector/pods_containerlogs_collector_test.go @@ -10,14 +10,16 @@ import ( func TestPodsContainerLogsCollector(t *testing.T) { tests := []struct { - name string - want int - wantErr bool + name string + want int + wantErr bool + collectorName string }{ { - name: "get pods container logs", - want: 1, - wantErr: false, + name: "get pods container logs", + want: 1, + wantErr: false, + collectorName: "podscontainerlogs", }, } @@ -51,6 +53,12 @@ func TestPodsContainerLogsCollector(t *testing.T) { if len(raw) < tt.want { t.Errorf("len(GetData()) = %v, want %v", len(raw), tt.want) } + + name := c.GetName() + if name != tt.collectorName { + t.Errorf("GetName()) = %v, want %v", name, tt.collectorName) + } + }) } } From f4237c04dfe076465e4a6a1801ee3148e692ed6d Mon Sep 17 00:00:00 2001 From: Sanya Kochhar <42152676+SanyaKochhar@users.noreply.github.com> Date: Mon, 4 Oct 2021 17:30:57 -0400 Subject: [PATCH 2/2] osm/smi: update dir structure and add test (#118) --- deployment/cluster-role.yaml | 3 -- go.mod | 4 +- pkg/collector/osm_collector.go | 72 ++++++++++++++++------------- pkg/collector/osm_collector_test.go | 56 ++++++++++++++++++++++ pkg/collector/smi_collector.go | 4 +- 5 files changed, 100 insertions(+), 39 deletions(-) create mode 100644 pkg/collector/osm_collector_test.go diff --git a/deployment/cluster-role.yaml b/deployment/cluster-role.yaml index d817103d..10f028ce 100644 --- a/deployment/cluster-role.yaml +++ b/deployment/cluster-role.yaml @@ -12,6 +12,3 @@ rules: - apiGroups: ["apiextensions.k8s.io"] resources: ["customresourcedefinitions"] verbs: ["get", "list", "watch"] -- apiGroups: ["access.smi-spec.io", "specs.smi-spec.io", "split.smi-spec.io"] - resources: ["*"] - verbs: ["get", "list", "watch"] diff --git a/go.mod b/go.mod index f817704f..3d6b844f 100644 --- a/go.mod +++ b/go.mod @@ -6,9 +6,9 @@ require ( github.com/Azure/azure-storage-blob-go v0.7.0 github.com/Azure/go-autorest/autorest/adal v0.9.14 // indirect github.com/kr/pretty v0.2.1 // indirect - github.com/onsi/gomega v1.13.0 + github.com/onsi/gomega v1.13.0 // indirect helm.sh/helm/v3 v3.6.3 - k8s.io/api v0.21.3 // indirect + k8s.io/api v0.21.3 k8s.io/apimachinery v0.21.3 k8s.io/cli-runtime v0.21.3 k8s.io/client-go v0.21.3 diff --git a/pkg/collector/osm_collector.go b/pkg/collector/osm_collector.go index 91f1d124..d640bf1f 100644 --- a/pkg/collector/osm_collector.go +++ b/pkg/collector/osm_collector.go @@ -50,22 +50,22 @@ func (collector *OsmCollector) Collect() error { // callNamespaceCollectors calls functions to collect data for osm-controller namespace and namespaces monitored by a given mesh func (collector *OsmCollector) callNamespaceCollectors(monitoredNamespaces []string, controllerNamespaces []string, meshName string) { for _, namespace := range monitoredNamespaces { - if err := collector.collectDataFromEnvoys(namespace); err != nil { + if err := collector.collectDataFromEnvoys(namespace, meshName); err != nil { log.Printf("Failed to collect Envoy configs in OSM monitored namespace %s: %+v", namespace, err) } - collector.collectNamespaceResources(namespace) + collector.collectNamespaceResources(namespace, meshName) } for _, namespace := range controllerNamespaces { - if err := collector.collectPodLogs(namespace); err != nil { + if err := collector.collectPodLogs(namespace, meshName); err != nil { log.Printf("Failed to collect pod logs for controller namespace %s: %+v", namespace, err) } - collector.collectNamespaceResources(namespace) + collector.collectNamespaceResources(namespace, meshName) } } // collectNamespaceResources collects information about general resources in a given namespace -func (collector *OsmCollector) collectNamespaceResources(namespace string) { - if err := collector.collectPodConfigs(namespace); err != nil { +func (collector *OsmCollector) collectNamespaceResources(namespace string, meshName string) { + if err := collector.collectPodConfigs(namespace, meshName); err != nil { log.Printf("Failed to collect pod configs for ns %s: %+v", namespace, err) } @@ -140,23 +140,23 @@ func (collector *OsmCollector) collectNamespaceResources(namespace string) { podList = fmt.Sprintf("Failed to collect pod list for namespace %s: %v", namespace, err) log.Print(podList) } - - collector.data[namespace+"_metadata"] = metadata - collector.data[namespace+"_services_list"] = servicesList - collector.data[namespace+"_services"] = services - collector.data[namespace+"_endpoints_list"] = endpointList - collector.data[namespace+"_endpoints"] = endpoints - collector.data[namespace+"_configmaps_list"] = configmapsList - collector.data[namespace+"_configmaps"] = configmaps - collector.data[namespace+"_ingresses_list"] = ingressList - collector.data[namespace+"_ingresses"] = ingresses - collector.data[namespace+"_service_accounts_list"] = svcAccountList - collector.data[namespace+"_service_accounts"] = svcAccounts - collector.data[namespace+"_pods_list"] = podList + filePath := meshName + "/" + namespace + collector.data[filePath+"_metadata"] = metadata + collector.data[filePath+"_services_list"] = servicesList + collector.data[filePath+"_services"] = services + collector.data[filePath+"_endpoints_list"] = endpointList + collector.data[filePath+"_endpoints"] = endpoints + collector.data[filePath+"_configmaps_list"] = configmapsList + collector.data[filePath+"_configmaps"] = configmaps + collector.data[filePath+"_ingresses_list"] = ingressList + collector.data[filePath+"_ingresses"] = ingresses + collector.data[filePath+"_service_accounts_list"] = svcAccountList + collector.data[filePath+"_service_accounts"] = svcAccounts + collector.data[filePath+"_pods_list"] = podList } // collectPodConfigs collects configs for pods in given namespace -func (collector *OsmCollector) collectPodConfigs(namespace string) error { +func (collector *OsmCollector) collectPodConfigs(namespace string, meshName string) error { pods, err := utils.GetResourceList([]string{"get", "pods", "-n", namespace, "-o", "jsonpath={..metadata.name}"}, " ") if err != nil { return err @@ -168,14 +168,15 @@ func (collector *OsmCollector) collectPodConfigs(namespace string) error { output = fmt.Sprintf("Failed to collect config for pod %s in OSM monitored namespace %s: %v", podName, namespace, err) log.Print(output) } - collector.data[podName] = output + filePath := meshName + "/" + podName + "_podConfig" + collector.data[filePath] = output } return nil } // collectDataFromEnvoys collects Envoy proxy config for pods in monitored namespace: port-forward and curl config dump -func (collector *OsmCollector) collectDataFromEnvoys(namespace string) error { +func (collector *OsmCollector) collectDataFromEnvoys(namespace string, meshName string) error { pods, err := utils.GetResourceList([]string{"get", "pods", "-n", namespace, "-o", "jsonpath={..metadata.name}"}, " ") if err != nil { return err @@ -197,8 +198,8 @@ func (collector *OsmCollector) collectDataFromEnvoys(namespace string) error { // Remove certificate secrets from Envoy config i.e., "inline_bytes" field from response re := regexp.MustCompile("(?m)[\r\n]+^.*inline_bytes.*$") secretRemovedResponse := re.ReplaceAllString(string(responseBody), "---redacted---") - - collector.data[query+"_"+podName] = secretRemovedResponse + filePath := meshName + "/envoy/"+ podName + query + collector.data[filePath] = secretRemovedResponse } if err = utils.KillProcess(pid); err != nil { log.Printf("Failed to kill process: %+v", err) @@ -209,7 +210,7 @@ func (collector *OsmCollector) collectDataFromEnvoys(namespace string) error { } // collectPodLogs collects logs of every pod in a given namespace -func (collector *OsmCollector) collectPodLogs(namespace string) error { +func (collector *OsmCollector) collectPodLogs(namespace string, meshName string) error { pods, err := utils.GetResourceList([]string{"get", "pods", "-n", namespace, "-o", "jsonpath={..metadata.name}"}, " ") if err != nil { return err @@ -220,8 +221,8 @@ func (collector *OsmCollector) collectPodLogs(namespace string) error { output = fmt.Sprintf("Failed to collect logs for pod %s: %+v", podName, err) log.Print(output) } - - collector.data[podName+"_logs"] = output + filePath := meshName + "/" + podName + "_podLogs" + collector.data[filePath] = output } return nil } @@ -242,7 +243,7 @@ func (collector *OsmCollector) collectGroundTruth(meshName string) { mutationWebhookConfig, err := utils.RunCommandOnContainer("kubectl", "get", "MutatingWebhookConfiguration", "--all-namespaces", "-l", "app.kubernetes.io/instance="+meshName, "-o", "json") if err != nil { - mutationWebhookConfig = fmt.Sprintf("Failed to collect mutation webhook config for mesh %s: %v", meshName, err) + mutationWebhookConfig = fmt.Sprintf("Failed to collect mutating webhook config for mesh %s: %v", meshName, err) log.Print(mutationWebhookConfig) } @@ -252,10 +253,17 @@ func (collector *OsmCollector) collectGroundTruth(meshName string) { log.Print(validatingWebhookConfig) } - collector.data[meshName+"_all_resources_list"] = allResourcesList - collector.data[meshName+"_all_resources_configs"] = allResourcesConfigs - collector.data[meshName+"_mutating_webhook_configurations"] = mutationWebhookConfig - collector.data[meshName+"_validating_webhook_configurations"] = validatingWebhookConfig + meshConfig, err := utils.RunCommandOnContainer("kubectl", "get", "meshconfigs", "--all-namespaces", "-o", "json") + if err != nil { + meshConfig = fmt.Sprintf("Failed to collect meshconfigs for mesh %s: %v", meshName, err) + log.Print(meshConfig) + } + filePath := meshName + "/control_plane/" + collector.data[filePath+"/all_resources_list"] = allResourcesList + collector.data[filePath+"/all_resources_configs"] = allResourcesConfigs + collector.data[filePath+"/mutating_webhook_configurations"] = mutationWebhookConfig + collector.data[filePath+"/validating_webhook_configurations"] = validatingWebhookConfig + collector.data[filePath+"/mesh_configs"] = meshConfig } func (collector *OsmCollector) GetData() map[string]string { diff --git a/pkg/collector/osm_collector_test.go b/pkg/collector/osm_collector_test.go new file mode 100644 index 00000000..c043e60e --- /dev/null +++ b/pkg/collector/osm_collector_test.go @@ -0,0 +1,56 @@ +package collector + +import ( + "os" + "testing" + + appsv1 "k8s.io/api/apps/v1" + "k8s.io/apimachinery/pkg/runtime" +) + +func TestOsmCollector(t *testing.T) { + tests := []struct { + name string + want int + wantErr bool + deployments []*appsv1.Deployment + collectorName string + }{ + { + name: "no deployments found", + want: 0, + wantErr: true, + deployments: []*appsv1.Deployment{}, + collectorName: "osm", + }, + } + + c := NewOsmCollector() + + if err := os.Setenv("COLLECTOR_LIST", "OSM"); err != nil { + t.Fatalf("Setenv: %v", err) + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + objs := make([]runtime.Object, len(tt.deployments)) + for i := range tt.deployments { + objs[i] = tt.deployments[i] + } + err := c.Collect() + + if (err != nil) != tt.wantErr { + t.Errorf("Collect() error = %v, wantErr %v", err, tt.wantErr) + } + raw := c.GetData() + + if len(raw) < tt.want { + t.Errorf("len(GetData()) = %v, want %v", len(raw), tt.want) + } + name := c.GetName() + if name != tt.collectorName { + t.Errorf("GetName()) = %v, want %v", name, tt.collectorName) + } + }) + } +} diff --git a/pkg/collector/smi_collector.go b/pkg/collector/smi_collector.go index 712c0f93..49de1518 100644 --- a/pkg/collector/smi_collector.go +++ b/pkg/collector/smi_collector.go @@ -58,7 +58,7 @@ func collectSmiCrds(collector *SmiCollector, smiCrdsList []string) { if err != nil { log.Printf("Skipping: unable to collect yaml definition of %s: %+v", smiCrd, err) } - collector.data["smi_crd_definition_"+smiCrd] = yamlDefinition + collector.data["smi/crd_"+strings.TrimSuffix(smiCrd, ".io")] = yamlDefinition } } @@ -90,7 +90,7 @@ func collectSmiCustomResourcesFromNamespace(collector *SmiCollector, smiCrdsList if err != nil { log.Printf("Skipping: unable to collect yaml definition of %s (custom resource type: %s): %+v", customResourceName, smiCrdType, err) } - collector.data["namespace_"+namespace+"_"+smiCrdType+"_custom_resource_"+customResourceName] = yamlDefinition + collector.data["smi/namespace_"+namespace+"/"+smiCrdType+"_"+customResourceName+"_custom_resource"] = yamlDefinition } } }