fix mysql deployment strategy by thesuperzapper · Pull Request #1562 · kubeflow/community-distribution

thesuperzapper · 2020-09-17T09:14:30Z

When trying to restart MySQL instances, they get stuck because the old container will refuse to give up the lock on the DB:

kubectl rollout restart deployment/katib-mysql -n kubeflow
kubectl rollout restart deployment/metadata-db -n kubeflow

This PR fixes that by setting the deployment strategy to Recreate.

Checklist:

Unit tests have been rebuilt:
1. cd manifests/tests
2. make generate-changed-only
3. make test

kubeflow-bot · 2020-09-17T09:14:38Z

This change is

thesuperzapper · 2020-09-17T09:40:46Z

/assign @andreyvelich @zhenghuiwang

can you please review?

andreyvelich · 2020-09-17T16:51:10Z

@thesuperzapper Thank you for updating this, have you checked that when katib-mysql pod is restarting katib-db-manager connects to the new DB properly ?

thesuperzapper · 2020-09-21T05:56:37Z

@andreyvelich well as far as I can see it works

andreyvelich · 2020-09-21T13:29:45Z

@thesuperzapper Thanks!
Can you open a PR to make changes in Katib manifests also, please?

andreyvelich

/lgtm

andreyvelich · 2020-09-21T13:30:26Z

/cc @gaocegege @johnugeorge

thesuperzapper · 2020-09-24T01:32:58Z

Gentle ping: @gaocegege @johnugeorge

thesuperzapper · 2020-10-12T03:40:53Z

Have you had a chance to review this? @andreyvelich @zhenghuiwang

andreyvelich

@thesuperzapper Sorry for the long reply!
/lgtm
/approve

thesuperzapper · 2020-10-13T00:24:16Z

I still need one of the metadata owners to LGTM this

Can one of @neuromage @prodonjs @zhenghuiwang please take a look?

PatrickXYS · 2020-11-04T20:56:43Z

Can you try re-run new test?

/test ?

aws-kf-ci-bot · 2020-11-04T20:56:46Z

@PatrickXYS: The following commands are available to trigger jobs:

/test kubeflow-manifests-presubmit-e2e

Use /test all to run all jobs.

Details

In response to this:

Can you try re-run new test?

/test ?

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

k8s-ci-robot · 2020-11-04T20:56:48Z

@PatrickXYS: No presubmit jobs available for kubeflow/manifests@master

Details

In response to this:

Can you try re-run new test?

/test ?

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

thesuperzapper · 2020-11-05T08:28:29Z

I am still waiting on one of @neuromage @prodonjs @zhenghuiwang to LGTM this

I want to get this in before Kubeflow 1.2

PatrickXYS · 2020-11-05T16:30:58Z

@thesuperzapper I don't think any of them are active in kubeflow community now.

You might need to find review from other one, after that, try re-run test because we have migrated to shared test-infra

PatrickXYS · 2020-11-06T00:29:02Z

/test kubeflow-manifests-presubmit-e2e

k8s-ci-robot · 2020-11-06T00:29:08Z

@PatrickXYS: No presubmit jobs available for kubeflow/manifests@master

Details

In response to this:

/test kubeflow-manifests-presubmit-e2e

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Jeffwan · 2020-11-06T03:55:35Z

/lgtm

thesuperzapper · 2020-11-09T04:55:22Z

@andreyvelich @Jeffwan I have rebased

andreyvelich · 2020-11-09T14:58:43Z

/test kubeflow-manifests-presubmit-e2e

k8s-ci-robot · 2020-11-09T14:58:47Z

@andreyvelich: No presubmit jobs available for kubeflow/manifests@master

Details

In response to this:

/test kubeflow-manifests-presubmit-e2e

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

andreyvelich · 2020-11-09T16:16:05Z

@thesuperzapper I think you need to run make generate-changed-only again to fix the unit tests.

thesuperzapper · 2020-11-10T00:29:40Z

@andreyvelich when I run make generate-changed-only on a rebased version of my branch, I get an error, is master broken? Or is it something to do with my setup?

PatrickXYS · 2020-11-10T00:34:06Z

@thesuperzapper If there's something wrong in unit test, you might need to go figure it out, keeping unit test succeed is the bottom line.

Jeffwan · 2020-11-10T00:41:16Z

@thesuperzapper Can you share the errors? What's the errors msg? What kustomize version you have in your environment. Recent change all pass UT and I think master should be good.

thesuperzapper · 2020-11-10T01:00:31Z

/retest

thesuperzapper · 2020-11-10T01:01:41Z

@Jeffwan it seems like kustomize 3.8.1 is not working with our repo, we probably need to fix this for users

I have rolled my local version back to 3.6.1 and make generate-changed-only works again

PatrickXYS · 2020-11-10T01:02:51Z

Copy from manifest README

This repo contains kustomize packages for deploying Kubeflow applications.

If you are a contributor authoring or editing the packages please see Best Practices. Note, please use kustomize v3.2.1 with manifests in this repo, before #538 is fixed which will allow latest kustomize to be used.

https://github.com/kubeflow/manifests

thesuperzapper · 2020-11-10T02:33:03Z

@PatrickXYS, yeah, but we should aim to support any widely used versions of kustomize

PatrickXYS · 2020-11-10T03:09:35Z

@thesuperzapper Ideally we should do so, let's create a GitHub issue to talk about it.

/lgtm
/approve
/hold

Can you cherry-pick this PR into v1.2-branch as well?

k8s-ci-robot · 2020-11-10T03:09:43Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: andreyvelich, PatrickXYS, thesuperzapper

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:

