Compare k8s pod running info with pd client health info , improve inspection mechanism #1484

mikechengwei · 2020-01-05T07:06:21Z

What problem does this PR solve?

What is changed and how does it work?

Compare k8s pod running info with pd client health info , if the pod run but not join the tidb cluster,
take pod info into NotJoinClusterMembers type.

Check List

Tests

Unit test

Code changes

Has Go code change

Side effects

None

Related changes

None

Add `unjoinedMembers` of PD in the status of `TidbCluster` custom resource

mikechengwei · 2020-01-05T07:56:18Z

run-e2e-test

aylei · 2020-01-05T08:00:23Z

/ok-to-test

aylei

The rest LGTM, thanks!

aylei · 2020-01-05T16:02:50Z

pkg/apis/pingcap/v1alpha1/types.go

@@ -528,6 +529,13 @@ type PDFailureMember struct {
 	CreatedAt     metav1.Time `json:"createdAt,omitempty"`
 }

+// PDFailureMember is the pd failure member information


correct the comment

aylei · 2020-01-05T16:06:54Z

pkg/apis/pingcap/v1alpha1/types.go

+	Members               map[string]PDMember             `json:"members,omitempty"`
+	Leader                PDMember                        `json:"leader,omitempty"`
+	FailureMembers        map[string]PDFailureMember      `json:"failureMembers,omitempty"`
+	NotJoinClusterMembers map[string]NotJoinClusterMember `json:"notJoinClusterMembers,omitempty"`


I recommend to name it UnjoinedClusterMember

Edit: simply UnjoinedMember

aylei · 2020-01-05T16:09:58Z

pkg/manager/member/pd_member_manager.go

@@ -756,6 +762,52 @@ func getPDConfigMap(tc *v1alpha1.TidbCluster) (*corev1.ConfigMap, error) {
 	return cm, nil
 }

