src/kubernetes.go: add option to disable kubelet and controlplane jobs (

#206)

* src/kubernetes.go: add option to disable kubelet and controlplane jobs

As a WIP step for splitting nri-kubernetes into Kubelet and
Controlplane/KSM part.

Disabling KSM scraping is already possible.

Signed-off-by: Mateusz Gozdek <[email protected]>

* src/kubernetes.go: move jobs variable declaration closer to caller

Signed-off-by: Mateusz Gozdek <[email protected]>

* src/prometheus: don't execute queries twice when getting metrics

Closes #235

Signed-off-by: Mateusz Gozdek <[email protected]>

* src/prometheus: annotate errors in Do()

So tracing is easier in case error occurs.

Signed-off-by: Mateusz Gozdek <[email protected]>

* src/ksm/client/discovery.go: improve variable name a bit

It might be very confusing passing "timeout" to Discover() function,
where it's actually the generated client global timeout, rather than
discovery timeout itself. "clientTimeout" name should reflect that
better.

Signed-off-by: Mateusz Gozdek <[email protected]>

* src/ksm/client: move client to separate file

So discoverer stays in separate file and client is separate.

Signed-off-by: Mateusz Gozdek <[email protected]>

* src/client: decompose HTTPClient interface into multiple types

To start process of phasing out NodeIP() method, so clients which do not
need it can start only using HTTPDoer instead of HTTPClient.

This is because NodeIP() method in HTTPClient does not make any sense.

Signed-off-by: Mateusz Gozdek <[email protected]>

* src: take client.HTTPDoer instead of client.HTTPClient where possible

To reduce the possible complexity and to start phasing out
client.HTTPClient, which NodeIP() method shouldn't be added there in the
first place.

Signed-off-by: Mateusz Gozdek <[email protected]>

* src/controlplane/client: refactor auth configuration a bit

Signed-off-by: Mateusz Gozdek <[email protected]>

* src/ksm/client: don't add bearer token to requests

Requests for KSM metrics are not authenticated in majority of cases, so
there is no point of setting the authentication header there.

Part of #239

Signed-off-by: Mateusz Gozdek <[email protected]>

* ksm/client: set InsecureSkipVerify: true only when scheme is HTTPS

Otherwise it won't be used anyway.

Signed-off-by: Mateusz Gozdek <[email protected]>

* src: use http.MethodGet instead of plain "GET" where possible

So it is easier to identify what unique values are passed to the
clients.

Signed-off-by: Mateusz Gozdek <[email protected]>

* src: remove method parameter from HTTPDoer interface

As we always use just GET anyway and net/http.Client's Do() method has a
different signature anyway.

Signed-off-by: Mateusz Gozdek <[email protected]>

* src/kubelet/metric/pods.go: refactor a bit

So we don't pass so many parameters around.

Signed-off-by: Mateusz Gozdek <[email protected]>

* src/metric: get rid of k8sPopulator

This struct is useless.

Signed-off-by: Mateusz Gozdek <[email protected]>

* src/metric: use test package for blackbox testing

It's a good practice and in this package it's easy to change.

Signed-off-by: Mateusz Gozdek <[email protected]>

* src/data: remove unused Populator interface

Signed-off-by: Mateusz Gozdek <[email protected]>

* e2e/cmd: remove unused struct field

Signed-off-by: Mateusz Gozdek <[email protected]>

* src/kubelet/metric: remove unused cached field from PodsFetcher

Signed-off-by: Mateusz Gozdek <[email protected]>

* src/metric: remove unused forEachFetchedValue function

Signed-off-by: Mateusz Gozdek <[email protected]>

* src: rename HTTPDoer interface to HTTPGetter

Now that HTTP method is hardcoded, this name make more sense.

Signed-off-by: Mateusz Gozdek <[email protected]>

* src/ksm/group.go: keep happy path on the left

This is a good practice to not indent happy path when possible.

Signed-off-by: Mateusz Gozdek <[email protected]>

* src/client: rename HTTPGetter.Do() to Get()

To not make it confusing for new HTTPDoer.Do() interface and method and
to better reflect that this interface allows only performing GET
requests.

Signed-off-by: Mateusz Gozdek <[email protected]>

* src/prometheus: unexport Valid()

It is not used anywhere outside of prometheus package.

Signed-off-by: Mateusz Gozdek <[email protected]>

cmd/kubernetes-static/main.go

@@ -280,7 +280,7 @@ type basicHTTPClient struct {
 	httpClient http.Client
 }
 
-func (b basicHTTPClient) Do(method, path string) (*http.Response, error) {
+func (b basicHTTPClient) Get(path string) (*http.Response, error) {
 	endpoint := fmt.Sprintf("%s%s", b.url, path)
 	log.Info("Getting: %s", endpoint)
 

e2e/cmd/e2e.go

@@ -55,7 +55,6 @@ type integrationData struct {
 	podName      string
 	stdOut       []byte
 	stdErr       []byte
-	err          error
 }
 
 type executionErr struct {

src/client/cached.go

@@ -129,8 +129,8 @@ type cacheAwareClient struct {
 	timeout time.Duration
 }
 
-func (c *cacheAwareClient) Do(method, path string) (*http.Response, error) {
-	response, err := c.client.Do(method, path)
+func (c *cacheAwareClient) Get(path string) (*http.Response, error) {
+	response, err := c.client.Get(path)
 	if err == nil {
 		return response, nil
 	}
@@ -146,7 +146,7 @@ func (c *cacheAwareClient) Do(method, path string) (*http.Response, error) {
 		return nil, err
 	}
 	c.client = newClient
-	return c.client.Do(method, path)
+	return c.client.Get(path)
 }
 
 // this implementation doesn't guarantee the returned NodeIP is valid in the moment of the function invocation.

src/client/cached_test.go

@@ -23,7 +23,7 @@ func TestCacheAwareClient_CachedClientWorks(t *testing.T) {
 
 	// Setup discovered client
 	wrappedClient := new(MockDiscoveredHTTPClient)
-	wrappedClient.On("Do", mock.Anything, mock.Anything).Return(&http.Response{StatusCode: 200}, nil)
+	wrappedClient.On("Get", mock.Anything).Return(&http.Response{StatusCode: 200}, nil)
 
 	// Setup wrapped discoverer
 	discoverer := new(MockDiscoverer)
@@ -38,7 +38,7 @@ func TestCacheAwareClient_CachedClientWorks(t *testing.T) {
 	assert.NoError(t, err)
 
 	// When the client works as expected
-	resp, err := client.Do("GET", "/api/path")
+	resp, err := client.Get("/api/path")
 	assert.NoError(t, err)
 	assert.Equal(t, 200, resp.StatusCode)
 
@@ -52,9 +52,9 @@ func TestCacheAwareClient_CachedClientDoesNotWork(t *testing.T) {
 	// Setup discovered client
 	wrappedClient := new(MockDiscoveredHTTPClient)
 	// After the error on the first call, the second call returns a correct value
-	wrappedClient.On("Do", mock.Anything, mock.Anything).
+	wrappedClient.On("Get", mock.Anything).
 		Return(nil, fmt.Errorf("patapum")).Once()
-	wrappedClient.On("Do", mock.Anything, mock.Anything).
+	wrappedClient.On("Get", mock.Anything).
 		Return(&http.Response{StatusCode: 200}, nil)
 
 	// Setup wrapped discoverer
@@ -70,7 +70,7 @@ func TestCacheAwareClient_CachedClientDoesNotWork(t *testing.T) {
 	assert.NoError(t, err)
 
 	// When the cached client does not work (see discovered client mock setup)
-	resp, err := client.Do("GET", "/api/path")
+	resp, err := client.Get("/api/path")
 	assert.NoError(t, err)
 	assert.Equal(t, 200, resp.StatusCode)
 
@@ -83,7 +83,7 @@ func TestCacheAwareClient_RediscoveryDoesntWork(t *testing.T) {
 
 	// Setup discovered client
 	wrappedClient := new(MockDiscoveredHTTPClient)
-	wrappedClient.On("Do", mock.Anything, mock.Anything).
+	wrappedClient.On("Get", mock.Anything).
 		Return(nil, fmt.Errorf("patapum"))
 
 	// Setup wrapped discoverer
@@ -100,7 +100,7 @@ func TestCacheAwareClient_RediscoveryDoesntWork(t *testing.T) {
 	assert.NoError(t, err)
 
 	// When the cached client does not work and neither the re-discovery do
-	resp, err := client.Do("GET", "/api/path")
+	resp, err := client.Get("/api/path")
 	assert.Equal(t, "discovery failed", err.Error())
 	assert.Nil(t, resp)
 
@@ -117,7 +117,7 @@ func Test_CacheAwareClient_when_cache_TTL_is_not_reached(t *testing.T) {
 
 	// Setup discovered client
 	wrappedClient := new(MockDiscoveredHTTPClient)
-	wrappedClient.On("Do", mock.Anything, mock.Anything).Return(&http.Response{StatusCode: 200}, nil)
+	wrappedClient.On("Get", mock.Anything).Return(&http.Response{StatusCode: 200}, nil)
 
 	// Setup wrapped discoverer
 	discoverer := new(MockDiscoverer)
@@ -136,7 +136,7 @@ func Test_CacheAwareClient_when_cache_TTL_is_not_reached(t *testing.T) {
 	require.NoError(t, err, "running discovery again")
 
 	t.Run("returns_functional_cached_HTTP_client", func(t *testing.T) {
-		resp, err := client.Do("GET", "/api/path")
+		resp, err := client.Get("/api/path")
 		assert.NoError(t, err)
 		assert.Equal(t, 200, resp.StatusCode)
 	})
@@ -152,7 +152,7 @@ func Test_CacheAwareClient_perform_the_discovery_again_when_cache_TTL_is_reached
 
 	// Setup discovered client
 	wrappedClient := new(MockDiscoveredHTTPClient)
-	wrappedClient.On("Do", mock.Anything, mock.Anything).Return(&http.Response{StatusCode: 200}, nil)
+	wrappedClient.On("Get", mock.Anything).Return(&http.Response{StatusCode: 200}, nil)
 
 	// Setup wrapped discoverer
 	discoverer := new(MockDiscoverer)
@@ -199,7 +199,7 @@ func Test_CacheAwareClient_returns_error_when(t *testing.T) {
 
 		// Setup discovered client
 		wrappedClient := new(MockDiscoveredHTTPClient)
-		wrappedClient.On("Do", mock.Anything, mock.Anything).Return(&http.Response{StatusCode: 200}, nil)
+		wrappedClient.On("Get", mock.Anything).Return(&http.Response{StatusCode: 200}, nil)
 
 		// Setup wrapped discoverer
 		discoverer := new(MockDiscoverer)
@@ -226,7 +226,7 @@ func Test_CacheAwareClient_ignores_cache_decomposition_errors(t *testing.T) {
 
 	// Setup discovered client
 	wrappedClient := new(MockDiscoveredHTTPClient)
-	wrappedClient.On("Do", mock.Anything, mock.Anything).Return(&http.Response{StatusCode: 200}, nil)
+	wrappedClient.On("Get", mock.Anything, mock.Anything).Return(&http.Response{StatusCode: 200}, nil)
 
 	// Setup wrapped discoverer
 	discoverer := new(MockDiscoverer)

src/client/discovery.go

@@ -19,6 +19,17 @@ type MultiDiscoverer interface {
 
 // HTTPClient allows to connect to the discovered Kubernetes services
 type HTTPClient interface {
-	Do(method, path string) (*http.Response, error)
+	HTTPGetter
+	NodeIPGetter
+}
+
+// HTTPGetter is an interface for HTTP client with, which should provide
+// scheme, port and hostname for the HTTP call.
+type HTTPGetter interface {
+	Get(path string) (*http.Response, error)
+}
+
+// NodeIPGetter allows getting discovered Node IP.
+type NodeIPGetter interface {
 	NodeIP() string
 }

src/client/discovery_mock.go

@@ -28,8 +28,8 @@ type MockDiscoveredHTTPClient struct {
 }
 
 // Do provides a mock implementation for HTTPClient interface
-func (m *MockDiscoveredHTTPClient) Do(method, path string) (*http.Response, error) {
-	args := m.Called(method, path)
+func (m *MockDiscoveredHTTPClient) Get(path string) (*http.Response, error) {
+	args := m.Called(path)
 
 	resp := args.Get(0)
 	if resp == nil {

src/controlplane/client/discovery.go

@@ -56,7 +56,7 @@ type ControlPlaneComponentClient struct {
 	InsecureFallback         bool
 }
 
-func (c *ControlPlaneComponentClient) Do(method, urlPath string) (*http.Response, error) {
+func (c *ControlPlaneComponentClient) Get(urlPath string) (*http.Response, error) {
 	// Use the secure endpoint by default. If this component doesn't support it yet, fallback to the insecure one.
 	e := c.secureEndpoint
 	usingSecureEndpoint := true
@@ -65,7 +65,7 @@ func (c *ControlPlaneComponentClient) Do(method, urlPath string) (*http.Response
 		usingSecureEndpoint = false
 	}
 
-	r, err := c.buildPrometheusRequest(method, e, urlPath)
+	r, err := c.buildPrometheusRequest(e, urlPath)
 	if err != nil {
 		return nil, err
 	}
@@ -84,7 +84,7 @@ func (c *ControlPlaneComponentClient) Do(method, urlPath string) (*http.Response
 		c.logger.Debugf("Error when calling secure endpoint: %s", err.Error())
 		c.logger.Debugf("Falling back to insecure endpoint")
 		e = c.endpoint
-		r, err := c.buildPrometheusRequest(method, e, urlPath)
+		r, err := c.buildPrometheusRequest(e, urlPath)
 		if err != nil {
 			return nil, err
 		}
@@ -94,17 +94,18 @@ func (c *ControlPlaneComponentClient) Do(method, urlPath string) (*http.Response
 	return resp, err
 }
 
-func (c *ControlPlaneComponentClient) buildPrometheusRequest(method string, e url.URL, urlPath string) (*http.Request, error) {
+func (c *ControlPlaneComponentClient) buildPrometheusRequest(e url.URL, urlPath string) (*http.Request, error) {
 	e.Path = path.Join(e.Path, urlPath)
-	r, err := prometheus.NewRequest(method, e.String())
+	r, err := prometheus.NewRequest(e.String())
 	if err != nil {
-		return nil, fmt.Errorf("Error creating %s request to: %s. Got error: %v ", method, e.String(), err)
+		return nil, fmt.Errorf("Error creating request to: %s. Got error: %v ", e.String(), err)
 	}
 	return r, err
 }
 
 func (c *ControlPlaneComponentClient) configureAuthentication() error {
-	if c.authenticationMethod == mTLS {
+	switch c.authenticationMethod {
+	case mTLS:
 		tlsConfig, err := c.getTLSConfigFromSecret()
 		if err != nil {
 			return errors.Wrap(err, "could not load TLS configuration")
@@ -113,11 +114,7 @@ func (c *ControlPlaneComponentClient) configureAuthentication() error {
 		c.httpClient.Transport = &http.Transport{
 			TLSClientConfig: tlsConfig,
 		}
-		return nil
-	}
-
-	if c.authenticationMethod == serviceAccount {
-
+	case serviceAccount:
 		config, err := rest.InClusterConfig()
 		if err != nil {
 			return errors.Wrapf(err, "could not create in cluster Kubernetes configuration to query pod: %s", c.PodName)
@@ -130,10 +127,10 @@ func (c *ControlPlaneComponentClient) configureAuthentication() error {
 
 		// Use the default kubernetes Bearer token authentication RoundTripper
 		c.httpClient.Transport = transport.NewBearerAuthRoundTripper(config.BearerToken, t)
-		return nil
+	default:
+		c.httpClient.Transport = http.DefaultTransport
 	}
 
-	c.httpClient.Transport = http.DefaultTransport
 	return nil
 }
 

src/controlplane/client/discovery_tls_test.go

@@ -81,7 +81,7 @@ func TestMutualTLSCalls(t *testing.T) {
 		t.Run(test.name, func(t *testing.T) {
 			endpoint := startMTLSServer()
 			c := createClientComponent(endpoint, test.cacert, test.key, test.cert, test.insecureSkipVerify)
-			resp, err := c.Do("GET", "/test")
+			resp, err := c.Get("/test")
 			test.assert(t, resp, err)
 		})
 	}

src/controlplane/group.go

@@ -17,7 +17,7 @@ const prometheusMetricsPath = "/metrics"
 
 type componentGrouper struct {
 	queries []prometheus.Query
-	client  client.HTTPClient
+	client  client.HTTPGetter
 	logger  log.Logger
 	podName string
 }
@@ -43,7 +43,7 @@ func (r *componentGrouper) Group(specGroups definition.SpecGroups) (definition.R
 // NewComponentGrouper creates a grouper for the given control plane
 // component podName.
 func NewComponentGrouper(
-	c client.HTTPClient,
+	c client.HTTPGetter,
 	queries []prometheus.Query,
 	logger log.Logger,
 	podName string,

src/data/group.go

@@ -4,9 +4,6 @@ import (
 	"fmt"
 	"strings"
 
-	"github.com/newrelic/infra-integrations-sdk/integration"
-	"k8s.io/apimachinery/pkg/version"
-
 	"github.com/newrelic/nri-kubernetes/v2/src/definition"
 )
 
@@ -15,11 +12,6 @@ type Grouper interface {
 	Group(definition.SpecGroups) (definition.RawGroups, *ErrorGroup)
 }
 
-// Populator populates a given integration with grouped raw data.
-type Populator interface {
-	Populate(definition.RawGroups, definition.SpecGroups, *integration.Integration, string, *version.Info) PopulateResult
-}
-
 // ErrorGroup groups errors that can be recoverable (the execution can continue) or not
 type ErrorGroup struct {
 	Recoverable bool

src/ksm/client/client.go

@@ -0,0 +1,50 @@
+package client
+
+import (
+	"fmt"
+	"net/http"
+	"net/url"
+	"path"
+	"time"
+
+	"github.com/newrelic/infra-integrations-sdk/log"
+	"github.com/newrelic/nri-kubernetes/v2/src/prometheus"
+)
+
+// ksm implements Client interface
+type ksm struct {
+	httpClient *http.Client
+	endpoint   url.URL
+	nodeIP     string
+	logger     log.Logger
+}
+
+func newKSMClient(timeout time.Duration, nodeIP string, endpoint url.URL, logger log.Logger) *ksm {
+	return &ksm{
+		nodeIP:   nodeIP,
+		endpoint: endpoint,
+		httpClient: &http.Client{
+			Timeout: timeout,
+		},
+		logger: logger,
+	}
+}
+
+func (c *ksm) NodeIP() string {
+	return c.nodeIP
+}
+
+func (c *ksm) Get(urlPath string) (*http.Response, error) {
+	e := c.endpoint
+	e.Path = path.Join(c.endpoint.Path, urlPath)
+
+	// Creates Prometheus request.
+	r, err := prometheus.NewRequest(e.String())
+	if err != nil {
+		return nil, fmt.Errorf("Error creating request to: %s. Got error: %s ", e.String(), err)
+	}
+
+	c.logger.Debugf("Calling kube-state-metrics endpoint: %s", r.URL.String())
+
+	return c.httpClient.Do(r)
+}