Skip to content

Commit

Permalink
Change namespace label for metrics collector injection (#1740)
Browse files Browse the repository at this point in the history
* Change namespace label for metrics collector injection

* Fix var name
  • Loading branch information
andreyvelich authored Nov 23, 2021
1 parent 0e17302 commit c3e778b
Show file tree
Hide file tree
Showing 10 changed files with 12 additions and 12 deletions.
6 changes: 3 additions & 3 deletions docs/proposals/metrics-collector.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ The sidecar collects metrics of the master and then store them on the persistent
<img src="../images/metrics-collector-design.png" width="80%">

Fig. 1 Architecture of the new design

</center>

## Goal
Expand Down Expand Up @@ -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**,

Expand Down
2 changes: 1 addition & 1 deletion examples/v1beta1/argo/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion manifests/v1beta1/components/namespace/namespace.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,4 @@ kind: Namespace
metadata:
name: kubeflow
labels:
katib-metricscollector-injection: enabled
katib.kubeflow.org/metrics-collector-injection: enabled
2 changes: 1 addition & 1 deletion manifests/v1beta1/components/webhook/webhooks.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ webhooks:
path: /mutate-pod
namespaceSelector:
matchLabels:
katib-metricscollector-injection: enabled
katib.kubeflow.org/metrics-collector-injection: enabled
rules:
- apiGroups:
- ""
Expand Down
2 changes: 1 addition & 1 deletion operators/katib-controller/src/webhooks.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ webhooks:
path: /mutate-pod
namespaceSelector:
matchLabels:
katib-metricscollector-injection: enabled
katib.kubeflow.org/metrics-collector-injection: enabled
rules:
- apiGroups:
- ""
Expand Down
2 changes: 1 addition & 1 deletion pkg/metricscollector/v1beta1/common/const.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ const (
)

var (
AutoInjectMetricsCollecterList = [...]v1beta1common.CollectorKind{
AutoInjectMetricsCollectorList = [...]v1beta1common.CollectorKind{
v1beta1common.StdOutCollector,
v1beta1common.TfEventCollector,
v1beta1common.FileCollector,
Expand Down
2 changes: 1 addition & 1 deletion pkg/webhook/v1beta1/common/const.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,6 @@ limitations under the License.
package common

const (
KatibMetricsCollectorInjection = "katib-metricscollector-injection"
KatibMetricsCollectorInjection = "katib.kubeflow.org/metrics-collector-injection"
KatibMetricsCollectorInjectionEnabled = "enabled"
)
2 changes: 1 addition & 1 deletion pkg/webhook/v1beta1/experiment/validator/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/webhook/v1beta1/pod/const.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ const (
)

var (
NeedWrapWorkerMetricsCollecterList = [...]common.CollectorKind{
NeedWrapWorkerMetricsCollectorList = [...]common.CollectorKind{
common.StdOutCollector,
common.TfEventCollector,
common.FileCollector,
Expand Down
2 changes: 1 addition & 1 deletion pkg/webhook/v1beta1/pod/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down

0 comments on commit c3e778b

Please sign in to comment.