Skip to content

Commit ee47752

Browse files
zhujian7zhujian7
authored
🌱 Deprecate work healthCheck use healthChecker instead (#324)
* Deprecate work healthCheck use healthChecker instead Signed-off-by: zhujian <[email protected]> * Fix integration tests Signed-off-by: zhujian <[email protected]> --------- Signed-off-by: zhujian <[email protected]>
1 parent 5d62be0 commit ee47752

File tree

6 files changed

+89
-51
lines changed

6 files changed

+89
-51
lines changed

‎pkg/addonmanager/controllers/agentdeploy/healthcheck_sync.go‎

Lines changed: 21 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,19 @@ func (s *healthCheckSyncer) probeAddonStatusByWorks(
191191

192192
for _, field := range probeFields {
193193
results := findResultsByIdentifier(field.ResourceIdentifier, manifestConditions)
194+
// if no results are returned. it is possible that work agent has not returned the feedback value.
195+
// mark condition to unknown
196+
if len(results) == 0 {
197+
meta.SetStatusCondition(&addon.Status.Conditions, metav1.Condition{
198+
Type: addonapiv1alpha1.ManagedClusterAddOnConditionAvailable,
199+
Status: metav1.ConditionUnknown,
200+
Reason: addonapiv1alpha1.AddonAvailableReasonNoProbeResult,
201+
Message: fmt.Sprintf("Probe results are not returned for %s/%s: %s/%s",
202+
field.ResourceIdentifier.Group, field.ResourceIdentifier.Resource,
203+
field.ResourceIdentifier.Namespace, field.ResourceIdentifier.Name),
204+
})
205+
return nil
206+
}
194207

195208
// healthCheck will be ignored if healthChecker is set
196209
if healthChecker != nil {
@@ -210,20 +223,6 @@ func (s *healthCheckSyncer) probeAddonStatusByWorks(
210223
return nil
211224
}
212225

213-
// if no results are returned. it is possible that work agent has not returned the feedback value.
214-
// mark condition to unknown
215-
if len(results) == 0 {
216-
meta.SetStatusCondition(&addon.Status.Conditions, metav1.Condition{
217-
Type: addonapiv1alpha1.ManagedClusterAddOnConditionAvailable,
218-
Status: metav1.ConditionUnknown,
219-
Reason: addonapiv1alpha1.AddonAvailableReasonNoProbeResult,
220-
Message: fmt.Sprintf("Probe results are not returned for %s/%s: %s/%s",
221-
field.ResourceIdentifier.Group, field.ResourceIdentifier.Resource,
222-
field.ResourceIdentifier.Namespace, field.ResourceIdentifier.Name),
223-
})
224-
return nil
225-
}
226-
227226
for _, result := range results {
228227
err := healthCheck(result.ResourceIdentifier, result.FeedbackResult)
229228
if err != nil {
@@ -275,11 +274,11 @@ func (s *healthCheckSyncer) analyzeWorkProber(
275274
}
276275
return nil, nil, nil, fmt.Errorf("work prober is not configured")
277276
case agent.HealthProberTypeDeploymentAvailability:
278-
probeFields, heathCheck, err := s.analyzeDeploymentWorkProber(agentAddon, cluster, addon)
279-
return probeFields, heathCheck, nil, err
277+
probeFields, heathChecker, err := s.analyzeDeploymentWorkProber(agentAddon, cluster, addon)
278+
return probeFields, nil, heathChecker, err
280279
case agent.HealthProberTypeWorkloadAvailability:
281-
probeFields, heathCheck, err := s.analyzeWorkloadsWorkProber(agentAddon, cluster, addon)
282-
return probeFields, heathCheck, nil, err
280+
probeFields, heathChecker, err := s.analyzeWorkloadsWorkProber(agentAddon, cluster, addon)
281+
return probeFields, nil, heathChecker, err
283282
default:
284283
return nil, nil, nil, fmt.Errorf("unsupported health prober type %s", agentAddon.GetAgentAddonOptions().HealthProber.Type)
285284
}
@@ -289,7 +288,7 @@ func (s *healthCheckSyncer) analyzeDeploymentWorkProber(
289288
agentAddon agent.AgentAddon,
290289
cluster *clusterv1.ManagedCluster,
291290
addon *addonapiv1alpha1.ManagedClusterAddOn,
292-
) ([]agent.ProbeField, agent.AddonHealthCheckFunc, error) {
291+
) ([]agent.ProbeField, agent.AddonHealthCheckerFunc, error) {
293292
probeFields := []agent.ProbeField{}
294293

295294
manifests, err := agentAddon.Manifests(cluster, addon)
@@ -310,14 +309,14 @@ func (s *healthCheckSyncer) analyzeDeploymentWorkProber(
310309
})
311310
}
312311

313-
return probeFields, utils.DeploymentAvailabilityHealthCheck, nil
312+
return probeFields, utils.DeploymentAvailabilityHealthChecker, nil
314313
}
315314

316315
func (s *healthCheckSyncer) analyzeWorkloadsWorkProber(
317316
agentAddon agent.AgentAddon,
318317
cluster *clusterv1.ManagedCluster,
319318
addon *addonapiv1alpha1.ManagedClusterAddOn,
320-
) ([]agent.ProbeField, agent.AddonHealthCheckFunc, error) {
319+
) ([]agent.ProbeField, agent.AddonHealthCheckerFunc, error) {
321320
probeFields := []agent.ProbeField{}
322321

323322
manifests, err := agentAddon.Manifests(cluster, addon)
@@ -344,7 +343,7 @@ func (s *healthCheckSyncer) analyzeWorkloadsWorkProber(
344343
})
345344
}
346345

347-
return probeFields, utils.WorkloadAvailabilityHealthCheck, nil
346+
return probeFields, utils.WorkloadAvailabilityHealthChecker, nil
348347
}
349348

350349
func findResultsByIdentifier(identifier workapiv1.ResourceIdentifier,

‎pkg/addonmanager/controllers/agentdeploy/healthcheck_sync_test.go‎

Lines changed: 33 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ package agentdeploy
22

33
import (
44
"context"
5-
"fmt"
65
"testing"
76
"time"
87

@@ -454,7 +453,22 @@ func TestHealthCheckReconcile(t *testing.T) {
454453
Name: "test-deployment1",
455454
Namespace: "default",
456455
},
457-
StatusFeedbacks: v1.StatusFeedbackResult{},
456+
StatusFeedbacks: v1.StatusFeedbackResult{
457+
Values: []v1.FeedbackValue{
458+
{
459+
Name: "Replicas",
460+
Value: v1.FieldValue{
461+
Integer: boolPtr(1),
462+
},
463+
},
464+
{
465+
Name: "ReadyReplicas",
466+
Value: v1.FieldValue{
467+
Integer: boolPtr(2),
468+
},
469+
},
470+
},
471+
},
458472
},
459473
},
460474
},
@@ -532,7 +546,22 @@ func TestHealthCheckReconcile(t *testing.T) {
532546
Name: "test-deployment1",
533547
Namespace: "default",
534548
},
535-
StatusFeedbacks: v1.StatusFeedbackResult{},
549+
StatusFeedbacks: v1.StatusFeedbackResult{
550+
Values: []v1.FeedbackValue{
551+
{
552+
Name: "Replicas",
553+
Value: v1.FieldValue{
554+
Integer: boolPtr(1),
555+
},
556+
},
557+
{
558+
Name: "ReadyReplicas",
559+
Value: v1.FieldValue{
560+
Integer: boolPtr(2),
561+
},
562+
},
563+
},
564+
},
536565
},
537566
},
538567
},
@@ -1298,20 +1327,6 @@ func TestHealthCheckReconcile(t *testing.T) {
12981327
}
12991328
}
13001329

