Implement KatibConfig API #2176

tenzen-y · 2023-07-19T20:44:17Z

What this PR does / why we need it:
I implemented the new KatibConfig API to set all parameters for the katib.
Also, the new KatibConfig supports JSON and YAML, similar to the kubernetes manifest.

NOTE: Since the KatibConfig is NOT CustomResource, we need to create a ConfigMap embedded KatibConfig as in the past.

apiVersion: v1
kind: ConfigMap
metadata:
  name: katib-config
  namespace: kubeflow
data:
  katib-config.yaml: |
    ---
    apiVersion: config.kubeflow.org/v1beta1
    kind: KatibConfig
    init:
      controller:
        webhookPort: 8443
        trialResources:
          - Job.v1.batch
          - TFJob.v1.kubeflow.org
    runtime:
      metricsCollectors:
        - kind: StdOut
          image: docker.io/kubeflowkatib/file-metrics-collector:latest
        - kind: TensorFlowEvent
          image: docker.io/kubeflowkatib/tfevent-metrics-collector:latest
          resources:
            limits:
              memory: 1Gi
      suggestions:
        - algorithmName: random
          image: docker.io/kubeflowkatib/suggestion-hyperopt:latest
        - algorithmName: pbt
          image: docker.io/kubeflowkatib/suggestion-pbt:latest
          persistentVolumeClaimSpec:
            accessModes:
              - ReadWriteMany
            resources:
              requests:
                storage: 5Gi
      earlyStoppings:
        - algorithmName: medianstop
          image: docker.io/kubeflowkatib/earlystopping-medianstop:latest

Given KatibConfig is used for the following:

.init.controller: This holds parameters for the katib-controller. Here is evaluated only when launching the controller. So we need to restart the controller's pod to set new parameters. Also, we need to mount the comfigMap embedded KatibConfig to the controller, and set the mounted path to controller's option, --katib-config like:

apiVersion: apps/v1
kind: Deployment
...
spec:
...
  template:
...
    spec:
      serviceAccountName: katib-controller
      containers:
        - name: katib-controller
          args:
            - --katib-config=/katib-config.yaml
...
          volumeMounts:
            - mountPath: /katib-config.yaml
              name: katib-config
              subPath: katib-config.yaml
              readOnly: true
...
      volumes:
        - name: katib-config
          configMap:
            name: katib-config
...

.runtime: This holds parameters for the metrics-collectors, suggestion-services, and earlystoppings. Here is reevaluated for every query. So we don't need to restart the controler's pods to set new parameters.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #2150

Checklist:

Docs included if any changes are user facing

tenzen-y · 2023-07-20T17:16:41Z

Uhm... I can not reproduce CI error on my local...

Error from server: error when creating "../../testdata/valid-experiment.yaml": admission webhook "validator.experiment.katib.kubeflow.org" denied the request: GetMetricsCollectorConfigData failed: failed to find metrics collector config for kind: StdOut in ConfigMap: katib-config

https://github.com/kubeflow/katib/actions/runs/5613737623/job/15210405916?pr=2176#step:4:9781

tenzen-y · 2023-07-20T21:39:27Z

Uhm... I can not reproduce CI error on my local...

Error from server: error when creating "../../testdata/valid-experiment.yaml": admission webhook "validator.experiment.katib.kubeflow.org" denied the request: GetMetricsCollectorConfigData failed: failed to find metrics collector config for kind: StdOut in ConfigMap: katib-config

https://github.com/kubeflow/katib/actions/runs/5613737623/job/15210405916?pr=2176#step:4:9781

This was resolved.

tenzen-y · 2023-07-21T09:18:19Z

@andreyvelich @johnugeorge @gaocegege This PR is ready for review. Please take a look.

andreyvelich

Thank you for this great feature @tenzen-y!
This should give users much better experience.
I left a few first comments.

manifests/v1beta1/components/controller/katib-config.yaml

Makefile

cmd/katib-controller/v1beta1/main.go

pkg/apis/config/v1beta1/defaults.go

pkg/util/v1beta1/katibconfig/config.go

andreyvelich · 2023-07-21T15:24:55Z

pkg/apis/config/v1beta1/defaults.go

+	return nil
+}
+
+func SetDefaults_KatibConfig(cfg *KatibConfig) {


I think, in the future, we should migrate our defaults for Katib Experiment to follow similar model which is more Kubernetes-native.
Currently, we set defaults via mutation webhook: https://github.com/tenzen-y/katib/blob/067c11933792f8060c3f9cd5349ef1b508a5b17c/pkg/webhook/v1beta1/experiment/mutate_webhook.go#L60.

Yes, that's right. If it's possible, it would be good.
By doing that, we can reduce the kube-apiserver load and then improve cluster performance :)

test/e2e/v1beta1/scripts/gh-actions/setup-katib.sh