~~katib/OWNERS~~ [PatrickXYS,andreyvelich]
~~metadata/OWNERS~~ [PatrickXYS]
~~tests/OWNERS~~ [PatrickXYS,andreyvelich,thesuperzapper]

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

Jeffwan · 2020-11-10T05:52:33Z

this should be good to go.

/hold cancel

thesuperzapper · 2020-11-10T05:52:59Z

@PatrickXYS, I dont think I have the required permissions to chery-pick this commit

PatrickXYS · 2020-11-10T16:04:33Z

@thesuperzapper You won't be able to write directly to v1.2-branch, but send a cherry-pick PR

(cherry picked from commit 6c4f265)

* fix mysql deployment strategy (#1562) (cherry picked from commit 6c4f265) * resolve tests for azure

google-cla Bot added the cla: yes label Sep 17, 2020

k8s-ci-robot requested review from PatrickXYS and krishnadurai September 17, 2020 09:14

k8s-ci-robot added the size/M label Sep 17, 2020

k8s-ci-robot assigned andreyvelich and zhenghuiwang Sep 17, 2020

andreyvelich reviewed Sep 21, 2020

View reviewed changes

k8s-ci-robot added the lgtm label Sep 21, 2020

k8s-ci-robot requested review from gaocegege and johnugeorge September 21, 2020 13:30

andreyvelich approved these changes Oct 12, 2020

View reviewed changes

Jeffwan reviewed Nov 5, 2020

View reviewed changes

Comment thread metadata/overlays/db/metadata-db-deployment.yaml

k8s-ci-robot assigned Jeffwan Nov 6, 2020

thesuperzapper force-pushed the fix_mysql branch from b27bec4 to 59c8404 Compare November 10, 2020 00:23

fix mysql deployment strategy

806fab8

thesuperzapper force-pushed the fix_mysql branch from 59c8404 to 806fab8 Compare November 10, 2020 01:00

k8s-ci-robot added the do-not-merge/hold label Nov 10, 2020

k8s-ci-robot assigned PatrickXYS Nov 10, 2020

k8s-ci-robot added the lgtm label Nov 10, 2020

k8s-ci-robot added the approved label Nov 10, 2020

k8s-ci-robot removed the do-not-merge/hold label Nov 10, 2020

k8s-ci-robot merged commit 6c4f265 into kubeflow:master Nov 10, 2020

thesuperzapper added a commit to thesuperzapper/manifests that referenced this pull request Nov 16, 2020

fix mysql deployment strategy (kubeflow#1562)

eb9aa6b

(cherry picked from commit 6c4f265)

thesuperzapper mentioned this pull request Nov 16, 2020

fix mysql deployment strategy (#1562) #1643

Merged

k8s-ci-robot pushed a commit that referenced this pull request Nov 16, 2020

fix mysql deployment strategy (#1562) (#1643)

fc9fe83

* fix mysql deployment strategy (#1562) (cherry picked from commit 6c4f265) * resolve tests for azure

andreyvelich mentioned this pull request Nov 17, 2020

Add recreate strategy to MySQL deployment kubeflow/katib#1393

Merged

thesuperzapper deleted the fix_mysql branch November 16, 2023 01:22

Conversation

thesuperzapper commented Sep 17, 2020

Uh oh!

kubeflow-bot commented Sep 17, 2020

Uh oh!

thesuperzapper commented Sep 17, 2020

Uh oh!

andreyvelich commented Sep 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thesuperzapper commented Sep 21, 2020

Uh oh!

andreyvelich commented Sep 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

andreyvelich left a comment

Choose a reason for hiding this comment

Uh oh!

andreyvelich commented Sep 21, 2020

Uh oh!

thesuperzapper commented Sep 24, 2020

Uh oh!

thesuperzapper commented Oct 12, 2020

Uh oh!

andreyvelich left a comment

Choose a reason for hiding this comment

Uh oh!

thesuperzapper commented Oct 13, 2020

Uh oh!

PatrickXYS commented Nov 4, 2020

Uh oh!

aws-kf-ci-bot commented Nov 4, 2020

Uh oh!

k8s-ci-robot commented Nov 4, 2020

Uh oh!

thesuperzapper commented Nov 5, 2020

Uh oh!

PatrickXYS commented Nov 5, 2020

Uh oh!

Uh oh!

PatrickXYS commented Nov 6, 2020

Uh oh!

k8s-ci-robot commented Nov 6, 2020

Uh oh!

Jeffwan commented Nov 6, 2020

Uh oh!

thesuperzapper commented Nov 9, 2020

Uh oh!

andreyvelich commented Nov 9, 2020

Uh oh!

k8s-ci-robot commented Nov 9, 2020

Uh oh!

andreyvelich commented Nov 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thesuperzapper commented Nov 10, 2020

Uh oh!

PatrickXYS commented Nov 10, 2020

Uh oh!

Jeffwan commented Nov 10, 2020

Uh oh!

thesuperzapper commented Nov 10, 2020

Uh oh!

thesuperzapper commented Nov 10, 2020

Uh oh!

PatrickXYS commented Nov 10, 2020

Uh oh!

thesuperzapper commented Nov 10, 2020

Uh oh!

PatrickXYS commented Nov 10, 2020

Uh oh!

k8s-ci-robot commented Nov 10, 2020

Uh oh!

Jeffwan commented Nov 10, 2020

Uh oh!

thesuperzapper commented Nov 10, 2020

Uh oh!

PatrickXYS commented Nov 10, 2020

Uh oh!

Reviewers

andreyvelich commented Sep 17, 2020 •

edited

Loading

andreyvelich commented Sep 21, 2020 •

edited

Loading

andreyvelich commented Nov 9, 2020 •

edited

Loading