From 49d39e5a123ecf40e12d51f28d4185eb1bf7512f Mon Sep 17 00:00:00 2001 From: Tarun Pothulapati Date: Fri, 20 Sep 2019 22:16:57 +0530 Subject: [PATCH] Instrumenting Proxy-Injector (#3354) * add proxy injection prometheus counters Signed-off-by: Tarun Pothulapati * formatted injection reasons Signed-off-by: Tarun Pothulapati * update proxy injection report tests Signed-off-by: Tarun Pothulapati * keep the structure, and add global ownerKind Signed-off-by: Tarun Pothulapati * increase request count, when owner is nil Signed-off-by: Tarun Pothulapati * add readable reasons using map Signed-off-by: Tarun Pothulapati * fix linting issues Signed-off-by: Tarun Pothulapati * add proxy config override annotations as labels Signed-off-by: Tarun Pothulapati * remove space for machine reasons Signed-off-by: Tarun Pothulapati * use correct proxy image override annotation Signed-off-by: Tarun Pothulapati * add annotation_at label to prom metrics Signed-off-by: Tarun Pothulapati * refactor disablebyannotation function Signed-off-by: Tarun Pothulapati --- controller/proxy-injector/metrics.go | 74 ++++++++++++++++++++++ controller/proxy-injector/webhook.go | 23 +++++-- pkg/inject/inject.go | 39 +++++++++++- pkg/inject/report.go | 52 +++++++++++---- pkg/inject/report_test.go | 95 +++++++++++++++++++++++++--- 5 files changed, 253 insertions(+), 30 deletions(-) create mode 100644 controller/proxy-injector/metrics.go diff --git a/controller/proxy-injector/metrics.go b/controller/proxy-injector/metrics.go new file mode 100644 index 000000000..6606b34cd --- /dev/null +++ b/controller/proxy-injector/metrics.go @@ -0,0 +1,74 @@ +package injector + +import ( + "strings" + + "github.com/linkerd/linkerd2/pkg/k8s" + + "github.com/linkerd/linkerd2/pkg/inject" + "github.com/prometheus/client_golang/prometheus" + "github.com/prometheus/client_golang/prometheus/promauto" +) + +const ( + labelOwnerKind = "owner_kind" + labelNamespace = "namespace" + labelSkip = "skip" + labelAnnotationAt = "annotation_at" + labelReason = "skip_reason" +) + +var ( + requestLabels = []string{labelOwnerKind, labelNamespace, labelAnnotationAt} + responseLabels = []string{labelOwnerKind, labelNamespace, labelSkip, labelReason, labelAnnotationAt} + + proxyInjectionAdmissionRequests = promauto.NewCounterVec(prometheus.CounterOpts{ + Name: "proxy_inject_admission_requests_total", + Help: "A counter for number of admission requests to proxy injector.", + }, append(requestLabels, validLabelNames(inject.ProxyAnnotations)...)) + + proxyInjectionAdmissionResponses = promauto.NewCounterVec(prometheus.CounterOpts{ + Name: "proxy_inject_admission_responses_total", + Help: "A counter for number of admission responses from proxy injector.", + }, append(responseLabels, validLabelNames(inject.ProxyAnnotations)...)) +) + +func admissionRequestLabels(ownerKind, namespace, annotationAt string, configLabels prometheus.Labels) prometheus.Labels { + configLabels[labelOwnerKind] = ownerKind + configLabels[labelNamespace] = namespace + configLabels[labelAnnotationAt] = annotationAt + return configLabels +} + +func admissionResponseLabels(owner, namespace, skip, reason, annotationAt string, configLabels prometheus.Labels) prometheus.Labels { + configLabels[labelOwnerKind] = owner + configLabels[labelNamespace] = namespace + configLabels[labelSkip] = skip + configLabels[labelReason] = reason + configLabels[labelAnnotationAt] = annotationAt + return configLabels +} + +func configToPrometheusLabels(conf *inject.ResourceConfig) prometheus.Labels { + labels := conf.GetOverriddenConfiguration() + promLabels := map[string]string{} + + for label, value := range labels { + promLabels[validProxyConfigurationLabel(label)] = value + + } + return promLabels +} + +func validLabelNames(labels []string) []string { + var validLabels []string + + for _, label := range labels { + validLabels = append(validLabels, validProxyConfigurationLabel(label)) + } + return validLabels +} + +func validProxyConfigurationLabel(label string) string { + return strings.Replace(label[len(k8s.ProxyConfigAnnotationsPrefix)+1:], "-", "_", -1) +} diff --git a/controller/proxy-injector/webhook.go b/controller/proxy-injector/webhook.go index 8365ac97e..d91bab957 100644 --- a/controller/proxy-injector/webhook.go +++ b/controller/proxy-injector/webhook.go @@ -2,6 +2,7 @@ package injector import ( "fmt" + "strings" pb "github.com/linkerd/linkerd2/controller/gen/config" "github.com/linkerd/linkerd2/controller/k8s" @@ -61,7 +62,9 @@ func Inject(api *k8s.API, Allowed: true, } + configLabels := configToPrometheusLabels(resourceConfig) var parent *runtime.Object + ownerKind := "" if ownerRef := resourceConfig.GetOwnerRef(); ownerRef != nil { objs, err := api.GetObjects(request.Namespace, ownerRef.Kind, ownerRef.Name) if err != nil { @@ -71,13 +74,23 @@ func Inject(api *k8s.API, } else { parent = &objs[0] } + ownerKind = strings.ToLower(ownerRef.Kind) } + proxyInjectionAdmissionRequests.With(admissionRequestLabels(ownerKind, request.Namespace, report.InjectAnnotationAt, configLabels)).Inc() - if injectable, reason := report.Injectable(); !injectable { - if parent != nil { - recorder.Eventf(*parent, v1.EventTypeNormal, eventTypeSkipped, "Linkerd sidecar proxy injection skipped: %s", reason) + if injectable, reasons := report.Injectable(); !injectable { + var readableReasons, metricReasons string + metricReasons = strings.Join(reasons, ",") + for _, reason := range reasons { + readableReasons = readableReasons + ", " + inject.Reasons[reason] } - log.Infof("skipped %s: %s", report.ResName(), reason) + // removing the initial comma, space + readableReasons = readableReasons[2:] + if parent != nil { + recorder.Eventf(*parent, v1.EventTypeNormal, eventTypeSkipped, "Linkerd sidecar proxy injection skipped: %s", readableReasons) + } + log.Infof("skipped %s: %s", report.ResName(), readableReasons) + proxyInjectionAdmissionResponses.With(admissionResponseLabels(ownerKind, request.Namespace, "true", metricReasons, report.InjectAnnotationAt, configLabels)).Inc() return admissionResponse, nil } @@ -98,7 +111,7 @@ func Inject(api *k8s.API, } log.Infof("patch generated for: %s", report.ResName()) log.Debugf("patch: %s", patchJSON) - + proxyInjectionAdmissionResponses.With(admissionResponseLabels(ownerKind, request.Namespace, "false", "", report.InjectAnnotationAt, configLabels)).Inc() patchType := admissionv1beta1.PatchTypeJSONPatch admissionResponse.Patch = patchJSON admissionResponse.PatchType = &patchType diff --git a/pkg/inject/inject.go b/pkg/inject/inject.go index c4adc9a15..892c63265 100644 --- a/pkg/inject/inject.go +++ b/pkg/inject/inject.go @@ -29,7 +29,34 @@ const ( proxyInitResourceLimitMemory = "50Mi" ) -var rTrail = regexp.MustCompile(`\},\s*\]`) +var ( + rTrail = regexp.MustCompile(`\},\s*\]`) + + // ProxyAnnotations is the list of possible annotations that can be applied on a pod or namespace + ProxyAnnotations = []string{ + k8s.ProxyAdminPortAnnotation, + k8s.ProxyControlPortAnnotation, + k8s.ProxyDisableIdentityAnnotation, + k8s.ProxyDisableTapAnnotation, + k8s.ProxyEnableDebugAnnotation, + k8s.ProxyEnableExternalProfilesAnnotation, + k8s.ProxyImagePullPolicyAnnotation, + k8s.ProxyInboundPortAnnotation, + k8s.ProxyInitImageAnnotation, + k8s.ProxyInitImageVersionAnnotation, + k8s.ProxyOutboundPortAnnotation, + k8s.ProxyCPULimitAnnotation, + k8s.ProxyCPURequestAnnotation, + k8s.ProxyImageAnnotation, + k8s.ProxyLogLevelAnnotation, + k8s.ProxyMemoryLimitAnnotation, + k8s.ProxyMemoryRequestAnnotation, + k8s.ProxyUIDAnnotation, + k8s.ProxyVersionOverrideAnnotation, + k8s.ProxyIgnoreInboundPortsAnnotation, + k8s.ProxyIgnoreOutboundPortsAnnotation, + } +) // Origin defines where the input YAML comes from. Refer the ResourceConfig's // 'origin' field @@ -783,6 +810,16 @@ func (conf *ResourceConfig) proxyOutboundSkipPorts() string { return strings.Join(ports, ",") } +// GetOverriddenConfiguration returns a map of the overridden proxy annotations +func (conf *ResourceConfig) GetOverriddenConfiguration() map[string]string { + proxyOverrideConfig := map[string]string{} + for _, annotation := range ProxyAnnotations { + proxyOverrideConfig[annotation] = conf.getOverride(annotation) + } + + return proxyOverrideConfig +} + func sortedKeys(m map[string]string) []string { keys := []string{} for k := range m { diff --git a/pkg/inject/report.go b/pkg/inject/report.go index 4a7f3624f..7d84510e1 100644 --- a/pkg/inject/report.go +++ b/pkg/inject/report.go @@ -9,6 +9,27 @@ import ( v1 "k8s.io/api/core/v1" ) +const ( + hostNetworkEnabled = "host_network_enabled" + sidecarExists = "sidecar_already_exists" + unsupportedResource = "unsupported_resource" + injectEnableAnnotationAbsent = "injection_enable_annotation_absent" + injectDisableAnnotationPresent = "injection_disable_annotation_present" + annotationAtNamespace = "namespace" + annotationAtWorkload = "workload" +) + +var ( + // Reasons is a map of inject skip reasons with human readable sentences + Reasons = map[string]string{ + hostNetworkEnabled: "hostNetwork is enabled", + sidecarExists: "pod has a sidecar injected already", + unsupportedResource: "this resource kind is unsupported", + injectEnableAnnotationAbsent: fmt.Sprintf("neither the namespace nor the pod have the annotation \"%s:%s\"", k8s.ProxyInjectAnnotation, k8s.ProxyInjectEnabled), + injectDisableAnnotationPresent: fmt.Sprintf("pod has the annotation \"%s:%s\"", k8s.ProxyInjectAnnotation, k8s.ProxyInjectDisabled), + } +) + // Report contains the Kind and Name for a given workload along with booleans // describing the result of the injection transformation type Report struct { @@ -20,6 +41,7 @@ type Report struct { UnsupportedResource bool InjectDisabled bool InjectDisabledReason string + InjectAnnotationAt string // Uninjected consists of two boolean flags to indicate if a proxy and // proxy-init containers have been uninjected in this report @@ -51,7 +73,7 @@ func newReport(conf *ResourceConfig) *Report { } if conf.pod.meta != nil && conf.pod.spec != nil { - report.InjectDisabled, report.InjectDisabledReason = report.disableByAnnotation(conf) + report.InjectDisabled, report.InjectDisabledReason, report.InjectAnnotationAt = report.disableByAnnotation(conf) report.HostNetwork = conf.pod.spec.HostNetwork report.Sidecar = healthcheck.HasExistingSidecars(conf.pod.spec) report.UDP = checkUDPPorts(conf.pod.spec) @@ -70,25 +92,25 @@ func (r *Report) ResName() string { // Injectable returns false if the report flags indicate that the workload is on a host network // or there is already a sidecar or the resource is not supported or inject is explicitly disabled. // If false, the second returned value describes the reason. -func (r *Report) Injectable() (bool, string) { - reasons := []string{} +func (r *Report) Injectable() (bool, []string) { + var reasons []string if r.HostNetwork { - reasons = append(reasons, "hostNetwork is enabled") + reasons = append(reasons, hostNetworkEnabled) } if r.Sidecar { - reasons = append(reasons, "pod has a sidecar injected already") + reasons = append(reasons, sidecarExists) } if r.UnsupportedResource { - reasons = append(reasons, "this resource kind is unsupported") + reasons = append(reasons, unsupportedResource) } if r.InjectDisabled { reasons = append(reasons, r.InjectDisabledReason) } if len(reasons) > 0 { - return false, strings.Join(reasons, ", ") + return false, reasons } - return true, "" + return true, nil } func checkUDPPorts(t *v1.PodSpec) bool { @@ -103,7 +125,9 @@ func checkUDPPorts(t *v1.PodSpec) bool { return false } -func (r *Report) disableByAnnotation(conf *ResourceConfig) (bool, string) { +// disabledByAnnotation checks annotations for both workload, namespace and returns +// if disabled, Inject Disabled reason and the resource where that annotation was present +func (r *Report) disableByAnnotation(conf *ResourceConfig) (bool, string, string) { // truth table of the effects of the inject annotation: // // origin | namespace | pod | inject? | return @@ -125,19 +149,19 @@ func (r *Report) disableByAnnotation(conf *ResourceConfig) (bool, string) { nsAnnotation := conf.nsAnnotations[k8s.ProxyInjectAnnotation] if conf.origin == OriginCLI { - return podAnnotation == k8s.ProxyInjectDisabled, "" + return podAnnotation == k8s.ProxyInjectDisabled, "", "" } if nsAnnotation == k8s.ProxyInjectEnabled { if podAnnotation == k8s.ProxyInjectDisabled { - return true, fmt.Sprintf("pod has the annotation \"%s:%s\"", k8s.ProxyInjectAnnotation, k8s.ProxyInjectDisabled) + return true, injectDisableAnnotationPresent, annotationAtWorkload } - return false, "" + return false, "", annotationAtNamespace } if podAnnotation != k8s.ProxyInjectEnabled { - return true, fmt.Sprintf("neither the namespace nor the pod have the annotation \"%s:%s\"", k8s.ProxyInjectAnnotation, k8s.ProxyInjectEnabled) + return true, injectEnableAnnotationAbsent, "" } - return false, "" + return false, "", annotationAtWorkload } diff --git a/pkg/inject/report_test.go b/pkg/inject/report_test.go index 32b6f6d79..cf4692605 100644 --- a/pkg/inject/report_test.go +++ b/pkg/inject/report_test.go @@ -16,7 +16,7 @@ func TestInjectable(t *testing.T) { nsAnnotations map[string]string unsupportedResource bool injectable bool - reason string + reasons []string }{ { podSpec: &corev1.PodSpec{HostNetwork: false}, @@ -35,7 +35,7 @@ func TestInjectable(t *testing.T) { }, }, injectable: false, - reason: "hostNetwork is enabled", + reasons: []string{hostNetworkEnabled}, }, { podSpec: &corev1.PodSpec{ @@ -52,7 +52,7 @@ func TestInjectable(t *testing.T) { }, }, injectable: false, - reason: "pod has a sidecar injected already", + reasons: []string{sidecarExists}, }, { podSpec: &corev1.PodSpec{ @@ -69,7 +69,7 @@ func TestInjectable(t *testing.T) { }, }, injectable: false, - reason: "pod has a sidecar injected already", + reasons: []string{sidecarExists}, }, { unsupportedResource: true, @@ -80,7 +80,73 @@ func TestInjectable(t *testing.T) { }, }, injectable: false, - reason: "this resource kind is unsupported", + reasons: []string{unsupportedResource}, + }, + { + unsupportedResource: true, + podSpec: &corev1.PodSpec{HostNetwork: true}, + podMeta: &metav1.ObjectMeta{ + Annotations: map[string]string{ + k8s.ProxyInjectAnnotation: k8s.ProxyInjectEnabled, + }, + }, + + injectable: false, + reasons: []string{hostNetworkEnabled, unsupportedResource}, + }, + { + nsAnnotations: map[string]string{ + k8s.ProxyInjectAnnotation: k8s.ProxyInjectEnabled, + }, + podSpec: &corev1.PodSpec{HostNetwork: true}, + podMeta: &metav1.ObjectMeta{ + Annotations: map[string]string{ + k8s.ProxyInjectAnnotation: k8s.ProxyInjectDisabled, + }, + }, + + injectable: false, + reasons: []string{hostNetworkEnabled, injectDisableAnnotationPresent}, + }, + { + nsAnnotations: map[string]string{ + k8s.ProxyInjectAnnotation: k8s.ProxyInjectEnabled, + }, + unsupportedResource: true, + podSpec: &corev1.PodSpec{HostNetwork: true}, + podMeta: &metav1.ObjectMeta{ + Annotations: map[string]string{ + k8s.ProxyInjectAnnotation: k8s.ProxyInjectDisabled, + }, + }, + + injectable: false, + reasons: []string{hostNetworkEnabled, unsupportedResource, injectDisableAnnotationPresent}, + }, + { + unsupportedResource: true, + podSpec: &corev1.PodSpec{HostNetwork: true}, + podMeta: &metav1.ObjectMeta{ + Annotations: map[string]string{}, + }, + + injectable: false, + reasons: []string{hostNetworkEnabled, unsupportedResource, injectEnableAnnotationAbsent}, + }, + { + podSpec: &corev1.PodSpec{HostNetwork: true, + Containers: []corev1.Container{ + { + Name: k8s.ProxyContainerName, + Image: "gcr.io/linkerd-io/proxy:", + }, + }}, + podMeta: &metav1.ObjectMeta{ + Annotations: map[string]string{}, + }, + + injectable: false, + reasons: []string{hostNetworkEnabled, sidecarExists, injectEnableAnnotationAbsent}, }, } @@ -90,18 +156,27 @@ func TestInjectable(t *testing.T) { resourceConfig := &ResourceConfig{} resourceConfig.WithNsAnnotations(testCase.nsAnnotations) resourceConfig.pod.spec = testCase.podSpec + resourceConfig.origin = OriginWebhook resourceConfig.pod.meta = testCase.podMeta report := newReport(resourceConfig) report.UnsupportedResource = testCase.unsupportedResource - actual, reason := report.Injectable() + actual, reasons := report.Injectable() if testCase.injectable != actual { t.Errorf("Expected %t. Actual %t", testCase.injectable, actual) } - if testCase.reason != reason { - t.Errorf("Expected reason '%s'. Actual reason '%s'", testCase.reason, reason) + + if len(reasons) != len(testCase.reasons) { + t.Errorf("Expected %d number of reasons. Actual %d", len(testCase.reasons), len(reasons)) } + + for i := range reasons { + if testCase.reasons[i] != reasons[i] { + t.Errorf("Expected reason '%s'. Actual reason '%s'", testCase.reasons[i], reasons[i]) + } + } + }) } } @@ -203,7 +278,7 @@ func TestDisableByAnnotation(t *testing.T) { resourceConfig.pod.meta = testCase.podMeta report := newReport(resourceConfig) - if actual, _ := report.disableByAnnotation(resourceConfig); testCase.expected != actual { + if actual, _, _ := report.disableByAnnotation(resourceConfig); testCase.expected != actual { t.Errorf("Expected %t. Actual %t", testCase.expected, actual) } }) @@ -244,7 +319,7 @@ func TestDisableByAnnotation(t *testing.T) { resourceConfig.pod.meta = testCase.podMeta report := newReport(resourceConfig) - if actual, _ := report.disableByAnnotation(resourceConfig); testCase.expected != actual { + if actual, _, _ := report.disableByAnnotation(resourceConfig); testCase.expected != actual { t.Errorf("Expected %t. Actual %t", testCase.expected, actual) } })