Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change namespace label for metrics collector injection #1740

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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