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

Fixed potential nil pointer issues #4054

Merged
merged 8 commits into from
Jul 2, 2021
Merged

Fixed potential nil pointer issues #4054

merged 8 commits into from
Jul 2, 2021

Conversation

KanShiori
Copy link
Collaborator

@KanShiori KanShiori commented Jul 1, 2021 •
edited by DanielZhangQD
Loading

What problem does this PR solve?

Fix #4051
Fixed potential nil pointer issues.

What is changed and how does it work?

Code changes

  • Has Go code change
  • Has CI related scripts change

Tests

  • Unit test
  • E2E test
  • Manual test
  • Test upgrade
    1. Upgrade TiDB-Opertaor and check the cluster is normal.
  • Test depolyment
    • Deploy new cluster and monitor, then check function.
    • Deploy new cluster without PD , then check pods and watch logs of TiDB-Operator. 
      NAME                                READY   STATUS    RESTARTS   AGE
my-tidb-cluster-dev-pump-0          0/1     Running   1          114s
my-tidb-cluster-dev-ticdc-0         1/1     Running   0          114s
my-tidb-cluster-dev-ticdc-1         1/1     Running   0          114s
my-tidb-cluster-dev-ticdc-2         1/1     Running   0          114s
my-tidb-cluster-dev-tiflash-0       4/4     Running   0          114s
my-tidb-cluster-dev-tikv-0          3/3     Running   0          114s
my-tidb-cluster-dev-tikv-1          3/3     Running   0          114s
my-tidb-cluster-dev-tikv-2          3/3     Running   0          114s
my-tidb-cluster-monitor-monitor-0   4/4     Running   0          26m
    • Deploy new cluster without TiDB, then check pods and watch logs of TiDB-Operator. 
      NAME                                             READY   STATUS    RESTARTS   AGE
my-tidb-cluster-dev-discovery-85b8698c6c-tfgcs   1/1     Running   0          3m2s
my-tidb-cluster-dev-pd-0                         1/1     Running   0          3m1s
my-tidb-cluster-dev-pd-1                         1/1     Running   0          3m1s
my-tidb-cluster-dev-pd-2                         1/1     Running   0          3m1s
my-tidb-cluster-dev-pump-0                       1/1     Running   0          2m18s
my-tidb-cluster-dev-pump-1                       1/1     Running   0          2m16s
my-tidb-cluster-dev-pump-2                       1/1     Running   0          2m14s
my-tidb-cluster-dev-ticdc-0                      1/1     Running   0          2m15s
my-tidb-cluster-dev-ticdc-1                      1/1     Running   0          2m15s
my-tidb-cluster-dev-ticdc-2                      1/1     Running   0          2m15s
my-tidb-cluster-dev-tiflash-0                    4/4     Running   0          2m15s
my-tidb-cluster-dev-tikv-0                       3/3     Running   0          2m18s
my-tidb-cluster-dev-tikv-1                       3/3     Running   0          2m18s
my-tidb-cluster-dev-tikv-2                       3/3     Running   0          2m18s
my-tidb-cluster-monitor-monitor-0                4/4     Running   0          163m
    • Deploy new cluster without TiKV , then check pods and watch logs of TiDB-Operator. 
      kubectl get pods
NAME                                             READY   STATUS    RESTARTS   AGE
my-tidb-cluster-dev-discovery-85b8698c6c-bjf6m   1/1     Running   0          4m27s
my-tidb-cluster-dev-pd-0                         1/1     Running   0          4m26s
my-tidb-cluster-dev-pd-1                         1/1     Running   0          4m26s
my-tidb-cluster-dev-pd-2                         1/1     Running   0          4m26s
my-tidb-cluster-dev-pump-0                       1/1     Running   0          3m41s
my-tidb-cluster-dev-pump-1                       1/1     Running   0          3m39s
my-tidb-cluster-dev-pump-2                       1/1     Running   0          3m36s
my-tidb-cluster-dev-ticdc-0                      1/1     Running   0          3m40s
my-tidb-cluster-dev-ticdc-1                      1/1     Running   0          3m40s
my-tidb-cluster-dev-ticdc-2                      1/1     Running   0          3m40s
my-tidb-cluster-dev-tidb-0                       1/2     Running   1          3m37s
my-tidb-cluster-dev-tidb-1                       1/2     Running   1          3m37s
my-tidb-cluster-dev-tidb-2                       1/2     Running   1          3m37s
my-tidb-cluster-dev-tiflash-0                    4/4     Running   0          3m40s
my-tidb-cluster-monitor-monitor-0                4/4     Running   0          20m
    • Deploy new cluster without Pump , then check pods and watch logs of TiDB-Operator.
    • Deploy new cluster without TiFlash , then check pods and watch logs of TiDB-Operator.
    • Deploy new cluster without TiCDC , then check pods and watch logs of TiDB-Operator.
  • 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.

