I may be interested in contributing a fix if it would be welcome and some guidance could be provided as to how to go about it.

Thank you for creating this @garymm!
Yes, it's a great idea, we can just add the Experiment validation here: https://github.com/kubeflow/katib/blob/master/pkg/webhook/v1beta1/experiment/validator/validator.go#L81

/assign @garymm

Feel free to reach out if you have any questions.

/remove-label lifecycle/needs-triage

What exactly should the validaton be though? I think the better fix is to name the experiment service in a way that is guaranteed to be legal. Where might that happen in the code?

@garymm Our longest algorithm name is: bayesianoptimization and the Suggestion's Deployment name is <Experiment_name>-<Algorithm_Name>.
So I suggest that we validate that Experiment name is no longer than 40 symbols just to be safe.

You just simple need to add this additional check here: https://github.com/kubeflow/katib/blob/master/pkg/webhook/v1beta1/experiment/validator/validator.go#L85

E.g. || len(instance.name) > 40.

/good-first-issue

@andreyvelich:
This request has been marked as suitable for new contributors.

Please ensure the request meets the requirements listed here.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-good-first-issue command.

Hello! @andreyvelich has a PR been made to fix this issue? If not, I would be open to helping out as well :)

There are no PRs open that reference this issue.

@AydanPirani Yes, please feel free to submit a PR
/assign @AydanPirani

Sorry for the delay, will get this up and going ASAP :)

Hi @garymm (cc @andreyvelich)! I'm trying to reproduce this locally, and I had a few questions:

1. What command did you run to get Katib version? I get the following when I run the command you included:

kubectl get pods -n kubeflow -l katib.kubeflow.org/component=controller -o jsonpath="{.items[*].spec.containers[*].image}"

docker.io/kubeflowkatib/katib-controller:v0.17.0
docker.io/kubeflowkatib/katib-controller:latest
zsh: no such file or directory: docker.io/kubeflowkatib/katib-controller:v0.17.0`

2. Is there anything specific you did to run the experiment? A link to the yaml file would be nice, so I can also see the same results on my end :)

Thanks!

I ran:

kubectl get pods -n kubeflow -l katib.kubeflow.org/component=controller -o jsonpath="{.items[*].spec.containers[*].image}"

Just create an experiment with a long name, such that experiment name + algorithm name > 63 characters.

@garymm Is right, this command prints the version of Katib controller which is equal to Katib version (e.g. v0.17.0).
@AydanPirani After you make appropriate changes to the validation webhook, you need to build Katib Controller, and re-use this image in your deployment.
That should help you to test your code changes in the local Kind/Minikube cluster.

@andreyvelich Got it, thanks! Also, can you please point me to the docs page re. deployment? I'm looking at this, but I don't see much about local Docker builds...

For clarity - I got the Docker build to work, I now have an image katib-master (that follows the master branch), how do I run this locally?

Thanks again for the help, I appreciate it!

@AydanPirani You can find some info here: https://github.com/kubeflow/katib/blob/master/CONTRIBUTING.md#build-from-source-code, but we don't really explain how to update images.
You just need to take your local image and use it in the kustomize installs: https://github.com/kubeflow/katib/blob/master/manifests/v1beta1/installs/katib-standalone/kustomization.yaml#L21-L23

After that, you can follow similar steps as for Training Operator to deploy Kind cluster locally and Katib using the standalone Kustomize overlay: https://github.com/kubeflow/training-operator/blob/master/CONTRIBUTING.md#run-a-kubernetes-cluster.

Hi all,

Still working on a repro from my end.

@garymm Does the following YAML file lead to the same error mentioned above? Just want to confirm that we both see the same thing!

Update: I have a local repro, getting the PR up now.