Support specifying an initialize sql file on TiDB' s first bootstrap by fgksgf · Pull Request #4862 · pingcap/tidb-operator

fgksgf · 2023-02-07T06:26:26Z

What problem does this PR solve?

Support specifying an initialize sql file on TiDB' s first bootstrap.
Related PR: pingcap/tidb#35625

What is changed and how does it work?

Add a BootstrapSQLConfigMapName field to tidb spec
If it's enabled, will mount the specify configmap to tidb pod and add --initialize-sql-file to tidb start script

Code changes

Has Go code change
Has CI related scripts change

Tests

Unit test
E2E test
Manual test
- Run tidb operator locally
- Create a config map like:

apiVersion: v1
kind: ConfigMap
metadata:
  name: sem
data:
  bootstrap-sql: |
    CREATE USER test_admin;
    GRANT RESTRICTED_VARIABLES_ADMIN ON *.* TO 'test_admin'@'%';
    GRANT RESTRICTED_STATUS_ADMIN ON *.* TO 'test_admin'@'%';
    GRANT RESTRICTED_CONNECTION_ADMIN ON *.* TO 'test_admin'@'%';
    GRANT RESTRICTED_USER_ADMIN ON *.* TO 'test_admin'@'%';
    GRANT RESTRICTED_TABLES_ADMIN ON *.* TO 'test_admin'@'%';
    GRANT RESTRICTED_REPLICA_WRITER_ADMIN ON *.* TO 'test_admin'@'%';
    GRANT SHUTDOWN, CONFIG ON *.* TO 'test_admin'@'%';
    REVOKE SHUTDOWN, CONFIG ON *.* FROM root;

create a TC CR, specify the BootstrapSQLConfigMapName field and set tidb config to enable SEM
connect to the tidb with user test_admin and:
- query show config where type='tidb' and name='security.enable-sem';
- query select * from INFORMATION_SCHEMA.USER_PRIVILEGES;

+------+-----------------------------------------------------------------+---------------------+-------+
| Type | Instance                                                        | Name                | Value |
+------+-----------------------------------------------------------------+---------------------+-------+
| tidb | with-bootstrap2-tidb-0.with-bootstrap2-tidb-peer.test5.svc:4000 | security.enable-sem | true  |
+------+-----------------------------------------------------------------+---------------------+-------+

+------------------+---------------+---------------------------------+--------------+
| GRANTEE          | TABLE_CATALOG | PRIVILEGE_TYPE                  | IS_GRANTABLE |
+------------------+---------------+---------------------------------+--------------+
| 'test_admin'@'%' | def           | SHUTDOWN                        | NO           |
| 'test_admin'@'%' | def           | CONFIG                          | NO           |
| 'test_admin'@'%' | def           | RESTRICTED_VARIABLES_ADMIN      | NO           |
| 'test_admin'@'%' | def           | RESTRICTED_STATUS_ADMIN         | NO           |
| 'test_admin'@'%' | def           | RESTRICTED_CONNECTION_ADMIN     | NO           |
| 'test_admin'@'%' | def           | RESTRICTED_USER_ADMIN           | NO           |
| 'test_admin'@'%' | def           | RESTRICTED_TABLES_ADMIN         | NO           |
| 'test_admin'@'%' | def           | RESTRICTED_REPLICA_WRITER_ADMIN | NO           |
+------------------+---------------+---------------------------------+--------------+

tidb pod did not restart
No code

Side effects

Breaking backward compatibility
Other side effects:

Related changes

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

Release Notes

Please refer to Release Notes Language Style Guide before writing the release note.

Support specifying an initialize sql file on TiDB' s first bootstrap

ti-chi-bot · 2023-02-07T06:26:28Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

csuzhangxc
july2993

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

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

Details

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

CLAassistant · 2023-02-07T06:26:31Z

All committers have signed the CLA.

codecov-commenter · 2023-02-07T07:32:32Z

Codecov Report