1301-
func addonHealthCheckAllFunc(resultFields []agent.FieldResult, cluster *clusterv1.ManagedCluster,
1302-
addon *addonapiv1alpha1.ManagedClusterAddOn) error {
1303-
for _, field := range resultFields {
1304-
switch field.ResourceIdentifier.Resource {
1305-
case "deployments":
1306-
err := utils.DeploymentAvailabilityHealthCheck(field.ResourceIdentifier, field.FeedbackResult)
1307-
if err == nil {
1308-
return nil
1309-
}
1310-
}
1311-
}
1312-
return fmt.Errorf("not meet the results")
1313-
}
1314-
13151330
func newDeploymentsCheckAllProber(deployments ...types.NamespacedName) *agent.HealthProber {
13161331
probeFields := []agent.ProbeField{}
13171332
for _, deploy := range deployments {
@@ -1325,7 +1340,7 @@ func newDeploymentsCheckAllProber(deployments ...types.NamespacedName) *agent.He
13251340
Type: agent.HealthProberTypeWork,
13261341
WorkProber: &agent.WorkHealthProber{
13271342
ProbeFields: probeFields,
1328-
HealthChecker: addonHealthCheckAllFunc,
1343+
HealthChecker: utils.DeploymentAvailabilityHealthChecker,
13291344
},
13301345
}
13311346
}

