From 8621feef1809a255e7c074732301c3a0eb9a64dd Mon Sep 17 00:00:00 2001 From: Vincent Behar Date: Thu, 24 Oct 2019 10:17:34 +0200 Subject: [PATCH 1/2] Rename the annotation used to inject the proxy Following #47 here, let's rename the annotation used to inject the proxy in the pods: - previous annotation key: `osiris.deislabs.io/enabled` - new annotation key: `osiris.deislabs.io/injectProxy` so that we can avoid confusion between the deployments and pods annotations --- README.md | 4 ++-- example/hello-osiris.yaml | 2 +- pkg/kubernetes/osiris.go | 13 ++++++++++++- pkg/metrics/proxy/injector/pod_patch.go | 2 +- 4 files changed, 16 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 8bc2963a..a150b73b 100644 --- a/README.md +++ b/README.md @@ -141,7 +141,7 @@ spec: labels: app: nginx annotations: - osiris.deislabs.io/enabled: "true" + osiris.deislabs.io/injectProxy: "true" # ... # ... ``` @@ -192,7 +192,7 @@ The following table lists the supported annotations for Kubernetes `Deployments` | Annotation | Description | Default | | ---------- | ----------- | ------- | -| `osiris.deislabs.io/enabled` | Enable the metrics collecting proxy sidecar container to be injected into this pod. Allowed values: `y`, `yes`, `true`, `on`, `1`. | _no value_ (= disabled) | +| `osiris.deislabs.io/injectProxy` | Inject a transparent proxy as a sidecar container into this pod. This is _required_ for metrics collection. Allowed values: `y`, `yes`, `true`, `on`, `1`. | _no value_ (= disabled) | #### Service Annotations diff --git a/example/hello-osiris.yaml b/example/hello-osiris.yaml index 9d8bb407..0f3c6b45 100644 --- a/example/hello-osiris.yaml +++ b/example/hello-osiris.yaml @@ -56,7 +56,7 @@ spec: labels: app: hello-osiris annotations: - osiris.deislabs.io/enabled: "true" + osiris.deislabs.io/injectProxy: "true" spec: containers: - name: hello-osiris diff --git a/pkg/kubernetes/osiris.go b/pkg/kubernetes/osiris.go index 74590879..f525a36f 100644 --- a/pkg/kubernetes/osiris.go +++ b/pkg/kubernetes/osiris.go @@ -8,13 +8,24 @@ import ( const ( osirisEnabledAnnotationName = "osiris.deislabs.io/enabled" + injectProxyAnnotationName = "osiris.deislabs.io/injectProxy" metricsCheckIntervalAnnotationName = "osiris.deislabs.io/metricsCheckInterval" ) // ResourceIsOsirisEnabled checks the annotations to see if the // kube resource is enabled for osiris or not. func ResourceIsOsirisEnabled(annotations map[string]string) bool { - enabled, ok := annotations[osirisEnabledAnnotationName] + return annotationBooleanValue(annotations, osirisEnabledAnnotationName) +} + +// PodIsEligibleForProxyInjection checks the annotations to see if the +// pod is eligible for proxy injection or not. +func PodIsEligibleForProxyInjection(annotations map[string]string) bool { + return annotationBooleanValue(annotations, injectProxyAnnotationName) +} + +func annotationBooleanValue(annotations map[string]string, key string) bool { + enabled, ok := annotations[key] if !ok { return false } diff --git a/pkg/metrics/proxy/injector/pod_patch.go b/pkg/metrics/proxy/injector/pod_patch.go index 041360e8..136582bf 100644 --- a/pkg/metrics/proxy/injector/pod_patch.go +++ b/pkg/metrics/proxy/injector/pod_patch.go @@ -40,7 +40,7 @@ func (i *injector) getPodPatchOperations( req.UserInfo, ) - if !kubernetes.ResourceIsOsirisEnabled(pod.Annotations) || + if !kubernetes.PodIsEligibleForProxyInjection(pod.Annotations) || (podContainsProxyInitContainer(&pod) && podContainsProxyContainer(&pod)) { return nil, nil } From ef8e139fdb957f240867bcdf8ed9ad17c989f622 Mon Sep 17 00:00:00 2001 From: Vincent Behar Date: Fri, 25 Oct 2019 08:29:57 +0200 Subject: [PATCH 2/2] rename annotation: injectProxy -> collectMetrics to have an annotation that references purpose more than implementation --- README.md | 4 ++-- example/hello-osiris.yaml | 2 +- pkg/kubernetes/osiris.go | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 40b79cca..1555f6a4 100644 --- a/README.md +++ b/README.md @@ -141,7 +141,7 @@ spec: labels: app: nginx annotations: - osiris.deislabs.io/injectProxy: "true" + osiris.deislabs.io/collectMetrics: "true" # ... # ... ``` @@ -194,7 +194,7 @@ The following table lists the supported annotations for Kubernetes `Pods` and th | Annotation | Description | Default | | ---------- | ----------- | ------- | -| `osiris.deislabs.io/injectProxy` | Enable the metrics collecting proxy sidecar container to be injected into this pod. Inject a transparent proxy as a sidecar container into this pod. This is _required_ for metrics collection. Allowed values: `y`, `yes`, `true`, `on`, `1`. | _no value_ (= disabled) | +| `osiris.deislabs.io/collectMetrics` | Enable the metrics collecting proxy to be injected as a sidecar container into this pod. This is _required_ for metrics collection. Allowed values: `y`, `yes`, `true`, `on`, `1`. | _no value_ (= disabled) | | `osiris.deislabs.io/ignoredPaths` | The list of (url) paths that should be "ignored" by Osiris. Requests to such paths won't be "counted" by the proxy. Format: comma-separated string. | _no value_ | #### Service Annotations diff --git a/example/hello-osiris.yaml b/example/hello-osiris.yaml index f4f6255f..3beb45e4 100644 --- a/example/hello-osiris.yaml +++ b/example/hello-osiris.yaml @@ -56,7 +56,7 @@ spec: labels: app: hello-osiris annotations: - osiris.deislabs.io/injectProxy: "true" + osiris.deislabs.io/collectMetrics: "true" osiris.deislabs.io/ignoredPaths: "/first/path,/second-path" spec: containers: diff --git a/pkg/kubernetes/osiris.go b/pkg/kubernetes/osiris.go index 84559dff..823124a5 100644 --- a/pkg/kubernetes/osiris.go +++ b/pkg/kubernetes/osiris.go @@ -9,7 +9,7 @@ import ( const ( IgnoredPathsAnnotationName = "osiris.deislabs.io/ignoredPaths" osirisEnabledAnnotationName = "osiris.deislabs.io/enabled" - injectProxyAnnotationName = "osiris.deislabs.io/injectProxy" + collectMetricsAnnotationName = "osiris.deislabs.io/collectMetrics" metricsCheckIntervalAnnotationName = "osiris.deislabs.io/metricsCheckInterval" ) @@ -22,7 +22,7 @@ func ResourceIsOsirisEnabled(annotations map[string]string) bool { // PodIsEligibleForProxyInjection checks the annotations to see if the // pod is eligible for proxy injection or not. func PodIsEligibleForProxyInjection(annotations map[string]string) bool { - return annotationBooleanValue(annotations, injectProxyAnnotationName) + return annotationBooleanValue(annotations, collectMetricsAnnotationName) } func annotationBooleanValue(annotations map[string]string, key string) bool {