Skip to content

Commit e126f3a

Browse files
leoryuleoryu
committed
fix(platform): update cls/mc needupdate logic
1 parent 48967e3 commit e126f3a

File tree

4 files changed

+579
-36
lines changed

4 files changed

+579
-36
lines changed

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

Lines changed: 31 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -148,33 +148,43 @@ func (c *Controller) enqueue(obj *platformv1.Cluster) {
148148
}
149149

150150
func (c *Controller) needsUpdate(old *platformv1.Cluster, new *platformv1.Cluster) bool {
151-
if !reflect.DeepEqual(old.Spec, new.Spec) {
151+
switch {
152+
case !reflect.DeepEqual(old.Spec, new.Spec):
152153
return true
153-
}
154-
155-
if old.Status.Phase != platformv1.ClusterTerminating && new.Status.Phase == platformv1.ClusterTerminating {
154+
case !reflect.DeepEqual(old.ObjectMeta.Labels, new.ObjectMeta.Labels):
156155
return true
157-
}
158-
159-
if !reflect.DeepEqual(old.ObjectMeta.Annotations, new.ObjectMeta.Annotations) {
156+
case !reflect.DeepEqual(old.ObjectMeta.Annotations, new.ObjectMeta.Annotations):
160157
return true
161-
}
162-
163-
if !reflect.DeepEqual(old.ObjectMeta.Labels, new.ObjectMeta.Labels) {
158+
case old.Status.Phase != new.Status.Phase:
164159
return true
165-
}
160+
case new.Status.Phase == platformv1.ClusterInitializing:
161+
// if ResourceVersion is equal, it's an resync envent, should return true.
162+
if old.ResourceVersion == new.ResourceVersion {
163+
return true
164+
}
165+
if len(new.Status.Conditions) == 0 {
166+
return true
167+
}
168+
if new.Status.Conditions[len(new.Status.Conditions)-1].Status == platformv1.ConditionUnknown {
169+
return true
170+
}
171+
// if user set last condition false block procesee
172+
if new.Status.Conditions[len(new.Status.Conditions)-1].Status == platformv1.ConditionFalse {
173+
return false
174+
}
175+
fallthrough
176+
default:
177+
healthCondition := new.GetCondition(conditionTypeHealthCheck)
178+
if healthCondition == nil {
179+
// when healthCondition is not set, if ResourceVersion is equal, it's an resync envent, should return true.
180+
return old.ResourceVersion == new.ResourceVersion
181+
}
182+
if time.Since(healthCondition.LastProbeTime.Time) > c.healthCheckPeriod {
183+
return true
184+
}
166185

167-
// Control the synchronization interval through the health detection interval
168-
// to avoid version conflicts caused by concurrent modification
169-
healthCondition := new.GetCondition(conditionTypeHealthCheck)
170-
if healthCondition == nil {
171-
return true
172-
}
173-
if time.Since(healthCondition.LastProbeTime.Time) > c.healthCheckPeriod {
174-
return true
186+
return false
175187
}
176-
177-
return false
178188
}
179189

180190
// 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: 259 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,259 @@
1+
/*
2+
* Tencent is pleased to support the open source community by making TKEStack
3+
* available.
4+
*
5+
* Copyright (C) 2012-2021 Tencent. All Rights Reserved.
6+
*
7+
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use
8+
* this file except in compliance with the License. You may obtain a copy of the
9+
* License at
10+
*
11+
* https://opensource.org/licenses/Apache-2.0
12+
*
13+
* Unless required by applicable law or agreed to in writing, software
14+
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
15+
* WARRANTIES OF ANY KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations under the License.
17+
*/
18+
19+
package cluster
20+
21+
import (
22+
"testing"
23+
"time"
24+
25+
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
26+
platformv1 "tkestack.io/tke/api/platform/v1"
27+
)
28+
29+
func TestController_needsUpdate(t *testing.T) {
30+
// type fields struct {
31+
// queue workqueue.RateLimitingInterface
32+
// lister platformv1lister.ClusterLister
33+
// listerSynced cache.InformerSynced
34+
// log log.Logger
35+
// platformClient platformversionedclient.PlatformV1Interface
36+
// deleter deletion.ClusterDeleterInterface
37+
// healthCheckPeriod time.Duration
38+
// }
39+
type args struct {
40+
old *platformv1.Cluster
41+
new *platformv1.Cluster
42+
}
43+
tests := []struct {
44+
name string
45+
// fields fields
46+
args args
47+
want bool
48+
}{
49+
{
50+
name: "change spec",
51+
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+
},
60+
},
61+
want: true,
62+
},
63+
{
64+
name: "Initializing to Running",
65+
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+
},
74+
},
75+
want: true,
76+
},
77+
{
78+
name: "Initializing to Failed",
79+
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+
},
88+
},
89+
want: true,
90+
},
91+
{
92+
name: "Running to Failed",
93+
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+
},
102+
},
103+
want: true,
104+
},
105+
{
106+
name: "Running to Terminating",
107+
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+
},
116+
},
117+
want: true,
118+
},
119+
{
120+
name: "Failed to Terminating",
121+
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+
},
130+
},
131+
want: true,
132+
},
133+
{
134+
name: "Failed to Running",
135+
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+
},
144+
},
145+
want: true,
146+
},
147+
{
148+
name: "Failed to Initializing",
149+
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+
},
158+
},
159+
want: true,
160+
},
161+
{
162+
name: "last conditon unkonwn to false",
163+
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+
},
174+
},
175+
want: false,
176+
},
177+
{
178+
name: "last conditon unkonwn to true",
179+
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+
},
190+
},
191+
want: true,
192+
},
193+
{
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+
},
207+
want: true,
208+
},
209+
{
210+
name: "last conditon true to unknown",
211+
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+
},
222+
},
223+
want: true,
224+
},
225+
{
226+
name: "last conditon false to unknown",
227+
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+
},
238+
},
239+
want: true,
240+
},
241+
// TODO: Add test cases.
242+
}
243+
for _, tt := range tests {
244+
t.Run(tt.name, func(t *testing.T) {
245+
c := &Controller{
246+
// queue: tt.fields.queue,
247+
// lister: tt.fields.lister,
248+
// listerSynced: tt.fields.listerSynced,
249+
// log: tt.fields.log,
250+
// platformClient: tt.fields.platformClient,
251+
// deleter: tt.fields.deleter,
252+
healthCheckPeriod: time.Second,
253+
}
254+
if got := c.needsUpdate(tt.args.old, tt.args.new); got != tt.want {
255+
t.Errorf("Controller.needsUpdate() = %v, want %v", got, tt.want)
256+
}
257+
})
258+
}
259+
}

