General improvements

cmd/kubernetes-discovery/main.go

@@ -11,39 +11,43 @@ import (
 	k8sclient "github.com/newrelic/nri-kubernetes/v2/src/client"
 )
 
-var discovery = flag.String("discovery", KSMPodLabel, "Which discovery mechanism to run")
-
-var logger = log.New(true)
-
 const (
 	KSMPodLabel = "ksm_pod_label"
 )
 
-func main() {
+var (
+	discovery   = flag.String("discovery", KSMPodLabel, "Which discovery mechanism to run")
+	ksmPodLabel = flag.String("ksm_pod_label", "my-custom-ksm", "[ksm_pod_label] The label to search for")
+)
 
+func main() {
 	flag.Parse()
 
-	k8sClient, err := k8sclient.NewKubernetes( /* tryLocalKubeconfig */ true)
+	verbose := true
+
+	logger := log.New(verbose)
+
+	tryLocalKubeconfig := true
+
+	k8sClient, err := k8sclient.NewKubernetes(tryLocalKubeconfig)
 	if err != nil {
-		logrus.Fatalf("could not create kubernetes client: %v", err)
+		logger.Fatalf("Could not create Kubernetes client: %v", err)
 	}
 
 	switch *discovery {
 	case KSMPodLabel:
-		runKSMPodLabel(k8sClient)
+		runKSMPodLabel(k8sClient, logger)
 	default:
-		logrus.Infof("Invalid discovery type: %s", *discovery)
+		logger.Infof("Invalid discovery type: %s", *discovery)
 	}
 }
 
-var ksmPodLabel = flag.String("ksm_pod_label", "my-custom-ksm", "[ksm_pod_label] The label to search for")
-
-func runKSMPodLabel(kubernetes k8sclient.Kubernetes) {
+func runKSMPodLabel(kubernetes k8sclient.Kubernetes, logger *logrus.Logger) {
 	discoverer := client.NewPodLabelDiscoverer(*ksmPodLabel, 8080, "http", logger, kubernetes)
 	ksm, err := discoverer.Discover(time.Second * 5)
 	if err != nil {
-		logrus.Fatal(err)
+		logger.Fatalf("Discovering KSM: %v", err)
 	}
 
-	logrus.Infof("found KSM pod on HostIP: %s", ksm.NodeIP())
+	logger.Infof("Found KSM pod on HostIP: %s", ksm.NodeIP())
 }

e2e/cmd/e2e.go

@@ -173,6 +173,19 @@ func main() {
 	if err != nil {
 		panic(err.Error())
 	}
+
+	nodes, err := c.NodesList()
+	if err != nil {
+		panic(err.Error())
+	}
+
+	// If there is more than one node on the cluster, some metrics may not be found, which makes tests to fail.
+	expectedNumberOfNodes := 1
+
+	if nodesCount := len(nodes.Items); nodesCount != expectedNumberOfNodes {
+		logger.Fatalf("e2e tests require %d number of nodes on the cluster, found %d", expectedNumberOfNodes, nodesCount)
+	}
+
 	logger.Infof("Executing tests in %q cluster. K8s version: %s", c.Config.Host, c.ServerVersion())
 
 	if cliArgs.CleanBeforeRun {

src/client/client.go

@@ -25,10 +25,6 @@ type Kubernetes interface {
 	FindNode(name string) (*v1.Node, error)
 	// FindPodsByLabel returns a PodList reference containing the pods matching the provided name/value label pair
 	FindPodsByLabel(name, value string) (*v1.PodList, error)
-	// FindPodByName returns a PodList reference that should contain the pod whose name matches with the name argument
-	FindPodByName(name string) (*v1.PodList, error)
-	// FindPodsByHostname returns a Podlist reference containing the pod or pods whose hostname matches the argument
-	FindPodsByHostname(hostname string) (*v1.PodList, error)
 	// FindServicesByLabel returns a ServiceList containing the services matching the provided name/value label pair
 	// name/value pairs
 	FindServicesByLabel(name, value string) (*v1.ServiceList, error)
@@ -67,18 +63,6 @@ func (ka *goClientImpl) FindPodsByLabel(name, value string) (*v1.PodList, error)
 	})
 }
 
-func (ka *goClientImpl) FindPodByName(name string) (*v1.PodList, error) {
-	return ka.client.CoreV1().Pods("").List(context.TODO(), metav1.ListOptions{
-		FieldSelector: fmt.Sprintf("metadata.name=%s", name),
-	})
-}
-
-func (ka *goClientImpl) FindPodsByHostname(hostname string) (*v1.PodList, error) {
-	return ka.client.CoreV1().Pods("").List(context.TODO(), metav1.ListOptions{
-		FieldSelector: fmt.Sprintf("spec.hostname=%s", hostname),
-	})
-}
-
 func (ka *goClientImpl) FindServicesByLabel(name, value string) (*v1.ServiceList, error) {
 	return ka.client.CoreV1().Services("").List(context.TODO(), metav1.ListOptions{
 		LabelSelector: fmt.Sprintf("%s=%s", name, value),

src/client/client_mock.go

@@ -45,18 +45,6 @@ func (m *MockedKubernetes) FindPodsByLabel(name, value string) (*v1.PodList, err
 	return args.Get(0).(*v1.PodList), args.Error(1)
 }
 
-// FindPodByName mocks Kubernetes FindPodByName
-func (m *MockedKubernetes) FindPodByName(name string) (*v1.PodList, error) {
-	args := m.Called(name)
-	return args.Get(0).(*v1.PodList), args.Error(1)
-}
-
-// FindPodsByHostname mocks Kubernetes FindPodsByHostname
-func (m *MockedKubernetes) FindPodsByHostname(hostname string) (*v1.PodList, error) {
-	args := m.Called(hostname)
-	return args.Get(0).(*v1.PodList), args.Error(1)
-}
-
 // FindServicesByLabel mocks Kubernetes FindServicesByLabel
 func (m *MockedKubernetes) FindServicesByLabel(name, value string) (*v1.ServiceList, error) {
 	args := m.Called(name, value)

src/ksm/client/discovery.go

@@ -19,8 +19,10 @@ import (
 	"github.com/newrelic/nri-kubernetes/v2/src/prometheus"
 )
 
-var ksmAppLabelNames = []string{"app.kubernetes.io/name", "k8s-app", "app"}
-var errNoKSMPodsFound = errors.New("no KSM pods found")
+var (
+	ksmAppLabelNames  = []string{"app.kubernetes.io/name", "k8s-app", "app"}
+	errNoKSMPodsFound = errors.New("no KSM pods found")
+)
 
 const (
 	ksmAppLabelValue         = "kube-state-metrics"

src/ksm/client/discovery_test.go

@@ -15,6 +15,7 @@ import (
 	"github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
+	"github.com/stretchr/testify/require"
 	v1 "k8s.io/api/core/v1"
 	"k8s.io/client-go/rest"
 
@@ -109,6 +110,7 @@ func TestDiscover_portThroughDNSAndGuessedNodeIPFromMultiplePods(t *testing.T) {
 	// And the nodeIP is correctly returned
 	assert.Equal(t, "162.178.1.1", ksmClient.(*ksm).nodeIP)
 }
+
 func TestDiscover_metricsPortThroughAPIWhenDNSFails(t *testing.T) {
 	tt := []struct {
 		label     string
@@ -127,15 +129,16 @@ func TestDiscover_metricsPortThroughAPIWhenDNSFails(t *testing.T) {
 			// Given a client
 			c := new(client.MockedKubernetes)
 			c.On("FindServicesByLabel", entry.label, mock.Anything).
-				Return(&v1.ServiceList{Items: []v1.Service{{
-					Spec: v1.ServiceSpec{
-						ClusterIP: "1.2.3.4",
-						Ports: []v1.ServicePort{{
-							Name: ksmPortName,
-							Port: 8888,
-						}},
+				Return(&v1.ServiceList{Items: []v1.Service{
+					{
+						Spec: v1.ServiceSpec{
+							ClusterIP: "1.2.3.4",
+							Ports: []v1.ServicePort{{
+								Name: ksmPortName,
+								Port: 8888,
+							}},
+						},
 					},
-				},
 				}}, nil)
 			c.On("FindServicesByLabel", mock.Anything, mock.Anything).
 				Return(&v1.ServiceList{}, nil)
@@ -161,7 +164,7 @@ func TestDiscover_metricsPortThroughAPIWhenDNSFails(t *testing.T) {
 			ksmClient, err := d.Discover(timeout)
 
 			// The call works correctly
-			assert.Nil(t, err, "should not return error")
+			require.Nil(t, err, "should not return error")
 			// And the discovered host:port of the KSM Service is returned
 			assert.Equal(t, "1.2.3.4:8888", ksmClient.(*ksm).endpoint.Host)
 			assert.Equal(t, "http", ksmClient.(*ksm).endpoint.Scheme)
@@ -175,15 +178,16 @@ func TestDiscover_metricsPortThroughAPIWhenDNSError(t *testing.T) {
 	// Given a client
 	c := new(client.MockedKubernetes)
 	c.On("FindServicesByLabel", mock.Anything, mock.Anything).
-		Return(&v1.ServiceList{Items: []v1.Service{{
-			Spec: v1.ServiceSpec{
-				ClusterIP: "1.2.3.4",
-				Ports: []v1.ServicePort{{
-					Name: ksmPortName,
-					Port: 8888,
-				}},
+		Return(&v1.ServiceList{Items: []v1.Service{
+			{
+				Spec: v1.ServiceSpec{
+					ClusterIP: "1.2.3.4",
+					Ports: []v1.ServicePort{{
+						Name: ksmPortName,
+						Port: 8888,
+					}},
+				},
 			},
-		},
 		}}, nil)
 	c.On("FindPodsByLabel", mock.Anything, mock.Anything).
 		Return(&v1.PodList{Items: []v1.Pod{{
@@ -227,7 +231,8 @@ func TestDiscover_guessedTCPPortThroughAPIWhenDNSEmptyResponse(t *testing.T) {
 					Protocol: "TCP",
 					Port:     8081,
 				}},
-			}}}}, nil)
+			},
+		}}}, nil)
 	c.On("FindPodsByLabel", mock.Anything, mock.Anything).
 		Return(&v1.PodList{Items: []v1.Pod{{
 			Status: v1.PodStatus{HostIP: "6.7.8.9"},
@@ -313,6 +318,7 @@ func TestDiscover_errorRetrievingPortWhenDNSAndAPIErrors(t *testing.T) {
 	// And the KSM client is not returned
 	assert.Nil(t, ksmClient)
 }
+
 func TestDiscover_errorRetrievingNodeIPWhenPodListEmpty(t *testing.T) {
 	// Given a client
 	c := new(client.MockedKubernetes)
@@ -386,13 +392,13 @@ func TestNodeIPForDiscoverer_Error(t *testing.T) {
 // Testing NodeIP() method
 func TestNodeIP(t *testing.T) {
 	// Given a ksm struct initialized
-	var c = ksm{
+	c := ksm{
 		nodeIP:     "1.2.3.4",
 		endpoint:   url.URL{},
 		httpClient: http.DefaultClient,
 		logger:     logger,
 	}
-	var cl = &c
+	cl := &c
 	// When retrieving node IP
 	nodeIP := cl.NodeIP()
 	// The call works correctly
@@ -408,7 +414,7 @@ func TestDo(t *testing.T) {
 		assert.FailNow(t, err.Error())
 	}
 
-	var c = &ksm{
+	c := &ksm{
 		nodeIP:     "1.2.3.4",
 		endpoint:   *endpoint,
 		httpClient: s.Client(),

src/ksm/client/distributed_pod_label_discovery.go

@@ -5,7 +5,6 @@ import (
 	"net/url"
 	"time"
 
-	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 	v1 "k8s.io/api/core/v1"
 
@@ -21,12 +20,11 @@ type distributedPodLabelDiscoverer struct {
 
 func (p *distributedPodLabelDiscoverer) findAllLabeledPodsRunningOnNode() ([]v1.Pod, error) {
 	pods, err := p.k8sClient.FindPodsByLabel(p.ksmPodLabel, "true")
-
 	if err != nil {
-		return nil, errors.Wrap(err, "could not query api server for pods")
+		return nil, fmt.Errorf("querying API server for pods: %w", err)
 	}
 	if len(pods.Items) == 0 {
-		return nil, errors.Wrapf(errNoKSMPodsFound, "no KSM pod found with label: '%s'", p.ksmPodLabel)
+		return nil, fmt.Errorf("discovering KSM with label %q: %w", p.ksmPodLabel, errNoKSMPodsFound)
 	}
 
 	var foundPods []v1.Pod
@@ -36,7 +34,7 @@ func (p *distributedPodLabelDiscoverer) findAllLabeledPodsRunningOnNode() ([]v1.
 		}
 
 		if pod.Status.HostIP == p.ownNodeIP {
-			p.logger.Debugf("Found KSM pod running on this code, pod IP: %s", pod.Status.PodIP)
+			p.logger.Debugf("Found KSM pod running on this node, pod IP: %s", pod.Status.PodIP)
 			foundPods = append(foundPods, pod)
 		}
 	}

src/ksm/client/pod_label_discovery.go

@@ -5,7 +5,6 @@ import (
 	"net/url"
 	"time"
 
-	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 	v1 "k8s.io/api/core/v1"
 
@@ -21,14 +20,13 @@ type podLabelDiscoverer struct {
 }
 
 func (p *podLabelDiscoverer) findSingleKSMPodByLabel() (*v1.Pod, error) {
-
 	pods, err := p.k8sClient.FindPodsByLabel(p.ksmPodLabel, "true")
-
 	if err != nil {
-		return nil, errors.Wrap(err, "could not query api server for pods")
+		return nil, fmt.Errorf("querying API server for Pods: %w", err)
 	}
+
 	if len(pods.Items) == 0 {
-		return nil, errors.Wrapf(errNoKSMPodsFound, "no KSM pod found with label: '%s'", p.ksmPodLabel)
+		return nil, fmt.Errorf("discovering KSM with label %q: %w", p.ksmPodLabel, errNoKSMPodsFound)
 	}
 
 	// In case there are multiple pods, we must be be sure to deterministically select the same Pod on each node
@@ -50,7 +48,6 @@ func (p *podLabelDiscoverer) findSingleKSMPodByLabel() (*v1.Pod, error) {
 
 // Discover will find a single KSM pod using the provided label.
 func (p *podLabelDiscoverer) Discover(timeout time.Duration) (client.HTTPClient, error) {
-
 	pod, err := p.findSingleKSMPodByLabel()
 	if err != nil {
 		return nil, err