pkg/controller.v1beta1/suggestion/composer/composer.go

tenzen-y · 2023-07-21T21:04:15Z

manifests/v1beta1/installs/katib-leader-election/katib-config.yaml

Currently, the kustomize can not merge (patch) against the structured .data in the ConfigMap.
So, we need to have a separate KatibConfig for the leaderElection.

NOTE: In the future, we might be able to merge the structured .data in the ConfigMap using the kustomize:
kubernetes-sigs/kustomize#4517

pkg/controller.v1beta1/consts/const.go

andreyvelich · 2023-07-21T23:12:11Z

manifests/v1beta1/installs/katib-leader-election/katib-config.yaml

+# This KatibConfig is mostly same as https://github.com/kubeflow/katib/manifests/v1beta1/components/controller/katib-config.yaml.
+# Only `.init.controller.enableLeaderElection` field is different.
+---


Unfortunately, Kustomize doesn't allow us to patch the ConfigMap data that is why we need to keep the whole config for each overlay 😞
Which means we need to update the image tags for each release in the future in these overlays.
I think, we need to do something with this script:

katib/scripts/v1beta1/update-images.sh

Lines 78 to 81 in 199aa8b

echo -e "Update Katib Metrics Collectors, Suggestions and EarlyStopping images\n"

update_yaml_files "${CONFIG_PATH}" "${OLD_PREFIX}" "${NEW_PREFIX}"

update_yaml_files "${CONFIG_PATH}" ":[^[:space:]].*\"" ":${TAG}\""

.

I think, at this stage it maybe worth to discuss how we want to maintain our suggestion image versions.
Currently, tags for suggestions we maintain under /manifests/v1beta1/components/controller/katib-config.yaml, tags for controller we maintain under /manifests/v1beta1/installs/katib-controller/kustomization.yaml.

What do you think @kubeflow/wg-automl-leads @tenzen-y ?

Which means we need to update the image tags for each release in the future in these overlays.
I think, we need to do something with this script:

It's a good point.

I think, at this stage it maybe worth to discuss how we want to maintain our suggestion image versions.
Currently, tags for suggestions we maintain under /manifests/v1beta1/components/controller/katib-config.yaml, tags for controller we maintain under /manifests/v1beta1/installs/katib-controller/kustomization.yaml.

I think generating KatibConfig for each installs by script would be good. This means we generate manifests/v1beta1/installs/katib-leader-election/katib-config.yaml from manifests/v1beta1/components/katib-config.yaml by a scripts.

If so, we could maintain the suggestion service image versions the same way as in the past.

I think generating KatibConfig for each installs by script would be good.

What do you mean by that ?

I was thinking that since Kustomize doesn't allow us to patch the ConfgMap data, we can just replace it where it is required.
Similar what you did for Katib Leader Election install.
So for Katib Standalone install and Katib with Kubeflow install we need to do the same to replace Katib Config with ConfigMap with appropriate image tags.
E.g. for master branch the image tags will be :latest for release-0.x branch the image tags will be :v0.x.0.

WDYT @tenzen-y @johnugeorge ?

I think generating KatibConfig for each installs by script would be good.

What do you mean by that ?

@andreyvelich NVM. My suggestion wouldn't work well.

So for Katib Standalone install and Katib with Kubeflow install we need to do the same to replace Katib Config with ConfigMap with appropriate image tags.

Does that mean that we use kustomize imageTagTransformer like

katib/manifests/v1beta1/installs/katib-standalone/kustomization.yaml

Lines 22 to 34 in 199aa8b

images:

- name: docker.io/kubeflowkatib/katib-controller

newName: docker.io/kubeflowkatib/katib-controller

newTag: latest

- name: docker.io/kubeflowkatib/katib-db-manager

newName: docker.io/kubeflowkatib/katib-db-manager

newTag: latest

- name: docker.io/kubeflowkatib/katib-ui

newName: docker.io/kubeflowkatib/katib-ui

newTag: latest

- name: docker.io/kubeflowkatib/cert-generator

newName: docker.io/kubeflowkatib/cert-generator

newTag: latest

?

Kustomize recognizes .data field as string data, not structured data such as YAML. So we can not replace image tags on the kind: KatibConfig embedded on ConfigMap using imageTagTransformer.

So, we need to replace image tags using vars like

katib/manifests/v1beta1/installs/katib-with-kubeflow/kustomization.yaml

Lines 46 to 56 in 199aa8b

vars:

- fieldref:

fieldPath: metadata.namespace

name: KATIB_UI_NAMESPACE

objref:

apiVersion: apps/v1

kind: Deployment

name: katib-ui

configurations:

- params.yaml

.