Fix the issue that TiDB Operator may panic when heterogeneous clusters are deployed

@codecov-commenter
Copy link

codecov-commenter commented Jul 1, 2021 •
edited
Loading

Codecov Report

Merging #4054 (43cec83) into master (40913be) will increase coverage by 0.83%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #4054      +/-   ##
==========================================
+ Coverage   70.10%   70.94%   +0.83%     
==========================================
  Files         182      182              
  Lines       19374    19374              
==========================================
+ Hits        13583    13745     +162     
+ Misses       4656     4497     -159     
+ Partials     1135     1132       -3
Flag Coverage Δ
e2e 57.15% <100.00%> (+2.28%) ⬆️
unittest 61.12% <75.00%> (-0.02%) ⬇️

DanielZhangQD
DanielZhangQD approved these changes Jul 1, 2021
View reviewed changes
Copy link
Contributor

@DanielZhangQD DanielZhangQD left a comment

Choose a reason for hiding this comment

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

LGTM

csuzhangxc
csuzhangxc approved these changes Jul 1, 2021
View reviewed changes
Copy link
Member

@csuzhangxc csuzhangxc left a comment

Choose a reason for hiding this comment

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

LGTM

@ti-chi-bot
Copy link
Member

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • DanielZhangQD
  • csuzhangxc

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.

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

@csuzhangxc
Copy link
Member

/merge

@ti-chi-bot
Copy link
Member

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

Commit hash: a28cae9

@ti-chi-bot
Copy link
Member

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

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.

@DanielZhangQD
Copy link
Contributor

/test pull-e2e-kind

1 similar comment
@DanielZhangQD
Copy link
Contributor

/test pull-e2e-kind

@ti-chi-bot
Copy link
Member

Merge canceled because a new commit is pushed.

@DanielZhangQD
Copy link
Contributor

/test pull-e2e-kind

@DanielZhangQD
Copy link
Contributor

/test pull-e2e-kind

@DanielZhangQD DanielZhangQD merged commit 83a2db0 into pingcap:master Jul 2, 2021
ti-srebot pushed a commit to ti-srebot/tidb-operator that referenced this pull request Jul 2, 2021
@KanShiori @ti-srebot
cherry pick pingcap#4054 to release-1.1
bd1686e 
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot ti-srebot mentioned this pull request Jul 2, 2021
Merged
10 tasks
@ti-srebot
Copy link
Contributor

cherry pick to release-1.1 in PR #4058

DanielZhangQD pushed a commit that referenced this pull request Jul 2, 2021
@ti-srebot
Fixed potential nil pointer issues (#4054) (#4058)
8f4cd5e 
* cherry pick #4054 to release-1.1
@KanShiori KanShiori deleted the fix_potential_nilpointer_issue branch July 5, 2021 03:01
mianhk pushed a commit to mianhk/tidb-operator that referenced this pull request Jul 23, 2021
@KanShiori @mianhk
Fixed potential nil pointer issues (pingcap#4054)
fa6ac90 
* Fixed potential nil pointer issues
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DanielZhangQD DanielZhangQD DanielZhangQD approved these changes

@csuzhangxc csuzhangxc csuzhangxc approved these changes

@july2993 july2993 Awaiting requested review from july2993

Assignees
No one assigned
Labels
needs-cherry-pick-1.1 status/LGT2
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

panic issues triggered by no TiDB
6 participants
@KanShiori @codecov-commenter @ti-chi-bot @csuzhangxc @DanielZhangQD @ti-srebot