Merging #4862 (776f88e) into master (ec7e73f) will increase coverage by 8.36%.
The diff coverage is 0.00%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4862      +/-   ##
==========================================
+ Coverage   59.43%   67.79%   +8.36%     
==========================================
  Files         226      230       +4     
  Lines       25697    28793    +3096     
==========================================
+ Hits        15273    19521    +4248     
+ Misses       8969     7800    -1169     
- Partials     1455     1472      +17

Flag	Coverage Δ
e2e	`52.65% <0.00%> (?)`
unittest	`59.39% <0.00%> (-0.05%)`	⬇️

july2993 · 2023-02-08T08:30:42Z

 	return allErrs
 }

+func disallowMutateBootstrapSQLConfigMapName(old, new *v1alpha1.TiDBSpec, p *field.Path) field.ErrorList {


it seems there are no need to restrict this

Because it only take effect at the first bootstrap, so updating the field makes no sense but make tidb restart.

KanShiori

Rest LGTM

csuzhangxc · 2023-02-13T06:50:38Z

 }

+// disallowMutateBootstrapSQLConfigMapName checks if user mutate the bootstrapSQLConfigMapName field.
+// Only allow to update bootstrapSQLConfigMapName from non-nil to nil.


why do we need to support updates from non-nil to nil?

If the user deletes the bootstrap sql configmap and also wants to remove it from tidb pods.

hanlins · 2023-02-13T22:15:36Z

/run-all-tests

hanlins · 2023-02-13T23:23:49Z

/test pull-e2e-kind-across-kubernetes

handlerww · 2023-02-14T10:44:16Z

/test pull-e2e-kind-across-kubernetes

handlerww · 2023-02-14T10:44:34Z

/test pull-e2e-kind-serial

fgksgf · 2023-02-15T02:36:35Z

Any ideas about the error in E2E?

Join cluster with existing data [It]

failed to check status

Unexpected error: "timed out waiting for the condition, 
last error: all pump member hosts don't contain update-1-pump.across-kubernetes-9232.svc.cluster.local"

/home/jenkins/agent/workspace/tidb-operator-pull-e2e-kind-across-kubernetes/go/src/github.com/pingcap/tidb-operator/tests/e2e/tidbcluster/across-kubernetes.go:280

tidb-operator/tests/e2e/tidbcluster/across-kubernetes.go

Lines 991 to 993 in 0ff338f

    
           if len(ownMembers) == 0 { 
        
           	return fmt.Errorf("all pump member hosts don't contain %s", pumpHostSuffix) 
        
           }

Do we need to turn up the timeout?

tidb-operator/tests/e2e/tidbcluster/across-kubernetes.go

Line 279 in 0ff338f

    
           err = CheckStatusWhenAcrossK8sWithTimeout(cli, []*v1alpha1.TidbCluster{tc1, tc2, tc3}, 5*time.Second, 3*time.Minute)

handlerww · 2023-02-15T04:59:47Z

/test pull-e2e-kind-across-kubernetes

KanShiori · 2023-02-15T10:53:35Z

/merge

ti-chi-bot · 2023-02-15T10:53:38Z

This pull request has been accepted and is ready to merge.

Details

Commit hash: 7702ee7

ti-chi-bot · 2023-02-15T18:00:17Z

@fgksgf: Your PR was out of date, I have automatically updated it for you.

Details

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 ti-community-infra/tichi repository.

hanlins · 2023-02-15T18:20:12Z

/run-all-tests

hanlins · 2023-02-15T20:13:41Z

/test pull-e2e-kind-serial

hanlins · 2023-02-15T22:08:01Z

/test pull-e2e-kind-serial

hanlins · 2023-02-15T22:16:50Z

/test pull-e2e-kind-serial

hanlins · 2023-02-15T23:55:19Z

/test pull-e2e-kind-serial

KanShiori · 2023-02-16T02:14:24Z

/test pull-e2e-kind-serial

fgksgf added 2 commits February 7, 2023 11:49

add BootstrapSQLConfigMapName to TiDBSpec

363cf38

support bootstrap sql

08b4f42

ti-chi-bot requested review from handlerww and july2993 February 7, 2023 06:26

polish