I meant, that for every install we are going to replace the whole Katib Config, similar to Katib Leader Election. E.g. for Katib Standalone we just have the same config as in /components/katib-config, but it will have different image tags (for master branch latest tag, for release- branch v0.x tag. Does it make sense?

@andreyvelich Oh, I see. Thanks for the clarification.

I think, the idea of installs/overlays was to have 1 place where we set settings, images, or any other changes for our Katib Control Plane.

Other solution could be to just remove Katib Config from components/ manifests and add it only in installs. We can modify our docs to explain it.

It sounds reasonable. I will do it ASAP.

@johnugeorge @gaocegege I am happy to discuss any other suggestions on how to configure our Katib manifests.

@andreyvelich I've done it: 42fe278
Is this your expected?

Yeah, that's right @tenzen-y, that what was I proposed.

Thanks for the check :)

tenzen-y · 2023-08-01T14:14:35Z

Thank you for this great contribution @tenzen-y! I think, we should update our docs in the next few months: https://www.kubeflow.org/docs/components/katib/katib-config/ Please can you create an issue to track this ?

/lgtm /hold for review from @gaocegege and @johnugeorge /assign @johnugeorge @gaocegege

Thanks for the review. I created an issue: #2186

andreyvelich · 2023-08-01T16:40:47Z

@tenzen-y Let's also rebase this PR, so we can test it in K8s 1.26.

Signed-off-by: Yuki Iwai <yuki.iwai.tz@gmail.com>

…beta1 Signed-off-by: Yuki Iwai <yuki.iwai.tz@gmail.com>

Signed-off-by: Yuki Iwai <yuki.iwai.tz@gmail.com>

tenzen-y · 2023-08-01T16:45:33Z

@tenzen-y Let's also rebase this PR, so we can test it in K8s 1.26.

Yea, I rebased this PR.

johnugeorge · 2023-08-01T16:45:37Z

/lgtm

tenzen-y · 2023-08-01T17:35:11Z

/hold cancel

google-oss-prow bot added do-not-merge/work-in-progress size/XXL approved labels Jul 19, 2023

google-oss-prow bot requested review from andreyvelich, anencore94 and gaocegege July 19, 2023 20:44

tenzen-y force-pushed the improve-katib-config-ux branch 8 times, most recently from cb33ce4 to b7cb65a Compare July 20, 2023 17:15

tenzen-y force-pushed the improve-katib-config-ux branch 5 times, most recently from ef2a9ae to 2cec0b2 Compare July 20, 2023 21:06

tenzen-y changed the title ~~WIP: Implement KatibConfig API~~ Implement KatibConfig API Jul 21, 2023

google-oss-prow bot removed the do-not-merge/work-in-progress label Jul 21, 2023

tenzen-y force-pushed the improve-katib-config-ux branch 2 times, most recently from 529b005 to 6595c5b Compare July 21, 2023 08:10

andreyvelich reviewed Jul 21, 2023

View reviewed changes

tenzen-y commented Jul 21, 2023

View reviewed changes

andreyvelich reviewed Jul 21, 2023

View reviewed changes

tenzen-y force-pushed the improve-katib-config-ux branch from 199aa8b to 20a1723 Compare July 30, 2023 16:46

tenzen-y added 19 commits August 2, 2023 01:41

Implement KatibConfig API

ba0e239

Signed-off-by: Yuki Iwai <yuki.iwai.tz@gmail.com>

Replace 'collectorKind' with 'kind'

b984644

Signed-off-by: Yuki Iwai <yuki.iwai.tz@gmail.com>

Replace 'metricsCollectorSidecars' with 'metricsCollectors'

73fd4c7

Signed-off-by: Yuki Iwai <yuki.iwai.tz@gmail.com>

Fix a typo

a9e32b1

Signed-off-by: Yuki Iwai <yuki.iwai.tz@gmail.com>

Make the init.controller.leaderElection non-pointer

373b3f3

Signed-off-by: Yuki Iwai <yuki.iwai.tz@gmail.com>

Make the init.controller.injectSecurityContext non-pointer

c2c39b8

Signed-off-by: Yuki Iwai <yuki.iwai.tz@gmail.com>

Update a comment for the future works

e91ccef

Signed-off-by: Yuki Iwai <yuki.iwai.tz@gmail.com>

Update manifest for the katib-leader-election

59c2303

Signed-off-by: Yuki Iwai <yuki.iwai.tz@gmail.com>

Fix a comment for the KatibConfig API

aacd734

Signed-off-by: Yuki Iwai <yuki.iwai.tz@gmail.com>

Replace 'configapi' with 'configv1beta1'

794ec25

Signed-off-by: Yuki Iwai <yuki.iwai.tz@gmail.com>

Remove debug code

a56c9f9

Signed-off-by: Yuki Iwai <yuki.iwai.tz@gmail.com>

Put constant for the default KatibConfig value on /pkg/apis/config/v1…

