From 3d2fcacb5a98af841b4b69995a0be204f6062c81 Mon Sep 17 00:00:00 2001 From: yangsoon Date: Wed, 24 Nov 2021 15:38:13 +0800 Subject: [PATCH] Feat: built in gc policy to application (#2575) --- .../core.oam.dev/v1alpha1/envbinding_types.go | 3 + docs/examples/gc-policy/gc.md | 116 ++++ pkg/appfile/appfile.go | 51 +- pkg/appfile/appfile_test.go | 2 +- pkg/appfile/parser.go | 23 +- .../application/application_controller.go | 12 +- .../v1alpha2/application/apply.go | 32 +- .../v1alpha2/application/dispatch/dispatch.go | 6 + .../v1alpha2/application/dispatch/gc.go | 106 +++- .../v1alpha2/application/dispatch/gc_test.go | 138 ++++ .../v1alpha2/application/gc_policy_test.go | 590 ++++++++++++++++++ .../v1alpha2/application/generator_test.go | 6 +- .../v1alpha2/application/revision.go | 2 +- 13 files changed, 1054 insertions(+), 33 deletions(-) create mode 100644 docs/examples/gc-policy/gc.md create mode 100644 pkg/controller/core.oam.dev/v1alpha2/application/dispatch/gc_test.go create mode 100644 pkg/controller/core.oam.dev/v1alpha2/application/gc_policy_test.go diff --git a/apis/core.oam.dev/v1alpha1/envbinding_types.go b/apis/core.oam.dev/v1alpha1/envbinding_types.go index 25a414c42..3f78178f3 100644 --- a/apis/core.oam.dev/v1alpha1/envbinding_types.go +++ b/apis/core.oam.dev/v1alpha1/envbinding_types.go @@ -25,6 +25,9 @@ import ( const ( // EnvBindingPolicyType refers to the type of EnvBinding EnvBindingPolicyType = "env-binding" + + // GarbageCollectPolicyType refers to the type of garbage-collect + GarbageCollectPolicyType = "garbage-collect" ) // EnvTraitPatch is the patch to trait diff --git a/docs/examples/gc-policy/gc.md b/docs/examples/gc-policy/gc.md new file mode 100644 index 000000000..ae77df6b7 --- /dev/null +++ b/docs/examples/gc-policy/gc.md @@ -0,0 +1,116 @@ +## How to use garbage-collect policy + +Suppose you want to keep the resources created by the old version of the app. You only need to specify garbage-collect in the policy field of the app and enable the option `keepLegacyResource`. + +```yaml +#app.yaml +apiVersion: core.oam.dev/v1beta1 +kind: Application +metadata: + name: first-vela-app +spec: + components: + - name: express-server + type: webservice + properties: + image: crccheck/hello-world + port: 8000 + traits: + - type: ingress-1-20 + properties: + domain: testsvc.example.com + http: + "/": 8000 + policies: + - name: keep-legacy-resource + type: garbage-collect + properties: + keepLegacyResource: true +``` + +1. create app + +``` shell +kubectl apply -f app.yaml +``` + +```shell +$ kubectl get app +NAME COMPONENT TYPE PHASE HEALTHY STATUS AGE +first-vela-app express-server webservice running true 29s +``` + +2. update the app + +```yaml +#app1.yaml +apiVersion: core.oam.dev/v1beta1 +kind: Application +metadata: + name: first-vela-app +spec: + components: + - name: express-server-1 + type: webservice + properties: + image: crccheck/hello-world + port: 8000 + traits: + - type: ingress-1-20 + properties: + domain: testsvc.example.com + http: + "/": 8000 + policies: + - name: keep-legacy-resource + type: garbage-collect + properties: + keepLegacyResource: true +``` + +``` shell +kubectl apply -f app1.yaml +``` + +```shell +$ kubectl get app +NAME COMPONENT TYPE PHASE HEALTHY STATUS AGE +first-vela-app express-server-1 webservice running true 9m35s +``` + +check whether legacy resources are reserved. + +``` +$ kubectl get deploy +NAME READY UP-TO-DATE AVAILABLE AGE +express-server 1/1 1 1 10m +express-server-1 1/1 1 1 40s +``` + +``` +$ kubectl get svc +NAME TYPE CLUSTER-IP EXTERNAL-IP PORT(S) AGE +express-server ClusterIP 10.96.102.249 8000/TCP 10m +express-server-1 ClusterIP 10.96.146.10 8000/TCP 46s +``` + +``` +$ kubectl get ingress +NAME CLASS HOSTS ADDRESS PORTS AGE +express-server testsvc.example.com 80 10m +express-server-1 testsvc.example.com 80 50s +``` + +``` +$ kubectl get resourcetrackers.core.oam.dev +NAME AGE +first-vela-app-default 12m +first-vela-app-v1-default 12m +first-vela-app-v2-default 2m56s +``` + +3. delete the app + +``` +$ kubectl delete app first-vela-app +``` diff --git a/pkg/appfile/appfile.go b/pkg/appfile/appfile.go index 2572254c4..12b8ba453 100644 --- a/pkg/appfile/appfile.go +++ b/pkg/appfile/appfile.go @@ -170,10 +170,23 @@ type Handler interface { } // PrepareWorkflowAndPolicy generates workflow steps and policies from an appFile -func (af *Appfile) PrepareWorkflowAndPolicy() (policies []*unstructured.Unstructured, err error) { - policies, err = af.generateUnstructureds(af.Policies) - if err != nil { - return +func (af *Appfile) PrepareWorkflowAndPolicy() ([]*Workload, []*unstructured.Unstructured, error) { + var externalPolicies []*unstructured.Unstructured + var builtInPolicies []*Workload + var err error + + for _, policy := range af.Policies { + switch policy.Type { + case v1alpha1.GarbageCollectPolicyType: + builtInPolicies = append(builtInPolicies, policy) + case v1alpha1.EnvBindingPolicyType: + default: + un, err := af.generateUnstructured(policy) + if err != nil { + return nil, nil, err + } + externalPolicies = append(externalPolicies, un) + } } af.WorkflowSteps, err = step.NewChainWorkflowStepGenerator( @@ -181,26 +194,22 @@ func (af *Appfile) PrepareWorkflowAndPolicy() (policies []*unstructured.Unstruct &step.ApplyComponentWorkflowStepGenerator{}, ).Generate(af.app, af.WorkflowSteps) - return policies, err + if err != nil { + return nil, nil, err + } + return builtInPolicies, externalPolicies, nil } -func (af *Appfile) generateUnstructureds(workloads []*Workload) ([]*unstructured.Unstructured, error) { - var uns []*unstructured.Unstructured - for _, wl := range workloads { - if wl.Type == v1alpha1.EnvBindingPolicyType { - continue - } - un, err := generateUnstructuredFromCUEModule(wl, af.Name, af.AppRevisionName, af.Namespace, af.Components, af.Artifacts) - if err != nil { - return nil, err - } - un.SetName(wl.Name) - if len(un.GetNamespace()) == 0 { - un.SetNamespace(af.Namespace) - } - uns = append(uns, un) +func (af *Appfile) generateUnstructured(workload *Workload) (*unstructured.Unstructured, error) { + un, err := generateUnstructuredFromCUEModule(workload, af.Name, af.AppRevisionName, af.Namespace, af.Components, af.Artifacts) + if err != nil { + return nil, err } - return uns, nil + un.SetName(workload.Name) + if len(un.GetNamespace()) == 0 { + un.SetNamespace(af.Namespace) + } + return un, nil } func generateUnstructuredFromCUEModule(wl *Workload, appName, revision, ns string, components []common.ApplicationComponent, artifacts []*types.ComponentManifest) (*unstructured.Unstructured, error) { diff --git a/pkg/appfile/appfile_test.go b/pkg/appfile/appfile_test.go index 0ba07fc56..2ea95f008 100644 --- a/pkg/appfile/appfile_test.go +++ b/pkg/appfile/appfile_test.go @@ -460,7 +460,7 @@ spec: } _, err := testAppfile.GenerateComponentManifests() Expect(err).Should(BeNil()) - gotPolicies, err := testAppfile.PrepareWorkflowAndPolicy() + _, gotPolicies, err := testAppfile.PrepareWorkflowAndPolicy() Expect(err).Should(BeNil()) Expect(len(gotPolicies)).ShouldNot(Equal(0)) diff --git a/pkg/appfile/parser.go b/pkg/appfile/parser.go index 63e74f598..8a42bc6e3 100644 --- a/pkg/appfile/parser.go +++ b/pkg/appfile/parser.go @@ -231,7 +231,13 @@ func (p *Parser) GenerateAppFileFromRevision(appRev *v1beta1.ApplicationRevision func (p *Parser) parsePolicies(ctx context.Context, policies []v1beta1.AppPolicy) ([]*Workload, error) { ws := []*Workload{} for _, policy := range policies { - w, err := p.makeWorkload(ctx, policy.Name, policy.Type, types.TypePolicy, policy.Properties) + var w *Workload + var err error + if policy.Type == "garbage-collect" { + w, err = p.makeBuiltInPolicy(policy.Name, policy.Type, policy.Properties) + } else { + w, err = p.makeWorkload(ctx, policy.Name, policy.Type, types.TypePolicy, policy.Properties) + } if err != nil { return nil, err } @@ -260,6 +266,21 @@ func (p *Parser) makeWorkload(ctx context.Context, name, typ string, capType typ return p.convertTemplate2Workload(name, typ, props, templ) } +func (p *Parser) makeBuiltInPolicy(name, typ string, props *runtime.RawExtension) (*Workload, error) { + settings, err := util.RawExtension2Map(props) + if err != nil { + return nil, errors.WithMessagef(err, "fail to parse settings for %s", name) + } + return &Workload{ + Traits: []*Trait{}, + ScopeDefinition: []*v1beta1.ScopeDefinition{}, + Name: name, + Type: typ, + Params: settings, + engine: definition.NewWorkloadAbstractEngine(name, p.pd), + }, nil +} + func (p *Parser) makeWorkloadFromRevision(name, typ string, capType types.CapType, props *runtime.RawExtension, appRev *v1beta1.ApplicationRevision) (*Workload, error) { templ, err := LoadTemplateFromRevision(typ, capType, appRev, p.dm) if err != nil { diff --git a/pkg/controller/core.oam.dev/v1alpha2/application/application_controller.go b/pkg/controller/core.oam.dev/v1alpha2/application/application_controller.go index 7c6937852..620df47e8 100644 --- a/pkg/controller/core.oam.dev/v1alpha2/application/application_controller.go +++ b/pkg/controller/core.oam.dev/v1alpha2/application/application_controller.go @@ -156,15 +156,21 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu } logCtx.Info("Successfully apply application revision") - policies, err := appFile.PrepareWorkflowAndPolicy() + builtInPolicies, externalPolicies, err := appFile.PrepareWorkflowAndPolicy() if err != nil { logCtx.Error(err, "[Handle PrepareWorkflowAndPolicy]") r.Recorder.Event(app, event.Warning(velatypes.ReasonFailedRender, err)) return r.endWithNegativeCondition(logCtx, app, condition.ErrorCondition("PrepareWorkflowAndPolicy", err), common.ApplicationPolicyGenerating) } - if len(policies) > 0 { - if err := handler.Dispatch(ctx, "", common.PolicyResourceCreator, policies...); err != nil { + if err := handler.HandleBuiltInPolicies(builtInPolicies); err != nil { + klog.Error(err, "[Handle BuiltIn Policies]") + r.Recorder.Event(app, event.Warning(velatypes.ReasonFailedRender, err)) + return r.endWithNegativeCondition(ctx, app, condition.ErrorCondition("HandleBuiltInPolicies", err), common.ApplicationPolicyGenerating) + } + + if len(externalPolicies) > 0 { + if err := handler.Dispatch(ctx, "", common.PolicyResourceCreator, externalPolicies...); err != nil { logCtx.Error(err, "[Handle ApplyPolicyResources]") r.Recorder.Event(app, event.Warning(velatypes.ReasonFailedApply, err)) return r.endWithNegativeCondition(logCtx, app, condition.ErrorCondition("ApplyPolices", err), common.ApplicationPolicyGenerating) diff --git a/pkg/controller/core.oam.dev/v1alpha2/application/apply.go b/pkg/controller/core.oam.dev/v1alpha2/application/apply.go index 5593e17fd..03d4dc11c 100644 --- a/pkg/controller/core.oam.dev/v1alpha2/application/apply.go +++ b/pkg/controller/core.oam.dev/v1alpha2/application/apply.go @@ -18,6 +18,7 @@ package application import ( "context" + "encoding/json" "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" @@ -54,6 +55,8 @@ type AppHandler struct { appliedResources []common.ClusterObjectReference deletedResources []common.ClusterObjectReference parser *appfile.Parser + + gcOptions dispatch.GCOptions } // Dispatch apply manifests into k8s. @@ -212,7 +215,7 @@ func (h *AppHandler) initDispatcher() { if h.dispatcher == nil { // only do GC when ALL resources are dispatched successfully // so skip GC while dispatching addon resources - h.dispatcher = dispatch.NewAppManifestsDispatcher(h.r.Client, h.currentAppRev).StartAndSkipGC(h.latestTracker) + h.dispatcher = dispatch.NewAppManifestsDispatcher(h.r.Client, h.currentAppRev).StartAndSkipGC(h.latestTracker).WithGCOptions(h.gcOptions) } } @@ -365,3 +368,30 @@ func (h *AppHandler) handleRollout(ctx context.Context) (reconcile.Result, error h.app.Status.Rollout = &appRollout.Status return res, nil } + +// HandleBuiltInPolicies handle built in policies +func (h *AppHandler) HandleBuiltInPolicies(policies []*appfile.Workload) error { + for _, policy := range policies { + if policy.Type == "garbage-collect" { + if err := h.SetGCOptions(policy.Params); err != nil { + return err + } + } + } + return nil +} + +// SetGCOptions set gc options for AppHandler +func (h *AppHandler) SetGCOptions(options map[string]interface{}) error { + bt, err := json.Marshal(options) + if err != nil { + return err + } + + gcOpts := dispatch.GCOptions{} + if err = json.Unmarshal(bt, &gcOpts); err != nil { + return err + } + h.gcOptions = gcOpts + return nil +} diff --git a/pkg/controller/core.oam.dev/v1alpha2/application/dispatch/dispatch.go b/pkg/controller/core.oam.dev/v1alpha2/application/dispatch/dispatch.go index 07555df4a..250d5e698 100644 --- a/pkg/controller/core.oam.dev/v1alpha2/application/dispatch/dispatch.go +++ b/pkg/controller/core.oam.dev/v1alpha2/application/dispatch/dispatch.go @@ -89,6 +89,12 @@ func (a *AppManifestsDispatcher) StartAndSkipGC(previousRT *v1beta1.ResourceTrac return a } +// WithGCOptions set gcOptions for AppManifestsDispatcher +func (a *AppManifestsDispatcher) WithGCOptions(gcOptions GCOptions) *AppManifestsDispatcher { + a.gcHandler.SetGCOptions(gcOptions) + return a +} + // Dispatch apply manifests into k8s and return a resource tracker recording applied manifests' references. // If GC is enabled, it will do GC after applying. // If 'UpgradeAndSkipGC' is enabled, it will: diff --git a/pkg/controller/core.oam.dev/v1alpha2/application/dispatch/gc.go b/pkg/controller/core.oam.dev/v1alpha2/application/dispatch/gc.go index 7ab6b38fe..68aa5d20b 100644 --- a/pkg/controller/core.oam.dev/v1alpha2/application/dispatch/gc.go +++ b/pkg/controller/core.oam.dev/v1alpha2/application/dispatch/gc.go @@ -24,6 +24,7 @@ import ( kerrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" types "k8s.io/apimachinery/pkg/types" "k8s.io/klog/v2" "sigs.k8s.io/controller-runtime/pkg/client" @@ -33,20 +34,31 @@ import ( "github.com/oam-dev/kubevela/pkg/oam" ) +// GCOptions contains options for gc +type GCOptions struct { + KeepLegacyResource bool `json:"keepLegacyResource,omitempty"` +} + // GarbageCollector do GC according two resource trackers type GarbageCollector interface { GarbageCollect(ctx context.Context, oldRT, newRT *v1beta1.ResourceTracker, legacyRTs []*v1beta1.ResourceTracker) error + SetGCOptions(gcOptions GCOptions) } // NewGCHandler create a GCHandler func NewGCHandler(c client.Client, ns string, appRev v1beta1.ApplicationRevision) *GCHandler { - return &GCHandler{c, ns, nil, nil, appRev} + return &GCHandler{ + c: c, + namespace: ns, + appRev: appRev, + } } // GCHandler implement GarbageCollector interface type GCHandler struct { c client.Client namespace string + gcOptions GCOptions oldRT *v1beta1.ResourceTracker newRT *v1beta1.ResourceTracker @@ -62,6 +74,22 @@ func (h *GCHandler) GarbageCollect(ctx context.Context, oldRT, newRT *v1beta1.Re return err } klog.InfoS("Garbage collect for application", "old", h.oldRT.Name, "new", h.newRT.Name) + + // if enabled KeepLegacyResource gc options, GCHandler will: + // 1. keep legacy resources created by old application + // 2. keep legacy resourceTrackers, if legacy resourceTracker not track any resources, delete it. + if h.gcOptions.KeepLegacyResource { + // if previous resourceTracker not track any resources, delete it + if !isTrackedResources(oldRT, newRT) { + if err := h.c.Delete(ctx, oldRT); err != nil && !kerrors.IsNotFound(err) { + klog.ErrorS(err, "Failed to delete resource tracker", "name", h.oldRT.Name) + return errors.Wrapf(err, "cannot delete resource tracker %q", oldRT.Name) + } + klog.InfoS("Successfully GC a resource tracker which not track any resources", "name", oldRT.Name) + } + // if legacy resourceTracker not track any resources, delete it. + return h.cleanUpResourceTracker(ctx, legacyRTs) + } for _, oldRsc := range h.oldRT.Status.TrackedResources { reused := false for _, newRsc := range h.newRT.Status.TrackedResources { @@ -131,7 +159,7 @@ func (h *GCHandler) handleResourceSkipGC(ctx context.Context, u *unstructured.Un res := u.DeepCopy() if err := h.c.Get(ctx, types.NamespacedName{Namespace: res.GetNamespace(), Name: res.GetName()}, res); err != nil { if !kerrors.IsNotFound(err) { - klog.ErrorS(err, "handleResourceSkipGC faied cannot get res kind ", res.GetKind(), "namespace", res.GetNamespace(), "name", res.GetName()) + klog.ErrorS(err, "handleResourceSkipGC failed cannot get res kind ", res.GetKind(), "namespace", res.GetNamespace(), "name", res.GetName()) return false, err } // resource have gone, skip delete it @@ -160,6 +188,42 @@ func (h *GCHandler) handleResourceSkipGC(ctx context.Context, u *unstructured.Un return true, nil } +// SetGCOptions set gc options for GCHandler +func (h *GCHandler) SetGCOptions(gcOptions GCOptions) { + h.gcOptions = gcOptions +} + +func isTrackedResources(oldRT *v1beta1.ResourceTracker, newRT *v1beta1.ResourceTracker) bool { + if len(oldRT.Status.TrackedResources) == 0 { + return false + } + if len(newRT.Status.TrackedResources) == 0 { + return true + } + type TrackedResourcesKey struct { + schema.GroupVersionKind + types.NamespacedName + } + resourceRecord := make(map[TrackedResourcesKey]bool) + for _, obj := range newRT.Status.TrackedResources { + objKey := TrackedResourcesKey{ + obj.GroupVersionKind(), + types.NamespacedName{Namespace: obj.Namespace, Name: obj.Name}, + } + resourceRecord[objKey] = true + } + for _, obj := range oldRT.Status.TrackedResources { + objKey := TrackedResourcesKey{ + obj.GroupVersionKind(), + types.NamespacedName{Namespace: obj.Namespace, Name: obj.Name}, + } + if !resourceRecord[objKey] { + return true + } + } + return false +} + func checkResourceRelatedCompDeleted(res unstructured.Unstructured, comps []common.ApplicationComponent) bool { compName := res.GetLabels()[oam.LabelAppComponent] deleted := true @@ -170,3 +234,41 @@ func checkResourceRelatedCompDeleted(res unstructured.Unstructured, comps []comm } return deleted } + +func (h *GCHandler) cleanUpResourceTracker(ctx context.Context, legacyRTs []*v1beta1.ResourceTracker) error { + for _, rt := range legacyRTs { + needDeleted := true + for _, resource := range rt.Status.TrackedResources { + rsc := new(unstructured.Unstructured) + rsc.SetGroupVersionKind(resource.GroupVersionKind()) + objKey := client.ObjectKey{Name: resource.Name, Namespace: resource.Namespace} + if err := h.c.Get(ctx, objKey, rsc); err != nil { + if kerrors.IsNotFound(err) { + continue + } + return err + } + if IsOwningObject(rsc, rt) { + needDeleted = false + break + } + } + if needDeleted { + if err := h.c.Delete(ctx, rt); err != nil { + return err + } + } + } + return nil +} + +// IsOwningObject check if owner owning the object +func IsOwningObject(obj metav1.Object, owner metav1.Object) bool { + ownerReferences := obj.GetOwnerReferences() + for _, ownerRef := range ownerReferences { + if ownerRef.UID == owner.GetUID() { + return true + } + } + return false +} diff --git a/pkg/controller/core.oam.dev/v1alpha2/application/dispatch/gc_test.go b/pkg/controller/core.oam.dev/v1alpha2/application/dispatch/gc_test.go new file mode 100644 index 000000000..bcaff9018 --- /dev/null +++ b/pkg/controller/core.oam.dev/v1alpha2/application/dispatch/gc_test.go @@ -0,0 +1,138 @@ +/* + Copyright 2021. The KubeVela Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package dispatch + +import ( + "testing" + + "github.com/stretchr/testify/assert" + corev1 "k8s.io/api/core/v1" + + "github.com/oam-dev/kubevela/apis/core.oam.dev/v1beta1" +) + +func TestIsTrackedResources(t *testing.T) { + testcases := []struct { + oldRT *v1beta1.ResourceTracker + newRT *v1beta1.ResourceTracker + expect bool + }{{ + oldRT: &v1beta1.ResourceTracker{ + Status: v1beta1.ResourceTrackerStatus{ + TrackedResources: []corev1.ObjectReference{{ + Kind: "Deployment", + APIVersion: "apps/v1", + Name: "test", + Namespace: "default", + }, { + Kind: "Pod", + APIVersion: "v1", + Name: "test", + Namespace: "default", + }}, + }, + }, + newRT: &v1beta1.ResourceTracker{ + Status: v1beta1.ResourceTrackerStatus{ + TrackedResources: []corev1.ObjectReference{{ + Kind: "Deployment", + APIVersion: "apps/v1", + Name: "test", + Namespace: "default", + }, { + Kind: "Pod", + APIVersion: "v1", + Name: "test", + Namespace: "default", + }}, + }, + }, + expect: false, + }, { + oldRT: &v1beta1.ResourceTracker{ + Status: v1beta1.ResourceTrackerStatus{ + TrackedResources: []corev1.ObjectReference{{ + Kind: "Deployment", + APIVersion: "apps/v1", + Name: "test", + Namespace: "default", + }, { + Kind: "Pod", + APIVersion: "v1", + Name: "hello", + Namespace: "default", + }}, + }, + }, + newRT: &v1beta1.ResourceTracker{ + Status: v1beta1.ResourceTrackerStatus{ + TrackedResources: []corev1.ObjectReference{{ + Kind: "Deployment", + APIVersion: "apps/v1", + Name: "test", + Namespace: "default", + }, { + Kind: "Pod", + APIVersion: "v1", + Name: "test", + Namespace: "default", + }}, + }, + }, + expect: true, + }, { + oldRT: &v1beta1.ResourceTracker{}, + newRT: &v1beta1.ResourceTracker{ + Status: v1beta1.ResourceTrackerStatus{ + TrackedResources: []corev1.ObjectReference{{ + Kind: "Deployment", + APIVersion: "apps/v1", + Name: "test", + Namespace: "default", + }, { + Kind: "Pod", + APIVersion: "v1", + Name: "test", + Namespace: "default", + }}, + }, + }, + expect: false, + }, { + oldRT: &v1beta1.ResourceTracker{ + Status: v1beta1.ResourceTrackerStatus{ + TrackedResources: []corev1.ObjectReference{{ + Kind: "Deployment", + APIVersion: "apps/v1", + Name: "test", + Namespace: "default", + }, { + Kind: "Pod", + APIVersion: "v1", + Name: "test", + Namespace: "default", + }}, + }, + }, + newRT: &v1beta1.ResourceTracker{}, + expect: true, + }} + + for _, testcase := range testcases { + assert.Equal(t, testcase.expect, isTrackedResources(testcase.oldRT, testcase.newRT)) + } +} diff --git a/pkg/controller/core.oam.dev/v1alpha2/application/gc_policy_test.go b/pkg/controller/core.oam.dev/v1alpha2/application/gc_policy_test.go new file mode 100644 index 000000000..182e97817 --- /dev/null +++ b/pkg/controller/core.oam.dev/v1alpha2/application/gc_policy_test.go @@ -0,0 +1,590 @@ +/* + Copyright 2021. The KubeVela Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package application + +import ( + "context" + "encoding/json" + "fmt" + "reflect" + "strconv" + "time" + + v1 "k8s.io/api/apps/v1" + v1beta12 "k8s.io/api/networking/v1beta1" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + "github.com/pkg/errors" + corev1 "k8s.io/api/core/v1" + kerrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + "sigs.k8s.io/yaml" + + "github.com/oam-dev/kubevela/apis/core.oam.dev/common" + "github.com/oam-dev/kubevela/apis/core.oam.dev/v1beta1" + "github.com/oam-dev/kubevela/pkg/oam" + "github.com/oam-dev/kubevela/pkg/oam/testutil" + "github.com/oam-dev/kubevela/pkg/oam/util" +) + +var _ = Describe("Test Application with GC options", func() { + ctx := context.Background() + + ns := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "vela-test-app-with-gc-options", + }, + } + + worker := &v1beta1.ComponentDefinition{} + workerCdDefJson, _ := yaml.YAMLToJSON([]byte(componentDefYaml)) + + ingressTrait := &v1beta1.TraitDefinition{} + ingressTdDefJson, _ := yaml.YAMLToJSON([]byte(ingressTraitDefYaml)) + + configMap := &v1beta1.TraitDefinition{} + configMapTdDefJson, _ := yaml.YAMLToJSON([]byte(configMapTraitDefYaml)) + + BeforeEach(func() { + Expect(k8sClient.Create(ctx, ns.DeepCopy())).Should(SatisfyAny(BeNil(), &util.AlreadyExistMatcher{})) + + Expect(json.Unmarshal(workerCdDefJson, worker)).Should(BeNil()) + Expect(k8sClient.Create(ctx, worker.DeepCopy())).Should(SatisfyAny(BeNil(), &util.AlreadyExistMatcher{})) + + Expect(json.Unmarshal(ingressTdDefJson, ingressTrait)).Should(BeNil()) + Expect(k8sClient.Create(ctx, ingressTrait.DeepCopy())).Should(SatisfyAny(BeNil(), &util.AlreadyExistMatcher{})) + + Expect(json.Unmarshal(configMapTdDefJson, configMap)).Should(BeNil()) + Expect(k8sClient.Create(ctx, configMap.DeepCopy())).Should(SatisfyAny(BeNil(), &util.AlreadyExistMatcher{})) + }) + + Context("Test Application enable gc option keepLegacyResource", func() { + baseApp := &v1beta1.Application{ + TypeMeta: metav1.TypeMeta{ + Kind: "Application", + APIVersion: "core.oam.dev/v1beta1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "baseApp", + }, + Spec: v1beta1.ApplicationSpec{ + Components: []common.ApplicationComponent{ + { + Name: "worker", + Type: "worker", + Properties: &runtime.RawExtension{Raw: []byte(`{"cmd":["sleep","1000"],"image":"busybox"}`)}, + Traits: []common.ApplicationTrait{{ + Type: "ingress-without-healthcheck", + Properties: &runtime.RawExtension{Raw: []byte(`{"domain":"test.com","http":{"/": 80}}`)}, + }}, + }, + }, + Policies: []v1beta1.AppPolicy{{ + Name: "keep-legacy-resource", + Type: "garbage-collect", + Properties: &runtime.RawExtension{Raw: []byte(`{"keepLegacyResource": true}`)}, + }}, + }, + } + + It("Each update will create a new workload and trait object", func() { + app := baseApp.DeepCopy() + app.SetNamespace(ns.Name) + app.SetName("app-with-worker-ingress") + + Expect(k8sClient.Create(ctx, app)).Should(BeNil()) + appV1 := new(v1beta1.Application) + Eventually(func() error { + _, err := testutil.ReconcileOnceAfterFinalizer(reconciler, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(app)}) + if err != nil { + return err + } + if err = k8sClient.Get(ctx, client.ObjectKeyFromObject(app), appV1); err != nil { + return err + } + if appV1.Status.Phase != common.ApplicationRunning { + return errors.New("app is not in running status") + } + return nil + }, 3*time.Second, 300*time.Second).Should(BeNil()) + + By("update app with new component name") + for i := 2; i <= 6; i++ { + Eventually(func() error { + oldApp := new(v1beta1.Application) + Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(app), oldApp)).Should(BeNil()) + updateApp := oldApp.DeepCopy() + updateApp.Spec.Components[0].Name = fmt.Sprintf("%s-v%d", "worker", i) + return k8sClient.Update(ctx, updateApp) + }, time.Second*3, time.Microsecond*300).Should(BeNil()) + + testutil.ReconcileRetry(reconciler, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(app)}) + newApp := new(v1beta1.Application) + Eventually(func() error { + _, err := testutil.ReconcileOnceAfterFinalizer(reconciler, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(app)}) + if err != nil { + return err + } + if err = k8sClient.Get(ctx, client.ObjectKeyFromObject(app), newApp); err != nil { + return err + } + if newApp.Status.Phase != common.ApplicationRunning { + return errors.New("app is not in running status") + } + return nil + }, 3*time.Second, 300*time.Second).Should(BeNil()) + Expect(newApp.Status.LatestRevision.Revision).Should(Equal(int64(i))) + + rtObjKey := client.ObjectKey{Name: fmt.Sprintf("%s-v%d-%s", app.Name, i, ns.Name)} + rt := new(v1beta1.ResourceTracker) + Expect(k8sClient.Get(ctx, rtObjKey, rt)).Should(BeNil()) + Expect(len(rt.Status.TrackedResources)).Should(Equal(3)) + + for _, obj := range rt.Status.TrackedResources { + un := new(unstructured.Unstructured) + un.SetGroupVersionKind(obj.GroupVersionKind()) + Expect(k8sClient.Get(ctx, client.ObjectKey{Name: obj.Name, Namespace: obj.Namespace}, un)).Should(BeNil()) + } + } + + By("check the resourceTrackers number") + listOpts := []client.ListOption{ + client.MatchingLabels{ + oam.LabelAppName: app.Name, + oam.LabelAppNamespace: app.Namespace, + }} + + rtList := &v1beta1.ResourceTrackerList{} + Expect(k8sClient.List(ctx, rtList, listOpts...)) + Expect(len(rtList.Items)).Should(Equal(7)) + + By("delete one resourceTracker to test the gc of legacy resources") + testRT := &v1beta1.ResourceTracker{ + ObjectMeta: metav1.ObjectMeta{ + Name: fmt.Sprintf("%s-v%d-%s", app.Name, 1, ns.Name), + }, + } + Expect(k8sClient.Delete(ctx, testRT)).Should(BeNil()) + for _, obj := range testRT.Status.TrackedResources { + un := &unstructured.Unstructured{} + un.SetGroupVersionKind(obj.GroupVersionKind()) + Eventually(func() error { + if err := k8sClient.Get(ctx, client.ObjectKey{Name: obj.Name, Namespace: obj.Name}, un); kerrors.IsNotFound(err) { + return nil + } + return errors.Errorf("failed to gc resource %v:%s", un.GroupVersionKind(), un.GetName()) + }, 3*time.Second, 300*time.Millisecond).Should(BeNil()) + } + + By("check the latest resources created by application") + latestApp := new(v1beta1.Application) + Expect(k8sClient.Get(ctx, client.ObjectKey{Name: app.Name, Namespace: app.Namespace}, latestApp)).Should(BeNil()) + appliedResource := latestApp.Status.AppliedResources + for _, obj := range appliedResource { + un := &unstructured.Unstructured{} + un.SetGroupVersionKind(obj.GroupVersionKind()) + Expect(k8sClient.Get(ctx, client.ObjectKey{Name: obj.Name, Namespace: obj.Namespace}, un)).Should(BeNil()) + } + + By("delete part legacy resource") + for i := 2; i <= 4; i++ { + deploy := new(v1.Deployment) + deploy.SetName(fmt.Sprintf("worker-v%d", i)) + deploy.SetNamespace(ns.Name) + Expect(k8sClient.Delete(ctx, deploy)) + + ingress := new(v1beta12.Ingress) + ingress.SetName(fmt.Sprintf("worker-v%d", i)) + ingress.SetNamespace(ns.Name) + Expect(k8sClient.Delete(ctx, ingress)) + + svc := new(corev1.Service) + svc.SetName(fmt.Sprintf("worker-v%d", i)) + svc.SetNamespace(ns.Name) + Expect(k8sClient.Delete(ctx, svc)) + } + + By("update app with new component name") + + Eventually(func() error { + oldApp := new(v1beta1.Application) + Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(app), oldApp)).Should(BeNil()) + updateApp := oldApp.DeepCopy() + updateApp.Spec.Components[0].Name = fmt.Sprintf("%s-v%d", "worker", 12) + return k8sClient.Update(ctx, updateApp) + }, time.Second*3, time.Microsecond*300).Should(BeNil()) + + testutil.ReconcileRetry(reconciler, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(app)}) + newApp := new(v1beta1.Application) + Eventually(func() error { + _, err := testutil.ReconcileOnceAfterFinalizer(reconciler, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(app)}) + if err != nil { + return err + } + if err = k8sClient.Get(ctx, client.ObjectKeyFromObject(app), newApp); err != nil { + return err + } + if newApp.Status.Phase != common.ApplicationRunning { + return errors.New("app is not in running status") + } + return nil + }, 3*time.Second, 300*time.Second).Should(BeNil()) + Expect(newApp.Status.LatestRevision.Revision).Should(Equal(int64(7))) + + By("check the resourceTrackers number") + newRTList := &v1beta1.ResourceTrackerList{} + Expect(k8sClient.List(ctx, newRTList, listOpts...)) + Expect(len(newRTList.Items)).Should(Equal(4)) + + By("delete all resources") + Expect(k8sClient.Delete(ctx, app)).Should(BeNil()) + testutil.ReconcileRetry(reconciler, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(app)}) + Expect(k8sClient.List(ctx, rtList, listOpts...)) + Expect(len(rtList.Items)).Should(Equal(0)) + }) + + It("Each update will create a new workload and update a trait object", func() { + app := baseApp.DeepCopy() + app.SetNamespace(ns.Name) + app.SetName("app-with-work-ingress-configmap") + + app.Spec.Components[0].Name = "job" + app.Spec.Components[0].Traits = append(app.Spec.Components[0].Traits, common.ApplicationTrait{ + Type: "configmap", + Properties: &runtime.RawExtension{Raw: []byte(`{"volumes": [{"name": "test-cm", "mountPath":"/tmp/test"}]}`)}, + }) + + Expect(k8sClient.Create(ctx, app)).Should(BeNil()) + appV1 := new(v1beta1.Application) + Eventually(func() error { + _, err := testutil.ReconcileOnceAfterFinalizer(reconciler, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(app)}) + if err != nil { + return err + } + if err = k8sClient.Get(ctx, client.ObjectKeyFromObject(app), appV1); err != nil { + return err + } + if appV1.Status.Phase != common.ApplicationRunning { + return errors.New("app is not in running status") + } + return nil + }, 3*time.Second, 300*time.Second).Should(BeNil()) + + By("update app's component name and the properties of configmap") + for i := 2; i <= 6; i++ { + Eventually(func() error { + oldApp := new(v1beta1.Application) + Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(app), oldApp)).Should(BeNil()) + updateApp := oldApp.DeepCopy() + updateApp.Spec.Components[0].Name = fmt.Sprintf("%s-v%d", "job", i) + updateApp.Spec.Components[0].Traits = append(app.Spec.Components[0].Traits, common.ApplicationTrait{ + Type: "configmap", + Properties: &runtime.RawExtension{Raw: []byte(fmt.Sprintf(`{"volumes": [{"name": "test-cm","mountPath": "/tmp/test","data": {"test": "%d"}}]}`, i))}, + }) + return k8sClient.Update(ctx, updateApp) + }, time.Second*3, time.Microsecond*300).Should(BeNil()) + + newApp := new(v1beta1.Application) + Eventually(func() error { + _, err := testutil.ReconcileOnceAfterFinalizer(reconciler, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(app)}) + if err != nil { + return err + } + if err = k8sClient.Get(ctx, client.ObjectKeyFromObject(app), newApp); err != nil { + return err + } + if newApp.Status.Phase != common.ApplicationRunning { + return errors.New("app is not in running status") + } + return nil + }, 5*time.Second, 300*time.Second).Should(BeNil()) + Expect(newApp.Status.LatestRevision.Revision).Should(Equal(int64(i))) + + rtObjKey := client.ObjectKey{Name: fmt.Sprintf("%s-v%d-%s", app.Name, i, ns.Name)} + rt := new(v1beta1.ResourceTracker) + Expect(k8sClient.Get(ctx, rtObjKey, rt)).Should(BeNil()) + Expect(len(rt.Status.TrackedResources)).Should(Equal(4)) + + for _, obj := range rt.Status.TrackedResources { + if obj.Kind == reflect.TypeOf(corev1.ConfigMap{}).Name() { + Expect(obj.Name).Should(Equal("test-cm")) + cm := new(corev1.ConfigMap) + Expect(k8sClient.Get(ctx, client.ObjectKey{Name: obj.Name, Namespace: obj.Namespace}, cm)).Should(BeNil()) + Expect(cm.Data["test"]).Should(Equal(strconv.Itoa(i))) + continue + } + un := new(unstructured.Unstructured) + un.SetGroupVersionKind(obj.GroupVersionKind()) + Expect(k8sClient.Get(ctx, client.ObjectKey{Name: obj.Name, Namespace: obj.Namespace}, un)).Should(BeNil()) + } + } + + By("check the resourceTrackers number") + listOpts := []client.ListOption{ + client.MatchingLabels{ + oam.LabelAppName: app.Name, + oam.LabelAppNamespace: app.Namespace, + }} + + rtList := &v1beta1.ResourceTrackerList{} + Expect(k8sClient.List(ctx, rtList, listOpts...)) + Expect(len(rtList.Items)).Should(Equal(7)) + + By("delete one resourceTracker to test the gc of legacy resources") + testRT := &v1beta1.ResourceTracker{ + ObjectMeta: metav1.ObjectMeta{ + Name: fmt.Sprintf("%s-v%d-%s", app.Name, 1, ns.Name), + }, + } + + Expect(k8sClient.Delete(ctx, testRT)).Should(BeNil()) + for _, obj := range testRT.Status.TrackedResources { + if obj.Kind == reflect.TypeOf(corev1.ConfigMap{}).Name() { + cm := new(corev1.ConfigMap) + Expect(k8sClient.Get(ctx, client.ObjectKey{Name: obj.Name, Namespace: obj.Namespace}, cm)).Should(BeNil()) + continue + } + un := &unstructured.Unstructured{} + un.SetGroupVersionKind(obj.GroupVersionKind()) + Eventually(func() error { + if err := k8sClient.Get(ctx, client.ObjectKey{Name: obj.Name, Namespace: obj.Name}, un); kerrors.IsNotFound(err) { + return nil + } + return errors.Errorf("failed to gc resource %v:%s", un.GroupVersionKind(), un.GetName()) + }, 3*time.Second, 300*time.Millisecond).Should(BeNil()) + } + + By("check the latest resources created by application") + latestApp := new(v1beta1.Application) + Expect(k8sClient.Get(ctx, client.ObjectKey{Name: app.Name, Namespace: app.Namespace}, latestApp)).Should(BeNil()) + appliedResource := latestApp.Status.AppliedResources + for _, obj := range appliedResource { + un := &unstructured.Unstructured{} + un.SetGroupVersionKind(obj.GroupVersionKind()) + Expect(k8sClient.Get(ctx, client.ObjectKey{Name: obj.Name, Namespace: obj.Namespace}, un)).Should(BeNil()) + } + + By("delete all resources") + Expect(k8sClient.Delete(ctx, app)).Should(BeNil()) + testutil.ReconcileRetry(reconciler, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(app)}) + Expect(k8sClient.List(ctx, rtList, listOpts...)) + Expect(len(rtList.Items)).Should(Equal(0)) + }) + + It("Each update will only update workload", func() { + app := baseApp.DeepCopy() + app.Spec.Components[0].Traits = nil + app.Spec.Components[0].Name = "only-work" + app.SetNamespace(ns.Name) + app.SetName("app-with-worker") + + Expect(k8sClient.Create(ctx, app)).Should(BeNil()) + appV1 := new(v1beta1.Application) + Eventually(func() error { + _, err := testutil.ReconcileOnceAfterFinalizer(reconciler, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(app)}) + if err != nil { + return err + } + if err = k8sClient.Get(ctx, client.ObjectKeyFromObject(app), appV1); err != nil { + return err + } + if appV1.Status.Phase != common.ApplicationRunning { + return errors.New("app is not in running status") + } + return nil + }, 3*time.Second, 300*time.Second).Should(BeNil()) + + By("update component with new properties") + for i := 2; i <= 11; i++ { + Eventually(func() error { + oldApp := new(v1beta1.Application) + Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(app), oldApp)).Should(BeNil()) + updateApp := oldApp.DeepCopy() + updateApp.Spec.Components[0].Properties = &runtime.RawExtension{Raw: []byte(fmt.Sprintf(`{"cmd":["sleep","%d"],"image":"busybox"}`, i))} + return k8sClient.Update(ctx, updateApp) + }, time.Second*3, time.Microsecond*300).Should(BeNil()) + + testutil.ReconcileRetry(reconciler, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(app)}) + newApp := new(v1beta1.Application) + Eventually(func() error { + _, err := testutil.ReconcileOnceAfterFinalizer(reconciler, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(app)}) + if err != nil { + return err + } + if err = k8sClient.Get(ctx, client.ObjectKeyFromObject(app), newApp); err != nil { + return err + } + if newApp.Status.Phase != common.ApplicationRunning { + return errors.New("app is not in running status") + } + return nil + }, 3*time.Second, 300*time.Second).Should(BeNil()) + Expect(newApp.Status.LatestRevision.Revision).Should(Equal(int64(i))) + + rtObjKey := client.ObjectKey{Name: fmt.Sprintf("%s-v%d-%s", app.Name, i, ns.Name)} + rt := new(v1beta1.ResourceTracker) + Expect(k8sClient.Get(ctx, rtObjKey, rt)).Should(BeNil()) + Expect(len(rt.Status.TrackedResources)).Should(Equal(1)) + + for _, obj := range rt.Status.TrackedResources { + un := new(unstructured.Unstructured) + un.SetGroupVersionKind(obj.GroupVersionKind()) + Expect(k8sClient.Get(ctx, client.ObjectKey{Name: obj.Name, Namespace: obj.Namespace}, un)).Should(BeNil()) + } + } + + By("check the resourceTrackers number") + listOpts := []client.ListOption{ + client.MatchingLabels{ + oam.LabelAppName: app.Name, + oam.LabelAppNamespace: app.Namespace, + }} + + rtList := &v1beta1.ResourceTrackerList{} + Expect(k8sClient.List(ctx, rtList, listOpts...)) + Expect(len(rtList.Items)).Should(Equal(2)) + + By("delete all resources") + Expect(k8sClient.Delete(ctx, app)).Should(BeNil()) + testutil.ReconcileRetry(reconciler, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(app)}) + Expect(k8sClient.List(ctx, rtList, listOpts...)) + Expect(len(rtList.Items)).Should(Equal(0)) + }) + }) +}) + +const ( + ingressTraitDefYaml = ` +apiVersion: core.oam.dev/v1beta1 +kind: TraitDefinition +metadata: + name: ingress-without-healthcheck + namespace: vela-system +spec: + schematic: + cue: + template: | + parameter: { + domain: string + http: [string]: int + } + // trait template can have multiple outputs in one trait + outputs: service: { + apiVersion: "v1" + kind: "Service" + metadata: + name: context.name + spec: { + selector: + app: context.name + ports: [ + for k, v in parameter.http { + port: v + targetPort: v + }, + ] + } + } + outputs: ingress: { + apiVersion: "networking.k8s.io/v1beta1" + kind: "Ingress" + metadata: + name: context.name + spec: { + rules: [{ + host: parameter.domain + http: { + paths: [ + for k, v in parameter.http { + path: k + backend: { + serviceName: context.name + servicePort: v + } + }, + ] + } + }] + } + } +` + configMapTraitDefYaml = ` +apiVersion: core.oam.dev/v1beta1 +kind: TraitDefinition +metadata: + annotations: + definition.oam.dev/description: Create/Attach configmaps on K8s pod for your workload which follows the pod spec in path 'spec.template'. + name: configmap + namespace: vela-system +spec: + appliesToWorkloads: + - '*' + podDisruptive: true + schematic: + cue: + template: | + patch: spec: template: spec: { + containers: [{ + // +patchKey=name + volumeMounts: [ + for v in parameter.volumes { + { + name: "volume-\(v.name)" + mountPath: v.mountPath + readOnly: v.readOnly + } + }, + ] + }, ...] + // +patchKey=name + volumes: [ + for v in parameter.volumes { + { + name: "volume-\(v.name)" + configMap: name: v.name + } + }, + ] + } + outputs: { + for v in parameter.volumes { + if v.data != _|_ { + "\(v.name)": { + apiVersion: "v1" + kind: "ConfigMap" + metadata: name: v.name + data: v.data + } + } + } + } + parameter: { + // +usage=Specify mounted configmap names and their mount paths in the container + volumes: [...{ + name: string + mountPath: string + readOnly: *false | bool + data?: [string]: string + }] + } +` +) diff --git a/pkg/controller/core.oam.dev/v1alpha2/application/generator_test.go b/pkg/controller/core.oam.dev/v1alpha2/application/generator_test.go index c34b81078..aef45dcc3 100644 --- a/pkg/controller/core.oam.dev/v1alpha2/application/generator_test.go +++ b/pkg/controller/core.oam.dev/v1alpha2/application/generator_test.go @@ -107,7 +107,7 @@ var _ = Describe("Test Application workflow generator", func() { } af, err := appParser.GenerateAppFile(ctx, app) Expect(err).Should(BeNil()) - _, err = af.PrepareWorkflowAndPolicy() + _, _, err = af.PrepareWorkflowAndPolicy() Expect(err).Should(BeNil()) appRev := &oamcore.ApplicationRevision{} @@ -151,7 +151,7 @@ var _ = Describe("Test Application workflow generator", func() { } af, err := appParser.GenerateAppFile(ctx, app) Expect(err).Should(BeNil()) - _, err = af.PrepareWorkflowAndPolicy() + _, _, err = af.PrepareWorkflowAndPolicy() Expect(err).Should(BeNil()) appRev := &oamcore.ApplicationRevision{} @@ -210,7 +210,7 @@ var _ = Describe("Test Application workflow generator", func() { } af, err := appParser.GenerateAppFile(ctx, app) Expect(err).Should(BeNil()) - _, err = af.PrepareWorkflowAndPolicy() + _, _, err = af.PrepareWorkflowAndPolicy() Expect(err).Should(BeNil()) apprev := &oamcore.ApplicationRevision{ ObjectMeta: metav1.ObjectMeta{ diff --git a/pkg/controller/core.oam.dev/v1alpha2/application/revision.go b/pkg/controller/core.oam.dev/v1alpha2/application/revision.go index 93b92730b..87888b5de 100644 --- a/pkg/controller/core.oam.dev/v1alpha2/application/revision.go +++ b/pkg/controller/core.oam.dev/v1alpha2/application/revision.go @@ -226,7 +226,7 @@ func (h *AppHandler) gatherRevisionSpec(af *appfile.Appfile) (*v1beta1.Applicati } } for _, p := range af.Policies { - if p == nil { + if p == nil || p.FullTemplate == nil { continue } if p.FullTemplate.PolicyDefinition != nil {