From bc1d24e034e0b78e453d43da201b3c451999561e Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue, 2 Aug 2022 11:00:57 +0800 Subject: [PATCH] [Backport release-1.5] Fix: empty health policy do not check object existence (#4522) * Fix: empty health policy do not check object existence Signed-off-by: Somefive (cherry picked from commit c759603094c2f35107fe0c9e7cdee049dd1b2894) * Fix: fix health check error Signed-off-by: Somefive (cherry picked from commit d46e49f9ef57c190e01b75827698ab05f84dab10) * Fix: add test Signed-off-by: Somefive (cherry picked from commit c29ded0765a3d7c315410b417f7f733ed488a82a) Co-authored-by: Somefive --- pkg/appfile/appfile.go | 5 +---- pkg/cue/definition/template.go | 20 ++++++----------- .../multicluster_test.go | 20 +++++++++++++++++ .../testdata/app/app-bad-resource.yaml | 22 +++++++++++++++++++ 4 files changed, 50 insertions(+), 17 deletions(-) create mode 100644 test/e2e-multicluster-test/testdata/app/app-bad-resource.yaml diff --git a/pkg/appfile/appfile.go b/pkg/appfile/appfile.go index b4157aebf..5aaa1d334 100644 --- a/pkg/appfile/appfile.go +++ b/pkg/appfile/appfile.go @@ -109,7 +109,7 @@ func (wl *Workload) EvalStatus(ctx process.Context, cli client.Client, accessor // EvalHealth eval workload health check func (wl *Workload) EvalHealth(ctx process.Context, client client.Client, accessor util.NamespaceAccessor) (bool, error) { // if health of template is not set or standard workload is managed by trait always return true - if wl.FullTemplate.Health == "" || wl.SkipApplyWorkload { + if wl.SkipApplyWorkload { return true, nil } return wl.engine.HealthCheck(ctx, client, accessor, wl.FullTemplate.Health) @@ -152,9 +152,6 @@ func (trait *Trait) EvalStatus(ctx process.Context, cli client.Client, accessor // EvalHealth eval trait health check func (trait *Trait) EvalHealth(ctx process.Context, client client.Client, accessor util.NamespaceAccessor) (bool, error) { - if trait.FullTemplate.Health == "" { - return true, nil - } return trait.engine.HealthCheck(ctx, client, accessor, trait.HealthCheckPolicy) } diff --git a/pkg/cue/definition/template.go b/pkg/cue/definition/template.go index 7cc19173f..d54afe385 100644 --- a/pkg/cue/definition/template.go +++ b/pkg/cue/definition/template.go @@ -186,7 +186,7 @@ func (wd *workloadDef) getTemplateContext(ctx process.Context, cli client.Reader return nil, err } // AuxiliaryWorkload will have a unique label("trait.oam.dev/resource"="name of outputs") in per component/app level - object, err := getResourceFromObj(ctx.GetCtx(), traitRef, cli, accessor.For(componentWorkload), util.MergeMapOverrideWithDst(map[string]string{ + object, err := getResourceFromObj(ctx.GetCtx(), traitRef, cli, accessor.For(traitRef), util.MergeMapOverrideWithDst(map[string]string{ oam.TraitTypeLabel: AuxiliaryWorkload, }, commonLabels), assist.Name) if err != nil { @@ -202,9 +202,6 @@ func (wd *workloadDef) getTemplateContext(ctx process.Context, cli client.Reader // HealthCheck address health check for workload func (wd *workloadDef) HealthCheck(ctx process.Context, cli client.Client, accessor util.NamespaceAccessor, healthPolicyTemplate string) (bool, error) { - if healthPolicyTemplate == "" { - return true, nil - } templateContext, err := wd.getTemplateContext(ctx, cli, accessor) if err != nil { return false, errors.WithMessage(err, "get template context") @@ -213,6 +210,9 @@ func (wd *workloadDef) HealthCheck(ctx process.Context, cli client.Client, acces } func checkHealth(templateContext map[string]interface{}, healthPolicyTemplate string) (bool, error) { + if healthPolicyTemplate == "" { + return true, nil + } bt, err := json.Marshal(templateContext) if err != nil { return false, errors.WithMessage(err, "json marshal template context") @@ -233,9 +233,6 @@ func checkHealth(templateContext map[string]interface{}, healthPolicyTemplate st // Status get workload status by customStatusTemplate func (wd *workloadDef) Status(ctx process.Context, cli client.Client, accessor util.NamespaceAccessor, customStatusTemplate string, parameter interface{}) (string, error) { - if customStatusTemplate == "" { - return "", nil - } templateContext, err := wd.getTemplateContext(ctx, cli, accessor) if err != nil { return "", errors.WithMessage(err, "get template context") @@ -244,6 +241,9 @@ func (wd *workloadDef) Status(ctx process.Context, cli client.Client, accessor u } func getStatusMessage(pd *packages.PackageDiscover, templateContext map[string]interface{}, customStatusTemplate string, parameter interface{}) (string, error) { + if customStatusTemplate == "" { + return "", nil + } bi := build.NewContext().NewInstance("", nil) var ctxBuff string var paramBuff = "parameter: {}\n" @@ -455,9 +455,6 @@ func (td *traitDef) getTemplateContext(ctx process.Context, cli client.Reader, a // Status get trait status by customStatusTemplate func (td *traitDef) Status(ctx process.Context, cli client.Client, accessor util.NamespaceAccessor, customStatusTemplate string, parameter interface{}) (string, error) { - if customStatusTemplate == "" { - return "", nil - } templateContext, err := td.getTemplateContext(ctx, cli, accessor) if err != nil { return "", errors.WithMessage(err, "get template context") @@ -467,9 +464,6 @@ func (td *traitDef) Status(ctx process.Context, cli client.Client, accessor util // HealthCheck address health check for trait func (td *traitDef) HealthCheck(ctx process.Context, cli client.Client, accessor util.NamespaceAccessor, healthPolicyTemplate string) (bool, error) { - if healthPolicyTemplate == "" { - return true, nil - } templateContext, err := td.getTemplateContext(ctx, cli, accessor) if err != nil { return false, errors.WithMessage(err, "get template context") diff --git a/test/e2e-multicluster-test/multicluster_test.go b/test/e2e-multicluster-test/multicluster_test.go index 299bf6128..4a4443b13 100644 --- a/test/e2e-multicluster-test/multicluster_test.go +++ b/test/e2e-multicluster-test/multicluster_test.go @@ -570,5 +570,25 @@ var _ = Describe("Test multicluster scenario", func() { g.Expect(k8sClient.Get(hubCtx, types.NamespacedName{Namespace: namespace, Name: "non-shared-app3"}, &corev1.ConfigMap{})).Should(Satisfy(kerrors.IsNotFound)) }, 10*time.Second).Should(Succeed()) }) + + It("Test applications with bad resource", func() { + bs, err := ioutil.ReadFile("./testdata/app/app-bad-resource.yaml") + Expect(err).Should(Succeed()) + appYaml := strings.ReplaceAll(string(bs), "TEST_NAMESPACE", testNamespace) + app := &v1beta1.Application{} + Expect(yaml.Unmarshal([]byte(appYaml), app)).Should(Succeed()) + ctx := context.Background() + Expect(k8sClient.Create(ctx, app)).Should(Succeed()) + Eventually(func(g Gomega) { + g.Expect(k8sClient.Get(hubCtx, client.ObjectKeyFromObject(app), app)).Should(Succeed()) + g.Expect(app.Status.Phase).Should(Equal(common.ApplicationRunningWorkflow)) + g.Expect(len(app.Status.Workflow.Steps) > 0).Should(BeTrue()) + g.Expect(app.Status.Workflow.Steps[0].Message).Should(ContainSubstring("is invalid")) + }, 20*time.Second).Should(Succeed()) + Expect(k8sClient.Delete(ctx, app)).Should(Succeed()) + Eventually(func(g Gomega) { + g.Expect(k8sClient.Get(hubCtx, client.ObjectKeyFromObject(app), app)).Should(Satisfy(kerrors.IsNotFound)) + }, 10*time.Second).Should(Succeed()) + }) }) }) diff --git a/test/e2e-multicluster-test/testdata/app/app-bad-resource.yaml b/test/e2e-multicluster-test/testdata/app/app-bad-resource.yaml new file mode 100644 index 000000000..f630ffbd9 --- /dev/null +++ b/test/e2e-multicluster-test/testdata/app/app-bad-resource.yaml @@ -0,0 +1,22 @@ +apiVersion: core.oam.dev/v1beta1 +kind: Application +metadata: + name: bad-resource + namespace: TEST_NAMESPACE +spec: + components: + - name: bad-resource + properties: + objects: + - apiVersion: apiregistration.k8s.io/v1beta1 + kind: APIService + metadata: + name: test-bad-resource + spec: + group: bad + type: k8s-objects + policies: + - name: topology + type: topology + properties: + clusters: ["local"]