‎pkg/agent/inteface.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,7 @@ type WorkHealthProber struct {
178178
// HealthCheck is deprecated and will be removed in the future. please use HealthChecker instead.
179179
// HealthCheck will be ignored if HealthChecker is set.
180180
// HealthCheck check status of the addon based on each probeField result.
181+
// Deprecated: use HealthChecker instead.
181182
HealthCheck AddonHealthCheckFunc
182183

183184
// HealthChecker check status of the addon based of all results of probeFields

‎pkg/utils/probe_helper.go‎

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import (
99
"k8s.io/apimachinery/pkg/runtime/schema"
1010
"k8s.io/apimachinery/pkg/types"
1111
"open-cluster-management.io/addon-framework/pkg/agent"
12-
"open-cluster-management.io/api/addon/v1alpha1"
12+
addonapiv1alpha1 "open-cluster-management.io/api/addon/v1alpha1"
1313
clusterv1 "open-cluster-management.io/api/cluster/v1"
1414
workapiv1 "open-cluster-management.io/api/work/v1"
1515
)
@@ -32,8 +32,8 @@ func NewDeploymentProber(deployments ...types.NamespacedName) *agent.HealthProbe
3232
return &agent.HealthProber{
3333
Type: agent.HealthProberTypeWork,
3434
WorkProber: &agent.WorkHealthProber{
35-
ProbeFields: probeFields,
36-
HealthCheck: DeploymentAvailabilityHealthCheck,
35+
ProbeFields: probeFields,
36+
HealthChecker: DeploymentAvailabilityHealthChecker,
3737
},
3838
}
3939
}
@@ -59,7 +59,7 @@ func NewAllDeploymentsProber() *agent.HealthProber {
5959
Type: agent.HealthProberTypeWork,
6060
WorkProber: &agent.WorkHealthProber{
6161
ProbeFields: probeFields,
62-
HealthChecker: AllDeploymentsAvailabilityHealthCheck,
62+
HealthChecker: DeploymentAvailabilityHealthChecker,
6363
},
6464
}
6565
}
@@ -84,26 +84,43 @@ func (d *DeploymentProber) ProbeFields() []agent.ProbeField {
8484
return probeFields
8585
}
8686

87+
// Deprecated: use DeploymentAvailabilityHealthChecker instead.
8788
func DeploymentAvailabilityHealthCheck(identifier workapiv1.ResourceIdentifier,
8889
result workapiv1.StatusFeedbackResult) error {
89-
return WorkloadAvailabilityHealthCheck(identifier, result)
90+
return checkWorkloadAvailabilityHealth(identifier, result)
9091
}
9192

