Skip to content

Commit

Permalink
fix(platform): healcheck resync internal not work (#1706)
Browse files Browse the repository at this point in the history
  • Loading branch information
@leoryu
leoryu committed Dec 9, 2021
1 parent 310e18e commit 4647d5d
Show file tree
Hide file tree
Showing 4 changed files with 209 additions and 283 deletions.
38 changes: 18 additions & 20 deletions pkg/platform/controller/cluster/cluster_controller.go
View file
Expand Up @@ -161,16 +161,22 @@ func (c *Controller) enqueue(obj *platformv1.Cluster) {
}

func (c *Controller) needsUpdate(old *platformv1.Cluster, new *platformv1.Cluster) bool {
switch {
case !reflect.DeepEqual(old.Spec, new.Spec):
healthCondition := new.GetCondition(conditionTypeHealthCheck)
if !reflect.DeepEqual(old.Spec, new.Spec) {
return true
case !reflect.DeepEqual(old.ObjectMeta.Labels, new.ObjectMeta.Labels):

}
if !reflect.DeepEqual(old.ObjectMeta.Labels, new.ObjectMeta.Labels) {
return true
case !reflect.DeepEqual(old.ObjectMeta.Annotations, new.ObjectMeta.Annotations):
}
if !reflect.DeepEqual(old.ObjectMeta.Annotations, new.ObjectMeta.Annotations) {
return true
case old.Status.Phase != new.Status.Phase:
}
if old.Status.Phase != new.Status.Phase {
return true
case new.Status.Phase == platformv1.ClusterInitializing:

}
if new.Status.Phase == platformv1.ClusterInitializing {
// if ResourceVersion is equal, it's an resync envent, should return true.
if old.ResourceVersion == new.ResourceVersion {
return true
Expand All @@ -181,25 +187,17 @@ func (c *Controller) needsUpdate(old *platformv1.Cluster, new *platformv1.Cluste
if new.Status.Conditions[len(new.Status.Conditions)-1].Status == platformv1.ConditionUnknown {
return true
}
// if user set last condition false block procesee
// if user set last condition false block procesee until resync envent
if new.Status.Conditions[len(new.Status.Conditions)-1].Status == platformv1.ConditionFalse {
return false
}
fallthrough
case !reflect.DeepEqual(old.Status.Conditions, new.Status.Conditions):
return true
default:
healthCondition := new.GetCondition(conditionTypeHealthCheck)
if healthCondition == nil {
// when healthCondition is not set, if ResourceVersion is equal, it's an resync envent, should return true.
return old.ResourceVersion == new.ResourceVersion
}
if time.Since(healthCondition.LastProbeTime.Time) > c.healthCheckPeriod {
return true
}

}
// if last health check is not long enough， return false
if healthCondition != nil &&
time.Since(healthCondition.LastProbeTime.Time) < c.healthCheckPeriod {
return false
}
return true
}

// Run will set up the event handlers for types we are interested in, as well
Expand Down
206 changes: 87 additions & 119 deletions pkg/platform/controller/cluster/cluster_controller_test.go
View file
Expand Up @@ -26,7 +26,40 @@ import (
platformv1 "tkestack.io/tke/api/platform/v1"
)

func newClusterForTest(resourcesVersion string, spec *platformv1.ClusterSpec, phase platformv1.ClusterPhase, conditions []platformv1.ClusterCondition) *platformv1.Cluster {
mc := &platformv1.Cluster{
ObjectMeta: v1.ObjectMeta{ResourceVersion: resourcesVersion},
Spec: platformv1.ClusterSpec{
TenantID: "default",
DisplayName: "global",
Type: "Baremetal",
Version: "1.21.1-tke.1",
},
Status: platformv1.ClusterStatus{
Phase: platformv1.ClusterRunning,
Conditions: []platformv1.ClusterCondition{
{
Type: conditionTypeHealthCheck,
Status: platformv1.ConditionTrue,
LastProbeTime: v1.Now(),
},
},
},
}
if spec != nil {
mc.Spec = *spec
}
if len(phase) != 0 {
mc.Status.Phase = phase
}
if conditions != nil {
mc.Status.Conditions = conditions
}
return mc
}

func TestController_needsUpdate(t *testing.T) {
resyncInternal := time.Minute
// type fields struct {
// queue workqueue.RateLimitingInterface
// lister platformv1lister.ClusterLister
Expand All @@ -49,195 +82,130 @@ func TestController_needsUpdate(t *testing.T) {
{
name: "change spec",
args: args{
old: &platformv1.Cluster{
ObjectMeta: v1.ObjectMeta{ResourceVersion: "old"},
Spec: platformv1.ClusterSpec{DisplayName: "old"},
},
new: &platformv1.Cluster{
ObjectMeta: v1.ObjectMeta{ResourceVersion: "new"},
Spec: platformv1.ClusterSpec{DisplayName: "nes"},
},
old: newClusterForTest("old", &platformv1.ClusterSpec{Version: "old"}, platformv1.ClusterPhase(""), nil),
new: newClusterForTest("new", &platformv1.ClusterSpec{Version: "new"}, platformv1.ClusterPhase(""), nil),
},
want: true,
},
{
name: "Initializing to Running",
args: args{
old: &platformv1.Cluster{
ObjectMeta: v1.ObjectMeta{ResourceVersion: "old"},
Status: platformv1.ClusterStatus{Phase: platformv1.ClusterInitializing},
},
new: &platformv1.Cluster{
ObjectMeta: v1.ObjectMeta{ResourceVersion: "new"},
Status: platformv1.ClusterStatus{Phase: platformv1.ClusterRunning},
},
old: newClusterForTest("old", nil, platformv1.ClusterInitializing, nil),
new: newClusterForTest("new", nil, platformv1.ClusterRunning, nil),
},
want: true,
},
{
name: "Initializing to Failed",
args: args{
old: &platformv1.Cluster{
ObjectMeta: v1.ObjectMeta{ResourceVersion: "old"},
Status: platformv1.ClusterStatus{Phase: platformv1.ClusterInitializing},
},
new: &platformv1.Cluster{
ObjectMeta: v1.ObjectMeta{ResourceVersion: "new"},
Status: platformv1.ClusterStatus{Phase: platformv1.ClusterFailed},
},
old: newClusterForTest("old", nil, platformv1.ClusterInitializing, nil),
new: newClusterForTest("new", nil, platformv1.ClusterFailed, nil),
},
want: true,
},
{
name: "Running to Failed",
args: args{
old: &platformv1.Cluster{
ObjectMeta: v1.ObjectMeta{ResourceVersion: "old"},
Status: platformv1.ClusterStatus{Phase: platformv1.ClusterRunning},
},
new: &platformv1.Cluster{
ObjectMeta: v1.ObjectMeta{ResourceVersion: "new"},
Status: platformv1.ClusterStatus{Phase: platformv1.ClusterFailed},
},
old: newClusterForTest("old", nil, platformv1.ClusterRunning, nil),
new: newClusterForTest("new", nil, platformv1.ClusterFailed, nil),
},
want: true,
},
{
name: "Running to Terminating",
args: args{
old: &platformv1.Cluster{
ObjectMeta: v1.ObjectMeta{ResourceVersion: "old"},
Status: platformv1.ClusterStatus{Phase: platformv1.ClusterRunning},
},
new: &platformv1.Cluster{
ObjectMeta: v1.ObjectMeta{ResourceVersion: "new"},
Status: platformv1.ClusterStatus{Phase: platformv1.ClusterTerminating},
},
old: newClusterForTest("old", nil, platformv1.ClusterRunning, nil),
new: newClusterForTest("new", nil, platformv1.ClusterTerminating, nil),
},
want: true,
},
{
name: "Failed to Terminating",
args: args{
old: &platformv1.Cluster{
ObjectMeta: v1.ObjectMeta{ResourceVersion: "old"},
Status: platformv1.ClusterStatus{Phase: platformv1.ClusterFailed},
},
new: &platformv1.Cluster{
ObjectMeta: v1.ObjectMeta{ResourceVersion: "new"},
Status: platformv1.ClusterStatus{Phase: platformv1.ClusterTerminating},
},
old: newClusterForTest("old", nil, platformv1.ClusterFailed, nil),
new: newClusterForTest("new", nil, platformv1.ClusterTerminating, nil),
},
want: true,
},
{
name: "Failed to Running",
args: args{
old: &platformv1.Cluster{
ObjectMeta: v1.ObjectMeta{ResourceVersion: "old"},
Status: platformv1.ClusterStatus{Phase: platformv1.ClusterFailed},
},
new: &platformv1.Cluster{
ObjectMeta: v1.ObjectMeta{ResourceVersion: "new"},
Status: platformv1.ClusterStatus{Phase: platformv1.ClusterRunning},
},
old: newClusterForTest("old", nil, platformv1.ClusterFailed, nil),
new: newClusterForTest("new", nil, platformv1.ClusterRunning, nil),
},
want: true,
},
{
name: "Failed to Initializing",
args: args{
old: &platformv1.Cluster{
ObjectMeta: v1.ObjectMeta{ResourceVersion: "old"},
Status: platformv1.ClusterStatus{Phase: platformv1.ClusterFailed},
},
new: &platformv1.Cluster{
ObjectMeta: v1.ObjectMeta{ResourceVersion: "new"},
Status: platformv1.ClusterStatus{Phase: platformv1.ClusterInitializing},
},
old: newClusterForTest("old", nil, platformv1.ClusterFailed, nil),
new: newClusterForTest("new", nil, platformv1.ClusterInitializing, nil),
},
want: true,
},
{
name: "last conditon unkonwn to false",
name: "Initializing last conditon unkonwn to false",
args: args{
old: &platformv1.Cluster{
ObjectMeta: v1.ObjectMeta{ResourceVersion: "old"},
Status: platformv1.ClusterStatus{Phase: platformv1.ClusterInitializing,
Conditions: []platformv1.ClusterCondition{{Status: platformv1.ConditionUnknown}}},
},
new: &platformv1.Cluster{
ObjectMeta: v1.ObjectMeta{ResourceVersion: "new"},
Status: platformv1.ClusterStatus{Phase: platformv1.ClusterInitializing,
Conditions: []platformv1.ClusterCondition{{Status: platformv1.ConditionFalse}}},
},
old: newClusterForTest("old", nil, platformv1.ClusterInitializing, []platformv1.ClusterCondition{{Status: platformv1.ConditionUnknown}}),
new: newClusterForTest("new", nil, platformv1.ClusterInitializing, []platformv1.ClusterCondition{{Status: platformv1.ConditionFalse}}),
},
want: false,
},
{
name: "last conditon unkonwn to true",
args: args{
old: &platformv1.Cluster{
ObjectMeta: v1.ObjectMeta{ResourceVersion: "old"},
Status: platformv1.ClusterStatus{Phase: platformv1.ClusterInitializing,
Conditions: []platformv1.ClusterCondition{{Status: platformv1.ConditionUnknown}}},
},
new: &platformv1.Cluster{
ObjectMeta: v1.ObjectMeta{ResourceVersion: "new"},
Status: platformv1.ClusterStatus{Phase: platformv1.ClusterInitializing,
Conditions: []platformv1.ClusterCondition{{Status: platformv1.ConditionTrue}}},
},
old: newClusterForTest("old", nil, platformv1.ClusterPhase(""), []platformv1.ClusterCondition{{Status: platformv1.ConditionUnknown}}),
new: newClusterForTest("new", nil, platformv1.ClusterPhase(""), []platformv1.ClusterCondition{{Status: platformv1.ConditionFalse}}),
},
want: true,
},
{
name: "last conditon unkonwn to true resync",
args: args{
old: &platformv1.Cluster{
ObjectMeta: v1.ObjectMeta{ResourceVersion: "new"},
Status: platformv1.ClusterStatus{Phase: platformv1.ClusterInitializing,
Conditions: []platformv1.ClusterCondition{{Status: platformv1.ConditionUnknown}}},
},
new: &platformv1.Cluster{
ObjectMeta: v1.ObjectMeta{ResourceVersion: "new"},
Status: platformv1.ClusterStatus{Phase: platformv1.ClusterInitializing,
Conditions: []platformv1.ClusterCondition{{Status: platformv1.ConditionUnknown}}},
},
},
name: "Initializing last conditon false retrun true if resync",
args: func() args {
// resource version equal
new := newClusterForTest("new", nil, platformv1.ClusterPhase(""), []platformv1.ClusterCondition{{Status: platformv1.ConditionFalse}})
return args{new, new}
}(),
want: true,
},
{
name: "last conditon true to unknown",
args: args{
old: &platformv1.Cluster{
ObjectMeta: v1.ObjectMeta{ResourceVersion: "old"},
Status: platformv1.ClusterStatus{Phase: platformv1.ClusterInitializing,
Conditions: []platformv1.ClusterCondition{{Status: platformv1.ConditionTrue}}},
},
new: &platformv1.Cluster{
ObjectMeta: v1.ObjectMeta{ResourceVersion: "new"},
Status: platformv1.ClusterStatus{Phase: platformv1.ClusterInitializing,
Conditions: []platformv1.ClusterCondition{{Status: platformv1.ConditionUnknown}}},
},
old: newClusterForTest("old", nil, platformv1.ClusterPhase(""), []platformv1.ClusterCondition{{Status: platformv1.ConditionTrue}}),
new: newClusterForTest("new", nil, platformv1.ClusterPhase(""), []platformv1.ClusterCondition{{Status: platformv1.ConditionUnknown}}),
},
want: true,
},
{
name: "last conditon false to unknown",
args: args{
old: &platformv1.Cluster{
ObjectMeta: v1.ObjectMeta{ResourceVersion: "old"},
Status: platformv1.ClusterStatus{Phase: platformv1.ClusterInitializing,
Conditions: []platformv1.ClusterCondition{{Status: platformv1.ConditionFalse}}},
},
new: &platformv1.Cluster{
ObjectMeta: v1.ObjectMeta{ResourceVersion: "new"},
Status: platformv1.ClusterStatus{Phase: platformv1.ClusterInitializing,
Conditions: []platformv1.ClusterCondition{{Status: platformv1.ConditionUnknown}}},
},
old: newClusterForTest("old", nil, platformv1.ClusterPhase(""), []platformv1.ClusterCondition{{Status: platformv1.ConditionFalse}}),
new: newClusterForTest("new", nil, platformv1.ClusterPhase(""), []platformv1.ClusterCondition{{Status: platformv1.ConditionUnknown}}),
},
want: true,
},
{
name: "health check is not long enough",
args: func() args {
new := newClusterForTest("old", nil, platformv1.ClusterPhase(""), []platformv1.ClusterCondition{{
Type: conditionTypeHealthCheck,
Status: platformv1.ConditionTrue,
LastProbeTime: v1.NewTime(time.Now().Add(-resyncInternal / 2))}})
return args{new, new}
}(),
want: false,
},
{
name: "health check is long enough",
args: func() args {
new := newClusterForTest("old", nil, platformv1.ClusterPhase(""), []platformv1.ClusterCondition{{
Type: conditionTypeHealthCheck,
Status: platformv1.ConditionTrue,
LastProbeTime: v1.NewTime(time.Now().Add(-resyncInternal - 1))}})
return args{new, new}
}(),
want: true,
},
// TODO: Add test cases.
}
for _, tt := range tests {
Expand All @@ -249,7 +217,7 @@ func TestController_needsUpdate(t *testing.T) {
// log: tt.fields.log,
// platformClient: tt.fields.platformClient,
// deleter: tt.fields.deleter,
healthCheckPeriod: time.Second,
healthCheckPeriod: resyncInternal,
}
if got := c.needsUpdate(tt.args.old, tt.args.new); got != tt.want {
t.Errorf("Controller.needsUpdate() = %v, want %v", got, tt.want)
Expand Down

0 comments on commit 4647d5d

Please sign in to comment.