Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

make rootpasswordreset optional #922

Merged
merged 2 commits into from
Sep 17, 2019
Merged

make rootpasswordreset optional #922

merged 2 commits into from
Sep 17, 2019

Conversation

Yisaer
Copy link
Contributor

@Yisaer Yisaer commented Sep 17, 2019

What problem does this PR solve?

#921
For now, root password reset is not optional during sql intialization.

What is changed and how does it work?

Editing the job template and python script for sql initialization.

Check List

Tests

  • Manual test (add detailed scripts or steps below)

Code changes

  • Has Helm charts change

Side effects

  • N/A

Related changes

  • N/A

Does this PR introduce a user-facing change?:

NONE

@Yisaer
Copy link
Contributor Author

Yisaer commented Sep 17, 2019

/run-e2e-in-kind

cofyc
cofyc previously approved these changes Sep 17, 2019
View reviewed changes
Copy link
Contributor

@cofyc cofyc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

aylei
aylei reviewed Sep 17, 2019
View reviewed changes
Copy link
Contributor

@aylei aylei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rest LGTM

charts/tidb-cluster/templates/tidb-initializer-job.yaml Show resolved Hide resolved
@Yisaer
Copy link
Contributor Author

Yisaer commented Sep 17, 2019

/run-e2e-in-kind

aylei
aylei approved these changes Sep 17, 2019
View reviewed changes
Copy link
Contributor

@aylei aylei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

weekface
weekface approved these changes Sep 17, 2019
View reviewed changes
Copy link
Contributor

@weekface weekface left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Have you tested these cases manually?:

  • no passwd, no init sql
  • have passwd, no init sql
  • have passwd, have init sql
  • no passwd, have init sql

Or can you add these cases to e2e test to cover all these situations?

@Yisaer
Copy link
Contributor Author

Yisaer commented Sep 17, 2019

LGTM

Have you tested these cases manually?:

  • no passwd, no init sql
  • have passwd, no init sql
  • have passwd, have init sql
  • no passwd, have init sql

Or can you add these cases to e2e test to cover all these situations?

I test all four cases manually and the result is expected. Adding e2e test is a good idea and i will add these cases in e2e test in near future

@cofyc cofyc merged commit 811bf85 into pingcap:master Sep 17, 2019
@Yisaer Yisaer mentioned this pull request Sep 17, 2019
Closed
@Yisaer Yisaer deleted the fix_password_reset_as_optional branch September 17, 2019 08:40
This was referenced Sep 18, 2019
Closed
Closed
@Yisaer Yisaer restored the fix_password_reset_as_optional branch September 24, 2019 03:32
yahonda pushed a commit that referenced this pull request Dec 27, 2021
@csuzhangxc
zh: add docs about TiDB-Lightning with TLS (#922)
65e54d2
yahonda pushed a commit that referenced this pull request Dec 27, 2021
@ran-huang @TomShawn @csuzhangxc
en: add contents for enabling tidb lightning tls and troubleshooting (#…
4ec29e1 
…1008)

* align #922 and #970

Signed-off-by: Ran <huangran@pingcap.com>

* align #942

Signed-off-by: Ran <huangran@pingcap.com>

* Apply suggestions from code review

Co-authored-by: TomShawn <41534398+TomShawn@users.noreply.github.com>
Co-authored-by: Xuecheng Zhang <csuzhangxc@gmail.com>

* Apply suggestions from code review

* Update en/restore-data-using-tidb-lightning.md

Co-authored-by: Ran <huangran@pingcap.com>

Co-authored-by: TomShawn <41534398+TomShawn@users.noreply.github.com>
Co-authored-by: Xuecheng Zhang <csuzhangxc@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@weekface weekface weekface approved these changes

@cofyc cofyc cofyc approved these changes

@aylei aylei aylei approved these changes

@LinuxGit LinuxGit Awaiting requested review from LinuxGit

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Yisaer @weekface @cofyc @aylei