Skip to content

Commit

Permalink
Fix nil pointer issue when clusterset is deleted in leader
Browse files Browse the repository at this point in the history
When the ClusterSet is deleted in leader Namespace, there will be
a nil pointer issue due to data inconsistent. This issue will cause
leader controller crash and restart. Refined codes to make data
consistent.
```
leader_clusterset_controller.go:72] "Received ClusterSet delete" config=""
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x18 pc=0x188b9b1]

goroutine 274 [running]:
antrea.io/antrea/multicluster/controllers/multicluster.(*MemberClusterAnnounceReconciler).processMCSStatus(0xc00052ff20)
	/antrea/multicluster/controllers/multicluster/memberclusterannounce_controller.go:181 +0x331
...
```

Signed-off-by: Lan Luo <luola@vmware.com>
  • Loading branch information
luolanzone committed Jun 21, 2022
1 parent 3f7f0d4 commit 7e66a35
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 35 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,9 @@ func (r *LeaderClusterSetReconciler) Reconcile(ctx context.Context, req ctrl.Req
return ctrl.Result{}, err
}
klog.InfoS("Received ClusterSet delete", "clusterset", klog.KObj(clusterSet))
r.StatusManager.CleanupMembers()
for _, removedMember := range r.clusterSetConfig.Spec.Members {
r.StatusManager.RemoveMember(common.ClusterID(removedMember.ClusterID))
}
r.clusterSetConfig = nil
r.clusterID = common.InvalidClusterID
r.clusterSetID = common.InvalidClusterSetID
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,8 @@ func TestLeaderClusterSetDelete(t *testing.T) {
err = fakeRemoteClient.Delete(context.TODO(), clusterSet)
assert.Equal(t, nil, err)

mockStatusManager.EXPECT().CleanupMembers()
mockStatusManager.EXPECT().RemoveMember(common.ClusterID("east"))
mockStatusManager.EXPECT().RemoveMember(common.ClusterID("west"))

req := ctrl.Request{
NamespacedName: types.NamespacedName{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,6 @@ type MemberClusterAnnounceReconciler struct {
type MemberClusterStatusManager interface {
AddMember(memberID common.ClusterID)
RemoveMember(memberID common.ClusterID)
CleanupMembers()

GetMemberClusterStatuses() []multiclusterv1alpha1.ClusterStatus
}
Expand Down Expand Up @@ -261,31 +260,16 @@ func (r *MemberClusterAnnounceReconciler) AddMember(memberID common.ClusterID) {
Conditions: conditions}

r.timerData[memberID] = &timerData{connected: false, lastUpdateTime: time.Time{}}
klog.InfoS("new member is added", "member", memberID)
klog.InfoS("Added member", "member", memberID)
}

func (r *MemberClusterAnnounceReconciler) RemoveMember(memberID common.ClusterID) {
r.mapLock.Lock()
defer r.mapLock.Unlock()

if _, ok := r.memberStatus[memberID]; ok {
delete(r.memberStatus, memberID)
return
}

if _, ok := r.timerData[memberID]; ok {
klog.InfoS("remove members", "member", memberID)
delete(r.timerData, memberID)
return
}
}

func (r *MemberClusterAnnounceReconciler) CleanupMembers() {
r.mapLock.Lock()
defer r.mapLock.Unlock()

r.memberStatus = make(map[common.ClusterID]*multiclusterv1alpha1.ClusterStatus)
r.timerData = make(map[common.ClusterID]*timerData)
delete(r.memberStatus, memberID)
delete(r.timerData, memberID)
klog.InfoS("Removed member", "member", memberID)
}

func (r *MemberClusterAnnounceReconciler) GetMemberClusterStatuses() []multiclusterv1alpha1.ClusterStatus {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,12 +114,24 @@ func TestStatusAfterAdd(t *testing.T) {
},
}

//[]multiclusterv1alpha1.ClusterStatus
status := memberClusterAnnounceReconcilerUnderTest.GetMemberClusterStatuses()
expectedTimeData := memberClusterAnnounceReconcilerUnderTest.timerData
assert.Equal(t, 1, len(status))
assert.Equal(t, 1, len(expectedTimeData))
verifyStatus(t, expectedStatus, status[0])
}

func TestStatusAfterDelete(t *testing.T) {
setup()
memberClusterAnnounceReconcilerUnderTest.AddMember("east")
memberClusterAnnounceReconcilerUnderTest.RemoveMember("east")

status := memberClusterAnnounceReconcilerUnderTest.GetMemberClusterStatuses()
expectedTimeData := memberClusterAnnounceReconcilerUnderTest.timerData
assert.Equal(t, 0, len(status))
assert.Equal(t, 0, len(expectedTimeData))
}

func TestStatusAfterReconcile(t *testing.T) {
TestStatusAfterAdd(t)

Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 7e66a35

Please sign in to comment.