feat: add support for monitoring istio sidecars (#35)

* feat: add support for monitoring istio sidecars

* chore: update port in classes.yml

* refactor: use status annotation for istio sidecar

* chore: more changes after code review

Dockerfile

@@ -10,7 +10,7 @@ COPY go.sum go.sum
 RUN go mod download
 
 # Copy the go source
-COPY main.go sidecar.go handler.go class_data.go ./
+COPY main.go sidecar.go handler.go class_data.go errors.go ./
 
 # Build
 RUN CGO_ENABLED=0 GOOS=linux GOARCH=amd64 GO111MODULE=on go build -a -o manager *.go

class_data.go

@@ -20,16 +20,13 @@ import (
 	"os"
 	"path/filepath"
 
-	corev1 "k8s.io/api/core/v1"
-
 	"github.com/go-logr/logr"
 	"github.com/influxdata/toml"
 )
 
 type classDataHandler struct {
 	Logger                   logr.Logger
 	TelegrafClassesDirectory string
-	TelegrafDefaultClass     string
 }
 
 func (c *classDataHandler) validateClassData() error {
@@ -75,12 +72,7 @@ func (c *classDataHandler) validateClassData() error {
 	return nil
 }
 
-func (c *classDataHandler) getData(pod *corev1.Pod) (string, error) {
-	className := c.TelegrafDefaultClass
-	if extClass, ok := pod.Annotations[TelegrafClass]; ok {
-		className = extClass
-	}
-
+func (c *classDataHandler) getData(className string) (string, error) {
 	data, err := ioutil.ReadFile(filepath.Join(c.TelegrafClassesDirectory, className))
 
 	if err != nil {

class_data_test.go

@@ -4,9 +4,6 @@ import (
 	"os"
 	"testing"
 
-	corev1 "k8s.io/api/core/v1"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-
 	logrTesting "github.com/go-logr/logr/testing"
 )
 
@@ -24,7 +21,6 @@ func Test_classDataHandler_getData(t *testing.T) {
 		secretName string
 		className  string
 		namespace  string
-		pod        *corev1.Pod
 
 		want    string
 		wantErr bool
@@ -33,31 +29,18 @@ func Test_classDataHandler_getData(t *testing.T) {
 			name:      "data does not contain class name",
 			className: "unknown",
 			classes:   map[string]string{testTelegrafClass: sampleClassData},
-			pod:       &corev1.Pod{},
 			wantErr:   true,
 		},
 		{
 			name:      "returns secret data",
 			className: testTelegrafClass,
 			classes:   map[string]string{testTelegrafClass: sampleClassData},
-			pod:       &corev1.Pod{},
 			want:      sampleClassData,
 		},
-		{
-			name:    "returns secret data with annotation override",
-			classes: map[string]string{"name_override": sampleClassData},
-			pod: &corev1.Pod{
-				ObjectMeta: metav1.ObjectMeta{
-					Annotations: map[string]string{
-						TelegrafClass: "name_override",
-					},
-				},
-			},
-			want: sampleClassData,
-		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
+
 			logger := &logrTesting.TestLogger{T: t}
 
 			dir := createTempClassesDirectory(t, tt.classes)
@@ -66,10 +49,9 @@ func Test_classDataHandler_getData(t *testing.T) {
 			testClassDataHandler := &classDataHandler{
 				Logger:                   logger,
 				TelegrafClassesDirectory: dir,
-				TelegrafDefaultClass:     tt.className,
 			}
 
-			got, err := testClassDataHandler.getData(tt.pod)
+			got, err := testClassDataHandler.getData(tt.className)
 			if (err != nil) != tt.wantErr {
 				t.Errorf("classDataHandler.getData() error = %v, wantErr %v", err, tt.wantErr)
 				return
@@ -81,7 +63,6 @@ func Test_classDataHandler_getData(t *testing.T) {
 	}
 }
 
-// TODO: test validateClassData
 func Test_classDataHandler_validateClassData(t *testing.T) {
 	tests := []struct {
 		name    string
@@ -117,7 +98,6 @@ func Test_classDataHandler_validateClassData(t *testing.T) {
 			testClassDataHandler := &classDataHandler{
 				Logger:                   logger,
 				TelegrafClassesDirectory: dir,
-				TelegrafDefaultClass:     testTelegrafClass,
 			}
 
 			err := testClassDataHandler.validateClassData()

deploy/dev.yml

@@ -62,6 +62,7 @@ webhooks:
         apiVersions: ["*"]
         resources: ["pods"]
     failurePolicy: Ignore
+    reinvocationPolicy: IfNeeded
 ---
 apiVersion: apps/v1
 kind: Deployment
@@ -94,6 +95,9 @@ spec:
             # default class to use if not specified by the pod
             - --telegraf-default-class=basic
             - --telegraf-classes-directory=/config/classes
+            # allow injecting telegraf-istio sidecar for pods with
+            # istio sidecar annotations enabled
+            - --enable-istio-injection=true
           env:
             - name: POD_NAMESPACE
               valueFrom:

errors.go

@@ -0,0 +1,18 @@
+package main
+
+// error that notifies the handler that error occurred, but the pod should be created
+type nonFatalError struct {
+	err     error
+	message string
+}
+
+func newNonFatalError(err error, message string) error {
+	return &nonFatalError{
+		err:     err,
+		message: message,
+	}
+}
+
+func (e *nonFatalError) Error() string {
+	return e.err.Error()
+}

examples/classes.yml

@@ -67,3 +67,7 @@ stringData:
       metric_batch_size = 100000
       metric_buffer_limit = 100000
       collection_jitter = "0s"
+  # example for gathering istio
+  istio: |+
+    [[outputs.influxdb]]
+      urls = ["http://influxdb.influxdb:8086"]

examples/nginx-with-istio.yml

@@ -0,0 +1,23 @@
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: nginx-with-istio
+  namespace: test
+spec:
+  replicas: 3
+  selector:
+    matchLabels:
+      app: nginx-with-istio
+  template:
+    metadata:
+      labels:
+        app: nginx-with-istio
+      annotations:
+        # this will cause istio to inject its sidecar, that telegraf will then monitor
+        sidecar.istio.io/inject: "true"
+        # inject the status of the istio sidecar to mimic istio being present
+        sidecar.istio.io/status: "dummy"
+    spec:
+      containers:
+      - name: nginx
+        image: nginx:alpine

handler.go

@@ -53,21 +53,29 @@ func (a *podInjector) Handle(ctx context.Context, req admission.Request) admissi
 	handlerLog.V(9).Info("request=" + string(marshaled))
 
 	if req.Operation == admv1.Delete {
-		secret := &corev1.Secret{
-			TypeMeta: metav1.TypeMeta{
-				Kind:       "Secret",
-				APIVersion: "v1",
-			},
-			ObjectMeta: metav1.ObjectMeta{
-				Name:      fmt.Sprintf("%s-%s", telegrafSecretPrefix, req.Name),
-				Namespace: req.Namespace,
-			},
+		deleteFailed := false
+		for _, name := range a.SidecarHandler.telegrafSecretNames(req.Name) {
+			secret := &corev1.Secret{
+				TypeMeta: metav1.TypeMeta{
+					Kind:       "Secret",
+					APIVersion: "v1",
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      name,
+					Namespace: req.Namespace,
+				},
+			}
+			handlerLog.Info("Deleting secret=" + secret.Name + "/" + secret.Namespace)
+			err := a.client.Delete(ctx, secret)
+			if err != nil {
+				handlerLog.Info("secret=" + secret.Name + "/" + secret.Namespace + " error:" + err.Error())
+				deleteFailed = true
+			}
 		}
-		err := a.client.Delete(ctx, secret)
-		if err != nil {
-			handlerLog.Info("secret=" + secret.Name + "/" + secret.Namespace + " error:" + err.Error())
-			return admission.Allowed("telegraf-injector coudn't delete secret")
+		if deleteFailed {
+			return admission.Allowed("telegraf-injector couldn't delete one or more secrets")
 		}
+
 		return admission.Allowed("telegraf-injector doesn't block pod deletions")
 	}
 
@@ -88,38 +96,42 @@ func (a *podInjector) Handle(ctx context.Context, req admission.Request) admissi
 		handlerLog.Info("name: " + name + ",  pod_getname=" + pod.GetName())
 	}
 
-	classData, err := a.ClassDataHandler.getData(pod)
-	if err != nil {
-		a.Logger.Info(fmt.Sprintf("unable to find class data: %v ; not adding sidecar container", err))
-		return admission.Allowed("telegraf-operator could not create sidecar container")
-	}
-
-	telegrafConf, err := a.SidecarHandler.assembleConf(pod, classData)
-	if err != nil {
-		a.Logger.Info(fmt.Sprintf("unable to assemble telegraf configuration: %v", err))
-		return admission.Allowed("telegraf-operator could not create sidecar container")
-	}
-
 	a.Logger.Info("adding sidecar container")
 	// if the telegraf configuration could be created, add sidecar pod
-	secret, err := a.SidecarHandler.addSidecar(pod, pod.GetName(), req.Namespace, telegrafConf)
+	result, err := a.SidecarHandler.addSidecars(pod, pod.GetName(), req.Namespace)
 	if err != nil {
-		a.Logger.Error(err, "unable to add sidecar container")
+
+		if nonFatalErr, ok := err.(*nonFatalError); ok {
+			a.Logger.Info(
+				fmt.Sprintf(
+					"unable to add telegraf sidecar container(s): %v ; not adding sidecar container, but allowing creation: %s",
+					nonFatalErr.err,
+					nonFatalErr.message,
+				),
+			)
+			return admission.Allowed(nonFatalErr.message)
+		}
+
+		a.Logger.Info(fmt.Sprintf("unable to add telegraf sidecar container(s): %v ; reporting error", err))
 		return admission.Errored(http.StatusBadRequest, err)
 	}
 
 	if req.Operation == admv1.Create {
-		err = a.client.Create(ctx, secret)
-		if err != nil {
-			a.Logger.Error(err, "unable to create secret")
-			return admission.Errored(http.StatusBadRequest, err)
+		for _, secret := range result.secrets {
+			err = a.client.Create(ctx, secret)
+			if err != nil {
+				a.Logger.Error(err, "unable to create secret")
+				return admission.Errored(http.StatusBadRequest, err)
+			}
 		}
 	}
 	if req.Operation == admv1.Update {
-		err = a.client.Update(ctx, secret)
-		if err != nil {
-			a.Logger.Error(err, "unable to update secret")
-			return admission.Errored(http.StatusBadRequest, err)
+		for _, secret := range result.secrets {
+			err = a.client.Update(ctx, secret)
+			if err != nil {
+				a.Logger.Error(err, "unable to update secret")
+				return admission.Errored(http.StatusBadRequest, err)
+			}
 		}
 	}