✨ add new addonHealthCheck func to check all fields and support wildcard #289
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: zhiweiyin318 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
zhiweiyin318
force-pushed
the
support-probefileds
branch
5 times, most recently
from
2859b36 to
f890394
Compare
|
/assign @qiujian16 |
zhiweiyin318
force-pushed
the
support-probefileds
branch
from
f890394 to
754aecc
Compare
zhiweiyin318
changed the title
|
/lgtm |
healthChecker was introduced in open-cluster-management-io/addon-framework#289 and it allow us to define a list of ProbeFields as before but it also allows us to define HealthChecker. HealthChecker has the ability to go through all the resultFields and decide which resources are mandatory and should trigger unheathly probes and which resources are optional
healthChecker was introduced in open-cluster-management-io/addon-framework#289 and it allow us to define a list of ProbeFields as before but it also allows us to define HealthChecker. HealthChecker has the ability to go through all the resultFields and decide which resources are mandatory and should trigger unheathly probes and which resources are optional
healthChecker was introduced in open-cluster-management-io/addon-framework#289 and it allow us to define a list of ProbeFields as before but it also allows us to define HealthChecker. HealthChecker has the ability to go through all the resultFields and decide which resources are mandatory and should trigger unheathly probes and which resources are optional
pkg/agent/inteface.go
Outdated
| } | ||
|
|
||
| type AddonHealthCheckFunc func(workapiv1.ResourceIdentifier, workapiv1.StatusFeedbackResult) error | ||
| type AddonHealthCheckerFunc func([]ResultField) error |
There was a problem hiding this comment.
Is the managedclusteraddon or managedcluster also required in some cases?
I remember someone raised a similar demand during our offline discussion?
There was a problem hiding this comment.
@qiujian16 looks like it makes sense to input cluster and mca. WDYT?
| } | ||
|
|
||
| probeFields, healthChecker, err := s.analyzeWorkProber(s.agentAddon, cluster, addon) | ||
| probeFields, healthChecker, healthAllChecker, err := s.analyzeWorkProber(s.agentAddon, cluster, addon) |
There was a problem hiding this comment.
I don't think healthAllChecker is a proper name here. seems no meaning for All?
|
|
||
| var resultFields []agent.ResultField | ||
|
|
||
| for _, field := range probeFields { |
There was a problem hiding this comment.
can we separate the healthChecker(Deprecated one) and healthAllChecker logic into two different functions, so it is clear and easy to deprecate in the future?
There was a problem hiding this comment.
Add a TODO here, will follow up to refactor this part
|
|
||
| return &status.StatusFeedbacks | ||
| // compare two string, target may include * | ||
| func wildcardMatch(resource, target string) bool { |
There was a problem hiding this comment.
These seem to be duplicates of the ocm/work code?
There was a problem hiding this comment.
yes. only this two parts use it
zhiweiyin318
force-pushed
the
support-probefileds
branch
from
754aecc to
822a2cc
Compare
Signed-off-by: Zhiwei Yin <[email protected]>
zhiweiyin318
force-pushed
the
support-probefileds
branch
from
822a2cc to
8beaf27
Compare
|
/lgtm |
|
/unhold |
openshift-merge-bot
bot
merged commit 81a5903
into
open-cluster-management-io:main
3 participants
Summary
Related issue(s)
#288
#287
Fixes #