5c184b2

fgksgf changed the title ~~[WIP] Support specifying an initialize sql file on TiDB' s first bootstrap~~ Support specifying an initialize sql file on TiDB' s first bootstrap Feb 8, 2023

fix ci

583dd57

july2993 approved these changes Feb 8, 2023

View reviewed changes

ti-chi-bot added the status/LGT1 label Feb 8, 2023

address review comments

e2e2264

KanShiori reviewed Feb 9, 2023

View reviewed changes

Comment thread pkg/apis/pingcap/v1alpha1/tidbcluster.go Outdated

Comment thread pkg/manager/member/startscript/v1/template.go Outdated

csuzhangxc reviewed Feb 10, 2023

View reviewed changes

Comment thread pkg/apis/pingcap/v1alpha1/validation/validation.go Outdated

Comment thread pkg/manager/member/startscript/v1/template.go Outdated

ti-chi-bot requested a review from KanShiori February 10, 2023 08:00

address review comments

959177f

fgksgf requested review from KanShiori and csuzhangxc and removed request for KanShiori, csuzhangxc and handlerww February 13, 2023 05:44

csuzhangxc approved these changes Feb 13, 2023

View reviewed changes

ti-chi-bot added status/LGT2 and removed status/LGT1 labels Feb 13, 2023

Merge branch 'master' into huaxi/bootstrap-sql

9c36dce

hanlins approved these changes Feb 13, 2023

View reviewed changes

This comment was marked as outdated.

Sign in to view

KanShiori and others added 3 commits February 15, 2023 14:50

Merge branch 'master' into huaxi/bootstrap-sql

1f4ec3a

turn up the timeout in e2e

4607684

Merge branch 'master' into huaxi/bootstrap-sql

7702ee7

ti-chi-bot added the status/can-merge label Feb 15, 2023

Merge branch 'master' into huaxi/bootstrap-sql

776f88e

ti-chi-bot merged commit 0d45f8c into pingcap:master Feb 16, 2023

fgksgf deleted the huaxi/bootstrap-sql branch February 16, 2023 03:47

hanlins added the release-note-action-required label Feb 16, 2023

Conversation

fgksgf commented Feb 7, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What problem does this PR solve?

What is changed and how does it work?

Code changes

Tests

Side effects

Related changes

Release Notes

Uh oh!

ti-chi-bot commented Feb 7, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

CLAassistant commented Feb 7, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-commenter commented Feb 7, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

july2993 Feb 8, 2023

Choose a reason for hiding this comment

Uh oh!

fgksgf Feb 8, 2023

Choose a reason for hiding this comment

Uh oh!

KanShiori left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

csuzhangxc Feb 13, 2023

Choose a reason for hiding this comment

Uh oh!

fgksgf Feb 13, 2023 • 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!

This comment was marked as outdated.

hanlins commented Feb 13, 2023

Uh oh!

hanlins commented Feb 13, 2023

Uh oh!

handlerww commented Feb 14, 2023

Uh oh!

handlerww commented Feb 14, 2023

Uh oh!

fgksgf commented Feb 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

handlerww commented Feb 15, 2023

Uh oh!

KanShiori commented Feb 15, 2023

Uh oh!

ti-chi-bot commented Feb 15, 2023

Uh oh!

ti-chi-bot commented Feb 15, 2023

Uh oh!

hanlins commented Feb 15, 2023

Uh oh!

hanlins commented Feb 15, 2023

Uh oh!

hanlins commented Feb 15, 2023

Uh oh!

hanlins commented Feb 15, 2023

Uh oh!

hanlins commented Feb 15, 2023

Uh oh!

KanShiori commented Feb 16, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

fgksgf commented Feb 7, 2023 •

edited

Loading

ti-chi-bot commented Feb 7, 2023 •

edited

Loading

CLAassistant commented Feb 7, 2023 •

edited

Loading

codecov-commenter commented Feb 7, 2023 •

edited

Loading

fgksgf Feb 13, 2023 •

edited

Loading

fgksgf commented Feb 15, 2023 •

edited

Loading