From 011b296c151d703629f54fa805b650bd570b140a Mon Sep 17 00:00:00 2001 From: Andrey Velichkevich Date: Mon, 22 Nov 2021 19:55:31 +0000 Subject: [PATCH 1/2] Change namespace label for metrics collector injection --- docs/proposals/metrics-collector.md | 6 +++--- examples/v1beta1/argo/README.md | 2 +- manifests/v1beta1/components/namespace/namespace.yaml | 2 +- manifests/v1beta1/components/webhook/webhooks.yaml | 2 +- operators/katib-controller/src/webhooks.yaml | 2 +- pkg/webhook/v1beta1/common/const.go | 2 +- pkg/webhook/v1beta1/pod/const.go | 2 +- pkg/webhook/v1beta1/pod/utils.go | 2 +- 8 files changed, 10 insertions(+), 10 deletions(-) diff --git a/docs/proposals/metrics-collector.md b/docs/proposals/metrics-collector.md index 7b17ab42444..82ee716af02 100644 --- a/docs/proposals/metrics-collector.md +++ b/docs/proposals/metrics-collector.md @@ -36,6 +36,7 @@ The sidecar collects metrics of the master and then store them on the persistent Fig. 1 Architecture of the new design + ## Goal @@ -117,9 +118,8 @@ For more detail, see [here](https://github.com/kubeflow/katib/pull/697#issuecomm To avoid collecting duplicated metrics, as we discuss in [kubeflow/katib#685](https://github.com/kubeflow/katib/issues/685), only one metrics collector sidecar will be injected into the master pod during one Experiment. In the new design, there are two modes for Katib mutating webhook to inject the sidecar: **Pod Level Injecting** and **Job Level Injecting**. - -The webhook decides which mode to be used based on the `katib-metricscollector-injection=enabled` label tagged on the namespace. -In the namespace with `katib-metricscollector-injection=enabled` label, the webhook inject the sidecar in the pod level. Otherwise, without this label, injecting in the job level. +The webhook decides which mode to be used based on the `katib.kubeflow.org/metrics-collector-injection=enabled` label tagged on the namespace. +In the namespace with `katib.kubeflow.org/metrics-collector-injection=enabled` label, the webhook inject the sidecar in the pod level. Otherwise, without this label, injecting in the job level. In **Pod Level Injecting**, diff --git a/examples/v1beta1/argo/README.md b/examples/v1beta1/argo/README.md index 095c46d74c1..dbf523c5eaa 100644 --- a/examples/v1beta1/argo/README.md +++ b/examples/v1beta1/argo/README.md @@ -29,7 +29,7 @@ After that, run below command to enable [Katib Metrics Collector sidecar injection](https://www.kubeflow.org/docs/components/katib/experiment/#metrics-collector): ```bash -kubectl patch namespace argo -p '{"metadata":{"labels":{"katib-metricscollector-injection":"enabled"}}}' +kubectl patch namespace argo -p '{"metadata":{"labels":{"katib.kubeflow.org/metrics-collector-injection":"enabled"}}}' ``` **Note:** Argo Workflows are using `docker` as a diff --git a/manifests/v1beta1/components/namespace/namespace.yaml b/manifests/v1beta1/components/namespace/namespace.yaml index d5b7bf410a2..49c379cd586 100644 --- a/manifests/v1beta1/components/namespace/namespace.yaml +++ b/manifests/v1beta1/components/namespace/namespace.yaml @@ -3,4 +3,4 @@ kind: Namespace metadata: name: kubeflow labels: - katib-metricscollector-injection: enabled + katib.kubeflow.org/metrics-collector-injection: enabled diff --git a/manifests/v1beta1/components/webhook/webhooks.yaml b/manifests/v1beta1/components/webhook/webhooks.yaml index 98cb1048341..fb78517270c 100644 --- a/manifests/v1beta1/components/webhook/webhooks.yaml +++ b/manifests/v1beta1/components/webhook/webhooks.yaml @@ -64,7 +64,7 @@ webhooks: path: /mutate-pod namespaceSelector: matchLabels: - katib-metricscollector-injection: enabled + katib.kubeflow.org/metrics-collector-injection: enabled rules: - apiGroups: - "" diff --git a/operators/katib-controller/src/webhooks.yaml b/operators/katib-controller/src/webhooks.yaml index 868aee1746d..382b408795a 100644 --- a/operators/katib-controller/src/webhooks.yaml +++ b/operators/katib-controller/src/webhooks.yaml @@ -65,7 +65,7 @@ webhooks: path: /mutate-pod namespaceSelector: matchLabels: - katib-metricscollector-injection: enabled + katib.kubeflow.org/metrics-collector-injection: enabled rules: - apiGroups: - "" diff --git a/pkg/webhook/v1beta1/common/const.go b/pkg/webhook/v1beta1/common/const.go index d360de4d4aa..8b8a1e73816 100644 --- a/pkg/webhook/v1beta1/common/const.go +++ b/pkg/webhook/v1beta1/common/const.go @@ -17,6 +17,6 @@ limitations under the License. package common const ( - KatibMetricsCollectorInjection = "katib-metricscollector-injection" + KatibMetricsCollectorInjection = "katib.kubeflow.org/metrics-collector-injection" KatibMetricsCollectorInjectionEnabled = "enabled" ) diff --git a/pkg/webhook/v1beta1/pod/const.go b/pkg/webhook/v1beta1/pod/const.go index afa999c2c4e..3e7618a6b2e 100644 --- a/pkg/webhook/v1beta1/pod/const.go +++ b/pkg/webhook/v1beta1/pod/const.go @@ -30,7 +30,7 @@ const ( ) var ( - NeedWrapWorkerMetricsCollecterList = [...]common.CollectorKind{ + NeedWrapWorkerMetricsCollectorList = [...]common.CollectorKind{ common.StdOutCollector, common.TfEventCollector, common.FileCollector, diff --git a/pkg/webhook/v1beta1/pod/utils.go b/pkg/webhook/v1beta1/pod/utils.go index 71bd6efa93c..3df05f87839 100644 --- a/pkg/webhook/v1beta1/pod/utils.go +++ b/pkg/webhook/v1beta1/pod/utils.go @@ -129,7 +129,7 @@ func getMountPath(mc common.MetricsCollectorSpec) (string, common.FileSystemKind func needWrapWorkerContainer(mc common.MetricsCollectorSpec) bool { mcKind := mc.Collector.Kind - for _, kind := range NeedWrapWorkerMetricsCollecterList { + for _, kind := range NeedWrapWorkerMetricsCollectorList { if mcKind == kind { return true } From f72527470cc1fdd7e528ab46d14db648a9c34845 Mon Sep 17 00:00:00 2001 From: Andrey Velichkevich Date: Mon, 22 Nov 2021 20:15:19 +0000 Subject: [PATCH 2/2] Fix var name --- pkg/metricscollector/v1beta1/common/const.go | 2 +- pkg/webhook/v1beta1/experiment/validator/validator.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/metricscollector/v1beta1/common/const.go b/pkg/metricscollector/v1beta1/common/const.go index 7a3b4c395d7..7e4a7cdb2c0 100644 --- a/pkg/metricscollector/v1beta1/common/const.go +++ b/pkg/metricscollector/v1beta1/common/const.go @@ -52,7 +52,7 @@ const ( ) var ( - AutoInjectMetricsCollecterList = [...]v1beta1common.CollectorKind{ + AutoInjectMetricsCollectorList = [...]v1beta1common.CollectorKind{ v1beta1common.StdOutCollector, v1beta1common.TfEventCollector, v1beta1common.FileCollector, diff --git a/pkg/webhook/v1beta1/experiment/validator/validator.go b/pkg/webhook/v1beta1/experiment/validator/validator.go index 400e8b7ecaf..e2a01a0ec52 100644 --- a/pkg/webhook/v1beta1/experiment/validator/validator.go +++ b/pkg/webhook/v1beta1/experiment/validator/validator.go @@ -392,7 +392,7 @@ func validatePatchJob(runSpec *unstructured.Unstructured, job interface{}, jobTy func (g *DefaultValidator) validateMetricsCollector(inst *experimentsv1beta1.Experiment) error { mcSpec := inst.Spec.MetricsCollectorSpec mcKind := mcSpec.Collector.Kind - for _, mc := range mccommon.AutoInjectMetricsCollecterList { + for _, mc := range mccommon.AutoInjectMetricsCollectorList { if mcKind != mc { continue }