Integration test and static main improvements (#263)

* test: new integration testing backbone
kubernetes-static main has been migrated to the new backbone as well

Signed-off-by: Roberto Santalla <[email protected]>

* test/integration: make Version object return mocks

Signed-off-by: Roberto Santalla <[email protected]>

* test/integration: implement the Asserter
Asserter is a (hopefully) powerful tool to assert that all metrics defined in a SpecGroup are correctly captured.

Signed-off-by: Roberto Santalla <[email protected]>

* test/integration: implement helper for building integrations

Signed-off-by: Roberto Santalla <[email protected]>

* test/asserter: log missing metric namespace

Signed-off-by: Roberto Santalla <[email protected]>

* test/asserter: make asserter fully thread-safe

Signed-off-by: Roberto Santalla <[email protected]>

* test/ksm: optimize asserter calls on test parallelization

Signed-off-by: Roberto Santalla <[email protected]>

* fixup! test/asserter: typos

Signed-off-by: Roberto Santalla <[email protected]>

* testutil: make LatestVersion rely on the order of AllVersions

* testutil: do not wrap fake.NewSimpleClientset, but rather provide helpers with objects\
fixup! add namespaces

* testutil/asserter: check if specGroups are empty and fail in that case

* testutil/asserter: make Excluding copy-safe

* test/ksm: share the asserter again
I've added comments explaining how this works, which will hopefully improve readability.

cmd/kubernetes-static/main.go

@@ -15,10 +15,11 @@ import (
 	"github.com/newrelic/infra-integrations-sdk/log"
 	"github.com/sirupsen/logrus"
 	v1 "k8s.io/api/core/v1"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/version"
+	"k8s.io/client-go/kubernetes/fake"
 
 	"github.com/newrelic/nri-kubernetes/v2/internal/discovery"
+	"github.com/newrelic/nri-kubernetes/v2/internal/testutil"
 	"github.com/newrelic/nri-kubernetes/v2/src/apiserver"
 	"github.com/newrelic/nri-kubernetes/v2/src/controlplane"
 	ksmClient "github.com/newrelic/nri-kubernetes/v2/src/ksm/client"
@@ -40,20 +41,20 @@ type argumentList struct {
 
 var args argumentList
 
-// Embed static metrics into binary.
-//go:embed data
-var content embed.FS
-
 func main() {
 	// Determines which subdirectory of cmd/kubernetes-static/ to use
 	// for serving the static metrics
-	k8sMetricsVersion := os.Getenv("K8S_METRICS_VERSION")
-	if k8sMetricsVersion == "" {
-		k8sMetricsVersion = "1_18"
+	testData := testutil.LatestVersion()
+	if envVersion := os.Getenv("K8S_METRICS_VERSION"); envVersion != "" {
+		testData = testutil.Version(envVersion)
 	}
 
-	endpoint := startStaticMetricsServer(content, k8sMetricsVersion)
+	testSever, err := testData.Server()
+	if err != nil {
+		logrus.Fatalf("Error building testserver: %v", err)
+	}
 
+	fakeK8s := fake.NewSimpleClientset(testutil.K8sEverything()...)
 	integration, err := integration.New(integrationName, integrationVersion, integration.Args(&args))
 	if err != nil {
 		logrus.Fatal(err)
@@ -96,102 +97,27 @@ func main() {
 	}}
 
 	// Kubelet
-	kubeletClient := newBasicHTTPClient(endpoint + "/kubelet")
+	kubeletClient := newBasicHTTPClient(testSever.KubeletEndpoint())
 	podsFetcher := kubeletmetric.NewPodsFetcher(logger, kubeletClient)
 	kubeletGrouper := kubelet.NewGrouper(
 		kubeletClient,
 		logger,
 		apiServerClient,
 		"ens5",
 		podsFetcher.FetchFuncWithCache(),
-		kubeletmetric.CadvisorFetchFunc(kubeletClient, metric.CadvisorQueries))
-
-	serviceList := []*v1.Service{
-		{
-			ObjectMeta: metav1.ObjectMeta{
-				Name:      "kube-state-metrics",
-				Namespace: "kube-system",
-			},
-			Spec: v1.ServiceSpec{
-				Selector: map[string]string{
-					"l1": "v1",
-					"l2": "v2",
-				},
-			},
-		},
-		{
-			ObjectMeta: metav1.ObjectMeta{
-				Name:      "cockroachdb",
-				Namespace: "default",
-			},
-			Spec: v1.ServiceSpec{
-				Selector: map[string]string{
-					"l1": "v1",
-					"l2": "v2",
-				},
-			},
-		},
-		{
-			ObjectMeta: metav1.ObjectMeta{
-				Name:      "metrics-server",
-				Namespace: "kube-system",
-			},
-			Spec: v1.ServiceSpec{
-				Selector: map[string]string{
-					"l1": "v1",
-					"l2": "v2",
-				},
-			},
-		},
-		{
-			ObjectMeta: metav1.ObjectMeta{
-				Name:      "kubernetes",
-				Namespace: "default",
-			},
-			Spec: v1.ServiceSpec{
-				Selector: map[string]string{
-					"l1": "v1",
-					"l2": "v2",
-				},
-			},
-		},
-		{
-			ObjectMeta: metav1.ObjectMeta{
-				Name:      "kube-dns",
-				Namespace: "kube-system",
-			},
-			Spec: v1.ServiceSpec{
-				Selector: map[string]string{
-					"l1": "v1",
-					"l2": "v2",
-				},
-			},
-		},
-		{
-			ObjectMeta: metav1.ObjectMeta{
-				Name:      "cockroachdb-public",
-				Namespace: "default",
-			},
-			Spec: v1.ServiceSpec{
-				Selector: map[string]string{
-					"l1": "v1",
-					"l2": "v2",
-				},
-			},
-		},
-	}
+		kubeletmetric.CadvisorFetchFunc(kubeletClient, metric.CadvisorQueries),
+	)
 
 	kc, err := ksmClient.New(ksmClient.WithLogger(log.New(true, os.Stderr)))
 	if err != nil {
 		log.Fatal(err)
 	}
 
+	fakeLister, _ := discovery.NewServicesLister(fakeK8s)
 	kg, err := ksmGrouper.New(ksmGrouper.Config{
-		MetricFamiliesGetter: kc.MetricFamiliesGetter(endpoint),
+		MetricFamiliesGetter: kc.MetricFamiliesGetter(testSever.KSMEndpoint()),
 		Queries:              metric.KSMQueries,
-		ServicesLister: discovery.MockedServicesLister{
-			Services: serviceList,
-		},
+		ServicesLister:       fakeLister,
 	}, ksmGrouper.WithLogger(logger))
 	if err != nil {
 		log.Fatal(err)
@@ -213,7 +139,7 @@ func main() {
 
 	for _, component := range controlplane.BuildComponentList() {
 		componentGrouper := controlplane.NewComponentGrouper(
-			newBasicHTTPClient(fmt.Sprintf("%s/controlplane/%s", endpoint, component.Name)),
+			newBasicHTTPClient(testSever.ControlPlaneEndpoint(string(component.Name))),
 			component.Queries,
 			logger,
 			controlPlaneComponentPods[component.Name],

internal/discovery/services_discoverer.go

@@ -30,6 +30,8 @@ func NewServicesLister(client kubernetes.Interface, options ...informers.SharedI
 	return lister, stopCh
 }
 
+// MockedServicesLister is a simple lister that returns a hardcoded list of services.
+// For integration testing, it is recommended to use the a ServiceDiscoverer with testutil.FakeK8sClient as a backend.
 type MockedServicesLister struct {
 	Services []*corev1.Service
 }

internal/testutil/assert.go

@@ -0,0 +1,176 @@
+package testutil
+
+import (
+	"strings"
+	"sync"
+	"testing"
+
+	"github.com/newrelic/infra-integrations-sdk/data/metric"
+	"github.com/newrelic/infra-integrations-sdk/integration"
+	"github.com/newrelic/nri-kubernetes/v2/src/definition"
+)
+
+// Asserter is a helper for checking whether an integration contains all the metrics defined in a specGroup.
+// It provides a chainable API, with each call returning a copy of the asserter. This way, successive calls to the
+// chainable methods do not modify the previous Asserter, allowing to reuse the chain as a test fans out.
+// Asserter is safe to use concurrently.
+type Asserter struct {
+	*sync.Mutex
+	entities        []*integration.Entity
+	specGroups      definition.SpecGroups
+	exclude         exclusions
+	excludeOptional bool
+	silent          bool
+}
+
+// exclusions is a map from groupName to metricName to a boolean which is true if the metric should be excluded.
+// An entry for a groupName with an empty list of metrics to exclude will be interpreted as an exclusion for all metrics
+// in that group.
+type exclusions map[string]map[string]bool
+
+func NewAsserter() Asserter {
+	return Asserter{
+		Mutex: &sync.Mutex{},
+	}
+}
+
+// Using returns an asserter that will use the supplied specGroups to assert entities.
+func (a Asserter) Using(groups definition.SpecGroups) Asserter {
+	a.specGroups = groups
+	return a
+}
+
+// On returns an asserter configured to check for existence on the supplied entities.
+func (a Asserter) On(entities []*integration.Entity) Asserter {
+	a.entities = entities
+	return a
+}
+
+// Excluding returns an asserter that will not fail for the supplied groupName and metricName if they are missing.
+// If no metricNames are specified, Asserter will ignore the whole group.
+// Missing metrics are still logged.
+func (a Asserter) Excluding(groupName string, metricNames ...string) Asserter {
+	// Map is a pointer and therefore shared among all asserters. To avoid modifying the previous asserter we need to
+	// copy it.
+	prevExclude := a.exclude
+	a.exclude = map[string]map[string]bool{}
+
+	// Copy exclusions from previous asserter
+	for prevGroup, prevMetrics := range prevExclude {
+		a.exclude[prevGroup] = prevMetrics
+	}
+
+	if a.exclude[groupName] == nil {
+		a.exclude[groupName] = map[string]bool{}
+	}
+
+	for _, e := range metricNames {
+		a.exclude[groupName][e] = true
+	}
+
+	return a
+}
+
+// Silently returns an asserter that will not log optional or excepted metrics
+func (a Asserter) Silently() Asserter {
+	a.silent = true
+	return a
+}
+
+// ExcludingOptional returns an asserter that will not fail, but log, metrics that are marked as optional in the SpecGroup.
+func (a Asserter) ExcludingOptional() Asserter {
+	a.excludeOptional = true
+	return a
+}
+
+// Assert checks whether all metrics defined in the supplied groups are present, and fails the test if any is not.
+func (a Asserter) Assert(t *testing.T) {
+	t.Helper()
+
+	if len(a.specGroups) == 0 {
+		t.Fatalf("cannot assert empty specGroups, did you forget Using()?")
+	}
+
+	// TODO: Consider paralleling if it's too slow.
+	for groupName, group := range a.specGroups {
+		exclusions, excludeGroup := a.exclude[groupName]
+		if excludeGroup && len(exclusions) == 0 {
+			// Group exclusion present with any metric, meaning we exclude the whole group
+			t.Logf("skipping excluded group %q", groupName)
+			continue
+		}
+
+		// Integration will contain many entities, but we are only interested in the one corresponding to this group.
+		entities := entitiesFor(a.entities, groupName)
+		if entities == nil {
+			t.Fatalf("could not find any entity for specGroup %q", groupName)
+		}
+
+		for _, spec := range group.Specs {
+			for _, entity := range entities {
+				if entityHas(entity, spec.Name, spec.Type) {
+					continue
+				}
+
+				if spec.Optional && a.excludeOptional {
+					if !a.silent {
+						t.Logf("optional metric %q not found in entity %q", spec.Name, entity.Metadata.Name)
+					}
+					continue
+				}
+
+				if exclusions[spec.Name] {
+					if !a.silent {
+						t.Logf("excluded metric %q not found in entity %q", spec.Name, entity.Metadata.Name)
+					}
+					continue
+				}
+
+				t.Errorf("metric %q not found in entity %q %q", spec.Name, entity.Metadata.Namespace, entity.Metadata.Name)
+				t.Failed()
+			}
+		}
+	}
+}
+
+// entitiesFor heuristically finds the entity associated to a spec group name.
+func entitiesFor(entities []*integration.Entity, pseudotype string) []*integration.Entity {
+	var appropriateEntities []*integration.Entity
+	for _, e := range entities {
+		if strings.Contains(strings.ToLower(e.Metadata.Namespace), strings.ToLower(pseudotype)) {
+			appropriateEntities = append(appropriateEntities, e)
+		}
+	}
+
+	return appropriateEntities
+}
+
+// entityHas returns true if supplied entity has metric m with type _similar_ to mType, false otherwise.
+func entityHas(e *integration.Entity, m string, mType metric.SourceType) bool {
+	// Wildcard metrics are ignored.
+	// TODO: Improve this and check matching glob patterns.
+	if strings.HasSuffix(m, "*") {
+		return true
+	}
+
+	for _, ms := range e.Metrics {
+		entityMetric, found := ms.Metrics[m]
+		if !found {
+			continue
+		}
+
+		// Check if metricType is an attribute but metric is not a string
+		_, isString := entityMetric.(string)
+		if isString && mType != metric.ATTRIBUTE {
+			continue
+		}
+
+		if !isString && mType == metric.ATTRIBUTE {
+			continue
+		}
+
+		return true
+	}
+
+	return false
+}

internal/testutil/data/namespaces.yaml

@@ -0,0 +1,3 @@
+apiVersion: v1
+kind: List
+items: []