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

fix pd control bugs #245

Merged
merged 6 commits into from
Dec 25, 2018
Merged

fix pd control bugs #245

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
0873cff
fix pd control bugs
xiaojingchen Dec 21, 2018
01dc579
fix bugs
xiaojingchen Dec 21, 2018
2f2df9a
address comment
xiaojingchen Dec 21, 2018
4d440fb
Merge branch 'master' into pd-controll-bugs
tennix Dec 21, 2018
2e42390
Merge branch 'master' into pd-controll-bugs
xiaojingchen Dec 24, 2018
e05e575
Merge branch 'master' into pd-controll-bugs
xiaojingchen Dec 25, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 44 additions & 2 deletions pkg/controller/pd_control.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,20 @@ func (pc *pdClient) GetStore(storeID uint64) (*StoreInfo, error) {
}

func (pc *pdClient) DeleteStore(storeID uint64) error {
var exist bool
stores, err := pc.GetStores()
if err != nil {
return err
}
for _, store := range stores.Stores {
if store.Store.GetId() == storeID {
exist = true
break
}
}
if !exist {
return nil
}
apiURL := fmt.Sprintf("%s/%s/%d", pc.url, storePrefix, storeID)
req, err := http.NewRequest("DELETE", apiURL, nil)
if err != nil {
Expand All @@ -316,7 +330,7 @@ func (pc *pdClient) DeleteStore(storeID uint64) error {
defer DeferClose(res.Body, &err)

// Remove an offline store should returns http.StatusOK
if res.StatusCode == http.StatusOK {
if res.StatusCode == http.StatusOK || res.StatusCode == http.StatusNotFound {
Copy link
Contributor

Choose a reason for hiding this comment

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

Does DeleteStore request's response will return http.StatusNotFound ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the response return 500 code too, this is no use now. It's just a standard way.

return nil
}
body, err := ioutil.ReadAll(res.Body)
Expand All @@ -328,6 +342,20 @@ func (pc *pdClient) DeleteStore(storeID uint64) error {
}

func (pc *pdClient) DeleteMemberByID(memberID uint64) error {
var exist bool
members, err := pc.GetMembers()
if err != nil {
return err
}
for _, member := range members.Members {
if member.MemberId == memberID {
exist = true
break
}
}
if !exist {
return nil
}
apiURL := fmt.Sprintf("%s/%s/id/%d", pc.url, membersPrefix, memberID)
req, err := http.NewRequest("DELETE", apiURL, nil)
if err != nil {
Expand All @@ -338,14 +366,28 @@ func (pc *pdClient) DeleteMemberByID(memberID uint64) error {
return err
}
defer DeferClose(res.Body, &err)
if res.StatusCode == http.StatusOK {
if res.StatusCode == http.StatusOK || res.StatusCode == http.StatusNotFound {
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

return nil
}
err2 := readErrorBody(res.Body)
return fmt.Errorf("failed %v to delete member %d: %v", res.StatusCode, memberID, err2)
}

func (pc *pdClient) DeleteMember(name string) error {
var exist bool
members, err := pc.GetMembers()
if err != nil {
return err
}
for _, member := range members.Members {
if member.Name == name {
exist = true
break
}
}
if !exist {
return nil
}
apiURL := fmt.Sprintf("%s/%s/name/%s", pc.url, membersPrefix, name)
req, err := http.NewRequest("DELETE", apiURL, nil)
if err != nil {
Expand Down
221 changes: 183 additions & 38 deletions pkg/controller/pd_control_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -359,29 +359,78 @@ func TestSetStoreLabels(t *testing.T) {
func TestDeleteMember(t *testing.T) {
g := NewGomegaWithT(t)
name := "testMember"
member := &pdpb.Member{Name: name, MemberId: uint64(1)}
membersExist := &MembersInfo{
Members: []*pdpb.Member{
member,
},
Leader: member,
EtcdLeader: member,
}
membersExistBytes, err := json.Marshal(membersExist)
g.Expect(err).NotTo(HaveOccurred())

membersNotExist := &MembersInfo{
Members: []*pdpb.Member{},
}
membersNotExistBytes, err := json.Marshal(membersNotExist)
g.Expect(err).NotTo(HaveOccurred())

tcs := []struct {
caseName string
path string
method string
want bool
caseName string
prePath string
preMethod string
preResp []byte
exist bool
path string
method string
want bool
}{{
caseName: "success_DeleteMember",
path: fmt.Sprintf("/%s/name/%s", membersPrefix, name),
method: "DELETE",
want: true,
caseName: "success_DeleteMember",
prePath: fmt.Sprintf("/%s", membersPrefix),
preMethod: "GET",
preResp: membersExistBytes,
exist: true,
path: fmt.Sprintf("/%s/name/%s", membersPrefix, name),
method: "DELETE",
want: true,
}, {
caseName: "failed_DeleteMember",
path: fmt.Sprintf("/%s/name/%s", membersPrefix, name),
method: "DELETE",
want: false,
caseName: "failed_DeleteMember",
prePath: fmt.Sprintf("/%s", membersPrefix),
preMethod: "GET",
preResp: membersExistBytes,
exist: true,
path: fmt.Sprintf("/%s/name/%s", membersPrefix, name),
method: "DELETE",
want: false,
}, {
caseName: "delete_not_exist_member",
prePath: fmt.Sprintf("/%s", membersPrefix),
preMethod: "GET",
preResp: membersNotExistBytes,
exist: false,
path: fmt.Sprintf("/%s/name/%s", membersPrefix, name),
method: "DELETE",
want: true,
},
}

for _, tc := range tcs {
count := 1
svc := getClientServer(func(w http.ResponseWriter, request *http.Request) {
if count == 1 {
g.Expect(request.Method).To(Equal(tc.preMethod), "check method")
g.Expect(request.URL.Path).To(Equal(tc.prePath), "check url")
w.Header().Set("Content-Type", ContentTypeJSON)
w.WriteHeader(http.StatusOK)
w.Write(tc.preResp)
count++
return
}

g.Expect(tc.exist).To(BeTrue())
g.Expect(request.Method).To(Equal(tc.method), "check method")
g.Expect(request.URL.Path).To(Equal(tc.path), "check url")

w.Header().Set("Content-Type", ContentTypeJSON)
if tc.want {
w.WriteHeader(http.StatusOK)
Expand All @@ -404,29 +453,78 @@ func TestDeleteMember(t *testing.T) {
func TestDeleteMemberByID(t *testing.T) {
g := NewGomegaWithT(t)
id := uint64(1)
member := &pdpb.Member{Name: "test", MemberId: id}
membersExist := &MembersInfo{
Members: []*pdpb.Member{
member,
},
Leader: member,
EtcdLeader: member,
}
membersExistBytes, err := json.Marshal(membersExist)
g.Expect(err).NotTo(HaveOccurred())

membersNotExist := &MembersInfo{
Members: []*pdpb.Member{},
}
membersNotExistBytes, err := json.Marshal(membersNotExist)
g.Expect(err).NotTo(HaveOccurred())

tcs := []struct {
caseName string
path string
method string
want bool
caseName string
prePath string
preMethod string
preResp []byte
exist bool
path string
method string
want bool
}{{
caseName: "success_DeleteMemberByID",
path: fmt.Sprintf("/%s/id/%d", membersPrefix, id),
method: "DELETE",
want: true,
caseName: "success_DeleteMemberByID",
prePath: fmt.Sprintf("/%s", membersPrefix),
preMethod: "GET",
preResp: membersExistBytes,
exist: true,
path: fmt.Sprintf("/%s/id/%d", membersPrefix, id),
method: "DELETE",
want: true,
}, {
caseName: "failed_DeleteMemberByID",
path: fmt.Sprintf("/%s/id/%d", membersPrefix, id),
method: "DELETE",
want: false,
caseName: "failed_DeleteMemberByID",
prePath: fmt.Sprintf("/%s", membersPrefix),
preMethod: "GET",
preResp: membersExistBytes,
exist: true,
path: fmt.Sprintf("/%s/id/%d", membersPrefix, id),
method: "DELETE",
want: false,
}, {
caseName: "delete_not_exit_member",
prePath: fmt.Sprintf("/%s", membersPrefix),
preMethod: "GET",
preResp: membersNotExistBytes,
exist: false,
path: fmt.Sprintf("/%s/id/%d", membersPrefix, id),
method: "DELETE",
want: true,
},
}

for _, tc := range tcs {
count := 1
svc := getClientServer(func(w http.ResponseWriter, request *http.Request) {
if count == 1 {
g.Expect(request.Method).To(Equal(tc.preMethod), "check method")
g.Expect(request.URL.Path).To(Equal(tc.prePath), "check url")
w.Header().Set("Content-Type", ContentTypeJSON)
w.WriteHeader(http.StatusOK)
w.Write(tc.preResp)
count++
return
}

g.Expect(tc.exist).To(BeTrue())
g.Expect(request.Method).To(Equal(tc.method), "check method")
g.Expect(request.URL.Path).To(Equal(tc.path), "check url")

w.Header().Set("Content-Type", ContentTypeJSON)
if tc.want {
w.WriteHeader(http.StatusOK)
Expand All @@ -449,26 +547,73 @@ func TestDeleteMemberByID(t *testing.T) {
func TestDeleteStore(t *testing.T) {
g := NewGomegaWithT(t)
storeID := uint64(1)
store := &StoreInfo{
Store: &MetaStore{Store: &metapb.Store{Id: storeID, State: metapb.StoreState_Up}},
Status: &StoreStatus{},
}
stores := &StoresInfo{
Count: 1,
Stores: []*StoreInfo{
store,
},
}

storesBytes, err := json.Marshal(stores)
g.Expect(err).NotTo(HaveOccurred())

tcs := []struct {
caseName string
path string
method string
want bool
caseName string
prePath string
preMethod string
preResp []byte
exist bool
path string
method string
want bool
}{{
caseName: "success_DeleteStore",
path: fmt.Sprintf("/%s/%d", storePrefix, storeID),
method: "DELETE",
want: true,
caseName: "success_DeleteStore",
prePath: fmt.Sprintf("/%s", storesPrefix),
preMethod: "GET",
preResp: storesBytes,
exist: true,
path: fmt.Sprintf("/%s/%d", storePrefix, storeID),
method: "DELETE",
want: true,
}, {
caseName: "failed_DeleteStore",
path: fmt.Sprintf("/%s/%d", storePrefix, storeID),
method: "DELETE",
want: false,
caseName: "failed_DeleteStore",
prePath: fmt.Sprintf("/%s", storesPrefix),
preMethod: "GET",
preResp: storesBytes,
exist: true,
path: fmt.Sprintf("/%s/%d", storePrefix, storeID),
method: "DELETE",
want: false,
}, {
caseName: "delete_not_exist_store",
prePath: fmt.Sprintf("/%s", storesPrefix),
preMethod: "GET",
preResp: storesBytes,
exist: true,
path: fmt.Sprintf("/%s/%d", storePrefix, storeID),
method: "DELETE",
want: true,
},
}

for _, tc := range tcs {
count := 1
svc := getClientServer(func(w http.ResponseWriter, request *http.Request) {
if count == 1 {
g.Expect(request.Method).To(Equal(tc.preMethod), "check method")
g.Expect(request.URL.Path).To(Equal(tc.prePath), "check url")
w.Header().Set("Content-Type", ContentTypeJSON)
w.WriteHeader(http.StatusOK)
w.Write(tc.preResp)
count++
return
}

g.Expect(tc.exist).To(BeTrue())
g.Expect(request.Method).To(Equal(tc.method), "check method")
g.Expect(request.URL.Path).To(Equal(tc.path), "check url")

Expand Down
4 changes: 2 additions & 2 deletions pkg/manager/member/pd_failover.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,12 +157,12 @@ func (pf *pdFailover) tryToDeleteAFailureMember(tc *v1alpha1.TidbCluster) error
return nil
}

memberID, err := strconv.Atoi(failureMember.MemberID)
memberID, err := strconv.ParseUint(failureMember.MemberID, 10, 64)
xiaojingchen marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return err
}
// invoke deleteMember api to delete a member from the pd cluster
err = pf.pdControl.GetPDClient(tc).DeleteMemberByID(uint64(memberID))
err = pf.pdControl.GetPDClient(tc).DeleteMemberByID(memberID)
if err != nil {
return err
}
Expand Down