From 78b84f2c4799c93ba005a72e10225c7f07b6d41a Mon Sep 17 00:00:00 2001 From: Yue Wang Date: Wed, 9 Sep 2020 16:07:23 +0900 Subject: [PATCH] enhance health scope with informative health condition (#194) * use merging instead of overriding add unit test Signed-off-by: roy wang * move AddLabel func into oam/helper Signed-off-by: roy wang * Make health condition of HealthScope more informative Add CheckByHealthCheckTrait interface Signed-off-by: roy wang * fix e2e-test Signed-off-by: roy wang * modify health scope condition modify e2e-test & unit test Signed-off-by: roy wang * fix typo Signed-off-by: roy wang * add unit test for health scope Signed-off-by: roy wang --- apis/core/v1alpha2/core_scope_types.go | 38 +++- apis/core/v1alpha2/zz_generated.deepcopy.go | 43 ++++ .../crds/core.oam.dev_healthscopes.yaml | 72 +++++- .../core/scopes/healthscope/healthscope.go | 207 ++++++++++++------ .../healthscope/healthscope_controller.go | 131 ++++++----- .../healthscope_controller_test.go | 76 ++++--- .../scopes/healthscope/healthscope_test.go | 157 +++++++++---- test/e2e-test/appconfig_finalizer_test.go | 8 +- test/e2e-test/health_scope_test.go | 23 +- 9 files changed, 545 insertions(+), 210 deletions(-) diff --git a/apis/core/v1alpha2/core_scope_types.go b/apis/core/v1alpha2/core_scope_types.go index cd1ba76b..6c312e8f 100644 --- a/apis/core/v1alpha2/core_scope_types.go +++ b/apis/core/v1alpha2/core_scope_types.go @@ -23,6 +23,18 @@ import ( "github.com/crossplane/oam-kubernetes-runtime/pkg/oam" ) +// HealthStatus represents health status strings. +type HealthStatus string + +const ( + // StatusHealthy represents healthy status. + StatusHealthy HealthStatus = "HEALTHY" + // StatusUnhealthy represents unhealthy status. + StatusUnhealthy = "UNHEALTHY" + // StatusUnknown represents unknown status. + StatusUnknown = "UNKNOWN" +) + var _ oam.Scope = &HealthScope{} // A HealthScopeSpec defines the desired state of a HealthScope. @@ -41,7 +53,31 @@ type HealthScopeSpec struct { type HealthScopeStatus struct { runtimev1alpha1.ConditionedStatus `json:",inline"` - Health string `json:"health"` + // ScopeHealthCondition represents health condition summary of the scope + ScopeHealthCondition ScopeHealthCondition `json:"scopeHealthCondition"` + + // WorkloadHealthConditions represents health condition of workloads in the scope + WorkloadHealthConditions []*WorkloadHealthCondition `json:"healthConditions,omitempty"` +} + +// ScopeHealthCondition represents health condition summary of a scope. +type ScopeHealthCondition struct { + HealthStatus HealthStatus `json:"healthStatus"` + Total int64 `json:"total,omitempty"` + HealthyWorkloads int64 `json:"healthyWorkloads,omitempty"` + UnhealthyWorkloads int64 `json:"unhealthyWorkloads,omitempty"` + UnknownWorkloads int64 `json:"unknownWorkloads,omitempty"` +} + +// WorkloadHealthCondition represents informative health condition. +type WorkloadHealthCondition struct { + // ComponentName represents the component name if target is a workload + ComponentName string `json:"componentName,omitempty"` + TargetWorkload runtimev1alpha1.TypedReference `json:"targetWorkload,omitempty"` + HealthStatus HealthStatus `json:"healthStatus"` + Diagnosis string `json:"diagnosis,omitempty"` + // WorkloadStatus represents status of workloads whose HealthStatus is UNKNOWN. + WorkloadStatus string `json:"workloadStatus,omitempty"` } // +kubebuilder:object:root=true diff --git a/apis/core/v1alpha2/zz_generated.deepcopy.go b/apis/core/v1alpha2/zz_generated.deepcopy.go index b6b21ab2..04be7983 100644 --- a/apis/core/v1alpha2/zz_generated.deepcopy.go +++ b/apis/core/v1alpha2/zz_generated.deepcopy.go @@ -1092,6 +1092,18 @@ func (in *HealthScopeSpec) DeepCopy() *HealthScopeSpec { func (in *HealthScopeStatus) DeepCopyInto(out *HealthScopeStatus) { *out = *in in.ConditionedStatus.DeepCopyInto(&out.ConditionedStatus) + out.ScopeHealthCondition = in.ScopeHealthCondition + if in.WorkloadHealthConditions != nil { + in, out := &in.WorkloadHealthConditions, &out.WorkloadHealthConditions + *out = make([]*WorkloadHealthCondition, len(*in)) + for i := range *in { + if (*in)[i] != nil { + in, out := &(*in)[i], &(*out)[i] + *out = new(WorkloadHealthCondition) + **out = **in + } + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new HealthScopeStatus. @@ -1305,6 +1317,21 @@ func (in *ScopeDefinitionSpec) DeepCopy() *ScopeDefinitionSpec { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ScopeHealthCondition) DeepCopyInto(out *ScopeHealthCondition) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ScopeHealthCondition. +func (in *ScopeHealthCondition) DeepCopy() *ScopeHealthCondition { + if in == nil { + return nil + } + out := new(ScopeHealthCondition) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *SecretKeySelector) DeepCopyInto(out *SecretKeySelector) { *out = *in @@ -1552,6 +1579,22 @@ func (in *WorkloadDefinitionSpec) DeepCopy() *WorkloadDefinitionSpec { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *WorkloadHealthCondition) DeepCopyInto(out *WorkloadHealthCondition) { + *out = *in + out.TargetWorkload = in.TargetWorkload +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new WorkloadHealthCondition. +func (in *WorkloadHealthCondition) DeepCopy() *WorkloadHealthCondition { + if in == nil { + return nil + } + out := new(WorkloadHealthCondition) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *WorkloadScope) DeepCopyInto(out *WorkloadScope) { *out = *in diff --git a/charts/oam-kubernetes-runtime/crds/core.oam.dev_healthscopes.yaml b/charts/oam-kubernetes-runtime/crds/core.oam.dev_healthscopes.yaml index e2a0b71b..5310a5a9 100644 --- a/charts/oam-kubernetes-runtime/crds/core.oam.dev_healthscopes.yaml +++ b/charts/oam-kubernetes-runtime/crds/core.oam.dev_healthscopes.yaml @@ -119,10 +119,76 @@ spec: - type type: object type: array - health: - type: string + healthConditions: + description: WorkloadHealthConditions represents health condition + of workloads in the scope + items: + description: WorkloadHealthCondition represents informative health + condition. + properties: + componentName: + description: ComponentName represents the component name if + target is a workload + type: string + diagnosis: + type: string + healthStatus: + description: HealthStatus represents health status strings. + type: string + targetWorkload: + description: A TypedReference refers to an object by Name, Kind, + and APIVersion. It is commonly used to reference cluster-scoped + objects or objects where the namespace is already known. + properties: + apiVersion: + description: APIVersion of the referenced object. + type: string + kind: + description: Kind of the referenced object. + type: string + name: + description: Name of the referenced object. + type: string + uid: + description: UID of the referenced object. + type: string + required: + - apiVersion + - kind + - name + type: object + workloadStatus: + description: WorkloadStatus represents status of workloads whose + HealthStatus is UNKNOWN. + type: string + required: + - healthStatus + type: object + type: array + scopeHealthCondition: + description: ScopeHealthCondition represents health condition summary + of the scope + properties: + healthStatus: + description: HealthStatus represents health status strings. + type: string + healthyWorkloads: + format: int64 + type: integer + total: + format: int64 + type: integer + unhealthyWorkloads: + format: int64 + type: integer + unknownWorkloads: + format: int64 + type: integer + required: + - healthStatus + type: object required: - - health + - scopeHealthCondition type: object type: object served: true diff --git a/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope.go b/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope.go index 4b5aec4a..b7ecb2fb 100644 --- a/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope.go +++ b/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope.go @@ -18,31 +18,47 @@ package healthscope import ( "context" + "encoding/json" "fmt" "reflect" "time" apps "k8s.io/api/apps/v1" core "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" + "github.com/crossplane/oam-kubernetes-runtime/apis/core/v1alpha2" corev1alpha2 "github.com/crossplane/oam-kubernetes-runtime/apis/core/v1alpha2" + "github.com/crossplane/oam-kubernetes-runtime/pkg/oam" runtimev1alpha1 "github.com/crossplane/crossplane-runtime/apis/core/v1alpha1" "github.com/pkg/errors" ) const ( - errFmtUnsupportWorkload = "APIVersion %v Kind %v workload is not supportted by HealthScope" - errHealthCheck = "error occurs in health check" - errUnhealthyChildResource = "unhealthy child resource exists" - errFmtResourceNotReady = "resource not ready, resource status: %+v" + infoFmtUnknownWorkload = "APIVersion %v Kind %v workload is unknown for HealthScope " + infoFmtReady = "Ready: %d/%d " + infoFmtNoChildRes = "cannot get child resource references of ContainerizedWorkload %v" + errHealthCheck = "error occurs in health check " defaultTimeout = 10 * time.Second ) +// HealthStatus represents health status strings. +type HealthStatus = v1alpha2.HealthStatus + +const ( + // StatusHealthy represents healthy status. + StatusHealthy = v1alpha2.StatusHealthy + // StatusUnhealthy represents unhealthy status. + StatusUnhealthy = v1alpha2.StatusUnhealthy + // StatusUnknown represents unknown status. + StatusUnknown = v1alpha2.StatusUnknown +) + var ( kindContainerizedWorkload = corev1alpha2.ContainerizedWorkloadKind kindDeployment = reflect.TypeOf(apps.Deployment{}).Name() @@ -51,98 +67,95 @@ var ( kindDaemonSet = reflect.TypeOf(apps.DaemonSet{}).Name() ) -// HealthCondition holds health status of any resource -type HealthCondition struct { - // Target represents resource being diagnosed - Target runtimev1alpha1.TypedReference `json:"target"` +// WorkloadHealthCondition holds health status of any resource +type WorkloadHealthCondition = v1alpha2.WorkloadHealthCondition - IsHealthy bool `json:"isHealthy"` - - // Diagnosis contains diagnosis info as well as error info - Diagnosis string `json:"diagnosis,omitempty"` - - // SubConditions represents health status of its child resources, if exist - SubConditions []*HealthCondition `json:"subConditions,omitempty"` -} +// ScopeHealthCondition holds health condition of a scope +type ScopeHealthCondition = v1alpha2.ScopeHealthCondition // A WorloadHealthChecker checks health status of specified resource // and saves status into an HealthCondition object. type WorloadHealthChecker interface { - Check(context.Context, client.Client, runtimev1alpha1.TypedReference, string) *HealthCondition + Check(context.Context, client.Client, runtimev1alpha1.TypedReference, string) *WorkloadHealthCondition } // WorkloadHealthCheckFn checks health status of specified resource // and saves status into an HealthCondition object. -type WorkloadHealthCheckFn func(context.Context, client.Client, runtimev1alpha1.TypedReference, string) *HealthCondition +type WorkloadHealthCheckFn func(context.Context, client.Client, runtimev1alpha1.TypedReference, string) *WorkloadHealthCondition // Check the health status of specified resource -func (fn WorkloadHealthCheckFn) Check(ctx context.Context, c client.Client, tr runtimev1alpha1.TypedReference, ns string) *HealthCondition { +func (fn WorkloadHealthCheckFn) Check(ctx context.Context, c client.Client, tr runtimev1alpha1.TypedReference, ns string) *WorkloadHealthCondition { return fn(ctx, c, tr, ns) } -// CheckContainerziedWorkloadHealth check health status of ContainerizedWorkload -func CheckContainerziedWorkloadHealth(ctx context.Context, c client.Client, ref runtimev1alpha1.TypedReference, namespace string) *HealthCondition { +// CheckContainerziedWorkloadHealth check health condition of ContainerizedWorkload +func CheckContainerziedWorkloadHealth(ctx context.Context, c client.Client, ref runtimev1alpha1.TypedReference, namespace string) *WorkloadHealthCondition { if ref.GroupVersionKind() != corev1alpha2.SchemeGroupVersion.WithKind(kindContainerizedWorkload) { return nil } - r := &HealthCondition{ - IsHealthy: false, - Target: ref, + r := &WorkloadHealthCondition{ + HealthStatus: StatusHealthy, + TargetWorkload: ref, } cwObj := corev1alpha2.ContainerizedWorkload{} cwObj.SetGroupVersionKind(corev1alpha2.SchemeGroupVersion.WithKind(kindContainerizedWorkload)) if err := c.Get(ctx, types.NamespacedName{Namespace: namespace, Name: ref.Name}, &cwObj); err != nil { + r.HealthStatus = StatusUnhealthy r.Diagnosis = errors.Wrap(err, errHealthCheck).Error() return r } - r.Target.UID = cwObj.GetUID() - r.SubConditions = []*HealthCondition{} + r.ComponentName = getComponentNameFromLabel(&cwObj) + r.TargetWorkload.UID = cwObj.GetUID() + + subConditions := []*WorkloadHealthCondition{} childRefs := cwObj.Status.Resources + if len(childRefs) != 2 { + // one deployment and one svc are required by containerizedworkload + r.Diagnosis = fmt.Sprintf(infoFmtNoChildRes, ref.Name) + r.HealthStatus = StatusUnhealthy + return r + } for _, childRef := range childRefs { switch childRef.Kind { case kindDeployment: // reuse Deployment health checker childCondition := CheckDeploymentHealth(ctx, c, childRef, namespace) - r.SubConditions = append(r.SubConditions, childCondition) - default: - childCondition := &HealthCondition{ - Target: childRef, - IsHealthy: true, + subConditions = append(subConditions, childCondition) + case kindService: + childCondition := &WorkloadHealthCondition{ + TargetWorkload: childRef, + HealthStatus: StatusHealthy, } o := unstructured.Unstructured{} o.SetAPIVersion(childRef.APIVersion) o.SetKind(childRef.Kind) if err := c.Get(ctx, types.NamespacedName{Namespace: namespace, Name: childRef.Name}, &o); err != nil { - // for unspecified resource - // if cannot get it, then check fails - childCondition.IsHealthy = false + childCondition.HealthStatus = StatusUnhealthy childCondition.Diagnosis = errors.Wrap(err, errHealthCheck).Error() } - r.SubConditions = append(r.SubConditions, childCondition) + subConditions = append(subConditions, childCondition) } } - r.IsHealthy = true - for _, sc := range r.SubConditions { - if !sc.IsHealthy { - r.IsHealthy = false - r.Diagnosis = errUnhealthyChildResource - break + for _, sc := range subConditions { + if sc.HealthStatus != StatusHealthy { + r.HealthStatus = StatusUnhealthy } + r.Diagnosis = fmt.Sprintf("%s%s", r.Diagnosis, sc.Diagnosis) } return r } -// CheckDeploymentHealth checks health status of Deployment -func CheckDeploymentHealth(ctx context.Context, client client.Client, ref runtimev1alpha1.TypedReference, namespace string) *HealthCondition { +// CheckDeploymentHealth checks health condition of Deployment +func CheckDeploymentHealth(ctx context.Context, client client.Client, ref runtimev1alpha1.TypedReference, namespace string) *WorkloadHealthCondition { if ref.GroupVersionKind() != apps.SchemeGroupVersion.WithKind(kindDeployment) { return nil } - r := &HealthCondition{ - IsHealthy: false, - Target: ref, + r := &WorkloadHealthCondition{ + HealthStatus: StatusUnhealthy, + TargetWorkload: ref, } deployment := apps.Deployment{} deployment.SetGroupVersionKind(apps.SchemeGroupVersion.WithKind(kindDeployment)) @@ -151,24 +164,32 @@ func CheckDeploymentHealth(ctx context.Context, client client.Client, ref runtim r.Diagnosis = errors.Wrap(err, errHealthCheck).Error() return r } - r.Target.UID = deployment.GetUID() + r.ComponentName = getComponentNameFromLabel(&deployment) + r.TargetWorkload.UID = deployment.GetUID() - if deployment.Status.ReadyReplicas == 0 { - r.Diagnosis = fmt.Sprintf(errFmtResourceNotReady, deployment.Status) + requiredReplicas := int32(0) + if deployment.Spec.Replicas != nil { + requiredReplicas = *deployment.Spec.Replicas + } + + r.Diagnosis = fmt.Sprintf(infoFmtReady, deployment.Status.ReadyReplicas, requiredReplicas) + + // Health criteria + if deployment.Status.ReadyReplicas != requiredReplicas { return r } - r.IsHealthy = true + r.HealthStatus = StatusHealthy return r } -// CheckStatefulsetHealth checks health status of StatefulSet -func CheckStatefulsetHealth(ctx context.Context, client client.Client, ref runtimev1alpha1.TypedReference, namespace string) *HealthCondition { +// CheckStatefulsetHealth checks health condition of StatefulSet +func CheckStatefulsetHealth(ctx context.Context, client client.Client, ref runtimev1alpha1.TypedReference, namespace string) *WorkloadHealthCondition { if ref.GroupVersionKind() != apps.SchemeGroupVersion.WithKind(kindStatefulSet) { return nil } - r := &HealthCondition{ - IsHealthy: false, - Target: ref, + r := &WorkloadHealthCondition{ + HealthStatus: StatusUnhealthy, + TargetWorkload: ref, } statefulset := apps.StatefulSet{} statefulset.APIVersion = ref.APIVersion @@ -178,24 +199,30 @@ func CheckStatefulsetHealth(ctx context.Context, client client.Client, ref runti r.Diagnosis = errors.Wrap(err, errHealthCheck).Error() return r } - r.Target.UID = statefulset.GetUID() + r.ComponentName = getComponentNameFromLabel(&statefulset) + r.TargetWorkload.UID = statefulset.GetUID() + requiredReplicas := int32(0) + if statefulset.Spec.Replicas != nil { + requiredReplicas = *statefulset.Spec.Replicas + } + r.Diagnosis = fmt.Sprintf(infoFmtReady, statefulset.Status.ReadyReplicas, requiredReplicas) - if statefulset.Status.ReadyReplicas == 0 { - r.Diagnosis = fmt.Sprintf(errFmtResourceNotReady, statefulset.Status) + // Health criteria + if statefulset.Status.ReadyReplicas != requiredReplicas { return r } - r.IsHealthy = true + r.HealthStatus = StatusHealthy return r } -// CheckDaemonsetHealth checks health status of DaemonSet -func CheckDaemonsetHealth(ctx context.Context, client client.Client, ref runtimev1alpha1.TypedReference, namespace string) *HealthCondition { +// CheckDaemonsetHealth checks health condition of DaemonSet +func CheckDaemonsetHealth(ctx context.Context, client client.Client, ref runtimev1alpha1.TypedReference, namespace string) *WorkloadHealthCondition { if ref.GroupVersionKind() != apps.SchemeGroupVersion.WithKind(kindDaemonSet) { return nil } - r := &HealthCondition{ - IsHealthy: false, - Target: ref, + r := &WorkloadHealthCondition{ + HealthStatus: StatusUnhealthy, + TargetWorkload: ref, } daemonset := apps.DaemonSet{} daemonset.APIVersion = ref.APIVersion @@ -205,12 +232,58 @@ func CheckDaemonsetHealth(ctx context.Context, client client.Client, ref runtime r.Diagnosis = errors.Wrap(err, errHealthCheck).Error() return r } - r.Target.UID = daemonset.GetUID() + r.ComponentName = getComponentNameFromLabel(&daemonset) + r.TargetWorkload.UID = daemonset.GetUID() + r.Diagnosis = fmt.Sprintf(infoFmtReady, daemonset.Status.NumberReady, daemonset.Status.DesiredNumberScheduled) + // Health criteria if daemonset.Status.NumberUnavailable != 0 { - r.Diagnosis = fmt.Sprintf(errFmtResourceNotReady, daemonset.Status) return r } - r.IsHealthy = true + r.HealthStatus = StatusHealthy return r } + +// CheckByHealthCheckTrait checks health condition through HealthCheckTrait. +func CheckByHealthCheckTrait(ctx context.Context, c client.Client, wlRef runtimev1alpha1.TypedReference, ns string) *WorkloadHealthCondition { + //TODO(roywang) implement HealthCheckTrait feature + return nil +} + +// CheckUnknownWorkload handles unknown type workloads. +func CheckUnknownWorkload(ctx context.Context, c client.Client, wlRef runtimev1alpha1.TypedReference, ns string) *WorkloadHealthCondition { + healthCondition := &WorkloadHealthCondition{ + TargetWorkload: wlRef, + HealthStatus: StatusUnknown, + Diagnosis: fmt.Sprintf(infoFmtUnknownWorkload, wlRef.APIVersion, wlRef.Kind), + } + + wl := &unstructured.Unstructured{} + wl.SetGroupVersionKind(wlRef.GroupVersionKind()) + if err := c.Get(ctx, client.ObjectKey{Namespace: ns, Name: wlRef.Name}, wl); err != nil { + healthCondition.Diagnosis = errors.Wrap(err, errHealthCheck).Error() + return healthCondition + } + healthCondition.ComponentName = getComponentNameFromLabel(wl) + + // for unknown workloads, just show status instead of precise diagnosis + wlStatus, _, _ := unstructured.NestedMap(wl.UnstructuredContent(), "status") + wlStatusR, err := json.Marshal(wlStatus) + if err != nil { + healthCondition.Diagnosis = errors.Wrap(err, errHealthCheck).Error() + return healthCondition + } + healthCondition.WorkloadStatus = string(wlStatusR) + return healthCondition +} + +func getComponentNameFromLabel(o metav1.Object) string { + if o == nil { + return "" + } + compName, exist := o.GetLabels()[oam.LabelAppComponent] + if !exist { + compName = "" + } + return compName +} diff --git a/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope_controller.go b/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope_controller.go index 2175e2a7..fa666937 100644 --- a/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope_controller.go +++ b/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope_controller.go @@ -18,8 +18,6 @@ package healthscope import ( "context" - "encoding/json" - "fmt" "strings" "sync" "time" @@ -29,7 +27,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/reconcile" - "github.com/crossplane/crossplane-runtime/apis/core/v1alpha1" runtimev1alpha1 "github.com/crossplane/crossplane-runtime/apis/core/v1alpha1" "github.com/crossplane/crossplane-runtime/pkg/event" "github.com/crossplane/crossplane-runtime/pkg/logging" @@ -48,7 +45,6 @@ const ( // Reconcile error strings. const ( errGetHealthScope = "cannot get health scope" - errMarshalScopeDiagnosiis = "cannot marshal diagnosis of the scope" errUpdateHealthScopeStatus = "cannot update health scope status" ) @@ -74,9 +70,15 @@ func Setup(mgr ctrl.Manager, args controller.Args, l logging.Logger) error { type Reconciler struct { client client.Client - log logging.Logger - record event.Recorder + log logging.Logger + record event.Recorder + // traitChecker represents checker fetching health condition from HealthCheckTrait + traitChecker WorloadHealthChecker + // checkers represents a set of built-in checkers checkers []WorloadHealthChecker + // unknownChecker represents checker handling workloads that + // cannot be hanlded by traitChecker nor built-in checkers + unknownChecker WorloadHealthChecker } // A ReconcilerOption configures a Reconciler. @@ -96,6 +98,13 @@ func WithRecorder(er event.Recorder) ReconcilerOption { } } +// WithTraitChecker adds health checker based on HealthCheckTrait +func WithTraitChecker(c WorloadHealthChecker) ReconcilerOption { + return func(r *Reconciler) { + r.traitChecker = c + } +} + // WithChecker adds workload health checker func WithChecker(c WorloadHealthChecker) ReconcilerOption { return func(r *Reconciler) { @@ -109,15 +118,17 @@ func WithChecker(c WorloadHealthChecker) ReconcilerOption { // NewReconciler returns a Reconciler that reconciles HealthScope by keeping track of its healthstatus. func NewReconciler(m ctrl.Manager, o ...ReconcilerOption) *Reconciler { r := &Reconciler{ - client: m.GetClient(), - log: logging.NewNopLogger(), - record: event.NewNopRecorder(), + client: m.GetClient(), + log: logging.NewNopLogger(), + record: event.NewNopRecorder(), + traitChecker: WorkloadHealthCheckFn(CheckByHealthCheckTrait), checkers: []WorloadHealthChecker{ WorkloadHealthCheckFn(CheckContainerziedWorkloadHealth), WorkloadHealthCheckFn(CheckDeploymentHealth), WorkloadHealthCheckFn(CheckStatefulsetHealth), WorkloadHealthCheckFn(CheckDaemonsetHealth), }, + unknownChecker: WorkloadHealthCheckFn(CheckUnknownWorkload), } for _, ro := range o { ro(r) @@ -152,43 +163,28 @@ func (r *Reconciler) Reconcile(req reconcile.Request) (reconcile.Result, error) log = log.WithValues("uid", hs.GetUID(), "version", hs.GetResourceVersion()) - hsStatus := r.GetScopeHealthStatus(ctx, hs) + scopeCondition, wlConditions := r.GetScopeHealthStatus(ctx, hs) log.Debug("Successfully ran health check", "scope", hs.Name) r.record.Event(hs, event.Normal(reasonHealthCheck, "Successfully ran health check")) elapsed := time.Since(start) - - if hsStatus.IsHealthy { - hs.Status.Health = "healthy" - } else { - hs.Status.Health = "unhealthy" - } - - diagnosis, err := json.Marshal(hsStatus) - if err != nil { - return reconcile.Result{}, errors.Wrap(err, errMarshalScopeDiagnosiis) - } - - // sava diagnosis into status.conditions - // TODO(roywang) is there a better way to show diagnosis - hs.SetConditions(v1alpha1.ReconcileSuccess(). - WithMessage(string(diagnosis))) + hs.Status.ScopeHealthCondition = scopeCondition + hs.Status.WorkloadHealthConditions = wlConditions return reconcile.Result{RequeueAfter: interval - elapsed}, errors.Wrap(r.client.Status().Update(ctx, hs), errUpdateHealthScopeStatus) } // GetScopeHealthStatus get the status of the healthscope based on workload resources. -func (r *Reconciler) GetScopeHealthStatus(ctx context.Context, healthScope *v1alpha2.HealthScope) HealthCondition { - hsStatus := HealthCondition{ - Target: runtimev1alpha1.TypedReference{ - APIVersion: healthScope.APIVersion, - Kind: healthScope.Kind, - Name: healthScope.Name, - UID: healthScope.UID, - }, - IsHealthy: true, // if no workload referenced, scope is healthy by default - SubConditions: []*HealthCondition{}, +func (r *Reconciler) GetScopeHealthStatus(ctx context.Context, healthScope *v1alpha2.HealthScope) (ScopeHealthCondition, []*WorkloadHealthCondition) { + log := r.log.WithValues("get scope health status", healthScope.GetName()) + scopeCondition := ScopeHealthCondition{ + HealthStatus: StatusHealthy, //if no workload referenced, scope is healthy by default } + scopeWLRefs := healthScope.Spec.WorkloadReferences + if len(scopeWLRefs) == 0 { + return scopeCondition, []*WorkloadHealthCondition{} + } + timeout := defaultTimeout if healthScope.Spec.ProbeTimeout != nil { timeout = time.Duration(*healthScope.Spec.ProbeTimeout) * time.Second @@ -196,42 +192,67 @@ func (r *Reconciler) GetScopeHealthStatus(ctx context.Context, healthScope *v1al ctxWithTimeout, cancel := context.WithTimeout(ctx, timeout) defer cancel() - scopeWLRefs := healthScope.Spec.WorkloadReferences // process workloads concurrently - wlStatusCs := make(chan *HealthCondition, len(scopeWLRefs)) + workloadHealthConditionsC := make(chan *WorkloadHealthCondition, len(scopeWLRefs)) var wg sync.WaitGroup wg.Add(len(scopeWLRefs)) for _, workloadRef := range scopeWLRefs { go func(resRef runtimev1alpha1.TypedReference) { defer wg.Done() - var wlStatusC *HealthCondition + var wlHealthCondition *WorkloadHealthCondition + + wlHealthCondition = r.traitChecker.Check(ctx, r.client, resRef, healthScope.GetNamespace()) + if wlHealthCondition != nil { + log.Debug("get health condition from health check trait ", "workload", resRef, "healthCondition", wlHealthCondition) + // get healthCondition from HealthCheckTrait + workloadHealthConditionsC <- wlHealthCondition + return + } + for _, checker := range r.checkers { - wlStatusC = checker.Check(ctxWithTimeout, r.client, resRef, healthScope.GetNamespace()) - if wlStatusC != nil { - // found matched checker and get health status - wlStatusCs <- wlStatusC + wlHealthCondition = checker.Check(ctxWithTimeout, r.client, resRef, healthScope.GetNamespace()) + if wlHealthCondition != nil { + log.Debug("get health condition from built-in checker", "workload", resRef, "healthCondition", wlHealthCondition) + // found matched checker and get health condition + workloadHealthConditionsC <- wlHealthCondition return } } - // unsupportted workload - wlStatusCs <- &HealthCondition{ - Target: resRef, - IsHealthy: false, - Diagnosis: fmt.Sprintf(errFmtUnsupportWorkload, resRef.APIVersion, resRef.Kind), - } + // handle unknown workload + log.Debug("get unknown workload", "workload", resRef) + workloadHealthConditionsC <- r.unknownChecker.Check(ctx, r.client, resRef, healthScope.GetNamespace()) }(workloadRef) } go func() { wg.Wait() - close(wlStatusCs) + close(workloadHealthConditionsC) }() - for wlStatus := range wlStatusCs { - // any unhealthy workload makes the scope unhealthy - hsStatus.IsHealthy = hsStatus.IsHealthy && wlStatus.IsHealthy - hsStatus.SubConditions = append(hsStatus.SubConditions, wlStatus) + var healthyCount, unhealthyCount, unknownCount int64 + workloadHealthConditions := []*WorkloadHealthCondition{} + for wlC := range workloadHealthConditionsC { + workloadHealthConditions = append(workloadHealthConditions, wlC) + switch wlC.HealthStatus { + case StatusHealthy: + healthyCount++ + case StatusUnhealthy: + unhealthyCount++ + case StatusUnknown: + unknownCount++ + default: + unknownCount++ + } } - return hsStatus + if unhealthyCount > 0 || unknownCount > 0 { + // ANY unhealthy or unknown worloads make the whole scope unhealthy + scopeCondition.HealthStatus = StatusUnhealthy + } + scopeCondition.Total = int64(len(scopeWLRefs)) + scopeCondition.HealthyWorkloads = healthyCount + scopeCondition.UnhealthyWorkloads = unhealthyCount + scopeCondition.UnknownWorkloads = unknownCount + + return scopeCondition, workloadHealthConditions } diff --git a/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope_controller_test.go b/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope_controller_test.go index b3df5fdd..895d4af2 100644 --- a/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope_controller_test.go +++ b/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope_controller_test.go @@ -45,12 +45,13 @@ var _ = Describe("HealthScope Controller Reconcile Test", func() { Client: &test.MockClient{}, } MockHealthyChecker := WorkloadHealthCheckFn( - func(context.Context, client.Client, v1alpha1.TypedReference, string) *HealthCondition { - return &HealthCondition{IsHealthy: true} + func(context.Context, client.Client, v1alpha1.TypedReference, string) *WorkloadHealthCondition { + + return &WorkloadHealthCondition{HealthStatus: StatusHealthy} }) MockUnhealthyChecker := WorkloadHealthCheckFn( - func(context.Context, client.Client, v1alpha1.TypedReference, string) *HealthCondition { - return &HealthCondition{IsHealthy: false} + func(context.Context, client.Client, v1alpha1.TypedReference, string) *WorkloadHealthCondition { + return &WorkloadHealthCondition{HealthStatus: StatusUnhealthy} }) reconciler := NewReconciler(mockMgr, WithLogger(logging.NewNopLogger().WithValues("HealthScopeReconciler")), @@ -154,25 +155,20 @@ var _ = Describe("Test GetScopeHealthStatus", func() { cwRef.SetGroupVersionKind(corev1alpha2.SchemeGroupVersion.WithKind(kindContainerizedWorkload)) deployRef.SetGroupVersionKind(appsv1.SchemeGroupVersion.WithKind(kindDeployment)) svcRef.SetGroupVersionKind(appsv1.SchemeGroupVersion.WithKind(kindService)) - hGeneralRef := v1alpha1.TypedReference{ - APIVersion: "unknown", - Kind: "unknown", - Name: "healthyGeneral", - } cw := corev1alpha2.ContainerizedWorkload{} cw.SetGroupVersionKind(corev1alpha2.SchemeGroupVersion.WithKind(kindContainerizedWorkload)) cw.Status.Resources = []v1alpha1.TypedReference{deployRef, svcRef} hDeploy := appsv1.Deployment{ + Spec: appsv1.DeploymentSpec{ + Replicas: &varInt1, + }, Status: appsv1.DeploymentStatus{ ReadyReplicas: 1, // healthy }, } hDeploy.SetGroupVersionKind(appsv1.SchemeGroupVersion.WithKind(kindDeployment)) - hGeneralWL := &unstructured.Unstructured{Object: make(map[string]interface{})} - fieldpath.Pave(hGeneralWL.Object).SetValue("status.readyReplicas", 1) // healthy - fieldpath.Pave(hGeneralWL.Object).SetValue("metadata.name", "healthyGeneral") // healthy uhGeneralRef := v1alpha1.TypedReference{ APIVersion: "unknown", @@ -199,9 +195,10 @@ var _ = Describe("Test GetScopeHealthStatus", func() { // use ContainerizedWorkload and Deployment checker It("Test healthy scope", func() { tests := []struct { - caseName string - hsWorkloadRefs []v1alpha1.TypedReference - mockGetFn test.MockGetFn + caseName string + hsWorkloadRefs []v1alpha1.TypedReference + mockGetFn test.MockGetFn + wantScopeCondition ScopeHealthCondition }{ { caseName: "2 supportted workloads(cw,deploy)", @@ -215,6 +212,13 @@ var _ = Describe("Test GetScopeHealthStatus", func() { } return nil }, + wantScopeCondition: ScopeHealthCondition{ + HealthStatus: StatusHealthy, + Total: int64(2), + HealthyWorkloads: int64(2), + UnhealthyWorkloads: 0, + UnknownWorkloads: 0, + }, }, } for _, tc := range tests { @@ -224,18 +228,19 @@ var _ = Describe("Test GetScopeHealthStatus", func() { } reconciler.client = mockClient hs.Spec.WorkloadReferences = tc.hsWorkloadRefs - result := reconciler.GetScopeHealthStatus(ctx, &hs) + result, _ := reconciler.GetScopeHealthStatus(ctx, &hs) Expect(result).ShouldNot(BeNil()) - Expect(result.IsHealthy).Should(Equal(true)) + Expect(result).Should(Equal(tc.wantScopeCondition)) } }) // use ContainerizedWorkload and Deployment checker It("Test unhealthy scope", func() { tests := []struct { - caseName string - hsWorkloadRefs []v1alpha1.TypedReference - mockGetFn test.MockGetFn + caseName string + hsWorkloadRefs []v1alpha1.TypedReference + mockGetFn test.MockGetFn + wantScopeCondition ScopeHealthCondition }{ { caseName: "2 supportted workloads but one is unhealthy", @@ -252,20 +257,12 @@ var _ = Describe("Test GetScopeHealthStatus", func() { } return nil }, - }, - { - caseName: "2 general workloads but one is unhealthy", - hsWorkloadRefs: []v1alpha1.TypedReference{hGeneralRef, uhGeneralRef}, - mockGetFn: func(ctx context.Context, key types.NamespacedName, obj runtime.Object) error { - if key.Name == "healthyGeneral" { - o, _ := obj.(*unstructured.Unstructured) - *o = *hGeneralWL - } - if key.Name == "unhealthyGeneral" { - o, _ := obj.(*unstructured.Unstructured) - *o = *uhGeneralWL - } - return nil + wantScopeCondition: ScopeHealthCondition{ + HealthStatus: StatusUnhealthy, + Total: int64(2), + HealthyWorkloads: int64(1), + UnhealthyWorkloads: int64(1), + UnknownWorkloads: 0, }, }, { @@ -282,6 +279,13 @@ var _ = Describe("Test GetScopeHealthStatus", func() { } return nil }, + wantScopeCondition: ScopeHealthCondition{ + HealthStatus: StatusUnhealthy, + Total: int64(2), + HealthyWorkloads: int64(1), + UnhealthyWorkloads: 0, + UnknownWorkloads: int64(1), + }, }, } @@ -292,9 +296,9 @@ var _ = Describe("Test GetScopeHealthStatus", func() { } reconciler.client = mockClient hs.Spec.WorkloadReferences = tc.hsWorkloadRefs - result := reconciler.GetScopeHealthStatus(ctx, &hs) + result, _ := reconciler.GetScopeHealthStatus(ctx, &hs) Expect(result).ShouldNot(BeNil()) - Expect(result.IsHealthy).Should(Equal(false)) + Expect(result).Should(Equal(tc.wantScopeCondition)) } }) }) diff --git a/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope_test.go b/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope_test.go index 68b9b1a5..05cf273f 100644 --- a/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope_test.go +++ b/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope_test.go @@ -18,7 +18,7 @@ package healthscope import ( "context" - "errors" + "fmt" "testing" "github.com/stretchr/testify/assert" @@ -28,8 +28,11 @@ import ( "k8s.io/apimachinery/pkg/types" runtimev1alpha1 "github.com/crossplane/crossplane-runtime/apis/core/v1alpha1" + "github.com/crossplane/crossplane-runtime/pkg/fieldpath" "github.com/crossplane/crossplane-runtime/pkg/test" + "github.com/pkg/errors" + corev1alpha2 "github.com/crossplane/oam-kubernetes-runtime/apis/core/v1alpha2" ) @@ -41,6 +44,7 @@ const ( var ( ctx = context.Background() errMockErr = errors.New("get error") + varInt1 = int32(1) ) func TestCheckContainerziedWorkloadHealth(t *testing.T) { @@ -61,7 +65,7 @@ func TestCheckContainerziedWorkloadHealth(t *testing.T) { caseName string mockGetFn test.MockGetFn wlRef runtimev1alpha1.TypedReference - expect *HealthCondition + expect *WorkloadHealthCondition }{ { caseName: "not matched checker", @@ -78,6 +82,9 @@ func TestCheckContainerziedWorkloadHealth(t *testing.T) { } if o, ok := obj.(*apps.Deployment); ok { *o = apps.Deployment{ + Spec: apps.DeploymentSpec{ + Replicas: &varInt1, + }, Status: apps.DeploymentStatus{ ReadyReplicas: 1, // healthy }, @@ -85,8 +92,8 @@ func TestCheckContainerziedWorkloadHealth(t *testing.T) { } return nil }, - expect: &HealthCondition{ - IsHealthy: true, + expect: &WorkloadHealthCondition{ + HealthStatus: StatusHealthy, }, }, { @@ -99,6 +106,9 @@ func TestCheckContainerziedWorkloadHealth(t *testing.T) { } if o, ok := obj.(*apps.Deployment); ok { *o = apps.Deployment{ + Spec: apps.DeploymentSpec{ + Replicas: &varInt1, + }, Status: apps.DeploymentStatus{ ReadyReplicas: 0, // unhealthy }, @@ -106,8 +116,8 @@ func TestCheckContainerziedWorkloadHealth(t *testing.T) { } return nil }, - expect: &HealthCondition{ - IsHealthy: false, + expect: &WorkloadHealthCondition{ + HealthStatus: StatusUnhealthy, }, }, { @@ -116,8 +126,8 @@ func TestCheckContainerziedWorkloadHealth(t *testing.T) { mockGetFn: func(ctx context.Context, key types.NamespacedName, obj runtime.Object) error { return errMockErr }, - expect: &HealthCondition{ - IsHealthy: false, + expect: &WorkloadHealthCondition{ + HealthStatus: StatusUnhealthy, }, }, { @@ -133,8 +143,8 @@ func TestCheckContainerziedWorkloadHealth(t *testing.T) { } return nil }, - expect: &HealthCondition{ - IsHealthy: false, + expect: &WorkloadHealthCondition{ + HealthStatus: StatusUnhealthy, }, }, { @@ -155,8 +165,8 @@ func TestCheckContainerziedWorkloadHealth(t *testing.T) { } return nil }, - expect: &HealthCondition{ - IsHealthy: false, + expect: &WorkloadHealthCondition{ + HealthStatus: StatusUnhealthy, }, }, } @@ -168,7 +178,7 @@ func TestCheckContainerziedWorkloadHealth(t *testing.T) { if tc.expect == nil { assert.Nil(t, result, tc.caseName) } else { - assert.Equal(t, tc.expect.IsHealthy, result.IsHealthy, tc.caseName) + assert.Equal(t, tc.expect.HealthStatus, result.HealthStatus, tc.caseName) } }(t) @@ -184,7 +194,7 @@ func TestCheckDeploymentHealth(t *testing.T) { caseName string mockGetFn test.MockGetFn wlRef runtimev1alpha1.TypedReference - expect *HealthCondition + expect *WorkloadHealthCondition }{ { caseName: "not matched checker", @@ -197,6 +207,9 @@ func TestCheckDeploymentHealth(t *testing.T) { mockGetFn: func(ctx context.Context, key types.NamespacedName, obj runtime.Object) error { if o, ok := obj.(*apps.Deployment); ok { *o = apps.Deployment{ + Spec: apps.DeploymentSpec{ + Replicas: &varInt1, + }, Status: apps.DeploymentStatus{ ReadyReplicas: 1, // healthy }, @@ -204,8 +217,8 @@ func TestCheckDeploymentHealth(t *testing.T) { } return nil }, - expect: &HealthCondition{ - IsHealthy: true, + expect: &WorkloadHealthCondition{ + HealthStatus: StatusHealthy, }, }, { @@ -214,6 +227,9 @@ func TestCheckDeploymentHealth(t *testing.T) { mockGetFn: func(ctx context.Context, key types.NamespacedName, obj runtime.Object) error { if o, ok := obj.(*apps.Deployment); ok { *o = apps.Deployment{ + Spec: apps.DeploymentSpec{ + Replicas: &varInt1, + }, Status: apps.DeploymentStatus{ ReadyReplicas: 0, // unhealthy }, @@ -221,8 +237,8 @@ func TestCheckDeploymentHealth(t *testing.T) { } return nil }, - expect: &HealthCondition{ - IsHealthy: false, + expect: &WorkloadHealthCondition{ + HealthStatus: StatusUnhealthy, }, }, { @@ -231,8 +247,8 @@ func TestCheckDeploymentHealth(t *testing.T) { mockGetFn: func(ctx context.Context, key types.NamespacedName, obj runtime.Object) error { return errMockErr }, - expect: &HealthCondition{ - IsHealthy: false, + expect: &WorkloadHealthCondition{ + HealthStatus: StatusUnhealthy, }, }, } @@ -244,7 +260,7 @@ func TestCheckDeploymentHealth(t *testing.T) { if tc.expect == nil { assert.Nil(t, result, tc.caseName) } else { - assert.Equal(t, tc.expect.IsHealthy, result.IsHealthy, tc.caseName) + assert.Equal(t, tc.expect.HealthStatus, result.HealthStatus, tc.caseName) } }(t) } @@ -259,7 +275,7 @@ func TestCheckStatefulsetHealth(t *testing.T) { caseName string mockGetFn test.MockGetFn wlRef runtimev1alpha1.TypedReference - expect *HealthCondition + expect *WorkloadHealthCondition }{ { caseName: "not matched checker", @@ -272,6 +288,9 @@ func TestCheckStatefulsetHealth(t *testing.T) { mockGetFn: func(ctx context.Context, key types.NamespacedName, obj runtime.Object) error { if o, ok := obj.(*apps.StatefulSet); ok { *o = apps.StatefulSet{ + Spec: apps.StatefulSetSpec{ + Replicas: &varInt1, + }, Status: apps.StatefulSetStatus{ ReadyReplicas: 1, // healthy }, @@ -279,8 +298,8 @@ func TestCheckStatefulsetHealth(t *testing.T) { } return nil }, - expect: &HealthCondition{ - IsHealthy: true, + expect: &WorkloadHealthCondition{ + HealthStatus: StatusHealthy, }, }, { @@ -289,6 +308,9 @@ func TestCheckStatefulsetHealth(t *testing.T) { mockGetFn: func(ctx context.Context, key types.NamespacedName, obj runtime.Object) error { if o, ok := obj.(*apps.StatefulSet); ok { *o = apps.StatefulSet{ + Spec: apps.StatefulSetSpec{ + Replicas: &varInt1, + }, Status: apps.StatefulSetStatus{ ReadyReplicas: 0, // unhealthy }, @@ -296,8 +318,8 @@ func TestCheckStatefulsetHealth(t *testing.T) { } return nil }, - expect: &HealthCondition{ - IsHealthy: false, + expect: &WorkloadHealthCondition{ + HealthStatus: StatusUnhealthy, }, }, { @@ -306,8 +328,8 @@ func TestCheckStatefulsetHealth(t *testing.T) { mockGetFn: func(ctx context.Context, key types.NamespacedName, obj runtime.Object) error { return errMockErr }, - expect: &HealthCondition{ - IsHealthy: false, + expect: &WorkloadHealthCondition{ + HealthStatus: StatusUnhealthy, }, }, } @@ -319,7 +341,7 @@ func TestCheckStatefulsetHealth(t *testing.T) { if tc.expect == nil { assert.Nil(t, result, tc.caseName) } else { - assert.Equal(t, tc.expect.IsHealthy, result.IsHealthy, tc.caseName) + assert.Equal(t, tc.expect.HealthStatus, result.HealthStatus, tc.caseName) } }(t) } @@ -334,7 +356,7 @@ func TestCheckDaemonsetHealth(t *testing.T) { caseName string mockGetFn test.MockGetFn wlRef runtimev1alpha1.TypedReference - expect *HealthCondition + expect *WorkloadHealthCondition }{ { caseName: "not matched checker", @@ -354,8 +376,8 @@ func TestCheckDaemonsetHealth(t *testing.T) { } return nil }, - expect: &HealthCondition{ - IsHealthy: true, + expect: &WorkloadHealthCondition{ + HealthStatus: StatusHealthy, }, }, { @@ -371,8 +393,8 @@ func TestCheckDaemonsetHealth(t *testing.T) { } return nil }, - expect: &HealthCondition{ - IsHealthy: false, + expect: &WorkloadHealthCondition{ + HealthStatus: StatusUnhealthy, }, }, { @@ -381,8 +403,8 @@ func TestCheckDaemonsetHealth(t *testing.T) { mockGetFn: func(ctx context.Context, key types.NamespacedName, obj runtime.Object) error { return errMockErr }, - expect: &HealthCondition{ - IsHealthy: false, + expect: &WorkloadHealthCondition{ + HealthStatus: StatusUnhealthy, }, }, } @@ -394,7 +416,68 @@ func TestCheckDaemonsetHealth(t *testing.T) { if tc.expect == nil { assert.Nil(t, result, tc.caseName) } else { - assert.Equal(t, tc.expect.IsHealthy, result.IsHealthy, tc.caseName) + assert.Equal(t, tc.expect.HealthStatus, result.HealthStatus, tc.caseName) + } + }(t) + } +} + +func TestCheckUnknownWorkload(t *testing.T) { + mockError := fmt.Errorf("mock error") + mockClient := test.NewMockClient() + unknownWL := runtimev1alpha1.TypedReference{} + tests := []struct { + caseName string + mockGetFn test.MockGetFn + expect *WorkloadHealthCondition + }{ + { + caseName: "cannot get workload", + mockGetFn: func(ctx context.Context, key types.NamespacedName, obj runtime.Object) error { + return mockError + }, + expect: &WorkloadHealthCondition{ + HealthStatus: StatusUnknown, + Diagnosis: errors.Wrap(mockError, errHealthCheck).Error(), + }, + }, + { + caseName: "unknown workload with status", + mockGetFn: func(ctx context.Context, key types.NamespacedName, obj runtime.Object) error { + o, _ := obj.(*unstructured.Unstructured) + *o = unstructured.Unstructured{} + o.Object = make(map[string]interface{}) + fieldpath.Pave(o.Object).SetValue("status.unknown", 1) + return nil + }, + expect: &WorkloadHealthCondition{ + HealthStatus: StatusUnknown, + Diagnosis: fmt.Sprintf(infoFmtUnknownWorkload, "", ""), + WorkloadStatus: "{\"unknown\":1}", + }, + }, + { + caseName: "unknown workload without status", + mockGetFn: func(ctx context.Context, key types.NamespacedName, obj runtime.Object) error { + o, _ := obj.(*unstructured.Unstructured) + *o = unstructured.Unstructured{} + return nil + }, + expect: &WorkloadHealthCondition{ + HealthStatus: StatusUnknown, + Diagnosis: fmt.Sprintf(infoFmtUnknownWorkload, "", ""), + WorkloadStatus: "null", + }, + }, + } + for _, tc := range tests { + func(t *testing.T) { + mockClient.MockGet = tc.mockGetFn + result := CheckUnknownWorkload(ctx, mockClient, unknownWL, namespace) + if tc.expect == nil { + assert.Nil(t, result, tc.caseName) + } else { + assert.Equal(t, tc.expect, result, tc.caseName) } }(t) } diff --git a/test/e2e-test/appconfig_finalizer_test.go b/test/e2e-test/appconfig_finalizer_test.go index e37fb017..d5f6ad2b 100644 --- a/test/e2e-test/appconfig_finalizer_test.go +++ b/test/e2e-test/appconfig_finalizer_test.go @@ -214,8 +214,12 @@ var _ = Describe("Finalizer for HealthScope in ApplicationConfiguration", func() }, time.Second*30, time.Millisecond*500).Should(Equal(0)) By("Check AppConfig has been deleted successfully") - Eventually(k8sClient.Get(ctx, client.ObjectKey{Namespace: namespace, Name: appConfigName}, &appConfig), - time.Second*15, time.Microsecond*500).Should(&util.NotFoundMatcher{}) + deletedAppConfig := &v1alpha2.ApplicationConfiguration{} + Eventually( + func() error { + return k8sClient.Get(ctx, client.ObjectKey{Namespace: namespace, Name: appConfigName}, deletedAppConfig) + }, + time.Second*30, time.Microsecond*500).Should(&util.NotFoundMatcher{}) }) }) diff --git a/test/e2e-test/health_scope_test.go b/test/e2e-test/health_scope_test.go index 62a805d4..d4eebd80 100644 --- a/test/e2e-test/health_scope_test.go +++ b/test/e2e-test/health_scope_test.go @@ -95,6 +95,11 @@ var _ = Describe("HealthScope", func() { } logf.Log.Info("Creating health scope") Expect(k8sClient.Create(ctx, &hs)).Should(SatisfyAny(BeNil(), &util.AlreadyExistMatcher{})) + By("Check empty health scope is healthy") + Eventually(func() v1alpha2.HealthStatus { + k8sClient.Get(ctx, client.ObjectKey{Namespace: namespace, Name: healthScopeName}, &hs) + return hs.Status.ScopeHealthCondition.HealthStatus + }, time.Second*30, time.Millisecond*500).Should(Equal(v1alpha2.StatusHealthy)) label := map[string]string{"workload": "containerized-workload"} // create a workload definition @@ -284,20 +289,20 @@ var _ = Describe("HealthScope", func() { healthScope := &v1alpha2.HealthScope{} By("Verify health scope") Eventually( - func() bool { + func() v1alpha2.ScopeHealthCondition { + *healthScope = v1alpha2.HealthScope{} k8sClient.Get(ctx, healthScopeObject, healthScope) logf.Log.Info("Checking on health scope", "len(WorkloadReferences)", len(healthScope.Spec.WorkloadReferences), "health", - healthScope.Status.Health) - // TODO(artursouza): enable this check once crossplane is updated. - //if len(healthScope.Spec.WorkloadReferences) == 0 { - // return false - //} - - return healthScope.Status.Health == "healthy" + healthScope.Status.ScopeHealthCondition) + return healthScope.Status.ScopeHealthCondition }, - time.Second*120, time.Second*5).Should(BeEquivalentTo(true)) + time.Second*120, time.Second*5).Should(Equal(v1alpha2.ScopeHealthCondition{ + HealthStatus: v1alpha2.StatusHealthy, + Total: int64(2), + HealthyWorkloads: int64(2), + })) }) })