Skip to content

Commit 3904581

Browse files
leoryuleoryu
authored
fix(platform): healcheck resync internal not work (#1708)
1 parent 93916cd commit 3904581

File tree

4 files changed

+209
-283
lines changed

4 files changed

+209
-283
lines changed

‎pkg/platform/controller/cluster/cluster_controller.go‎

Lines changed: 18 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -157,16 +157,22 @@ func (c *Controller) enqueue(obj *platformv1.Cluster) {
157157
}
158158

159159
func (c *Controller) needsUpdate(old *platformv1.Cluster, new *platformv1.Cluster) bool {
160-
switch {
161-
case !reflect.DeepEqual(old.Spec, new.Spec):
160+
healthCondition := new.GetCondition(conditionTypeHealthCheck)
161+
if !reflect.DeepEqual(old.Spec, new.Spec) {
162162
return true
163-
case !reflect.DeepEqual(old.ObjectMeta.Labels, new.ObjectMeta.Labels):
163+
164+
}
165+
if !reflect.DeepEqual(old.ObjectMeta.Labels, new.ObjectMeta.Labels) {
164166
return true
165-
case !reflect.DeepEqual(old.ObjectMeta.Annotations, new.ObjectMeta.Annotations):
167+
}
168+
if !reflect.DeepEqual(old.ObjectMeta.Annotations, new.ObjectMeta.Annotations) {
166169
return true
167-
case old.Status.Phase != new.Status.Phase:
170+
}
171+
if old.Status.Phase != new.Status.Phase {
168172
return true
169-
case new.Status.Phase == platformv1.ClusterInitializing:
173+
174+
}
175+
if new.Status.Phase == platformv1.ClusterInitializing {
170176
// if ResourceVersion is equal, it's an resync envent, should return true.
171177
if old.ResourceVersion == new.ResourceVersion {
172178
return true
@@ -177,25 +183,17 @@ func (c *Controller) needsUpdate(old *platformv1.Cluster, new *platformv1.Cluste
177183
if new.Status.Conditions[len(new.Status.Conditions)-1].Status == platformv1.ConditionUnknown {
178184
return true
179185
}
180-
// if user set last condition false block procesee
186+
// if user set last condition false block procesee until resync envent
181187
if new.Status.Conditions[len(new.Status.Conditions)-1].Status == platformv1.ConditionFalse {
182188
return false
183189
}
184-
fallthrough
185-
case !reflect.DeepEqual(old.Status.Conditions, new.Status.Conditions):
186-
return true
187-
default:
188-
healthCondition := new.GetCondition(conditionTypeHealthCheck)
189-
if healthCondition == nil {
190-
// when healthCondition is not set, if ResourceVersion is equal, it's an resync envent, should return true.
191-
return old.ResourceVersion == new.ResourceVersion
192-
}
193-
if time.Since(healthCondition.LastProbeTime.Time) > c.healthCheckPeriod {
194-
return true
195-
}
196-
190+
}
191+
// if last health check is not long enough， return false
192+
if healthCondition != nil &&
193+
time.Since(healthCondition.LastProbeTime.Time) < c.healthCheckPeriod {
197194
return false
198195
}
196+
return true
199197
}
200198

201199
// Run will set up the event handlers for types we are interested in, as well

‎pkg/platform/controller/cluster/cluster_controller_test.go‎

Lines changed: 87 additions & 119 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,40 @@ import (
2626
platformv1 "tkestack.io/tke/api/platform/v1"
2727
)
2828

29+
func newClusterForTest(resourcesVersion string, spec *platformv1.ClusterSpec, phase platformv1.ClusterPhase, conditions []platformv1.ClusterCondition) *platformv1.Cluster {
30+
mc := &platformv1.Cluster{
31+
ObjectMeta: v1.ObjectMeta{ResourceVersion: resourcesVersion},
32+
Spec: platformv1.ClusterSpec{
33+
TenantID: "default",
34+
DisplayName: "global",
35+
Type: "Baremetal",
36+
Version: "1.21.1-tke.1",
37+
},
38+
Status: platformv1.ClusterStatus{
39+
Phase: platformv1.ClusterRunning,
40+
Conditions: []platformv1.ClusterCondition{
41+
{
42+
Type: conditionTypeHealthCheck,
43+
Status: platformv1.ConditionTrue,
44+
LastProbeTime: v1.Now(),
45+
},
46+
},
47+
},
48+
}
49+
if spec != nil {
50+
mc.Spec = *spec
51+
}
52+
if len(phase) != 0 {
53+
mc.Status.Phase = phase
54+
}
55+
if conditions != nil {
56+
mc.Status.Conditions = conditions
57+
}
58+
return mc
59+
}
60+
2961
func TestController_needsUpdate(t *testing.T) {
62+
resyncInternal := time.Minute
3063
// type fields struct {
3164
// queue workqueue.RateLimitingInterface
3265
// lister platformv1lister.ClusterLister
@@ -49,195 +82,130 @@ func TestController_needsUpdate(t *testing.T) {
4982
{
5083
name: "change spec",
5184
args: args{
52-
old: &platformv1.Cluster{
53-
ObjectMeta: v1.ObjectMeta{ResourceVersion: "old"},
54-
Spec: platformv1.ClusterSpec{DisplayName: "old"},
55-
},
56-
new: &platformv1.Cluster{
57-
ObjectMeta: v1.ObjectMeta{ResourceVersion: "new"},
58-
Spec: platformv1.ClusterSpec{DisplayName: "nes"},
59-
},
85+
old: newClusterForTest("old", &platformv1.ClusterSpec{Version: "old"}, platformv1.ClusterPhase(""), nil),
86+
new: newClusterForTest("new", &platformv1.ClusterSpec{Version: "new"}, platformv1.ClusterPhase(""), nil),
6087
},
6188
want: true,
6289
},
6390
{
6491
name: "Initializing to Running",
6592
args: args{
66-
old: &platformv1.Cluster{
67-
ObjectMeta: v1.ObjectMeta{ResourceVersion: "old"},
68-
Status: platformv1.ClusterStatus{Phase: platformv1.ClusterInitializing},
69-
},
70-
new: &platformv1.Cluster{
71-
ObjectMeta: v1.ObjectMeta{ResourceVersion: "new"},
72-
Status: platformv1.ClusterStatus{Phase: platformv1.ClusterRunning},
73-
},
93+
old: newClusterForTest("old", nil, platformv1.ClusterInitializing, nil),
94+
new: newClusterForTest("new", nil, platformv1.ClusterRunning, nil),
7495
},
7596
want: true,
7697
},
7798
{
7899
name: "Initializing to Failed",
79100
args: args{
80-
old: &platformv1.Cluster{
81-
ObjectMeta: v1.ObjectMeta{ResourceVersion: "old"},
82-
Status: platformv1.ClusterStatus{Phase: platformv1.ClusterInitializing},
83-
},
84-
new: &platformv1.Cluster{
85-
ObjectMeta: v1.ObjectMeta{ResourceVersion: "new"},
86-
Status: platformv1.ClusterStatus{Phase: platformv1.ClusterFailed},
87-
},
101+
old: newClusterForTest("old", nil, platformv1.ClusterInitializing, nil),
102+
new: newClusterForTest("new", nil, platformv1.ClusterFailed, nil),
88103
},
89104
want: true,
90105
},
91106
{
92107
name: "Running to Failed",
93108
args: args{
94-
old: &platformv1.Cluster{
95-
ObjectMeta: v1.ObjectMeta{ResourceVersion: "old"},
96-
Status: platformv1.ClusterStatus{Phase: platformv1.ClusterRunning},
97-
},
98-
new: &platformv1.Cluster{
99-
ObjectMeta: v1.ObjectMeta{ResourceVersion: "new"},
100-
Status: platformv1.ClusterStatus{Phase: platformv1.ClusterFailed},
101-
},
109+
old: newClusterForTest("old", nil, platformv1.ClusterRunning, nil),
110+
new: newClusterForTest("new", nil, platformv1.ClusterFailed, nil),
102111
},
103112
want: true,
104113
},
105114
{
106115
name: "Running to Terminating",
107116
args: args{
108-
old: &platformv1.Cluster{
109-
ObjectMeta: v1.ObjectMeta{ResourceVersion: "old"},
110-
Status: platformv1.ClusterStatus{Phase: platformv1.ClusterRunning},
111-
},
112-
new: &platformv1.Cluster{
113-
ObjectMeta: v1.ObjectMeta{ResourceVersion: "new"},
114-
Status: platformv1.ClusterStatus{Phase: platformv1.ClusterTerminating},
115-
},
117+
old: newClusterForTest("old", nil, platformv1.ClusterRunning, nil),
118+
new: newClusterForTest("new", nil, platformv1.ClusterTerminating, nil),
116119
},
117120
want: true,
118121
},
119122
{
120123
name: "Failed to Terminating",
121124
args: args{
122-
old: &platformv1.Cluster{
123-
ObjectMeta: v1.ObjectMeta{ResourceVersion: "old"},
124-
Status: platformv1.ClusterStatus{Phase: platformv1.ClusterFailed},
125-
},
126-
new: &platformv1.Cluster{
127-
ObjectMeta: v1.ObjectMeta{ResourceVersion: "new"},
128-
Status: platformv1.ClusterStatus{Phase: platformv1.ClusterTerminating},
129-
},
125+
old: newClusterForTest("old", nil, platformv1.ClusterFailed, nil),
126+
new: newClusterForTest("new", nil, platformv1.ClusterTerminating, nil),
130127
},
131128
want: true,
132129
},
133130
{
134131
name: "Failed to Running",
135132
args: args{
136-
old: &platformv1.Cluster{
137-
ObjectMeta: v1.ObjectMeta{ResourceVersion: "old"},
138-
Status: platformv1.ClusterStatus{Phase: platformv1.ClusterFailed},
139-
},
140-
new: &platformv1.Cluster{
141-
ObjectMeta: v1.ObjectMeta{ResourceVersion: "new"},
142-
Status: platformv1.ClusterStatus{Phase: platformv1.ClusterRunning},
143-
},
133+
old: newClusterForTest("old", nil, platformv1.ClusterFailed, nil),
134+
new: newClusterForTest("new", nil, platformv1.ClusterRunning, nil),
144135
},
145136
want: true,
146137
},
147138
{
148139
name: "Failed to Initializing",
149140
args: args{
150-
old: &platformv1.Cluster{
151-
ObjectMeta: v1.ObjectMeta{ResourceVersion: "old"},
152-
Status: platformv1.ClusterStatus{Phase: platformv1.ClusterFailed},
153-
},
154-
new: &platformv1.Cluster{
155-
ObjectMeta: v1.ObjectMeta{ResourceVersion: "new"},
156-
Status: platformv1.ClusterStatus{Phase: platformv1.ClusterInitializing},
157-
},
141+
old: newClusterForTest("old", nil, platformv1.ClusterFailed, nil),
142+
new: newClusterForTest("new", nil, platformv1.ClusterInitializing, nil),
158143
},
159144
want: true,
160145
},
161146
{
162-
name: "last conditon unkonwn to false",
147+
name: "Initializing last conditon unkonwn to false",
163148
args: args{
164-
old: &platformv1.Cluster{
165-
ObjectMeta: v1.ObjectMeta{ResourceVersion: "old"},
166-
Status: platformv1.ClusterStatus{Phase: platformv1.ClusterInitializing,
167-
Conditions: []platformv1.ClusterCondition{{Status: platformv1.ConditionUnknown}}},
168-
},
169-
new: &platformv1.Cluster{
170-
ObjectMeta: v1.ObjectMeta{ResourceVersion: "new"},
171-
Status: platformv1.ClusterStatus{Phase: platformv1.ClusterInitializing,
172-
Conditions: []platformv1.ClusterCondition{{Status: platformv1.ConditionFalse}}},
173-
},
149+
old: newClusterForTest("old", nil, platformv1.ClusterInitializing, []platformv1.ClusterCondition{{Status: platformv1.ConditionUnknown}}),
150+
new: newClusterForTest("new", nil, platformv1.ClusterInitializing, []platformv1.ClusterCondition{{Status: platformv1.ConditionFalse}}),
174151
},
175152
want: false,
176153
},
177154
{
178155
name: "last conditon unkonwn to true",
179156
args: args{
180-
old: &platformv1.Cluster{
181-
ObjectMeta: v1.ObjectMeta{ResourceVersion: "old"},
182-
Status: platformv1.ClusterStatus{Phase: platformv1.ClusterInitializing,
183-
Conditions: []platformv1.ClusterCondition{{Status: platformv1.ConditionUnknown}}},
184-
},
185-
new: &platformv1.Cluster{
186-
ObjectMeta: v1.ObjectMeta{ResourceVersion: "new"},
187-
Status: platformv1.ClusterStatus{Phase: platformv1.ClusterInitializing,
188-
Conditions: []platformv1.ClusterCondition{{Status: platformv1.ConditionTrue}}},
189-
},
157+
old: newClusterForTest("old", nil, platformv1.ClusterPhase(""), []platformv1.ClusterCondition{{Status: platformv1.ConditionUnknown}}),
158+
new: newClusterForTest("new", nil, platformv1.ClusterPhase(""), []platformv1.ClusterCondition{{Status: platformv1.ConditionFalse}}),
190159
},
191160
want: true,
192161
},
193162
{
194-
name: "last conditon unkonwn to true resync",
195-
args: args{
196-
old: &platformv1.Cluster{
197-
ObjectMeta: v1.ObjectMeta{ResourceVersion: "new"},
198-
Status: platformv1.ClusterStatus{Phase: platformv1.ClusterInitializing,
199-
Conditions: []platformv1.ClusterCondition{{Status: platformv1.ConditionUnknown}}},
200-
},
201-
new: &platformv1.Cluster{
202-
ObjectMeta: v1.ObjectMeta{ResourceVersion: "new"},
203-
Status: platformv1.ClusterStatus{Phase: platformv1.ClusterInitializing,
204-
Conditions: []platformv1.ClusterCondition{{Status: platformv1.ConditionUnknown}}},
205-
},
206-
},
163+
name: "Initializing last conditon false retrun true if resync",
164+
args: func() args {
165+
// resource version equal
166+
new := newClusterForTest("new", nil, platformv1.ClusterPhase(""), []platformv1.ClusterCondition{{Status: platformv1.ConditionFalse}})
167+
return args{new, new}
168+
}(),
207169
want: true,
208170
},
209171
{
210172
name: "last conditon true to unknown",
211173
args: args{
212-
old: &platformv1.Cluster{
213-
ObjectMeta: v1.ObjectMeta{ResourceVersion: "old"},
214-
Status: platformv1.ClusterStatus{Phase: platformv1.ClusterInitializing,
215-
Conditions: []platformv1.ClusterCondition{{Status: platformv1.ConditionTrue}}},
216-
},
217-
new: &platformv1.Cluster{
218-
ObjectMeta: v1.ObjectMeta{ResourceVersion: "new"},
219-
Status: platformv1.ClusterStatus{Phase: platformv1.ClusterInitializing,
220-
Conditions: []platformv1.ClusterCondition{{Status: platformv1.ConditionUnknown}}},
221-
},
174+
old: newClusterForTest("old", nil, platformv1.ClusterPhase(""), []platformv1.ClusterCondition{{Status: platformv1.ConditionTrue}}),
175+
new: newClusterForTest("new", nil, platformv1.ClusterPhase(""), []platformv1.ClusterCondition{{Status: platformv1.ConditionUnknown}}),
222176
},
223177
want: true,
224178
},
225179
{
226180
name: "last conditon false to unknown",
227181
args: args{
228-
old: &platformv1.Cluster{
229-
ObjectMeta: v1.ObjectMeta{ResourceVersion: "old"},
230-
Status: platformv1.ClusterStatus{Phase: platformv1.ClusterInitializing,
231-
Conditions: []platformv1.ClusterCondition{{Status: platformv1.ConditionFalse}}},
232-
},
233-
new: &platformv1.Cluster{
234-
ObjectMeta: v1.ObjectMeta{ResourceVersion: "new"},
235-
Status: platformv1.ClusterStatus{Phase: platformv1.ClusterInitializing,
236-
Conditions: []platformv1.ClusterCondition{{Status: platformv1.ConditionUnknown}}},
237-
},
182+
old: newClusterForTest("old", nil, platformv1.ClusterPhase(""), []platformv1.ClusterCondition{{Status: platformv1.ConditionFalse}}),
183+
new: newClusterForTest("new", nil, platformv1.ClusterPhase(""), []platformv1.ClusterCondition{{Status: platformv1.ConditionUnknown}}),
238184
},
239185
want: true,
240186
},
187+
{
188+
name: "health check is not long enough",
189+
args: func() args {
190+
new := newClusterForTest("old", nil, platformv1.ClusterPhase(""), []platformv1.ClusterCondition{{
191+
Type: conditionTypeHealthCheck,
192+
Status: platformv1.ConditionTrue,
193+
LastProbeTime: v1.NewTime(time.Now().Add(-resyncInternal / 2))}})
194+
return args{new, new}
195+
}(),
196+
want: false,
197+
},
198+
{
199+
name: "health check is long enough",
200+
args: func() args {
201+
new := newClusterForTest("old", nil, platformv1.ClusterPhase(""), []platformv1.ClusterCondition{{
202+
Type: conditionTypeHealthCheck,
203+
Status: platformv1.ConditionTrue,
204+
LastProbeTime: v1.NewTime(time.Now().Add(-resyncInternal - 1))}})
205+
return args{new, new}
206+
}(),
207+
want: true,
208+
},
241209
// TODO: Add test cases.
242210
}
243211
for _, tt := range tests {
@@ -249,7 +217,7 @@ func TestController_needsUpdate(t *testing.T) {
249217
// log: tt.fields.log,
250218
// platformClient: tt.fields.platformClient,
251219
// deleter: tt.fields.deleter,
252-
healthCheckPeriod: time.Second,
220+
healthCheckPeriod: resyncInternal,
253221
}
254222
if got := c.needsUpdate(tt.args.old, tt.args.new); got != tt.want {
255223
t.Errorf("Controller.needsUpdate() = %v, want %v", got, tt.want)

0 commit comments

Comments
 (0)