93+
// Deprecated: use DeploymentAvailabilityHealthChecker instead.
9294
func AllDeploymentsAvailabilityHealthCheck(results []agent.FieldResult,
93-
cluster *clusterv1.ManagedCluster, addon *v1alpha1.ManagedClusterAddOn) error {
95+
cluster *clusterv1.ManagedCluster, addon *addonapiv1alpha1.ManagedClusterAddOn) error {
9496
if len(results) < 2 {
9597
return fmt.Errorf("all deployments are not available")
9698
}
9799

98100
for _, result := range results {
99-
if err := WorkloadAvailabilityHealthCheck(result.ResourceIdentifier, result.FeedbackResult); err != nil {
101+
if err := checkWorkloadAvailabilityHealth(result.ResourceIdentifier, result.FeedbackResult); err != nil {
102+
return err
103+
}
104+
}
105+
return nil
106+
}
107+
108+
func DeploymentAvailabilityHealthChecker(results []agent.FieldResult,
109+
cluster *clusterv1.ManagedCluster, addon *addonapiv1alpha1.ManagedClusterAddOn) error {
110+
return WorkloadAvailabilityHealthChecker(results, cluster, addon)
111+
}
112+
113+
func WorkloadAvailabilityHealthChecker(results []agent.FieldResult,
114+
cluster *clusterv1.ManagedCluster, addon *addonapiv1alpha1.ManagedClusterAddOn) error {
115+
for _, result := range results {
116+
if err := checkWorkloadAvailabilityHealth(result.ResourceIdentifier, result.FeedbackResult); err != nil {
100117
return err
101118
}
102119
}
103120
return nil
104121
}
105122

106-
func WorkloadAvailabilityHealthCheck(identifier workapiv1.ResourceIdentifier,
123+
func checkWorkloadAvailabilityHealth(identifier workapiv1.ResourceIdentifier,
107124
result workapiv1.StatusFeedbackResult) error {
108125
// only support deployments and daemonsets for now
109126
if identifier.Resource != "deployments" && identifier.Resource != "daemonsets" {

‎pkg/utils/probe_helper_test.go‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
99
"k8s.io/apimachinery/pkg/runtime"
1010
"k8s.io/apimachinery/pkg/types"
11+
"open-cluster-management.io/addon-framework/pkg/agent"
1112
workapiv1 "open-cluster-management.io/api/work/v1"
1213
)
1314

@@ -113,8 +114,12 @@ func TestDeploymentProbe(t *testing.T) {
113114
prober := NewDeploymentProber(types.NamespacedName{Name: "test", Namespace: "testns"})
114115

115116
fields := prober.WorkProber.ProbeFields
116-
117-
err := prober.WorkProber.HealthCheck(fields[0].ResourceIdentifier, c.result)
117+
err := prober.WorkProber.HealthChecker([]agent.FieldResult{
118+
{
119+
ResourceIdentifier: fields[0].ResourceIdentifier,
120+
FeedbackResult: c.result,
121+
},
122+
}, nil, nil)
118123
if err != nil && err.Error() != c.expectedErr {
119124
t.Errorf("expected error %s but got %v", c.expectedErr, err)
120125
}

‎test/integration/kube/agent_deploy_test.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -922,7 +922,8 @@ var _ = ginkgo.Describe("Agent deploy", func() {
922922
return err
923923
}
924924

925-
if !meta.IsStatusConditionFalse(addon.Status.Conditions, "Available") {
925+
// the daemonset is not probed, so the addon available condition is unknown
926+
if !meta.IsStatusConditionPresentAndEqual(addon.Status.Conditions, "Available", metav1.ConditionUnknown) {
926927
return fmt.Errorf("Unexpected addon available condition, %v", addon.Status.Conditions)
927928
}
928929
return nil

0 commit comments

Comments
 (0)