From 645a9670b78e13d128a54e39aaa8f7a70fc288fe Mon Sep 17 00:00:00 2001 From: roy wang Date: Sat, 29 Aug 2020 22:02:26 +0900 Subject: [PATCH 1/7] use merging instead of overriding add unit test Signed-off-by: roy wang --- apis/core/v1alpha2/core_scope_types.go | 24 ++- apis/core/v1alpha2/zz_generated.deepcopy.go | 28 ++++ .../crds/core.oam.dev_healthscopes.yaml | 88 +++++++++- .../applicationconfiguration/render.go | 11 ++ .../applicationconfiguration/render_test.go | 49 ++++++ .../core/scopes/healthscope/healthscope.go | 154 ++++++++++++------ .../healthscope/healthscope_controller.go | 103 +++++++----- .../healthscope_controller_test.go | 13 +- .../scopes/healthscope/healthscope_test.go | 36 ++-- test/e2e-test/health_scope_test.go | 4 +- 10 files changed, 386 insertions(+), 124 deletions(-) diff --git a/apis/core/v1alpha2/core_scope_types.go b/apis/core/v1alpha2/core_scope_types.go index cd1ba76b..d45dff70 100644 --- a/apis/core/v1alpha2/core_scope_types.go +++ b/apis/core/v1alpha2/core_scope_types.go @@ -23,6 +23,14 @@ import ( "github.com/crossplane/oam-kubernetes-runtime/pkg/oam" ) +type HealthStatus string + +const ( + StatusHealthy HealthStatus = "HEALTHY" + StatusUnhealthy = "UNHEALTHY" + StatusUnknown = "UNKNOWN" +) + var _ oam.Scope = &HealthScope{} // A HealthScopeSpec defines the desired state of a HealthScope. @@ -41,7 +49,21 @@ type HealthScopeSpec struct { type HealthScopeStatus struct { runtimev1alpha1.ConditionedStatus `json:",inline"` - Health string `json:"health"` + // ScopeHealthCondition represents health condition summary of the scope + ScopeHealthCondition HealthCondition `json:"scopeHealthCondition"` + + // HealthConditions represents health condition of workloads in the scope + HealthConditions []*HealthCondition `json:"healthConditions,omitempty"` +} + +type HealthCondition struct { + // Name represents the component name if target is a workload + Name string `json:"name,omitempty"` + TargetWorkload runtimev1alpha1.TypedReference `json:"targetWorkload,omitempty"` + HealthStatus HealthStatus `json:"healthStatus"` + Diagnosis string `json:"diagnosis,omitempty"` + // WorkloadInfo represents status of workloads whose HealthStatus is UNKNOWN. + WorkloadInfo 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..dae02ec9 100644 --- a/apis/core/v1alpha2/zz_generated.deepcopy.go +++ b/apis/core/v1alpha2/zz_generated.deepcopy.go @@ -999,6 +999,22 @@ func (in *HTTPHeader) DeepCopy() *HTTPHeader { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *HealthCondition) DeepCopyInto(out *HealthCondition) { + *out = *in + out.TargetWorkload = in.TargetWorkload +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new HealthCondition. +func (in *HealthCondition) DeepCopy() *HealthCondition { + if in == nil { + return nil + } + out := new(HealthCondition) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *HealthScope) DeepCopyInto(out *HealthScope) { *out = *in @@ -1092,6 +1108,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.HealthConditions != nil { + in, out := &in.HealthConditions, &out.HealthConditions + *out = make([]*HealthCondition, len(*in)) + for i := range *in { + if (*in)[i] != nil { + in, out := &(*in)[i], &(*out)[i] + *out = new(HealthCondition) + **out = **in + } + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new HealthScopeStatus. 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..130804b5 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,92 @@ spec: - type type: object type: array - health: - type: string + healthConditions: + description: HealthConditions represents health condition of workloads + in the scope + items: + properties: + diagnosis: + type: string + healthStatus: + type: string + name: + description: Name represents the component name if target is + a workload + 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: WorkloadInfo 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: + diagnosis: + type: string + healthStatus: + type: string + name: + description: Name represents the component name if target is a + workload + 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: WorkloadInfo represents status of workloads whose + HealthStatus is UNKNOWN. + type: string + required: + - healthStatus + type: object required: - - health + - scopeHealthCondition type: object type: object served: true diff --git a/pkg/controller/v1alpha2/applicationconfiguration/render.go b/pkg/controller/v1alpha2/applicationconfiguration/render.go index 78762dab..0a260212 100644 --- a/pkg/controller/v1alpha2/applicationconfiguration/render.go +++ b/pkg/controller/v1alpha2/applicationconfiguration/render.go @@ -478,6 +478,17 @@ func fillValue(obj *unstructured.Unstructured, fs []string, val interface{}) err return nil } +func addWorkloadLabels(w *unstructured.Unstructured, labels map[string]string) { + workloadLabels := w.GetLabels() + if workloadLabels == nil { + workloadLabels = map[string]string{} + } + for k, v := range labels { + workloadLabels[k] = v + } + w.SetLabels(workloadLabels) +} + func (r *components) getDataInput(ctx context.Context, s *dagSource) (interface{}, bool, error) { obj := s.ObjectRef key := types.NamespacedName{ diff --git a/pkg/controller/v1alpha2/applicationconfiguration/render_test.go b/pkg/controller/v1alpha2/applicationconfiguration/render_test.go index 0d299134..342ddf06 100644 --- a/pkg/controller/v1alpha2/applicationconfiguration/render_test.go +++ b/pkg/controller/v1alpha2/applicationconfiguration/render_test.go @@ -1057,3 +1057,52 @@ func TestMatchValue(t *testing.T) { }) } } + +func TestAddWorkloadLabels(t *testing.T) { + workload1 := new(unstructured.Unstructured) + wantWorkload1 := new(unstructured.Unstructured) + wantWorkload1.SetLabels(map[string]string{ + "newKey": "newValue", + }) + workload2 := new(unstructured.Unstructured) + wantWorkload2 := new(unstructured.Unstructured) + workload2.SetLabels(map[string]string{ + "key": "value", + }) + wantWorkload2.SetLabels(map[string]string{ + "key": "value", + "newKey": "newValue", + }) + + cases := map[string]struct { + workload *unstructured.Unstructured + newLabels map[string]string + want *unstructured.Unstructured + }{ + "add labels to workload without labels": { + workload1, + map[string]string{ + "newKey": "newValue", + }, + wantWorkload1, + }, + "add labels to workload with labels": { + workload2, + map[string]string{ + "newKey": "newValue", + }, + wantWorkload2, + }, + } + + for name, tc := range cases { + workload := tc.workload + wantWorkload := tc.want + t.Run(name, func(t *testing.T) { + addWorkloadLabels(tc.workload, tc.newLabels) + if diff := cmp.Diff(wantWorkload, workload); diff != "" { + t.Error(diff) + } + }) + } +} diff --git a/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope.go b/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope.go index 4b5aec4a..5b8c5e16 100644 --- a/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope.go +++ b/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope.go @@ -28,6 +28,7 @@ import ( "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" runtimev1alpha1 "github.com/crossplane/crossplane-runtime/apis/core/v1alpha1" @@ -35,14 +36,21 @@ import ( ) 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 " + errHealthCheck = "error occurs in health check " defaultTimeout = 10 * time.Second ) +type HealthStatus = v1alpha2.HealthStatus + +const ( + StatusHealthy = v1alpha2.StatusHealthy + StatusUnhealthy = v1alpha2.StatusUnhealthy + StatusUnknown = v1alpha2.StatusUnknown +) + var ( kindContainerizedWorkload = corev1alpha2.ContainerizedWorkloadKind kindDeployment = reflect.TypeOf(apps.Deployment{}).Name() @@ -52,18 +60,7 @@ var ( ) // HealthCondition holds health status of any resource -type HealthCondition struct { - // Target represents resource being diagnosed - Target runtimev1alpha1.TypedReference `json:"target"` - - 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"` -} +type HealthCondition = v1alpha2.HealthCondition // A WorloadHealthChecker checks health status of specified resource // and saves status into an HealthCondition object. @@ -86,8 +83,8 @@ func CheckContainerziedWorkloadHealth(ctx context.Context, c client.Client, ref return nil } r := &HealthCondition{ - IsHealthy: false, - Target: ref, + HealthStatus: StatusHealthy, + TargetWorkload: ref, } cwObj := corev1alpha2.ContainerizedWorkload{} cwObj.SetGroupVersionKind(corev1alpha2.SchemeGroupVersion.WithKind(kindContainerizedWorkload)) @@ -95,9 +92,10 @@ func CheckContainerziedWorkloadHealth(ctx context.Context, c client.Client, ref r.Diagnosis = errors.Wrap(err, errHealthCheck).Error() return r } - r.Target.UID = cwObj.GetUID() + //TODO get component name from labels + r.TargetWorkload.UID = cwObj.GetUID() - r.SubConditions = []*HealthCondition{} + subConditions := []*HealthCondition{} childRefs := cwObj.Status.Resources for _, childRef := range childRefs { @@ -105,32 +103,28 @@ func CheckContainerziedWorkloadHealth(ctx context.Context, c client.Client, ref case kindDeployment: // reuse Deployment health checker childCondition := CheckDeploymentHealth(ctx, c, childRef, namespace) - r.SubConditions = append(r.SubConditions, childCondition) - default: + subConditions = append(subConditions, childCondition) + case kindService: childCondition := &HealthCondition{ - Target: childRef, - IsHealthy: true, + 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 += sc.Diagnosis } return r } @@ -141,8 +135,8 @@ func CheckDeploymentHealth(ctx context.Context, client client.Client, ref runtim return nil } r := &HealthCondition{ - IsHealthy: false, - Target: ref, + HealthStatus: StatusUnhealthy, + TargetWorkload: ref, } deployment := apps.Deployment{} deployment.SetGroupVersionKind(apps.SchemeGroupVersion.WithKind(kindDeployment)) @@ -151,13 +145,14 @@ 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.TargetWorkload.UID = deployment.GetUID() + r.Diagnosis = fmt.Sprintf(infoFmtReady, deployment.Status.ReadyReplicas, *deployment.Spec.Replicas) - if deployment.Status.ReadyReplicas == 0 { - r.Diagnosis = fmt.Sprintf(errFmtResourceNotReady, deployment.Status) + // Health criteria + if deployment.Status.ReadyReplicas != *deployment.Spec.Replicas { return r } - r.IsHealthy = true + r.HealthStatus = StatusHealthy return r } @@ -167,8 +162,8 @@ func CheckStatefulsetHealth(ctx context.Context, client client.Client, ref runti return nil } r := &HealthCondition{ - IsHealthy: false, - Target: ref, + HealthStatus: StatusUnhealthy, + TargetWorkload: ref, } statefulset := apps.StatefulSet{} statefulset.APIVersion = ref.APIVersion @@ -178,13 +173,14 @@ 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.TargetWorkload.UID = statefulset.GetUID() + r.Diagnosis = fmt.Sprintf(infoFmtReady, statefulset.Status.ReadyReplicas, *statefulset.Spec.Replicas) - if statefulset.Status.ReadyReplicas == 0 { - r.Diagnosis = fmt.Sprintf(errFmtResourceNotReady, statefulset.Status) + // Health criteria + if statefulset.Status.ReadyReplicas != *statefulset.Spec.Replicas { return r } - r.IsHealthy = true + r.HealthStatus = StatusUnhealthy return r } @@ -194,8 +190,8 @@ func CheckDaemonsetHealth(ctx context.Context, client client.Client, ref runtime return nil } r := &HealthCondition{ - IsHealthy: false, - Target: ref, + HealthStatus: StatusUnhealthy, + TargetWorkload: ref, } daemonset := apps.DaemonSet{} daemonset.APIVersion = ref.APIVersion @@ -205,12 +201,66 @@ 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.TargetWorkload.UID = daemonset.GetUID() + r.Diagnosis = fmt.Sprintf(infoFmtReady, daemonset.Status.NumberReady, daemonset.Status.DesiredNumberScheduled) - if daemonset.Status.NumberUnavailable != 0 { - r.Diagnosis = fmt.Sprintf(errFmtResourceNotReady, daemonset.Status) + // Health criteria + if daemonset.Status.NumberReady != daemonset.Status.DesiredNumberScheduled { return r } - r.IsHealthy = true + r.HealthStatus = StatusHealthy return r } + +// func (r *Reconciler) listHealthCheckTrait(ctx context.Context, ns string) ([]unstructured.Unstructured, error) { +// //TODO Get HealthCheckTrait GVK dynamically +// hcTraitList := &unstructured.UnstructuredList{} +// hcTraitList.SetAPIVersion("extend.oam.dev/v1alpha2") +// hcTraitList.SetKind("HealthCheckTrait") +// +// if err := r.client.List(ctx, hcTraitList); err != nil { +// return nil, err +// } +// return hcTraitList.Items, nil +// +// } + +func (r *Reconciler) getHealthConditionFromTrait(ctx context.Context, wlRef runtimev1alpha1.TypedReference, ns string) (*HealthCondition, error) { + // get workload instance + + // get component name + + // get trait name by component name + + hcTraitList := &unstructured.UnstructuredList{} + hcTraitList.SetAPIVersion("extend.oam.dev/v1alpha2") + hcTraitList.SetKind("HealthCheckTrait") + + if err := r.client.List(ctx, hcTraitList); err != nil { + return nil, err + } + + return nil, nil +} + +func (r *Reconciler) getUnknownWorkloadHealthCondition(ctx context.Context, wlRef runtimev1alpha1.TypedReference, ns string) *HealthCondition { + healthCondition := &HealthCondition{ + TargetWorkload: wlRef, + HealthStatus: StatusUnknown, + Diagnosis: fmt.Sprintf(infoFmtUnknownWorkload, wlRef.APIVersion, wlRef.Kind), + } + + wl := &unstructured.Unstructured{} + wl.SetGroupVersionKind(wlRef.GroupVersionKind()) + if err := r.client.Get(ctx, client.ObjectKey{Namespace: ns, Name: wlRef.Name}, wl); err != nil { + healthCondition.Diagnosis += errors.Wrap(err, errHealthCheck).Error() + return healthCondition + } + wlBytes, err := wl.MarshalJSON() + if err != nil { + healthCondition.Diagnosis += errors.Wrap(err, errHealthCheck).Error() + return healthCondition + } + healthCondition.WorkloadInfo = string(wlBytes) + return healthCondition +} diff --git a/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope_controller.go b/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope_controller.go index 8feb8857..aeeab936 100644 --- a/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope_controller.go +++ b/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope_controller.go @@ -18,7 +18,6 @@ package healthscope import ( "context" - "encoding/json" "fmt" "strings" "sync" @@ -29,7 +28,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" @@ -52,6 +50,10 @@ const ( errUpdateHealthScopeStatus = "cannot update health scope status" ) +const ( + infoFmtScopeDiagnosis = "total workloads: %d, healthy workloads: %d, unhealthy workloads: %d, unknown workloads: %d" +) + // Reconcile event reasons. const ( reasonHealthCheck = "HealthCheck" @@ -152,43 +154,49 @@ 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.HealthConditions = wlConditions + + // if scopeCondition.HealthStatus { + // hs.Status.ScopeHealthCondition = "healthy" + // } else { + // hs.Status.ScopeHealthCondition = "unhealthy" + // } + + // diagnosis, err := json.Marshal(scopeCondition) + // 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))) 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{ +func (r *Reconciler) GetScopeHealthStatus(ctx context.Context, healthScope *v1alpha2.HealthScope) (HealthCondition, []*HealthCondition) { + scopeCondition := HealthCondition{ + TargetWorkload: 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{}, + HealthStatus: StatusHealthy, //if no workload referenced, scope is healthy by default } + scopeWLRefs := healthScope.Spec.WorkloadReferences + if len(scopeWLRefs) == 0 { + return scopeCondition, []*HealthCondition{} + } + timeout := defaultTimeout if healthScope.Spec.ProbeTimeout != nil { timeout = time.Duration(*healthScope.Spec.ProbeTimeout) * time.Second @@ -196,42 +204,53 @@ 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 *HealthCondition, 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 *HealthCondition + + //TODO get HealthCheckTrait status + for _, checker := range r.checkers { - wlStatusC = checker.Check(ctxWithTimeout, r.client, resRef, healthScope.GetNamespace()) - if wlStatusC != nil { + wlHealthCondition = checker.Check(ctxWithTimeout, r.client, resRef, healthScope.GetNamespace()) + if wlHealthCondition != nil { // found matched checker and get health status - wlStatusCs <- wlStatusC + workloadHealthConditionsC <- wlHealthCondition return } } - // unsupportted workload - wlStatusCs <- &HealthCondition{ - Target: resRef, - IsHealthy: false, - Diagnosis: fmt.Sprintf(errFmtUnsupportWorkload, resRef.APIVersion, resRef.Kind), - } + // unknown workload + workloadHealthConditionsC <- r.getUnknownWorkloadHealthCondition(ctx, 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 int + workloadHealthConditions := []*HealthCondition{} + for wlC := range workloadHealthConditionsC { + workloadHealthConditions = append(workloadHealthConditions, wlC) + switch wlC.HealthStatus { + case StatusHealthy: + healthyCount++ + case StatusUnhealthy: + unhealthyCount++ + case StatusUnknown: + unknownCount++ + } + } + if unhealthyCount > 0 || unknownCount > 0 { + // ANY unhealthy or unknown worloads make the whole scope unhealthy + scopeCondition.HealthStatus = StatusUnhealthy } - return hsStatus + scopeCondition.Diagnosis = fmt.Sprintf(infoFmtScopeDiagnosis, len(scopeWLRefs), healthyCount, unhealthyCount, 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..416589ee 100644 --- a/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope_controller_test.go +++ b/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope_controller_test.go @@ -46,11 +46,12 @@ var _ = Describe("HealthScope Controller Reconcile Test", func() { } MockHealthyChecker := WorkloadHealthCheckFn( func(context.Context, client.Client, v1alpha1.TypedReference, string) *HealthCondition { - return &HealthCondition{IsHealthy: true} + + return &HealthCondition{HealthStatus: StatusHealthy} }) MockUnhealthyChecker := WorkloadHealthCheckFn( func(context.Context, client.Client, v1alpha1.TypedReference, string) *HealthCondition { - return &HealthCondition{IsHealthy: false} + return &HealthCondition{HealthStatus: StatusUnhealthy} }) reconciler := NewReconciler(mockMgr, WithLogger(logging.NewNopLogger().WithValues("HealthScopeReconciler")), @@ -224,9 +225,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(true)) + Expect(result.HealthStatus).Should(Equal(true)) } }) @@ -292,9 +293,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.HealthStatus).Should(Equal(false)) } }) }) diff --git a/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope_test.go b/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope_test.go index 68b9b1a5..2e676cbe 100644 --- a/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope_test.go +++ b/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope_test.go @@ -86,7 +86,7 @@ func TestCheckContainerziedWorkloadHealth(t *testing.T) { return nil }, expect: &HealthCondition{ - IsHealthy: true, + HealthStatus: StatusHealthy, }, }, { @@ -107,7 +107,7 @@ func TestCheckContainerziedWorkloadHealth(t *testing.T) { return nil }, expect: &HealthCondition{ - IsHealthy: false, + HealthStatus: StatusUnhealthy, }, }, { @@ -117,7 +117,7 @@ func TestCheckContainerziedWorkloadHealth(t *testing.T) { return errMockErr }, expect: &HealthCondition{ - IsHealthy: false, + HealthStatus: StatusUnhealthy, }, }, { @@ -134,7 +134,7 @@ func TestCheckContainerziedWorkloadHealth(t *testing.T) { return nil }, expect: &HealthCondition{ - IsHealthy: false, + HealthStatus: StatusUnhealthy, }, }, { @@ -156,7 +156,7 @@ func TestCheckContainerziedWorkloadHealth(t *testing.T) { return nil }, expect: &HealthCondition{ - IsHealthy: false, + HealthStatus: StatusUnhealthy, }, }, } @@ -168,7 +168,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) @@ -205,7 +205,7 @@ func TestCheckDeploymentHealth(t *testing.T) { return nil }, expect: &HealthCondition{ - IsHealthy: true, + HealthStatus: StatusHealthy, }, }, { @@ -222,7 +222,7 @@ func TestCheckDeploymentHealth(t *testing.T) { return nil }, expect: &HealthCondition{ - IsHealthy: false, + HealthStatus: StatusUnhealthy, }, }, { @@ -232,7 +232,7 @@ func TestCheckDeploymentHealth(t *testing.T) { return errMockErr }, expect: &HealthCondition{ - IsHealthy: false, + HealthStatus: StatusUnhealthy, }, }, } @@ -244,7 +244,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) } @@ -280,7 +280,7 @@ func TestCheckStatefulsetHealth(t *testing.T) { return nil }, expect: &HealthCondition{ - IsHealthy: true, + HealthStatus: StatusHealthy, }, }, { @@ -297,7 +297,7 @@ func TestCheckStatefulsetHealth(t *testing.T) { return nil }, expect: &HealthCondition{ - IsHealthy: false, + HealthStatus: StatusUnhealthy, }, }, { @@ -307,7 +307,7 @@ func TestCheckStatefulsetHealth(t *testing.T) { return errMockErr }, expect: &HealthCondition{ - IsHealthy: false, + HealthStatus: StatusUnhealthy, }, }, } @@ -319,7 +319,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) } @@ -355,7 +355,7 @@ func TestCheckDaemonsetHealth(t *testing.T) { return nil }, expect: &HealthCondition{ - IsHealthy: true, + HealthStatus: StatusHealthy, }, }, { @@ -372,7 +372,7 @@ func TestCheckDaemonsetHealth(t *testing.T) { return nil }, expect: &HealthCondition{ - IsHealthy: false, + HealthStatus: StatusUnhealthy, }, }, { @@ -382,7 +382,7 @@ func TestCheckDaemonsetHealth(t *testing.T) { return errMockErr }, expect: &HealthCondition{ - IsHealthy: false, + HealthStatus: StatusUnhealthy, }, }, } @@ -394,7 +394,7 @@ 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) } diff --git a/test/e2e-test/health_scope_test.go b/test/e2e-test/health_scope_test.go index 62a805d4..12417cd4 100644 --- a/test/e2e-test/health_scope_test.go +++ b/test/e2e-test/health_scope_test.go @@ -290,13 +290,13 @@ var _ = Describe("HealthScope", func() { "len(WorkloadReferences)", len(healthScope.Spec.WorkloadReferences), "health", - healthScope.Status.Health) + healthScope.Status.ScopeHealthCondition) // TODO(artursouza): enable this check once crossplane is updated. //if len(healthScope.Spec.WorkloadReferences) == 0 { // return false //} - return healthScope.Status.Health == "healthy" + return healthScope.Status.ScopeHealthCondition.HealthStatus == "healthy" }, time.Second*120, time.Second*5).Should(BeEquivalentTo(true)) }) From 2707bbc18d51cda7d5638ea6ad860e14a1c85faa Mon Sep 17 00:00:00 2001 From: roy wang Date: Mon, 31 Aug 2020 17:20:46 +0900 Subject: [PATCH 2/7] move AddLabel func into oam/helper Signed-off-by: roy wang --- .../applicationconfiguration/render.go | 11 ----- .../applicationconfiguration/render_test.go | 49 ------------------- 2 files changed, 60 deletions(-) diff --git a/pkg/controller/v1alpha2/applicationconfiguration/render.go b/pkg/controller/v1alpha2/applicationconfiguration/render.go index 0a260212..78762dab 100644 --- a/pkg/controller/v1alpha2/applicationconfiguration/render.go +++ b/pkg/controller/v1alpha2/applicationconfiguration/render.go @@ -478,17 +478,6 @@ func fillValue(obj *unstructured.Unstructured, fs []string, val interface{}) err return nil } -func addWorkloadLabels(w *unstructured.Unstructured, labels map[string]string) { - workloadLabels := w.GetLabels() - if workloadLabels == nil { - workloadLabels = map[string]string{} - } - for k, v := range labels { - workloadLabels[k] = v - } - w.SetLabels(workloadLabels) -} - func (r *components) getDataInput(ctx context.Context, s *dagSource) (interface{}, bool, error) { obj := s.ObjectRef key := types.NamespacedName{ diff --git a/pkg/controller/v1alpha2/applicationconfiguration/render_test.go b/pkg/controller/v1alpha2/applicationconfiguration/render_test.go index 342ddf06..0d299134 100644 --- a/pkg/controller/v1alpha2/applicationconfiguration/render_test.go +++ b/pkg/controller/v1alpha2/applicationconfiguration/render_test.go @@ -1057,52 +1057,3 @@ func TestMatchValue(t *testing.T) { }) } } - -func TestAddWorkloadLabels(t *testing.T) { - workload1 := new(unstructured.Unstructured) - wantWorkload1 := new(unstructured.Unstructured) - wantWorkload1.SetLabels(map[string]string{ - "newKey": "newValue", - }) - workload2 := new(unstructured.Unstructured) - wantWorkload2 := new(unstructured.Unstructured) - workload2.SetLabels(map[string]string{ - "key": "value", - }) - wantWorkload2.SetLabels(map[string]string{ - "key": "value", - "newKey": "newValue", - }) - - cases := map[string]struct { - workload *unstructured.Unstructured - newLabels map[string]string - want *unstructured.Unstructured - }{ - "add labels to workload without labels": { - workload1, - map[string]string{ - "newKey": "newValue", - }, - wantWorkload1, - }, - "add labels to workload with labels": { - workload2, - map[string]string{ - "newKey": "newValue", - }, - wantWorkload2, - }, - } - - for name, tc := range cases { - workload := tc.workload - wantWorkload := tc.want - t.Run(name, func(t *testing.T) { - addWorkloadLabels(tc.workload, tc.newLabels) - if diff := cmp.Diff(wantWorkload, workload); diff != "" { - t.Error(diff) - } - }) - } -} From 528ec1046e284ab00047d9c934a960d377a3d837 Mon Sep 17 00:00:00 2001 From: roy wang Date: Thu, 3 Sep 2020 20:11:48 +0900 Subject: [PATCH 3/7] Make health condition of HealthScope more informative Add CheckByHealthCheckTrait interface Signed-off-by: roy wang --- apis/core/v1alpha2/core_scope_types.go | 19 +-- .../crds/core.oam.dev_healthscopes.yaml | 23 ++-- .../core/scopes/healthscope/healthscope.go | 108 ++++++++++-------- .../healthscope/healthscope_controller.go | 61 +++++----- .../healthscope_controller_test.go | 30 +---- .../scopes/healthscope/healthscope_test.go | 19 +++ test/e2e-test/health_scope_test.go | 2 +- 7 files changed, 147 insertions(+), 115 deletions(-) diff --git a/apis/core/v1alpha2/core_scope_types.go b/apis/core/v1alpha2/core_scope_types.go index d45dff70..65648a2b 100644 --- a/apis/core/v1alpha2/core_scope_types.go +++ b/apis/core/v1alpha2/core_scope_types.go @@ -23,12 +23,16 @@ import ( "github.com/crossplane/oam-kubernetes-runtime/pkg/oam" ) +// HealthStatus represents health status strings. type HealthStatus string const ( - StatusHealthy HealthStatus = "HEALTHY" - StatusUnhealthy = "UNHEALTHY" - StatusUnknown = "UNKNOWN" + // StatusHealthy represents healthy status. + StatusHealthy HealthStatus = "HEALTHY" + // StatusUnhealthy represents unhealthy status. + StatusUnhealthy = "UNHEALTHY" + // StatusUnknown represents unknown status. + StatusUnknown = "UNKNOWN" ) var _ oam.Scope = &HealthScope{} @@ -56,14 +60,15 @@ type HealthScopeStatus struct { HealthConditions []*HealthCondition `json:"healthConditions,omitempty"` } +// HealthCondition represents informative health condition. type HealthCondition struct { - // Name represents the component name if target is a workload - Name string `json:"name,omitempty"` + // 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"` - // WorkloadInfo represents status of workloads whose HealthStatus is UNKNOWN. - WorkloadInfo string `json:"workloadStatus,omitempty"` + // WorkloadStatus represents status of workloads whose HealthStatus is UNKNOWN. + WorkloadStatus string `json:"workloadStatus,omitempty"` } // +kubebuilder:object:root=true 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 130804b5..89a0e0b3 100644 --- a/charts/oam-kubernetes-runtime/crds/core.oam.dev_healthscopes.yaml +++ b/charts/oam-kubernetes-runtime/crds/core.oam.dev_healthscopes.yaml @@ -123,14 +123,16 @@ spec: description: HealthConditions represents health condition of workloads in the scope items: + description: HealthCondition represents informative health condition. properties: + componentName: + description: ComponentName represents the component name if + target is a workload + type: string diagnosis: type: string healthStatus: - type: string - name: - description: Name represents the component name if target is - a workload + description: HealthStatus represents health status strings. type: string targetWorkload: description: A TypedReference refers to an object by Name, Kind, @@ -155,7 +157,7 @@ spec: - name type: object workloadStatus: - description: WorkloadInfo represents status of workloads whose + description: WorkloadStatus represents status of workloads whose HealthStatus is UNKNOWN. type: string required: @@ -166,13 +168,14 @@ spec: description: ScopeHealthCondition represents health condition summary of the scope properties: + componentName: + description: ComponentName represents the component name if target + is a workload + type: string diagnosis: type: string healthStatus: - type: string - name: - description: Name represents the component name if target is a - workload + description: HealthStatus represents health status strings. type: string targetWorkload: description: A TypedReference refers to an object by Name, Kind, @@ -197,7 +200,7 @@ spec: - name type: object workloadStatus: - description: WorkloadInfo represents status of workloads whose + description: WorkloadStatus represents status of workloads whose HealthStatus is UNKNOWN. type: string required: diff --git a/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope.go b/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope.go index 5b8c5e16..aeae36da 100644 --- a/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope.go +++ b/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope.go @@ -18,6 +18,7 @@ package healthscope import ( "context" + "encoding/json" "fmt" "reflect" "time" @@ -30,6 +31,7 @@ import ( "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" @@ -43,12 +45,16 @@ const ( defaultTimeout = 10 * time.Second ) +// HealthStatus represents health status strings. type HealthStatus = v1alpha2.HealthStatus const ( - StatusHealthy = v1alpha2.StatusHealthy + // StatusHealthy represents healthy status. + StatusHealthy = v1alpha2.StatusHealthy + // StatusUnhealthy represents unhealthy status. StatusUnhealthy = v1alpha2.StatusUnhealthy - StatusUnknown = v1alpha2.StatusUnknown + // StatusUnknown represents unknown status. + StatusUnknown = v1alpha2.StatusUnknown ) var ( @@ -77,7 +83,7 @@ func (fn WorkloadHealthCheckFn) Check(ctx context.Context, c client.Client, tr r return fn(ctx, c, tr, ns) } -// CheckContainerziedWorkloadHealth check health status of ContainerizedWorkload +// CheckContainerziedWorkloadHealth check health condition of ContainerizedWorkload func CheckContainerziedWorkloadHealth(ctx context.Context, c client.Client, ref runtimev1alpha1.TypedReference, namespace string) *HealthCondition { if ref.GroupVersionKind() != corev1alpha2.SchemeGroupVersion.WithKind(kindContainerizedWorkload) { return nil @@ -89,10 +95,15 @@ func CheckContainerziedWorkloadHealth(ctx context.Context, c client.Client, 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 } - //TODO get component name from labels + compName, exist := cwObj.GetLabels()[oam.LabelAppComponent] + if !exist { + compName = "UNKNOWN NAME" + } + r.ComponentName = compName r.TargetWorkload.UID = cwObj.GetUID() subConditions := []*HealthCondition{} @@ -129,7 +140,7 @@ func CheckContainerziedWorkloadHealth(ctx context.Context, c client.Client, ref return r } -// CheckDeploymentHealth checks health status of Deployment +// CheckDeploymentHealth checks health condition of Deployment func CheckDeploymentHealth(ctx context.Context, client client.Client, ref runtimev1alpha1.TypedReference, namespace string) *HealthCondition { if ref.GroupVersionKind() != apps.SchemeGroupVersion.WithKind(kindDeployment) { return nil @@ -145,18 +156,29 @@ func CheckDeploymentHealth(ctx context.Context, client client.Client, ref runtim r.Diagnosis = errors.Wrap(err, errHealthCheck).Error() return r } + compName, exist := deployment.GetLabels()[oam.LabelAppComponent] + if !exist { + compName = "UNKNOWN NAME" + } + r.ComponentName = compName r.TargetWorkload.UID = deployment.GetUID() - r.Diagnosis = fmt.Sprintf(infoFmtReady, deployment.Status.ReadyReplicas, *deployment.Spec.Replicas) + + 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 != *deployment.Spec.Replicas { + if deployment.Status.ReadyReplicas != requiredReplicas { return r } r.HealthStatus = StatusHealthy return r } -// CheckStatefulsetHealth checks health status of StatefulSet +// CheckStatefulsetHealth checks health condition of StatefulSet func CheckStatefulsetHealth(ctx context.Context, client client.Client, ref runtimev1alpha1.TypedReference, namespace string) *HealthCondition { if ref.GroupVersionKind() != apps.SchemeGroupVersion.WithKind(kindStatefulSet) { return nil @@ -173,18 +195,27 @@ func CheckStatefulsetHealth(ctx context.Context, client client.Client, ref runti r.Diagnosis = errors.Wrap(err, errHealthCheck).Error() return r } + compName, exist := statefulset.GetLabels()[oam.LabelAppComponent] + if !exist { + compName = "UNKNOWN NAME" + } + r.ComponentName = compName r.TargetWorkload.UID = statefulset.GetUID() - r.Diagnosis = fmt.Sprintf(infoFmtReady, statefulset.Status.ReadyReplicas, *statefulset.Spec.Replicas) + requiredReplicas := int32(0) + if statefulset.Spec.Replicas != nil { + requiredReplicas = *statefulset.Spec.Replicas + } + r.Diagnosis = fmt.Sprintf(infoFmtReady, statefulset.Status.ReadyReplicas, requiredReplicas) // Health criteria - if statefulset.Status.ReadyReplicas != *statefulset.Spec.Replicas { + if statefulset.Status.ReadyReplicas != requiredReplicas { return r } - r.HealthStatus = StatusUnhealthy + r.HealthStatus = StatusHealthy return r } -// CheckDaemonsetHealth checks health status of DaemonSet +// CheckDaemonsetHealth checks health condition of DaemonSet func CheckDaemonsetHealth(ctx context.Context, client client.Client, ref runtimev1alpha1.TypedReference, namespace string) *HealthCondition { if ref.GroupVersionKind() != apps.SchemeGroupVersion.WithKind(kindDaemonSet) { return nil @@ -201,49 +232,30 @@ func CheckDaemonsetHealth(ctx context.Context, client client.Client, ref runtime r.Diagnosis = errors.Wrap(err, errHealthCheck).Error() return r } + compName, exist := daemonset.GetLabels()[oam.LabelAppComponent] + if !exist { + compName = "UNKNOWN NAME" + } + r.ComponentName = compName r.TargetWorkload.UID = daemonset.GetUID() r.Diagnosis = fmt.Sprintf(infoFmtReady, daemonset.Status.NumberReady, daemonset.Status.DesiredNumberScheduled) // Health criteria - if daemonset.Status.NumberReady != daemonset.Status.DesiredNumberScheduled { + if daemonset.Status.NumberUnavailable != 0 { return r } r.HealthStatus = StatusHealthy return r } -// func (r *Reconciler) listHealthCheckTrait(ctx context.Context, ns string) ([]unstructured.Unstructured, error) { -// //TODO Get HealthCheckTrait GVK dynamically -// hcTraitList := &unstructured.UnstructuredList{} -// hcTraitList.SetAPIVersion("extend.oam.dev/v1alpha2") -// hcTraitList.SetKind("HealthCheckTrait") -// -// if err := r.client.List(ctx, hcTraitList); err != nil { -// return nil, err -// } -// return hcTraitList.Items, nil -// -// } - -func (r *Reconciler) getHealthConditionFromTrait(ctx context.Context, wlRef runtimev1alpha1.TypedReference, ns string) (*HealthCondition, error) { - // get workload instance - - // get component name - - // get trait name by component name - - hcTraitList := &unstructured.UnstructuredList{} - hcTraitList.SetAPIVersion("extend.oam.dev/v1alpha2") - hcTraitList.SetKind("HealthCheckTrait") - - if err := r.client.List(ctx, hcTraitList); err != nil { - return nil, err - } - - return nil, nil +// CheckByHealthCheckTrait checks health condition through HealthCheckTrait. +func CheckByHealthCheckTrait(ctx context.Context, c client.Client, wlRef runtimev1alpha1.TypedReference, ns string) *HealthCondition { + //TODO(roywang) implement HealthCheckTrait feature + return nil } -func (r *Reconciler) getUnknownWorkloadHealthCondition(ctx context.Context, wlRef runtimev1alpha1.TypedReference, ns string) *HealthCondition { +// CheckUnknownWorkload handles unknown type workloads. +func CheckUnknownWorkload(ctx context.Context, c client.Client, wlRef runtimev1alpha1.TypedReference, ns string) *HealthCondition { healthCondition := &HealthCondition{ TargetWorkload: wlRef, HealthStatus: StatusUnknown, @@ -252,15 +264,19 @@ func (r *Reconciler) getUnknownWorkloadHealthCondition(ctx context.Context, wlRe wl := &unstructured.Unstructured{} wl.SetGroupVersionKind(wlRef.GroupVersionKind()) - if err := r.client.Get(ctx, client.ObjectKey{Namespace: ns, Name: wlRef.Name}, wl); err != nil { + if err := c.Get(ctx, client.ObjectKey{Namespace: ns, Name: wlRef.Name}, wl); err != nil { healthCondition.Diagnosis += errors.Wrap(err, errHealthCheck).Error() return healthCondition } - wlBytes, err := wl.MarshalJSON() + healthCondition.ComponentName = wl.GetLabels()[oam.LabelAppComponent] + + // 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.WorkloadInfo = string(wlBytes) + healthCondition.WorkloadStatus = string(wlStatusR) return healthCondition } diff --git a/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope_controller.go b/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope_controller.go index aeeab936..2d238720 100644 --- a/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope_controller.go +++ b/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope_controller.go @@ -46,7 +46,6 @@ const ( // Reconcile error strings. const ( errGetHealthScope = "cannot get health scope" - errMarshalScopeDiagnosiis = "cannot marshal diagnosis of the scope" errUpdateHealthScopeStatus = "cannot update health scope status" ) @@ -76,9 +75,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. @@ -98,6 +103,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) { @@ -111,15 +123,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) @@ -162,27 +176,12 @@ func (r *Reconciler) Reconcile(req reconcile.Request) (reconcile.Result, error) hs.Status.ScopeHealthCondition = scopeCondition hs.Status.HealthConditions = wlConditions - // if scopeCondition.HealthStatus { - // hs.Status.ScopeHealthCondition = "healthy" - // } else { - // hs.Status.ScopeHealthCondition = "unhealthy" - // } - - // diagnosis, err := json.Marshal(scopeCondition) - // 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))) - 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, []*HealthCondition) { + log := r.log.WithValues("get scope health status", healthScope.GetName()) scopeCondition := HealthCondition{ TargetWorkload: runtimev1alpha1.TypedReference{ APIVersion: healthScope.APIVersion, @@ -214,18 +213,26 @@ func (r *Reconciler) GetScopeHealthStatus(ctx context.Context, healthScope *v1al defer wg.Done() var wlHealthCondition *HealthCondition - //TODO get HealthCheckTrait status + 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 { wlHealthCondition = checker.Check(ctxWithTimeout, r.client, resRef, healthScope.GetNamespace()) if wlHealthCondition != nil { - // found matched checker and get health status + log.Debug("get health condition from built-in checker", "workload", resRef, "healthCondition", wlHealthCondition) + // found matched checker and get health condition workloadHealthConditionsC <- wlHealthCondition return } } - // unknown workload - workloadHealthConditionsC <- r.getUnknownWorkloadHealthCondition(ctx, resRef, healthScope.GetNamespace()) + // handle unknown workload + log.Debug("get unknown workload", "workload", resRef) + workloadHealthConditionsC <- r.unknownChecker.Check(ctx, r.client, resRef, healthScope.GetNamespace()) }(workloadRef) } @@ -245,6 +252,8 @@ func (r *Reconciler) GetScopeHealthStatus(ctx context.Context, healthScope *v1al unhealthyCount++ case StatusUnknown: unknownCount++ + default: + unknownCount++ } } if unhealthyCount > 0 || unknownCount > 0 { 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 416589ee..fc08ebff 100644 --- a/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope_controller_test.go +++ b/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope_controller_test.go @@ -155,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", @@ -227,7 +222,7 @@ var _ = Describe("Test GetScopeHealthStatus", func() { hs.Spec.WorkloadReferences = tc.hsWorkloadRefs result, _ := reconciler.GetScopeHealthStatus(ctx, &hs) Expect(result).ShouldNot(BeNil()) - Expect(result.HealthStatus).Should(Equal(true)) + Expect(result.HealthStatus).Should(Equal(StatusHealthy)) } }) @@ -254,21 +249,6 @@ 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 - }, - }, { caseName: "1 healthy supportted workload and 1 unsupportted workloads", hsWorkloadRefs: []v1alpha1.TypedReference{cwRef, uhGeneralRef}, @@ -295,7 +275,7 @@ var _ = Describe("Test GetScopeHealthStatus", func() { hs.Spec.WorkloadReferences = tc.hsWorkloadRefs result, _ := reconciler.GetScopeHealthStatus(ctx, &hs) Expect(result).ShouldNot(BeNil()) - Expect(result.HealthStatus).Should(Equal(false)) + Expect(result.HealthStatus).Should(Equal(HealthStatus(StatusUnhealthy))) } }) }) diff --git a/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope_test.go b/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope_test.go index 2e676cbe..3208a72e 100644 --- a/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope_test.go +++ b/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope_test.go @@ -41,6 +41,7 @@ const ( var ( ctx = context.Background() errMockErr = errors.New("get error") + varInt1 = int32(1) ) func TestCheckContainerziedWorkloadHealth(t *testing.T) { @@ -78,6 +79,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 }, @@ -99,6 +103,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 }, @@ -197,6 +204,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 }, @@ -214,6 +224,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 }, @@ -272,6 +285,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 }, @@ -289,6 +305,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 }, diff --git a/test/e2e-test/health_scope_test.go b/test/e2e-test/health_scope_test.go index 12417cd4..6d80d88f 100644 --- a/test/e2e-test/health_scope_test.go +++ b/test/e2e-test/health_scope_test.go @@ -296,7 +296,7 @@ var _ = Describe("HealthScope", func() { // return false //} - return healthScope.Status.ScopeHealthCondition.HealthStatus == "healthy" + return healthScope.Status.ScopeHealthCondition.HealthStatus == v1alpha2.StatusHealthy }, time.Second*120, time.Second*5).Should(BeEquivalentTo(true)) }) From 9925757e7bfd183bd2444d92e971fd5094d0364f Mon Sep 17 00:00:00 2001 From: roy wang Date: Thu, 3 Sep 2020 22:00:04 +0900 Subject: [PATCH 4/7] fix e2e-test Signed-off-by: roy wang --- test/e2e-test/appconfig_finalizer_test.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) 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{}) }) }) From 8a5852af8d06fe8be6635aec1409049f7bb98c11 Mon Sep 17 00:00:00 2001 From: roy wang Date: Fri, 4 Sep 2020 15:18:26 +0900 Subject: [PATCH 5/7] modify health scope condition modify e2e-test & unit test Signed-off-by: roy wang --- apis/core/v1alpha2/core_scope_types.go | 19 +++++-- apis/core/v1alpha2/zz_generated.deepcopy.go | 55 ++++++++++++------- .../crds/core.oam.dev_healthscopes.yaml | 51 ++++++----------- .../core/scopes/healthscope/healthscope.go | 50 +++++++++-------- .../healthscope/healthscope_controller.go | 25 +++------ .../healthscope_controller_test.go | 39 ++++++++++--- test/e2e-test/health_scope_test.go | 21 ++++--- 7 files changed, 145 insertions(+), 115 deletions(-) diff --git a/apis/core/v1alpha2/core_scope_types.go b/apis/core/v1alpha2/core_scope_types.go index 65648a2b..6c312e8f 100644 --- a/apis/core/v1alpha2/core_scope_types.go +++ b/apis/core/v1alpha2/core_scope_types.go @@ -54,14 +54,23 @@ type HealthScopeStatus struct { runtimev1alpha1.ConditionedStatus `json:",inline"` // ScopeHealthCondition represents health condition summary of the scope - ScopeHealthCondition HealthCondition `json:"scopeHealthCondition"` + ScopeHealthCondition ScopeHealthCondition `json:"scopeHealthCondition"` - // HealthConditions represents health condition of workloads in the scope - HealthConditions []*HealthCondition `json:"healthConditions,omitempty"` + // WorkloadHealthConditions represents health condition of workloads in the scope + WorkloadHealthConditions []*WorkloadHealthCondition `json:"healthConditions,omitempty"` } -// HealthCondition represents informative health condition. -type HealthCondition struct { +// 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"` diff --git a/apis/core/v1alpha2/zz_generated.deepcopy.go b/apis/core/v1alpha2/zz_generated.deepcopy.go index dae02ec9..04be7983 100644 --- a/apis/core/v1alpha2/zz_generated.deepcopy.go +++ b/apis/core/v1alpha2/zz_generated.deepcopy.go @@ -999,22 +999,6 @@ func (in *HTTPHeader) DeepCopy() *HTTPHeader { return out } -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *HealthCondition) DeepCopyInto(out *HealthCondition) { - *out = *in - out.TargetWorkload = in.TargetWorkload -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new HealthCondition. -func (in *HealthCondition) DeepCopy() *HealthCondition { - if in == nil { - return nil - } - out := new(HealthCondition) - in.DeepCopyInto(out) - return out -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *HealthScope) DeepCopyInto(out *HealthScope) { *out = *in @@ -1109,13 +1093,13 @@ func (in *HealthScopeStatus) DeepCopyInto(out *HealthScopeStatus) { *out = *in in.ConditionedStatus.DeepCopyInto(&out.ConditionedStatus) out.ScopeHealthCondition = in.ScopeHealthCondition - if in.HealthConditions != nil { - in, out := &in.HealthConditions, &out.HealthConditions - *out = make([]*HealthCondition, len(*in)) + 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(HealthCondition) + *out = new(WorkloadHealthCondition) **out = **in } } @@ -1333,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 @@ -1580,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 89a0e0b3..5310a5a9 100644 --- a/charts/oam-kubernetes-runtime/crds/core.oam.dev_healthscopes.yaml +++ b/charts/oam-kubernetes-runtime/crds/core.oam.dev_healthscopes.yaml @@ -120,10 +120,11 @@ spec: type: object type: array healthConditions: - description: HealthConditions represents health condition of workloads - in the scope + description: WorkloadHealthConditions represents health condition + of workloads in the scope items: - description: HealthCondition represents informative health condition. + description: WorkloadHealthCondition represents informative health + condition. properties: componentName: description: ComponentName represents the component name if @@ -168,41 +169,21 @@ spec: description: ScopeHealthCondition represents health condition summary of the scope 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 + healthyWorkloads: + format: int64 + type: integer + total: + format: int64 + type: integer + unhealthyWorkloads: + format: int64 + type: integer + unknownWorkloads: + format: int64 + type: integer required: - healthStatus type: object diff --git a/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope.go b/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope.go index aeae36da..8c0578d7 100644 --- a/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope.go +++ b/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope.go @@ -25,6 +25,7 @@ import ( 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" @@ -40,6 +41,7 @@ import ( const ( 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 @@ -66,7 +68,10 @@ var ( ) // HealthCondition holds health status of any resource -type HealthCondition = v1alpha2.HealthCondition +type HealthCondition = v1alpha2.WorkloadHealthCondition + +// 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. @@ -99,15 +104,18 @@ func CheckContainerziedWorkloadHealth(ctx context.Context, c client.Client, ref r.Diagnosis = errors.Wrap(err, errHealthCheck).Error() return r } - compName, exist := cwObj.GetLabels()[oam.LabelAppComponent] - if !exist { - compName = "UNKNOWN NAME" - } - r.ComponentName = compName + + r.ComponentName = getComponentNameFromLabel(&cwObj) r.TargetWorkload.UID = cwObj.GetUID() subConditions := []*HealthCondition{} 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 { @@ -156,11 +164,7 @@ func CheckDeploymentHealth(ctx context.Context, client client.Client, ref runtim r.Diagnosis = errors.Wrap(err, errHealthCheck).Error() return r } - compName, exist := deployment.GetLabels()[oam.LabelAppComponent] - if !exist { - compName = "UNKNOWN NAME" - } - r.ComponentName = compName + r.ComponentName = getComponentNameFromLabel(&deployment) r.TargetWorkload.UID = deployment.GetUID() requiredReplicas := int32(0) @@ -195,11 +199,7 @@ func CheckStatefulsetHealth(ctx context.Context, client client.Client, ref runti r.Diagnosis = errors.Wrap(err, errHealthCheck).Error() return r } - compName, exist := statefulset.GetLabels()[oam.LabelAppComponent] - if !exist { - compName = "UNKNOWN NAME" - } - r.ComponentName = compName + r.ComponentName = getComponentNameFromLabel(&statefulset) r.TargetWorkload.UID = statefulset.GetUID() requiredReplicas := int32(0) if statefulset.Spec.Replicas != nil { @@ -232,11 +232,7 @@ func CheckDaemonsetHealth(ctx context.Context, client client.Client, ref runtime r.Diagnosis = errors.Wrap(err, errHealthCheck).Error() return r } - compName, exist := daemonset.GetLabels()[oam.LabelAppComponent] - if !exist { - compName = "UNKNOWN NAME" - } - r.ComponentName = compName + r.ComponentName = getComponentNameFromLabel(&daemonset) r.TargetWorkload.UID = daemonset.GetUID() r.Diagnosis = fmt.Sprintf(infoFmtReady, daemonset.Status.NumberReady, daemonset.Status.DesiredNumberScheduled) @@ -265,7 +261,7 @@ func CheckUnknownWorkload(ctx context.Context, c client.Client, wlRef runtimev1a 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() + healthCondition.Diagnosis = errors.Wrap(err, errHealthCheck).Error() return healthCondition } healthCondition.ComponentName = wl.GetLabels()[oam.LabelAppComponent] @@ -274,9 +270,17 @@ func CheckUnknownWorkload(ctx context.Context, c client.Client, wlRef runtimev1a wlStatus, _, _ := unstructured.NestedMap(wl.UnstructuredContent(), "status") wlStatusR, err := json.Marshal(wlStatus) if err != nil { - healthCondition.Diagnosis += errors.Wrap(err, errHealthCheck).Error() + healthCondition.Diagnosis = errors.Wrap(err, errHealthCheck).Error() return healthCondition } healthCondition.WorkloadStatus = string(wlStatusR) return healthCondition } + +func getComponentNameFromLabel(o metav1.Object) string { + 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 2d238720..c329b263 100644 --- a/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope_controller.go +++ b/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope_controller.go @@ -18,7 +18,6 @@ package healthscope import ( "context" - "fmt" "strings" "sync" "time" @@ -49,10 +48,6 @@ const ( errUpdateHealthScopeStatus = "cannot update health scope status" ) -const ( - infoFmtScopeDiagnosis = "total workloads: %d, healthy workloads: %d, unhealthy workloads: %d, unknown workloads: %d" -) - // Reconcile event reasons. const ( reasonHealthCheck = "HealthCheck" @@ -174,21 +169,15 @@ func (r *Reconciler) Reconcile(req reconcile.Request) (reconcile.Result, error) elapsed := time.Since(start) hs.Status.ScopeHealthCondition = scopeCondition - hs.Status.HealthConditions = wlConditions + 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, []*HealthCondition) { +func (r *Reconciler) GetScopeHealthStatus(ctx context.Context, healthScope *v1alpha2.HealthScope) (ScopeHealthCondition, []*HealthCondition) { log := r.log.WithValues("get scope health status", healthScope.GetName()) - scopeCondition := HealthCondition{ - TargetWorkload: runtimev1alpha1.TypedReference{ - APIVersion: healthScope.APIVersion, - Kind: healthScope.Kind, - Name: healthScope.Name, - UID: healthScope.UID, - }, + scopeCondition := ScopeHealthCondition{ HealthStatus: StatusHealthy, //if no workload referenced, scope is healthy by default } scopeWLRefs := healthScope.Spec.WorkloadReferences @@ -241,7 +230,7 @@ func (r *Reconciler) GetScopeHealthStatus(ctx context.Context, healthScope *v1al close(workloadHealthConditionsC) }() - var healthyCount, unhealthyCount, unknownCount int + var healthyCount, unhealthyCount, unknownCount int64 workloadHealthConditions := []*HealthCondition{} for wlC := range workloadHealthConditionsC { workloadHealthConditions = append(workloadHealthConditions, wlC) @@ -260,6 +249,10 @@ func (r *Reconciler) GetScopeHealthStatus(ctx context.Context, healthScope *v1al // ANY unhealthy or unknown worloads make the whole scope unhealthy scopeCondition.HealthStatus = StatusUnhealthy } - scopeCondition.Diagnosis = fmt.Sprintf(infoFmtScopeDiagnosis, len(scopeWLRefs), healthyCount, unhealthyCount, unknownCount) + 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 fc08ebff..b496cab6 100644 --- a/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope_controller_test.go +++ b/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope_controller_test.go @@ -195,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)", @@ -211,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 { @@ -222,16 +230,17 @@ var _ = Describe("Test GetScopeHealthStatus", func() { hs.Spec.WorkloadReferences = tc.hsWorkloadRefs result, _ := reconciler.GetScopeHealthStatus(ctx, &hs) Expect(result).ShouldNot(BeNil()) - Expect(result.HealthStatus).Should(Equal(StatusHealthy)) + 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", @@ -248,6 +257,13 @@ var _ = Describe("Test GetScopeHealthStatus", func() { } return nil }, + wantScopeCondition: ScopeHealthCondition{ + HealthStatus: StatusUnhealthy, + Total: int64(2), + HealthyWorkloads: int64(1), + UnhealthyWorkloads: int64(1), + UnknownWorkloads: 0, + }, }, { caseName: "1 healthy supportted workload and 1 unsupportted workloads", @@ -263,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), + }, }, } @@ -275,7 +298,7 @@ var _ = Describe("Test GetScopeHealthStatus", func() { hs.Spec.WorkloadReferences = tc.hsWorkloadRefs result, _ := reconciler.GetScopeHealthStatus(ctx, &hs) Expect(result).ShouldNot(BeNil()) - Expect(result.HealthStatus).Should(Equal(HealthStatus(StatusUnhealthy))) + Expect(result).Should(Equal(tc.wantScopeCondition)) } }) }) diff --git a/test/e2e-test/health_scope_test.go b/test/e2e-test/health_scope_test.go index 6d80d88f..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.ScopeHealthCondition) - // TODO(artursouza): enable this check once crossplane is updated. - //if len(healthScope.Spec.WorkloadReferences) == 0 { - // return false - //} - - return healthScope.Status.ScopeHealthCondition.HealthStatus == v1alpha2.StatusHealthy + 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), + })) }) }) From 8b62ad51025053c63c2e682d71cc8d43487113be Mon Sep 17 00:00:00 2001 From: roy wang Date: Fri, 4 Sep 2020 18:01:59 +0900 Subject: [PATCH 6/7] fix typo Signed-off-by: roy wang --- .../core/scopes/healthscope/healthscope.go | 36 +++++++++---------- .../healthscope/healthscope_controller.go | 10 +++--- .../healthscope_controller_test.go | 8 ++--- .../scopes/healthscope/healthscope_test.go | 36 +++++++++---------- 4 files changed, 45 insertions(+), 45 deletions(-) diff --git a/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope.go b/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope.go index 8c0578d7..959cd2df 100644 --- a/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope.go +++ b/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope.go @@ -67,8 +67,8 @@ var ( kindDaemonSet = reflect.TypeOf(apps.DaemonSet{}).Name() ) -// HealthCondition holds health status of any resource -type HealthCondition = v1alpha2.WorkloadHealthCondition +// WorkloadHealthCondition holds health status of any resource +type WorkloadHealthCondition = v1alpha2.WorkloadHealthCondition // ScopeHealthCondition holds health condition of a scope type ScopeHealthCondition = v1alpha2.ScopeHealthCondition @@ -76,24 +76,24 @@ 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 condition of ContainerizedWorkload -func CheckContainerziedWorkloadHealth(ctx context.Context, c client.Client, ref runtimev1alpha1.TypedReference, namespace string) *HealthCondition { +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{ + r := &WorkloadHealthCondition{ HealthStatus: StatusHealthy, TargetWorkload: ref, } @@ -108,7 +108,7 @@ func CheckContainerziedWorkloadHealth(ctx context.Context, c client.Client, ref r.ComponentName = getComponentNameFromLabel(&cwObj) r.TargetWorkload.UID = cwObj.GetUID() - subConditions := []*HealthCondition{} + subConditions := []*WorkloadHealthCondition{} childRefs := cwObj.Status.Resources if len(childRefs) != 2 { // one deployment and one svc are required by containerizedworkload @@ -124,7 +124,7 @@ func CheckContainerziedWorkloadHealth(ctx context.Context, c client.Client, ref childCondition := CheckDeploymentHealth(ctx, c, childRef, namespace) subConditions = append(subConditions, childCondition) case kindService: - childCondition := &HealthCondition{ + childCondition := &WorkloadHealthCondition{ TargetWorkload: childRef, HealthStatus: StatusHealthy, } @@ -149,11 +149,11 @@ func CheckContainerziedWorkloadHealth(ctx context.Context, c client.Client, ref } // CheckDeploymentHealth checks health condition of Deployment -func CheckDeploymentHealth(ctx context.Context, client client.Client, ref runtimev1alpha1.TypedReference, namespace string) *HealthCondition { +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{ + r := &WorkloadHealthCondition{ HealthStatus: StatusUnhealthy, TargetWorkload: ref, } @@ -183,11 +183,11 @@ func CheckDeploymentHealth(ctx context.Context, client client.Client, ref runtim } // CheckStatefulsetHealth checks health condition of StatefulSet -func CheckStatefulsetHealth(ctx context.Context, client client.Client, ref runtimev1alpha1.TypedReference, namespace string) *HealthCondition { +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{ + r := &WorkloadHealthCondition{ HealthStatus: StatusUnhealthy, TargetWorkload: ref, } @@ -216,11 +216,11 @@ func CheckStatefulsetHealth(ctx context.Context, client client.Client, ref runti } // CheckDaemonsetHealth checks health condition of DaemonSet -func CheckDaemonsetHealth(ctx context.Context, client client.Client, ref runtimev1alpha1.TypedReference, namespace string) *HealthCondition { +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{ + r := &WorkloadHealthCondition{ HealthStatus: StatusUnhealthy, TargetWorkload: ref, } @@ -245,14 +245,14 @@ func CheckDaemonsetHealth(ctx context.Context, client client.Client, ref runtime } // CheckByHealthCheckTrait checks health condition through HealthCheckTrait. -func CheckByHealthCheckTrait(ctx context.Context, c client.Client, wlRef runtimev1alpha1.TypedReference, ns string) *HealthCondition { +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) *HealthCondition { - healthCondition := &HealthCondition{ +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), diff --git a/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope_controller.go b/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope_controller.go index c329b263..fa666937 100644 --- a/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope_controller.go +++ b/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope_controller.go @@ -175,14 +175,14 @@ func (r *Reconciler) Reconcile(req reconcile.Request) (reconcile.Result, error) } // GetScopeHealthStatus get the status of the healthscope based on workload resources. -func (r *Reconciler) GetScopeHealthStatus(ctx context.Context, healthScope *v1alpha2.HealthScope) (ScopeHealthCondition, []*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, []*HealthCondition{} + return scopeCondition, []*WorkloadHealthCondition{} } timeout := defaultTimeout @@ -193,14 +193,14 @@ func (r *Reconciler) GetScopeHealthStatus(ctx context.Context, healthScope *v1al defer cancel() // process workloads concurrently - workloadHealthConditionsC := 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 wlHealthCondition *HealthCondition + var wlHealthCondition *WorkloadHealthCondition wlHealthCondition = r.traitChecker.Check(ctx, r.client, resRef, healthScope.GetNamespace()) if wlHealthCondition != nil { @@ -231,7 +231,7 @@ func (r *Reconciler) GetScopeHealthStatus(ctx context.Context, healthScope *v1al }() var healthyCount, unhealthyCount, unknownCount int64 - workloadHealthConditions := []*HealthCondition{} + workloadHealthConditions := []*WorkloadHealthCondition{} for wlC := range workloadHealthConditionsC { workloadHealthConditions = append(workloadHealthConditions, wlC) switch wlC.HealthStatus { 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 b496cab6..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,13 +45,13 @@ var _ = Describe("HealthScope Controller Reconcile Test", func() { Client: &test.MockClient{}, } MockHealthyChecker := WorkloadHealthCheckFn( - func(context.Context, client.Client, v1alpha1.TypedReference, string) *HealthCondition { + func(context.Context, client.Client, v1alpha1.TypedReference, string) *WorkloadHealthCondition { - return &HealthCondition{HealthStatus: StatusHealthy} + return &WorkloadHealthCondition{HealthStatus: StatusHealthy} }) MockUnhealthyChecker := WorkloadHealthCheckFn( - func(context.Context, client.Client, v1alpha1.TypedReference, string) *HealthCondition { - return &HealthCondition{HealthStatus: StatusUnhealthy} + func(context.Context, client.Client, v1alpha1.TypedReference, string) *WorkloadHealthCondition { + return &WorkloadHealthCondition{HealthStatus: StatusUnhealthy} }) reconciler := NewReconciler(mockMgr, WithLogger(logging.NewNopLogger().WithValues("HealthScopeReconciler")), diff --git a/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope_test.go b/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope_test.go index 3208a72e..fb271f0c 100644 --- a/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope_test.go +++ b/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope_test.go @@ -62,7 +62,7 @@ func TestCheckContainerziedWorkloadHealth(t *testing.T) { caseName string mockGetFn test.MockGetFn wlRef runtimev1alpha1.TypedReference - expect *HealthCondition + expect *WorkloadHealthCondition }{ { caseName: "not matched checker", @@ -89,7 +89,7 @@ func TestCheckContainerziedWorkloadHealth(t *testing.T) { } return nil }, - expect: &HealthCondition{ + expect: &WorkloadHealthCondition{ HealthStatus: StatusHealthy, }, }, @@ -113,7 +113,7 @@ func TestCheckContainerziedWorkloadHealth(t *testing.T) { } return nil }, - expect: &HealthCondition{ + expect: &WorkloadHealthCondition{ HealthStatus: StatusUnhealthy, }, }, @@ -123,7 +123,7 @@ func TestCheckContainerziedWorkloadHealth(t *testing.T) { mockGetFn: func(ctx context.Context, key types.NamespacedName, obj runtime.Object) error { return errMockErr }, - expect: &HealthCondition{ + expect: &WorkloadHealthCondition{ HealthStatus: StatusUnhealthy, }, }, @@ -140,7 +140,7 @@ func TestCheckContainerziedWorkloadHealth(t *testing.T) { } return nil }, - expect: &HealthCondition{ + expect: &WorkloadHealthCondition{ HealthStatus: StatusUnhealthy, }, }, @@ -162,7 +162,7 @@ func TestCheckContainerziedWorkloadHealth(t *testing.T) { } return nil }, - expect: &HealthCondition{ + expect: &WorkloadHealthCondition{ HealthStatus: StatusUnhealthy, }, }, @@ -191,7 +191,7 @@ func TestCheckDeploymentHealth(t *testing.T) { caseName string mockGetFn test.MockGetFn wlRef runtimev1alpha1.TypedReference - expect *HealthCondition + expect *WorkloadHealthCondition }{ { caseName: "not matched checker", @@ -214,7 +214,7 @@ func TestCheckDeploymentHealth(t *testing.T) { } return nil }, - expect: &HealthCondition{ + expect: &WorkloadHealthCondition{ HealthStatus: StatusHealthy, }, }, @@ -234,7 +234,7 @@ func TestCheckDeploymentHealth(t *testing.T) { } return nil }, - expect: &HealthCondition{ + expect: &WorkloadHealthCondition{ HealthStatus: StatusUnhealthy, }, }, @@ -244,7 +244,7 @@ func TestCheckDeploymentHealth(t *testing.T) { mockGetFn: func(ctx context.Context, key types.NamespacedName, obj runtime.Object) error { return errMockErr }, - expect: &HealthCondition{ + expect: &WorkloadHealthCondition{ HealthStatus: StatusUnhealthy, }, }, @@ -272,7 +272,7 @@ func TestCheckStatefulsetHealth(t *testing.T) { caseName string mockGetFn test.MockGetFn wlRef runtimev1alpha1.TypedReference - expect *HealthCondition + expect *WorkloadHealthCondition }{ { caseName: "not matched checker", @@ -295,7 +295,7 @@ func TestCheckStatefulsetHealth(t *testing.T) { } return nil }, - expect: &HealthCondition{ + expect: &WorkloadHealthCondition{ HealthStatus: StatusHealthy, }, }, @@ -315,7 +315,7 @@ func TestCheckStatefulsetHealth(t *testing.T) { } return nil }, - expect: &HealthCondition{ + expect: &WorkloadHealthCondition{ HealthStatus: StatusUnhealthy, }, }, @@ -325,7 +325,7 @@ func TestCheckStatefulsetHealth(t *testing.T) { mockGetFn: func(ctx context.Context, key types.NamespacedName, obj runtime.Object) error { return errMockErr }, - expect: &HealthCondition{ + expect: &WorkloadHealthCondition{ HealthStatus: StatusUnhealthy, }, }, @@ -353,7 +353,7 @@ func TestCheckDaemonsetHealth(t *testing.T) { caseName string mockGetFn test.MockGetFn wlRef runtimev1alpha1.TypedReference - expect *HealthCondition + expect *WorkloadHealthCondition }{ { caseName: "not matched checker", @@ -373,7 +373,7 @@ func TestCheckDaemonsetHealth(t *testing.T) { } return nil }, - expect: &HealthCondition{ + expect: &WorkloadHealthCondition{ HealthStatus: StatusHealthy, }, }, @@ -390,7 +390,7 @@ func TestCheckDaemonsetHealth(t *testing.T) { } return nil }, - expect: &HealthCondition{ + expect: &WorkloadHealthCondition{ HealthStatus: StatusUnhealthy, }, }, @@ -400,7 +400,7 @@ func TestCheckDaemonsetHealth(t *testing.T) { mockGetFn: func(ctx context.Context, key types.NamespacedName, obj runtime.Object) error { return errMockErr }, - expect: &HealthCondition{ + expect: &WorkloadHealthCondition{ HealthStatus: StatusUnhealthy, }, }, From 6ad49849e80a04ff6ae00777a56c110713197eb5 Mon Sep 17 00:00:00 2001 From: roy wang Date: Mon, 7 Sep 2020 16:01:56 +0900 Subject: [PATCH 7/7] add unit test for health scope Signed-off-by: roy wang --- .../core/scopes/healthscope/healthscope.go | 7 +- .../scopes/healthscope/healthscope_test.go | 66 ++++++++++++++++++- 2 files changed, 70 insertions(+), 3 deletions(-) diff --git a/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope.go b/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope.go index 959cd2df..b7ecb2fb 100644 --- a/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope.go +++ b/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope.go @@ -143,7 +143,7 @@ func CheckContainerziedWorkloadHealth(ctx context.Context, c client.Client, ref if sc.HealthStatus != StatusHealthy { r.HealthStatus = StatusUnhealthy } - r.Diagnosis += sc.Diagnosis + r.Diagnosis = fmt.Sprintf("%s%s", r.Diagnosis, sc.Diagnosis) } return r } @@ -264,7 +264,7 @@ func CheckUnknownWorkload(ctx context.Context, c client.Client, wlRef runtimev1a healthCondition.Diagnosis = errors.Wrap(err, errHealthCheck).Error() return healthCondition } - healthCondition.ComponentName = wl.GetLabels()[oam.LabelAppComponent] + healthCondition.ComponentName = getComponentNameFromLabel(wl) // for unknown workloads, just show status instead of precise diagnosis wlStatus, _, _ := unstructured.NestedMap(wl.UnstructuredContent(), "status") @@ -278,6 +278,9 @@ func CheckUnknownWorkload(ctx context.Context, c client.Client, wlRef runtimev1a } func getComponentNameFromLabel(o metav1.Object) string { + if o == nil { + return "" + } compName, exist := o.GetLabels()[oam.LabelAppComponent] if !exist { compName = "" diff --git a/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope_test.go b/pkg/controller/v1alpha2/core/scopes/healthscope/healthscope_test.go index fb271f0c..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" ) @@ -418,3 +421,64 @@ func TestCheckDaemonsetHealth(t *testing.T) { }(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) + } +}