From 0cb85d897a58b4b6791e87c960ea47899b24ae84 Mon Sep 17 00:00:00 2001 From: Somefive Date: Mon, 11 Jul 2022 16:46:14 +0800 Subject: [PATCH] Fix: health check use original ns if no override and original exists (#4349) Signed-off-by: Somefive --- pkg/appfile/appfile.go | 16 ++++----- .../v1alpha2/application/apply.go | 25 +++++++------ .../core/scopes/healthscope/healthscope.go | 11 +++--- pkg/cue/definition/template.go | 30 ++++++++-------- pkg/oam/util/helper.go | 35 +++++++++++++++++++ 5 files changed, 77 insertions(+), 40 deletions(-) diff --git a/pkg/appfile/appfile.go b/pkg/appfile/appfile.go index f92685e59..b4157aebf 100644 --- a/pkg/appfile/appfile.go +++ b/pkg/appfile/appfile.go @@ -98,21 +98,21 @@ func (wl *Workload) EvalContext(ctx process.Context) error { } // EvalStatus eval workload status -func (wl *Workload) EvalStatus(ctx process.Context, cli client.Client, ns string) (string, error) { +func (wl *Workload) EvalStatus(ctx process.Context, cli client.Client, accessor util.NamespaceAccessor) (string, error) { // if the standard workload is managed by trait always return empty message if wl.SkipApplyWorkload { return "", nil } - return wl.engine.Status(ctx, cli, ns, wl.FullTemplate.CustomStatus, wl.Params) + return wl.engine.Status(ctx, cli, accessor, wl.FullTemplate.CustomStatus, wl.Params) } // EvalHealth eval workload health check -func (wl *Workload) EvalHealth(ctx process.Context, client client.Client, namespace string) (bool, error) { +func (wl *Workload) EvalHealth(ctx process.Context, client client.Client, accessor util.NamespaceAccessor) (bool, error) { // if health of template is not set or standard workload is managed by trait always return true if wl.FullTemplate.Health == "" || wl.SkipApplyWorkload { return true, nil } - return wl.engine.HealthCheck(ctx, client, namespace, wl.FullTemplate.Health) + return wl.engine.HealthCheck(ctx, client, accessor, wl.FullTemplate.Health) } // Scope defines the scope of workload @@ -146,16 +146,16 @@ func (trait *Trait) EvalContext(ctx process.Context) error { } // EvalStatus eval trait status -func (trait *Trait) EvalStatus(ctx process.Context, cli client.Client, ns string) (string, error) { - return trait.engine.Status(ctx, cli, ns, trait.CustomStatusFormat, trait.Params) +func (trait *Trait) EvalStatus(ctx process.Context, cli client.Client, accessor util.NamespaceAccessor) (string, error) { + return trait.engine.Status(ctx, cli, accessor, trait.CustomStatusFormat, trait.Params) } // EvalHealth eval trait health check -func (trait *Trait) EvalHealth(ctx process.Context, client client.Client, namespace string) (bool, error) { +func (trait *Trait) EvalHealth(ctx process.Context, client client.Client, accessor util.NamespaceAccessor) (bool, error) { if trait.FullTemplate.Health == "" { return true, nil } - return trait.engine.HealthCheck(ctx, client, namespace, trait.HealthCheckPolicy) + return trait.engine.HealthCheck(ctx, client, accessor, trait.HealthCheckPolicy) } // Appfile describes application diff --git a/pkg/controller/core.oam.dev/v1alpha2/application/apply.go b/pkg/controller/core.oam.dev/v1alpha2/application/apply.go index dcf1b70dc..bf7b72239 100644 --- a/pkg/controller/core.oam.dev/v1alpha2/application/apply.go +++ b/pkg/controller/core.oam.dev/v1alpha2/application/apply.go @@ -38,6 +38,7 @@ import ( "github.com/oam-dev/kubevela/pkg/monitor/metrics" "github.com/oam-dev/kubevela/pkg/multicluster" "github.com/oam-dev/kubevela/pkg/oam" + "github.com/oam-dev/kubevela/pkg/oam/util" "github.com/oam-dev/kubevela/pkg/resourcekeeper" ) @@ -215,17 +216,14 @@ func (h *AppHandler) ProduceArtifacts(ctx context.Context, comps []*types.Compon // nolint func (h *AppHandler) collectHealthStatus(ctx context.Context, wl *appfile.Workload, appRev *v1beta1.ApplicationRevision, overrideNamespace string) (*common.ApplicationComponentStatus, bool, error) { - namespace := h.app.Namespace - if overrideNamespace != "" { - namespace = overrideNamespace - } + accessor := util.NewApplicationResourceNamespaceAccessor(h.app.Namespace, overrideNamespace) var ( status = common.ApplicationComponentStatus{ Name: wl.Name, WorkloadDefinition: wl.FullTemplate.Reference.Definition, Healthy: true, - Namespace: namespace, + Namespace: accessor.Namespace(), Cluster: multicluster.ClusterNameInContext(ctx), } appName = appRev.Spec.Application.Name @@ -235,10 +233,10 @@ func (h *AppHandler) collectHealthStatus(ctx context.Context, wl *appfile.Worklo if wl.CapabilityCategory == types.TerraformCategory { var configuration terraforv1beta2.Configuration - if err := h.r.Client.Get(ctx, client.ObjectKey{Name: wl.Name, Namespace: namespace}, &configuration); err != nil { + if err := h.r.Client.Get(ctx, client.ObjectKey{Name: wl.Name, Namespace: accessor.Namespace()}, &configuration); err != nil { if kerrors.IsNotFound(err) { var legacyConfiguration terraforv1beta1.Configuration - if err := h.r.Client.Get(ctx, client.ObjectKey{Name: wl.Name, Namespace: namespace}, &legacyConfiguration); err != nil { + if err := h.r.Client.Get(ctx, client.ObjectKey{Name: wl.Name, Namespace: accessor.Namespace()}, &legacyConfiguration); err != nil { return nil, false, errors.WithMessagef(err, "app=%s, comp=%s, check health error", appName, wl.Name) } isHealth = setStatus(&status, legacyConfiguration.Status.ObservedGeneration, legacyConfiguration.Generation, @@ -251,12 +249,12 @@ func (h *AppHandler) collectHealthStatus(ctx context.Context, wl *appfile.Worklo appRev.Name, configuration.Status.Apply.State, configuration.Status.Apply.Message) } } else { - if ok, err := wl.EvalHealth(wl.Ctx, h.r.Client, namespace); !ok || err != nil { + if ok, err := wl.EvalHealth(wl.Ctx, h.r.Client, accessor); !ok || err != nil { isHealth = false status.Healthy = false } - status.Message, err = wl.EvalStatus(wl.Ctx, h.r.Client, namespace) + status.Message, err = wl.EvalStatus(wl.Ctx, h.r.Client, accessor) if err != nil { return nil, false, errors.WithMessagef(err, "app=%s, comp=%s, evaluate workload status message error", appName, wl.Name) } @@ -264,19 +262,21 @@ func (h *AppHandler) collectHealthStatus(ctx context.Context, wl *appfile.Worklo var traitStatusList []common.ApplicationTraitStatus for _, tr := range wl.Traits { + traitOverrideNamespace := overrideNamespace if tr.FullTemplate.TraitDefinition.Spec.ControlPlaneOnly { - namespace = appRev.GetNamespace() + traitOverrideNamespace = appRev.GetNamespace() wl.Ctx.SetCtx(context.WithValue(wl.Ctx.GetCtx(), multicluster.ClusterContextKey, multicluster.ClusterLocalName)) } + _accessor := util.NewApplicationResourceNamespaceAccessor(h.app.Namespace, traitOverrideNamespace) var traitStatus = common.ApplicationTraitStatus{ Type: tr.Name, Healthy: true, } - if ok, err := tr.EvalHealth(wl.Ctx, h.r.Client, namespace); !ok || err != nil { + if ok, err := tr.EvalHealth(wl.Ctx, h.r.Client, _accessor); !ok || err != nil { isHealth = false traitStatus.Healthy = false } - traitStatus.Message, err = tr.EvalStatus(wl.Ctx, h.r.Client, namespace) + traitStatus.Message, err = tr.EvalStatus(wl.Ctx, h.r.Client, _accessor) if err != nil { return nil, false, errors.WithMessagef(err, "app=%s, comp=%s, trait=%s, evaluate status message error", appName, wl.Name, tr.Name) } @@ -284,7 +284,6 @@ func (h *AppHandler) collectHealthStatus(ctx context.Context, wl *appfile.Worklo status.Message = traitStatus.Message } traitStatusList = append(traitStatusList, traitStatus) - namespace = appRev.GetNamespace() wl.Ctx.SetCtx(context.WithValue(wl.Ctx.GetCtx(), multicluster.ClusterContextKey, status.Cluster)) } diff --git a/pkg/controller/core.oam.dev/v1alpha2/core/scopes/healthscope/healthscope.go b/pkg/controller/core.oam.dev/v1alpha2/core/scopes/healthscope/healthscope.go index ae433d793..cce052405 100644 --- a/pkg/controller/core.oam.dev/v1alpha2/core/scopes/healthscope/healthscope.go +++ b/pkg/controller/core.oam.dev/v1alpha2/core/scopes/healthscope/healthscope.go @@ -44,6 +44,7 @@ import ( af "github.com/oam-dev/kubevela/pkg/appfile" "github.com/oam-dev/kubevela/pkg/cue/process" "github.com/oam-dev/kubevela/pkg/oam" + "github.com/oam-dev/kubevela/pkg/oam/util" ) const ( @@ -478,7 +479,8 @@ func CUEBasedHealthCheck(ctx context.Context, c client.Client, wlRef WorkloadRef okToCheckTrait = true return } - isHealthy, err := wl.EvalHealth(pCtx, c, ns) + accessor := util.NewApplicationResourceNamespaceAccessor(ns, "") + isHealthy, err := wl.EvalHealth(pCtx, c, accessor) if err != nil { wlHealth.HealthStatus = StatusUnhealthy wlHealth.Diagnosis = errors.Wrap(err, errHealthCheck).Error() @@ -490,7 +492,7 @@ func CUEBasedHealthCheck(ctx context.Context, c client.Client, wlRef WorkloadRef // TODO(wonderflow): we should add a custom way to let the template say why it's unhealthy, only a bool flag is not enough wlHealth.HealthStatus = StatusUnhealthy } - wlHealth.CustomStatusMsg, err = wl.EvalStatus(pCtx, c, ns) + wlHealth.CustomStatusMsg, err = wl.EvalStatus(pCtx, c, accessor) if err != nil { wlHealth.Diagnosis = errors.Wrap(err, errHealthCheck).Error() } @@ -522,7 +524,8 @@ func CUEBasedHealthCheck(ctx context.Context, c client.Client, wlRef WorkloadRef traits[i] = tHealth continue } - isHealthy, err := tr.EvalHealth(pCtx, c, ns) + accessor := util.NewApplicationResourceNamespaceAccessor("", ns) + isHealthy, err := tr.EvalHealth(pCtx, c, accessor) if err != nil { tHealth.HealthStatus = StatusUnhealthy tHealth.Diagnosis = errors.Wrap(err, errHealthCheck).Error() @@ -535,7 +538,7 @@ func CUEBasedHealthCheck(ctx context.Context, c client.Client, wlRef WorkloadRef // TODO(wonderflow): we should add a custom way to let the template say why it's unhealthy, only a bool flag is not enough tHealth.HealthStatus = StatusUnhealthy } - tHealth.CustomStatusMsg, err = tr.EvalStatus(pCtx, c, ns) + tHealth.CustomStatusMsg, err = tr.EvalStatus(pCtx, c, accessor) if err != nil { tHealth.Diagnosis = errors.Wrap(err, errHealthCheck).Error() } diff --git a/pkg/cue/definition/template.go b/pkg/cue/definition/template.go index 4afbb6624..7cc19173f 100644 --- a/pkg/cue/definition/template.go +++ b/pkg/cue/definition/template.go @@ -62,8 +62,8 @@ const ( // AbstractEngine defines Definition's Render interface type AbstractEngine interface { Complete(ctx process.Context, abstractTemplate string, params interface{}) error - HealthCheck(ctx process.Context, cli client.Client, ns string, healthPolicyTemplate string) (bool, error) - Status(ctx process.Context, cli client.Client, ns string, customStatusTemplate string, parameter interface{}) (string, error) + HealthCheck(ctx process.Context, cli client.Client, accessor util.NamespaceAccessor, healthPolicyTemplate string) (bool, error) + Status(ctx process.Context, cli client.Client, accessor util.NamespaceAccessor, customStatusTemplate string, parameter interface{}) (string, error) } type def struct { @@ -155,7 +155,7 @@ func (wd *workloadDef) Complete(ctx process.Context, abstractTemplate string, pa return nil } -func (wd *workloadDef) getTemplateContext(ctx process.Context, cli client.Reader, ns string) (map[string]interface{}, error) { +func (wd *workloadDef) getTemplateContext(ctx process.Context, cli client.Reader, accessor util.NamespaceAccessor) (map[string]interface{}, error) { var root = initRoot(ctx.BaseContextLabels()) var commonLabels = GetCommonLabels(ctx.BaseContextLabels()) @@ -166,7 +166,7 @@ func (wd *workloadDef) getTemplateContext(ctx process.Context, cli client.Reader return nil, err } // workload main resource will have a unique label("app.oam.dev/resourceType"="WORKLOAD") in per component/app level - object, err := getResourceFromObj(ctx.GetCtx(), componentWorkload, cli, ns, util.MergeMapOverrideWithDst(map[string]string{ + object, err := getResourceFromObj(ctx.GetCtx(), componentWorkload, cli, accessor.For(componentWorkload), util.MergeMapOverrideWithDst(map[string]string{ oam.LabelOAMResourceType: oam.ResourceTypeWorkload, }, commonLabels), "") if err != nil { @@ -186,7 +186,7 @@ func (wd *workloadDef) getTemplateContext(ctx process.Context, cli client.Reader return nil, err } // AuxiliaryWorkload will have a unique label("trait.oam.dev/resource"="name of outputs") in per component/app level - object, err := getResourceFromObj(ctx.GetCtx(), traitRef, cli, ns, util.MergeMapOverrideWithDst(map[string]string{ + object, err := getResourceFromObj(ctx.GetCtx(), traitRef, cli, accessor.For(componentWorkload), util.MergeMapOverrideWithDst(map[string]string{ oam.TraitTypeLabel: AuxiliaryWorkload, }, commonLabels), assist.Name) if err != nil { @@ -201,11 +201,11 @@ func (wd *workloadDef) getTemplateContext(ctx process.Context, cli client.Reader } // HealthCheck address health check for workload -func (wd *workloadDef) HealthCheck(ctx process.Context, cli client.Client, ns string, healthPolicyTemplate string) (bool, error) { +func (wd *workloadDef) HealthCheck(ctx process.Context, cli client.Client, accessor util.NamespaceAccessor, healthPolicyTemplate string) (bool, error) { if healthPolicyTemplate == "" { return true, nil } - templateContext, err := wd.getTemplateContext(ctx, cli, ns) + templateContext, err := wd.getTemplateContext(ctx, cli, accessor) if err != nil { return false, errors.WithMessage(err, "get template context") } @@ -232,11 +232,11 @@ func checkHealth(templateContext map[string]interface{}, healthPolicyTemplate st } // Status get workload status by customStatusTemplate -func (wd *workloadDef) Status(ctx process.Context, cli client.Client, ns string, customStatusTemplate string, parameter interface{}) (string, error) { +func (wd *workloadDef) Status(ctx process.Context, cli client.Client, accessor util.NamespaceAccessor, customStatusTemplate string, parameter interface{}) (string, error) { if customStatusTemplate == "" { return "", nil } - templateContext, err := wd.getTemplateContext(ctx, cli, ns) + templateContext, err := wd.getTemplateContext(ctx, cli, accessor) if err != nil { return "", errors.WithMessage(err, "get template context") } @@ -425,7 +425,7 @@ func initRoot(contextLabels map[string]string) map[string]interface{} { return root } -func (td *traitDef) getTemplateContext(ctx process.Context, cli client.Reader, ns string) (map[string]interface{}, error) { +func (td *traitDef) getTemplateContext(ctx process.Context, cli client.Reader, accessor util.NamespaceAccessor) (map[string]interface{}, error) { var root = initRoot(ctx.BaseContextLabels()) var commonLabels = GetCommonLabels(ctx.BaseContextLabels()) @@ -439,7 +439,7 @@ func (td *traitDef) getTemplateContext(ctx process.Context, cli client.Reader, n if err != nil { return nil, err } - object, err := getResourceFromObj(ctx.GetCtx(), traitRef, cli, ns, util.MergeMapOverrideWithDst(map[string]string{ + object, err := getResourceFromObj(ctx.GetCtx(), traitRef, cli, accessor.For(traitRef), util.MergeMapOverrideWithDst(map[string]string{ oam.TraitTypeLabel: assist.Type, }, commonLabels), assist.Name) if err != nil { @@ -454,11 +454,11 @@ func (td *traitDef) getTemplateContext(ctx process.Context, cli client.Reader, n } // Status get trait status by customStatusTemplate -func (td *traitDef) Status(ctx process.Context, cli client.Client, ns string, customStatusTemplate string, parameter interface{}) (string, error) { +func (td *traitDef) Status(ctx process.Context, cli client.Client, accessor util.NamespaceAccessor, customStatusTemplate string, parameter interface{}) (string, error) { if customStatusTemplate == "" { return "", nil } - templateContext, err := td.getTemplateContext(ctx, cli, ns) + templateContext, err := td.getTemplateContext(ctx, cli, accessor) if err != nil { return "", errors.WithMessage(err, "get template context") } @@ -466,11 +466,11 @@ func (td *traitDef) Status(ctx process.Context, cli client.Client, ns string, cu } // HealthCheck address health check for trait -func (td *traitDef) HealthCheck(ctx process.Context, cli client.Client, ns string, healthPolicyTemplate string) (bool, error) { +func (td *traitDef) HealthCheck(ctx process.Context, cli client.Client, accessor util.NamespaceAccessor, healthPolicyTemplate string) (bool, error) { if healthPolicyTemplate == "" { return true, nil } - templateContext, err := td.getTemplateContext(ctx, cli, ns) + templateContext, err := td.getTemplateContext(ctx, cli, accessor) if err != nil { return false, errors.WithMessage(err, "get template context") } diff --git a/pkg/oam/util/helper.go b/pkg/oam/util/helper.go index 0a05b176b..71521f4a8 100644 --- a/pkg/oam/util/helper.go +++ b/pkg/oam/util/helper.go @@ -953,3 +953,38 @@ func AsController(r *corev1.ObjectReference) metav1.OwnerReference { ref.Controller = &c return ref } + +// NamespaceAccessor namespace accessor for resource +type NamespaceAccessor interface { + For(obj client.Object) string + Namespace() string +} + +type applicationResourceNamespaceAccessor struct { + applicationNamespace string + overrideNamespace string +} + +// For access namespace for resource +func (accessor *applicationResourceNamespaceAccessor) For(obj client.Object) string { + if accessor.overrideNamespace != "" { + return accessor.overrideNamespace + } + if originalNamespace := obj.GetNamespace(); originalNamespace != "" { + return originalNamespace + } + return accessor.applicationNamespace +} + +// Namespace the namespace by default +func (accessor *applicationResourceNamespaceAccessor) Namespace() string { + if accessor.overrideNamespace != "" { + return accessor.overrideNamespace + } + return accessor.applicationNamespace +} + +// NewApplicationResourceNamespaceAccessor create namespace accessor for resource in application +func NewApplicationResourceNamespaceAccessor(appNs, overrideNs string) NamespaceAccessor { + return &applicationResourceNamespaceAccessor{applicationNamespace: appNs, overrideNamespace: overrideNs} +}