[DNM] fix: allow tidb init job to run a 2nd time

[donbowman]

Fixes #3353. If we can connect with the root password, assume the job has previously been run and exit. Otherwise connect with no password, set it, etc.

This will allow the tidb-initializer job to run a 2nd time without crashing, allowing the helm chart to be upgraded.

What problem does this PR solve?

Correct #3353 - if the init job is run a 2nd time it crashes since it assumes the password of the db is null

What is changed and how does it work?

in _initialize_tidb_users.py.tpl, if we can connect w/ the root password, assume job has previously run and exit.

Check List

Tests

• Unit test
• E2E test
• Stability test
• Manual test (add detailed scripts or steps below)
• No code

Code changes

• Has Go code change
• Has CI related scripts change
• Has Terraform scripts change

Side effects

• Breaking backward compatibility

Related changes

• Need to cherry-pick to the release branch
• Need to update the documentation

Does this PR introduce a user-facing change?:

NONE

[CLAassistant]

All committers have signed the CLA.

[codecov-io]

Codecov Report

Merging #3376 into master will decrease coverage by 0.01%. The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #3376      +/-   ##
==========================================
- Coverage   49.89%   49.87%   -0.02%     
==========================================
  Files         163      163              
  Lines       16414    16414              
==========================================
- Hits         8189     8186       -3     
- Misses       7415     7417       +2     
- Partials      810      811       +1     

Flag	Coverage Δ
#unittest	49.87% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

[DanielZhangQD]

@donbowman You do have tested the code change, right? Could you please update the test result in the PR description? Thanks!

[DanielZhangQD]

@donbowman With the TidbInitializer CR, it can be applied multiple times because TiDB Operator will check if the job exists and if yes, it will not create the job again, did you test with the TidbInitializer CR?

[DanielZhangQD]

@donbowman With the TidbInitializer CR, it can be applied multiple times because TiDB Operator will check if the job exists and if yes, it will not create the job again, did you test with the TidbInitializer CR?

@donbowman Did you test with the TidbInitializer CR?

[omaskery]

Hi, just wanted to get a sense of if this work is likely to be continued any time soon? @donbowman is there any way I can help?

[donbowman]

i'll try to find a time, but since i have a solution w/ my re-written initialiser (that i posted earlier), i cannot apply as-is to my production type system since it would wreck the database.

I am really not clear on what you are asking that is different. the problem i saw:

1. install. it runs a snippet of python which creates a db, changes the password
2. upgrade. it runs the same snippet of python. this fails, since the password was changed

so i think you are suggesting to edit the helm chart and remove that python snippet, and, use a new crd TidbInitializer and write that in some fashion so that it will do the same thing as my change?

Does everyone not have this problem? I am confused?

[omaskery]

@donbowman to be honest I'm now in the same boat, it was easier for me to make our infrastructure overwrite the python script in the rendered helm chart with my own that essentially implements your fix 😅

[DanielZhangQD]

The tidb-cluster chart is deprecated from v1.1.0 and TidbInitializer CR is used to initializer the database, in which case, the initializer job will not be created twice unless the original job is removed manually.

[DanielZhangQD]

The tidb-cluster chart is deprecated from v1.1.0 and TidbInitializer CR is used to initializer the database, in which case, the initializer job will not be created twice unless the original job is removed manually.

@donbowman Did you have a chance to try the TidbInitializer CR?