+func (pmm *pdMemberManager) checkNotJoinClusterNode(tc *v1alpha1.TidbCluster, set *apps.StatefulSet, pdStatus map[string]v1alpha1.PDMember) error {


I'm not mean to get into naming issues, but checkSomething usually does not mutate the input pointers, and Node is ambiguous with Kubernetes Node, maybe collectUnjoinedMembers could be more accurate

aylei · 2020-01-05T16:13:07Z

pkg/manager/member/pd_member_manager_test.go

+			}},
+			err: false,
+			expectTidbClusterFn: func(g *GomegaWithT, tc *v1alpha1.TidbCluster) {
+				g.Expect(tc.Status.PD.NotJoinClusterMembers["test-pd-2"]).NotTo(Equal(nil))


Suggested change

g.Expect(tc.Status.PD.NotJoinClusterMembers["test-pd-2"]).NotTo(Equal(nil))

g.Expect(tc.Status.PD.NotJoinClusterMembers["test-pd-2"]).NotTo(BeNil())

aylei · 2020-01-05T16:15:09Z

pkg/manager/member/pd_member_manager_test.go

+			}},
+			err: false,
+			expectTidbClusterFn: func(g *GomegaWithT, tc *v1alpha1.TidbCluster) {
+				g.Expect(len(tc.Status.PD.NotJoinClusterMembers)).To(Equal(0))


Suggested change

g.Expect(len(tc.Status.PD.NotJoinClusterMembers)).To(Equal(0))

g.Expect(tc.Status.PD.NotJoinClusterMembers).To(BeEmpty())

Use more descriptive ginkgo matchers could improve clarity

mikechengwei · 2020-01-06T02:41:25Z

run-e2e-test

aylei · 2020-01-06T03:17:22Z

LGTM, please add a proper release note

weekface · 2020-01-06T03:49:58Z

pkg/apis/pingcap/v1alpha1/types.go

+	Members         map[string]PDMember        `json:"members,omitempty"`
+	Leader          PDMember                   `json:"leader,omitempty"`
+	FailureMembers  map[string]PDFailureMember `json:"failureMembers,omitempty"`
+	UnjoinedMembers map[string]UnjoinedMember  `json:"unJoinedMembers,omitempty"`


Suggested change

UnjoinedMembers map[string]UnjoinedMember `json:"unJoinedMembers,omitempty"`

UnjoinedMembers map[string]UnjoinedMember `json:"unjoinedMembers,omitempty"`

weekface · 2020-01-06T03:50:24Z

pkg/manager/member/pd_member_manager.go

+		var joined = false
+		for podName := range pdStatus {
+			if strings.EqualFold(pod.Name, podName) {
+				joined = true


Suggested change

joined = true

joined = true

break

weekface · 2020-01-06T03:56:21Z

pkg/apis/pingcap/v1alpha1/types.go

@@ -528,6 +529,13 @@ type PDFailureMember struct {
 	CreatedAt     metav1.Time `json:"createdAt,omitempty"`
 }

+// UnjoinedMember is the pd unjoin cluster member information
+type UnjoinedMember struct {


Instead of add an attribute to TidbCluster, does just emit a UnjoinedMember and JoinedMember events to TidbCluster make sense?

User can use kubectl describe to get these messages easily. ref: #1466

I think event flow can't express my meaning in a concise manner , as a user, I need to understand and integrate the event flow and sort out each step. Use status type display current status is a concise way.

weekface

LGTM

aylei

LGTM

aylei · 2020-01-06T06:24:25Z

once again😁, thanks for your contribution!

aylei · 2020-01-06T06:24:31Z

/merge

sre-bot · 2020-01-06T06:24:36Z

Your auto merge job has been accepted, waiting for 1486

sre-bot · 2020-01-06T08:11:38Z

/run-all-tests

sre-bot · 2020-01-06T08:23:09Z

@mikechengwei merge failed.

aylei · 2020-01-08T04:43:32Z

/merge

sre-bot · 2020-01-08T04:43:35Z

Your auto merge job has been accepted, waiting for 1493

sre-bot · 2020-01-08T06:48:41Z

/run-all-tests

add k8s check when pd node not join cluster

8f190ce

sre-bot added the contribution label Jan 5, 2020

mikechengwei added 4 commits January 5, 2020 15:10

format PDStatus class

da3c1e6

Merge branch 'master' of github.com:pingcap/tidb-operator

761c65a

optimize range method

6546985

gen code

78d37c1

mikechengwei added 2 commits January 5, 2020 16:01

rm useless notJoinCluster field

3feb36b

fmt types class

2a1fd7a

aylei reviewed Jan 5, 2020

View reviewed changes

aylei self-assigned this Jan 5, 2020

optimize code style

76c2234

mikechengwei requested a review from aylei January 6, 2020 02:42

tennix requested a review from weekface January 6, 2020 03:30

weekface reviewed Jan 6, 2020

View reviewed changes

optimize code

9f958ab

mikechengwei requested a review from weekface January 6, 2020 06:17

weekface approved these changes Jan 6, 2020

View reviewed changes

aylei approved these changes Jan 6, 2020

View reviewed changes

sre-bot added the status/can-merge label Jan 6, 2020

Merge branch 'master' of github.com:pingcap/tidb-operator

f28e245

Merge branch 'master' into pdCheck

0bd31c1

sre-bot merged commit d11a844 into pingcap:master Jan 8, 2020

yahonda pushed a commit that referenced this pull request Dec 27, 2021

en,zh: graceful restart tikv (#1484)

15ce74a

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Compare k8s pod running info with pd client health info , improve inspection mechanism #1484

Compare k8s pod running info with pd client health info , improve inspection mechanism #1484

mikechengwei commented Jan 5, 2020 •

edited by aylei

Loading

mikechengwei commented Jan 5, 2020

aylei commented Jan 5, 2020

aylei left a comment

aylei Jan 5, 2020

aylei Jan 5, 2020

aylei Jan 5, 2020

aylei Jan 5, 2020

aylei Jan 5, 2020

aylei Jan 5, 2020

mikechengwei commented Jan 6, 2020

aylei commented Jan 6, 2020

weekface Jan 6, 2020

mikechengwei Jan 6, 2020

weekface Jan 6, 2020

mikechengwei Jan 6, 2020

weekface Jan 6, 2020 •

edited

Loading

mikechengwei Jan 6, 2020

weekface left a comment

aylei left a comment

aylei commented Jan 6, 2020

aylei commented Jan 6, 2020

sre-bot commented Jan 6, 2020

sre-bot commented Jan 6, 2020

sre-bot commented Jan 6, 2020

aylei commented Jan 8, 2020

sre-bot commented Jan 8, 2020

sre-bot commented Jan 8, 2020

	g.Expect(tc.Status.PD.NotJoinClusterMembers["test-pd-2"]).NotTo(Equal(nil))
	g.Expect(tc.Status.PD.NotJoinClusterMembers["test-pd-2"]).NotTo(BeNil())

	g.Expect(len(tc.Status.PD.NotJoinClusterMembers)).To(Equal(0))
	g.Expect(tc.Status.PD.NotJoinClusterMembers).To(BeEmpty())

	UnjoinedMembers map[string]UnjoinedMember `json:"unJoinedMembers,omitempty"`
	UnjoinedMembers map[string]UnjoinedMember `json:"unjoinedMembers,omitempty"`

Compare k8s pod running info with pd client health info , improve inspection mechanism #1484

Compare k8s pod running info with pd client health info , improve inspection mechanism #1484

Conversation

mikechengwei commented Jan 5, 2020 • edited by aylei Loading

What problem does this PR solve?

What is changed and how does it work?

Check List

mikechengwei commented Jan 5, 2020

aylei commented Jan 5, 2020

aylei left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mikechengwei commented Jan 6, 2020

aylei commented Jan 6, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

weekface Jan 6, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

weekface left a comment

Choose a reason for hiding this comment

aylei left a comment

Choose a reason for hiding this comment

aylei commented Jan 6, 2020

aylei commented Jan 6, 2020

sre-bot commented Jan 6, 2020

sre-bot commented Jan 6, 2020

sre-bot commented Jan 6, 2020

aylei commented Jan 8, 2020

sre-bot commented Jan 8, 2020

sre-bot commented Jan 8, 2020

mikechengwei commented Jan 5, 2020 •

edited by aylei

Loading

weekface Jan 6, 2020 •

edited

Loading