‎pkg/platform/controller/machine/machine_controller.go‎

Lines changed: 33 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -116,26 +116,44 @@ func (c *Controller) updateMachine(old, obj interface{}) {
116116
c.enqueue(machine)
117117
}
118118

119-
func (c *Controller) needsUpdate(oldMachine *platformv1.Machine, newMachine *platformv1.Machine) bool {
120-
if !reflect.DeepEqual(oldMachine.Spec, newMachine.Spec) {
119+
func (c *Controller) needsUpdate(old *platformv1.Machine, new *platformv1.Machine) bool {
120+
switch {
121+
case !reflect.DeepEqual(old.Spec, new.Spec):
121122
return true
122-
}
123-
124-
if oldMachine.Status.Phase != platformv1.MachineTerminating && newMachine.Status.Phase == platformv1.MachineTerminating {
123+
case !reflect.DeepEqual(old.ObjectMeta.Labels, new.ObjectMeta.Labels):
125124
return true
126-
}
127-
128-
// Control the synchronization interval through the health detection interval
129-
// to avoid version conflicts caused by concurrent modification
130-
healthCondition := newMachine.GetCondition(conditionTypeHealthCheck)
131-
if healthCondition == nil {
125+
case !reflect.DeepEqual(old.ObjectMeta.Annotations, new.ObjectMeta.Annotations):
132126
return true
133-
}
134-
if time.Since(healthCondition.LastProbeTime.Time) > resyncInternal {
127+
case old.Status.Phase != new.Status.Phase:
135128
return true
136-
}
129+
case new.Status.Phase == platformv1.MachineInitializing:
130+
// if ResourceVersion is equal, it's an resync envent, should return true.
131+
if old.ResourceVersion == new.ResourceVersion {
132+
return true
133+
}
134+
if len(new.Status.Conditions) == 0 {
135+
return true
136+
}
137+
if new.Status.Conditions[len(new.Status.Conditions)-1].Status == platformv1.ConditionUnknown {
138+
return true
139+
}
140+
// if user set last condition false block procesee
141+
if new.Status.Conditions[len(new.Status.Conditions)-1].Status == platformv1.ConditionFalse {
142+
return false
143+
}
144+
fallthrough
145+
default:
146+
healthCondition := new.GetCondition(conditionTypeHealthCheck)
147+
if healthCondition == nil {
148+
// when healthCondition is not set, if ResourceVersion is equal, it's an resync envent, should return true.
149+
return old.ResourceVersion == new.ResourceVersion
150+
}
151+
if time.Since(healthCondition.LastProbeTime.Time) > resyncInternal {
152+
return true
153+
}
137154

138-
return false
155+
return false
156+
}
139157
}
140158

141159
func (c *Controller) enqueue(obj *platformv1.Machine) {

0 commit comments

Comments
 (0)