3d6450b

…beta1 Signed-off-by: Yuki Iwai <yuki.iwai.tz@gmail.com>

Use 'sigs.k8s.io/yaml' instead of 'github.com/ghodss/yaml'

f8d070a

Signed-off-by: Yuki Iwai <yuki.iwai.tz@gmail.com>

Avoid to depend on k8s.io/utils directly

9a51ab8

Signed-off-by: Yuki Iwai <yuki.iwai.tz@gmail.com>

Fix a typo

e9eb97a

Signed-off-by: Yuki Iwai <yuki.iwai.tz@gmail.com>

Refactor katib-config using kustomize vars

1d0bef3

Signed-off-by: Yuki Iwai <yuki.iwai.tz@gmail.com>

Fix a typo

122fd9d

Signed-off-by: Yuki Iwai <yuki.iwai.tz@gmail.com>

Put KatibConfig on every install

a326b36

Signed-off-by: Yuki Iwai <yuki.iwai.tz@gmail.com>

Remove configMapGenerator from the katib-with-kubeflow

4e5ea07

Signed-off-by: Yuki Iwai <yuki.iwai.tz@gmail.com>

tenzen-y force-pushed the improve-katib-config-ux branch from e684420 to 4e5ea07 Compare August 1, 2023 16:44

google-oss-prow bot removed the lgtm label Aug 1, 2023

google-oss-prow bot added the lgtm label Aug 1, 2023

google-oss-prow bot removed the do-not-merge/hold label Aug 1, 2023

google-oss-prow bot merged commit e69235d into kubeflow:master Aug 1, 2023
60 checks passed

tenzen-y deleted the improve-katib-config-ux branch August 1, 2023 18:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement KatibConfig API #2176

Implement KatibConfig API #2176

tenzen-y commented Jul 19, 2023 •

edited

Loading

tenzen-y commented Jul 20, 2023

tenzen-y commented Jul 20, 2023

tenzen-y commented Jul 21, 2023

andreyvelich left a comment •

edited

Loading

andreyvelich Jul 21, 2023

tenzen-y Jul 21, 2023 •

edited

Loading

tenzen-y Jul 21, 2023

andreyvelich Jul 21, 2023

andreyvelich Jul 21, 2023

tenzen-y Jul 25, 2023

andreyvelich Jul 25, 2023

tenzen-y Jul 30, 2023

tenzen-y Jul 31, 2023

andreyvelich Jul 31, 2023 •

edited

Loading

tenzen-y Jul 31, 2023

andreyvelich Jul 31, 2023

tenzen-y Jul 31, 2023

tenzen-y commented Aug 1, 2023

andreyvelich commented Aug 1, 2023

tenzen-y commented Aug 1, 2023

johnugeorge commented Aug 1, 2023

tenzen-y commented Aug 1, 2023

	echo -e "Update Katib Metrics Collectors, Suggestions and EarlyStopping images\n"
	update_yaml_files "${CONFIG_PATH}" "${OLD_PREFIX}" "${NEW_PREFIX}"
	update_yaml_files "${CONFIG_PATH}" ":[^[:space:]].*\"" ":${TAG}\""

	images:
	- name: docker.io/kubeflowkatib/katib-controller
	newName: docker.io/kubeflowkatib/katib-controller
	newTag: latest
	- name: docker.io/kubeflowkatib/katib-db-manager
	newName: docker.io/kubeflowkatib/katib-db-manager
	newTag: latest
	- name: docker.io/kubeflowkatib/katib-ui
	newName: docker.io/kubeflowkatib/katib-ui
	newTag: latest
	- name: docker.io/kubeflowkatib/cert-generator
	newName: docker.io/kubeflowkatib/cert-generator
	newTag: latest

	vars:
	- fieldref:
	fieldPath: metadata.namespace
	name: KATIB_UI_NAMESPACE
	objref:
	apiVersion: apps/v1
	kind: Deployment
	name: katib-ui

	configurations:
	- params.yaml

Implement KatibConfig API #2176

Implement KatibConfig API #2176

Conversation

tenzen-y commented Jul 19, 2023 • edited Loading

tenzen-y commented Jul 20, 2023

tenzen-y commented Jul 20, 2023

tenzen-y commented Jul 21, 2023

andreyvelich left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tenzen-y Jul 21, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andreyvelich Jul 31, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tenzen-y commented Aug 1, 2023

andreyvelich commented Aug 1, 2023

tenzen-y commented Aug 1, 2023

johnugeorge commented Aug 1, 2023

tenzen-y commented Aug 1, 2023

tenzen-y commented Jul 19, 2023 •

edited

Loading

andreyvelich left a comment •

edited

Loading

tenzen-y Jul 21, 2023 •

edited

Loading

andreyvelich Jul 31, 2023 •

edited

Loading