Skip to content

Commit 98227f2

Browse files
ewoutpewoutp
committed
Fixed duplicate MemberStatus entries
1 parent badc27c commit 98227f2

File tree

7 files changed

+35
-25
lines changed

7 files changed

+35
-25
lines changed

‎pkg/apis/deployment/v1alpha/deployment_status_members.go

+9-9
Original file line numberDiff line numberDiff line change
@@ -75,23 +75,23 @@ func (ds DeploymentStatusMembers) ElementByID(id string) (MemberStatus, ServerGr
7575
// ForeachServerGroup calls the given callback for all server groups.
7676
// If the callback returns an error, this error is returned and the callback is
7777
// not called for the remaining groups.
78-
func (ds DeploymentStatusMembers) ForeachServerGroup(cb func(group ServerGroup, list *MemberStatusList) error) error {
79-
if err := cb(ServerGroupSingle, &ds.Single); err != nil {
78+
func (ds DeploymentStatusMembers) ForeachServerGroup(cb func(group ServerGroup, list MemberStatusList) error) error {
79+
if err := cb(ServerGroupSingle, ds.Single); err != nil {
8080
return maskAny(err)
8181
}
82-
if err := cb(ServerGroupAgents, &ds.Agents); err != nil {
82+
if err := cb(ServerGroupAgents, ds.Agents); err != nil {
8383
return maskAny(err)
8484
}
85-
if err := cb(ServerGroupDBServers, &ds.DBServers); err != nil {
85+
if err := cb(ServerGroupDBServers, ds.DBServers); err != nil {
8686
return maskAny(err)
8787
}
88-
if err := cb(ServerGroupCoordinators, &ds.Coordinators); err != nil {
88+
if err := cb(ServerGroupCoordinators, ds.Coordinators); err != nil {
8989
return maskAny(err)
9090
}
91-
if err := cb(ServerGroupSyncMasters, &ds.SyncMasters); err != nil {
91+
if err := cb(ServerGroupSyncMasters, ds.SyncMasters); err != nil {
9292
return maskAny(err)
9393
}
94-
if err := cb(ServerGroupSyncWorkers, &ds.SyncWorkers); err != nil {
94+
if err := cb(ServerGroupSyncWorkers, ds.SyncWorkers); err != nil {
9595
return maskAny(err)
9696
}
9797
return nil
@@ -190,8 +190,8 @@ func (ds *DeploymentStatusMembers) RemoveByID(id string, group ServerGroup) erro
190190

191191
// AllMembersReady returns true when all members are in the Ready state.
192192
func (ds DeploymentStatusMembers) AllMembersReady() bool {
193-
if err := ds.ForeachServerGroup(func(group ServerGroup, list *MemberStatusList) error {
194-
for _, x := range *list {
193+
if err := ds.ForeachServerGroup(func(group ServerGroup, list MemberStatusList) error {
194+
for _, x := range list {
195195
if !x.Conditions.IsTrue(ConditionTypeReady) {
196196
return fmt.Errorf("not ready")
197197
}

‎pkg/apis/deployment/v1alpha/member_status_list_test.go

+8
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,8 @@ func TestMemberStatusList(t *testing.T) {
4949
assert.NoError(t, list.RemoveByID(m3.ID))
5050
assert.Equal(t, 2, len(*list))
5151
assert.False(t, list.ContainsID(m3.ID))
52+
assert.Equal(t, m1.ID, (*list)[0].ID)
53+
assert.Equal(t, m2.ID, (*list)[1].ID)
5254

5355
m2.PodName = "foo"
5456
assert.NoError(t, list.Update(m2))
@@ -58,4 +60,10 @@ func TestMemberStatusList(t *testing.T) {
5860
assert.True(t, found)
5961
assert.Equal(t, "foo", x.PodName)
6062
assert.Equal(t, m2.ID, x.ID)
63+
64+
assert.NoError(t, list.Add(m3))
65+
assert.Equal(t, 3, len(*list))
66+
assert.Equal(t, m1.ID, (*list)[0].ID)
67+
assert.Equal(t, m2.ID, (*list)[1].ID)
68+
assert.Equal(t, m3.ID, (*list)[2].ID)
6169
}

‎pkg/deployment/reconcile/action_context.go

+3-1
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,9 @@ func (ac *actionContext) UpdateMember(member api.MemberStatus) error {
153153
if !found {
154154
return maskAny(fmt.Errorf("Member %s not found", member.ID))
155155
}
156-
status.Members.UpdateMemberStatus(member, group)
156+
if err := status.Members.UpdateMemberStatus(member, group); err != nil {
157+
return maskAny(err)
158+
}
157159
if err := ac.context.UpdateStatus(status, lastVersion); err != nil {
158160
log.Debug().Err(err).Msg("Updating CR status failed")
159161
return maskAny(err)

‎pkg/deployment/reconcile/plan_builder.go

+6-6
Original file line numberDiff line numberDiff line change
@@ -93,8 +93,8 @@ func createPlan(log zerolog.Logger, apiObject metav1.Object,
9393
var plan api.Plan
9494

9595
// Check for members in failed state
96-
status.Members.ForeachServerGroup(func(group api.ServerGroup, members *api.MemberStatusList) error {
97-
for _, m := range *members {
96+
status.Members.ForeachServerGroup(func(group api.ServerGroup, members api.MemberStatusList) error {
97+
for _, m := range members {
9898
if m.Phase == api.MemberPhaseFailed && len(plan) == 0 {
9999
newID := ""
100100
if group == api.ServerGroupAgents {
@@ -149,8 +149,8 @@ func createPlan(log zerolog.Logger, apiObject metav1.Object,
149149
}
150150
return nil
151151
}
152-
status.Members.ForeachServerGroup(func(group api.ServerGroup, members *api.MemberStatusList) error {
153-
for _, m := range *members {
152+
status.Members.ForeachServerGroup(func(group api.ServerGroup, members api.MemberStatusList) error {
153+
for _, m := range members {
154154
if len(plan) > 0 {
155155
// Only 1 change at a time
156156
continue
@@ -180,8 +180,8 @@ func createPlan(log zerolog.Logger, apiObject metav1.Object,
180180

181181
// Check for the need to rotate TLS certificate of a members
182182
if len(plan) == 0 && spec.TLS.IsSecure() {
183-
status.Members.ForeachServerGroup(func(group api.ServerGroup, members *api.MemberStatusList) error {
184-
for _, m := range *members {
183+
status.Members.ForeachServerGroup(func(group api.ServerGroup, members api.MemberStatusList) error {
184+
for _, m := range members {
185185
if len(plan) > 0 {
186186
// Only 1 change at a time
187187
continue

‎pkg/deployment/resilience/member_failure.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,8 @@ const (
4343
func (r *Resilience) CheckMemberFailure() error {
4444
status, lastVersion := r.context.GetStatus()
4545
updateStatusNeeded := false
46-
if err := status.Members.ForeachServerGroup(func(group api.ServerGroup, list *api.MemberStatusList) error {
47-
for _, m := range *list {
46+
if err := status.Members.ForeachServerGroup(func(group api.ServerGroup, list api.MemberStatusList) error {
47+
for _, m := range list {
4848
log := r.log.With().
4949
Str("id", m.ID).
5050
Str("role", group.AsRole()).
@@ -70,7 +70,7 @@ func (r *Resilience) CheckMemberFailure() error {
7070
} else if failureAcceptable {
7171
log.Info().Msg("Member is not ready for long time, marking is failed")
7272
m.Phase = api.MemberPhaseFailed
73-
list.Update(m)
73+
status.Members.UpdateMemberStatus(m, group)
7474
updateStatusNeeded = true
7575
} else {
7676
log.Warn().Msgf("Member is not ready for long time, but it is not safe to mark it a failed because: %s", reason)
@@ -89,7 +89,7 @@ func (r *Resilience) CheckMemberFailure() error {
8989
} else if failureAcceptable {
9090
log.Info().Msg("Member has terminated too often in recent history, marking is failed")
9191
m.Phase = api.MemberPhaseFailed
92-
list.Update(m)
92+
status.Members.UpdateMemberStatus(m, group)
9393
updateStatusNeeded = true
9494
} else {
9595
log.Warn().Msgf("Member has terminated too often in recent history, but it is not safe to mark it a failed because: %s", reason)

‎pkg/deployment/resources/member_cleanup.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -80,12 +80,12 @@ func (r *Resources) cleanupRemovedClusterMembers() error {
8080
status, lastVersion := r.context.GetStatus()
8181
updateStatusNeeded := false
8282
var podNamesToRemove, pvcNamesToRemove []string
83-
status.Members.ForeachServerGroup(func(group api.ServerGroup, list *api.MemberStatusList) error {
83+
status.Members.ForeachServerGroup(func(group api.ServerGroup, list api.MemberStatusList) error {
8484
if group != api.ServerGroupCoordinators && group != api.ServerGroupDBServers {
8585
// We're not interested in these other groups
8686
return nil
8787
}
88-
for _, m := range *list {
88+
for _, m := range list {
8989
if serverFound(m.ID) {
9090
// Member is (still) found, skip it
9191
if m.Conditions.Update(api.ConditionTypeMemberOfCluster, true, "", "") {
@@ -103,7 +103,7 @@ func (r *Resources) cleanupRemovedClusterMembers() error {
103103
// Member no longer part of cluster, remove it
104104
log.Info().Str("member", m.ID).Str("role", group.AsRole()).Msg("Member is no longer part of the ArangoDB cluster. Removing it.")
105105
}
106-
list.RemoveByID(m.ID)
106+
status.Members.RemoveByID(m.ID, group)
107107
updateStatusNeeded = true
108108
// Remove Pod & PVC (if any)
109109
if m.PodName != "" {

‎pkg/deployment/resources/pod_inspector.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -162,8 +162,8 @@ func (r *Resources) InspectPods(ctx context.Context) error {
162162
}
163163

164164
// Go over all members, check for missing pods
165-
status.Members.ForeachServerGroup(func(group api.ServerGroup, members *api.MemberStatusList) error {
166-
for _, m := range *members {
165+
status.Members.ForeachServerGroup(func(group api.ServerGroup, members api.MemberStatusList) error {
166+
for _, m := range members {
167167
if podName := m.PodName; podName != "" {
168168
if !podExists(podName) {
169169
switch m.Phase {

0 commit comments

Comments
 (0)