Update job name and service name as configurable for cert generator #1889

shaowei-su · 2022-06-06T20:02:00Z

What this PR does / why we need it:
This PR adds two additional command line input args serviceName jobName for the cert-generator job so that it's configurable in case Katib controller is deployed to multiple envs, e.g having katib-controller-production and katib-controller-staging.

By default, these two values will fall back to the hardcoded constants defined here: https://github.com/kubeflow/katib/blob/master/pkg/cert-generator/v1beta1/consts/const.go#L21

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 #
Allows cert-generator usage like:

./katib-cert-generator generate --namespace={{ Namespace }} --jobName={{ JobName }} --serviceName={{ ServiceName }}

Checklist:

Docs included if any changes are user facing

shaowei-su · 2022-06-07T18:54:42Z

Hi @andreyvelich could you help take a look for this PR, thanks!

coveralls · 2022-06-07T19:04:52Z

Coverage decreased (-0.3%) to 73.697% when pulling 748fb80 on shaowei-su:shaowei--update-cert-gen into e2378c3 on kubeflow:master.

johnugeorge · 2022-06-08T04:19:20Z

/cc @tenzen-y

tenzen-y

Thanks for your contribution! @shaowei-su

I do not know of any use cases where we want to change only jobName or serviceName.
Should we change the implementation so that all names can have a prefix or suffix?

What do you think? @kubeflow/wg-automl-leads @shaowei-su

johnugeorge · 2022-06-08T12:27:56Z

Isn't this harmless anyways?

tenzen-y · 2022-06-08T13:17:59Z

Yes, this feature is harmless. It makes sense. @johnugeorge
I will consider prefix or suffix at another time.

@shaowei-su
If a specified Service resource does not exist on the specified namespace, the katib-cert-generator can not generate an appropriate certificate. So, can you add a process to verify Service name?

shaowei-su · 2022-06-08T18:32:57Z

Good idea @tenzen-y PR updated with service resource validation. PTAL!

tenzen-y

Thanks for these updates! @shaowei-su
/lgtm

@kubeflow/wg-automl-leads Can you start gh-actions?

shaowei-su · 2022-06-09T18:36:39Z

It seems that all the e2e tests filed due to mysql connections

�mysqladmin: connect to server at 'localhost' failed
error: 'Can't connect to local MySQL server through socket '/var/run/mysqld/mysqld.sock' (2)'
Check that mysqld is running and that the socket: '/var/run/mysqld/mysqld.sock' exists!

cc + @johnugeorge

johnugeorge · 2022-06-09T19:17:14Z

@shaowei-su The test error is happening because katib-cert-generator is in error state

johnugeorge · 2022-06-09T20:30:52Z

https://github.com/kubeflow/katib/runs/6806542936?check_suite_focus=true#step:5:163
Line 163

shaowei-su · 2022-06-09T21:00:50Z

Thanks @johnugeorge ! any chance we can view the error logs or reproduce the failure locally? could it be the ordering of deployments, e.g cert-generator deployed before the controller service created and thus failed the validation added in this PR?

tenzen-y · 2022-06-10T02:40:35Z

@shaowei-su
You can reproduce it locally in the following steps:

Create KinD Cluster with kind create cluster.
Build cert-generator image with docker build -t shaowei-su/cert-generator:test -f cmd/cert-generator/v1beta1/Dockerfile ..
Upload your cert-generator image to KinD Cluster with kind load docker-image shaowei-su/cert-generator:test.
Modify cert-generator manifest with yq eval -i '.images[3].newTag|="test",.images[3].newName|="shaowei-su/cert-generator"' manifests/v1beta1/installs/katib-standalone/kustomization.yaml.
Deploy Katib with ./scripts/v1beta1/deploy.sh

johnugeorge · 2022-06-13T19:00:45Z

@shaowei-su Can you update this at the earliest as we are nearing the release timelines?

shaowei-su · 2022-06-13T19:05:19Z

Thanks @tenzen-y @johnugeorge for the detailed instructions!
It turns out to be the missing rbac permission for service and I updated in the latest commit. PTAL

tenzen-y

Thanks for updating PR! @shaowei-su
/lgtm

@kubeflow/wg-automl-leads Can you restart Go Test?

tenzen-y · 2022-06-14T17:25:21Z

@johnugeorge
I think that this PR is ready to merge.
Can you restart Go Test / Unit Test?

shaowei-su · 2022-06-14T17:44:19Z

Go tests have been restarted few times (https://github.com/kubeflow/katib/runs/6874004788?check_suite_focus=true) but always failed on k8s version 1.21.4 while succeeded on 1.22.1 and 1.23.5.

tenzen-y · 2022-06-14T17:59:01Z

We have flaky unit tests...

ref: #1649

johnugeorge · 2022-06-14T18:17:22Z

/approve

google-oss-prow · 2022-06-14T18:17:38Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: johnugeorge, shaowei-su

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [johnugeorge]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

shaowei su added 2 commits June 2, 2022 19:32

add more flags

a282eae

rename

9976f7a

google-oss-prow bot requested review from anencore94 and sperlingxx June 6, 2022 20:02

google-oss-prow bot added the size/S label Jun 6, 2022

google-oss-prow bot requested a review from tenzen-y June 8, 2022 04:19

tenzen-y reviewed Jun 8, 2022

View reviewed changes

add service validation

2be6a7f

google-oss-prow bot added size/M and removed size/S labels Jun 8, 2022

tenzen-y reviewed Jun 9, 2022

View reviewed changes

google-oss-prow bot assigned tenzen-y Jun 9, 2022

google-oss-prow bot added the lgtm label Jun 9, 2022

add service read permission

748fb80

google-oss-prow bot removed the lgtm label Jun 13, 2022

tenzen-y reviewed Jun 14, 2022

View reviewed changes

google-oss-prow bot added the lgtm label Jun 14, 2022

google-oss-prow bot added the approved label Jun 14, 2022

google-oss-prow bot merged commit 170647d into kubeflow:master Jun 14, 2022

Update job name and service name as configurable for cert generator #1889

Update job name and service name as configurable for cert generator #1889

Uh oh!

Conversation

shaowei-su commented Jun 6, 2022

Uh oh!

shaowei-su commented Jun 7, 2022

Uh oh!

coveralls commented Jun 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

johnugeorge commented Jun 8, 2022

Uh oh!

tenzen-y left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

johnugeorge commented Jun 8, 2022

Uh oh!

tenzen-y commented Jun 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

shaowei-su commented Jun 8, 2022

Uh oh!

tenzen-y left a comment

Choose a reason for hiding this comment

Uh oh!

shaowei-su commented Jun 9, 2022

Uh oh!

johnugeorge commented Jun 9, 2022

Uh oh!

johnugeorge commented Jun 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

shaowei-su commented Jun 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tenzen-y commented Jun 10, 2022

Uh oh!

johnugeorge commented Jun 13, 2022

Uh oh!

shaowei-su commented Jun 13, 2022

Uh oh!

tenzen-y left a comment

Choose a reason for hiding this comment

Uh oh!

tenzen-y commented Jun 14, 2022

Uh oh!

shaowei-su commented Jun 14, 2022

Uh oh!

tenzen-y commented Jun 14, 2022

Uh oh!

johnugeorge commented Jun 14, 2022

Uh oh!

google-oss-prow bot commented Jun 14, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

coveralls commented Jun 7, 2022 •

edited

Loading

tenzen-y left a comment •

edited

Loading

tenzen-y commented Jun 8, 2022 •

edited

Loading

johnugeorge commented Jun 9, 2022 •

edited

Loading

shaowei-su commented Jun 9, 2022 •

edited

Loading