From a3f48425be1c8869be7b0bdf60fd4517d53ff74a Mon Sep 17 00:00:00 2001 From: Somefive Date: Fri, 8 Oct 2021 13:11:14 +0800 Subject: [PATCH] Feat: use #ApplyComponent for EnvBinding (#2382) * Feat: use #ApplyComponent in EnvBinding * Fix: application test compRev control by resourcetracker * Fix: add more detail in error info --- cmd/core/main.go | 3 +- .../envbinding/cluster_gateway_engine.go | 25 +++++--------- .../v1alpha2/application/dispatch/dispatch.go | 8 ++++- .../v1alpha2/application/generator.go | 20 ++++++----- .../v1alpha2/application/revision.go | 34 +++++++++++++++++-- pkg/oam/labels.go | 3 ++ pkg/stdlib/op.cue | 15 ++++---- pkg/stdlib/pkgs/oam.cue | 1 + pkg/workflow/providers/oam/apply.go | 8 +++-- pkg/workflow/providers/oam/apply_test.go | 2 +- test/e2e-test/application_test.go | 4 +-- 11 files changed, 80 insertions(+), 43 deletions(-) diff --git a/cmd/core/main.go b/cmd/core/main.go index b21b2b817..871a0e11a 100644 --- a/cmd/core/main.go +++ b/cmd/core/main.go @@ -30,6 +30,7 @@ import ( "strings" "time" + appsv1 "k8s.io/api/apps/v1" "k8s.io/klog/v2" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -206,7 +207,7 @@ func main() { LeaseDuration: &leaseDuration, RenewDeadline: &renewDeadline, RetryPeriod: &retryPeriod, - ClientDisableCacheFor: []client.Object{&v1beta1.ResourceTracker{}}, + ClientDisableCacheFor: []client.Object{&v1beta1.ResourceTracker{}, &appsv1.ControllerRevision{}}, }) if err != nil { klog.ErrorS(err, "Unable to create a controller manager") diff --git a/pkg/controller/core.oam.dev/v1alpha1/envbinding/cluster_gateway_engine.go b/pkg/controller/core.oam.dev/v1alpha1/envbinding/cluster_gateway_engine.go index b361f0425..d275a7206 100644 --- a/pkg/controller/core.oam.dev/v1alpha1/envbinding/cluster_gateway_engine.go +++ b/pkg/controller/core.oam.dev/v1alpha1/envbinding/cluster_gateway_engine.go @@ -22,6 +22,7 @@ import ( "github.com/pkg/errors" v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" @@ -79,30 +80,20 @@ func (engine *ClusterGatewayEngine) prepare(ctx context.Context, configs []v1alp } func (engine *ClusterGatewayEngine) initEnvBindApps(ctx context.Context, envBinding *v1alpha1.EnvBinding, baseApp *v1beta1.Application, appParser *appfile.Parser) ([]*EnvBindApp, error) { - envBindApps, err := CreateEnvBindApps(envBinding, baseApp) - if err != nil { - return nil, err - } - if err = RenderEnvBindApps(ctx, envBindApps, appParser); err != nil { - return nil, err - } - if err = AssembleEnvBindApps(envBindApps); err != nil { - return nil, err - } - return envBindApps, nil + return CreateEnvBindApps(envBinding, baseApp) } func (engine *ClusterGatewayEngine) schedule(ctx context.Context, apps []*EnvBindApp) ([]v1alpha1.ClusterDecision, error) { for _, app := range apps { app.ScheduledManifests = make(map[string]*unstructured.Unstructured) clusterName := engine.clusterDecisions[app.envConfig.Name].Cluster - for _, component := range app.PatchedApp.Spec.Components { - for _, manifest := range app.assembledManifests[component.Name] { - manifestName := component.Name + "/" + manifest.GetName() - multicluster.SetClusterName(manifest, clusterName) - app.ScheduledManifests[manifestName] = manifest - } + raw, err := runtime.DefaultUnstructuredConverter.ToUnstructured(app.PatchedApp) + if err != nil { + return nil, errors.Wrapf(err, "failed to convert app [Env: %s](%s/%s) into unstructured", app.envConfig.Name, app.PatchedApp.Namespace, app.PatchedApp.Name) } + patchedApp := &unstructured.Unstructured{Object: raw} + multicluster.SetClusterName(patchedApp, clusterName) + app.ScheduledManifests[patchedApp.GetName()] = patchedApp } var decisions []v1alpha1.ClusterDecision for _, decision := range engine.clusterDecisions { 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 474af7c6c..aeb13bc11 100644 --- a/pkg/controller/core.oam.dev/v1alpha2/application/dispatch/dispatch.go +++ b/pkg/controller/core.oam.dev/v1alpha2/application/dispatch/dispatch.go @@ -199,7 +199,7 @@ func (a *AppManifestsDispatcher) retrieveLegacyResourceTrackers(ctx context.Cont } for _, rt := range rtList.Items { if rt.Name != a.currentRTName && - (a.previousRT != nil && rt.Name != a.previousRT.Name) { + (a.previousRT != nil && rt.Name != a.previousRT.Name) && !IsLifeLongResourceTracker(rt) { a.legacyRTs = append(a.legacyRTs, rt.DeepCopy()) } } @@ -357,3 +357,9 @@ func setOrOverrideOAMControllerOwner(obj ObjectOwner, controllerOwner metav1.Own } obj.SetOwnerReferences(newOwnerRefs) } + +// IsLifeLongResourceTracker check if resourcetracker shares the same whole life with the entire application +func IsLifeLongResourceTracker(rt v1beta1.ResourceTracker) bool { + _, ok := rt.GetAnnotations()[oam.AnnotationResourceTrackerLifeLong] + return ok +} diff --git a/pkg/controller/core.oam.dev/v1alpha2/application/generator.go b/pkg/controller/core.oam.dev/v1alpha2/application/generator.go index 73d2a37bd..9d1b14e34 100644 --- a/pkg/controller/core.oam.dev/v1alpha2/application/generator.go +++ b/pkg/controller/core.oam.dev/v1alpha2/application/generator.go @@ -31,6 +31,7 @@ import ( "github.com/oam-dev/kubevela/pkg/controller/core.oam.dev/v1alpha2/application/assemble" "github.com/oam-dev/kubevela/pkg/cue/model/value" "github.com/oam-dev/kubevela/pkg/cue/packages" + "github.com/oam-dev/kubevela/pkg/multicluster" "github.com/oam-dev/kubevela/pkg/oam/discoverymapper" "github.com/oam-dev/kubevela/pkg/oam/util" "github.com/oam-dev/kubevela/pkg/utils" @@ -123,7 +124,8 @@ func convertStepProperties(step *v1beta1.WorkflowStep, app *v1beta1.Application) } func (h *AppHandler) applyComponentFunc(appParser *appfile.Parser, appRev *v1beta1.ApplicationRevision, af *appfile.Appfile, cli client.Client) oamProvider.ComponentApply { - return func(comp common.ApplicationComponent, patcher *value.Value) (*unstructured.Unstructured, []*unstructured.Unstructured, bool, error) { + return func(comp common.ApplicationComponent, patcher *value.Value, clusterName string) (*unstructured.Unstructured, []*unstructured.Unstructured, bool, error) { + ctx := multicluster.ContextWithClusterName(context.Background(), clusterName) wl, err := appParser.ParseWorkloadFromRevision(comp, appRev) if err != nil { @@ -137,11 +139,11 @@ func (h *AppHandler) applyComponentFunc(appParser *appfile.Parser, appRev *v1bet if err := af.SetOAMContract(manifest); err != nil { return nil, nil, false, errors.WithMessage(err, "SetOAMContract") } - if err := h.HandleComponentsRevision(context.TODO(), []*types.ComponentManifest{manifest}); err != nil { + if err := h.HandleComponentsRevision(ctx, []*types.ComponentManifest{manifest}); err != nil { return nil, nil, false, errors.WithMessage(err, "HandleComponentsRevision") } if len(manifest.PackagedWorkloadResources) != 0 { - if err := h.Dispatch(context.TODO(), "", common.WorkflowResourceCreator, manifest.PackagedWorkloadResources...); err != nil { + if err := h.Dispatch(ctx, clusterName, common.WorkflowResourceCreator, manifest.PackagedWorkloadResources...); err != nil { return nil, nil, false, errors.WithMessage(err, "cannot dispatch packaged workload resources") } } @@ -152,12 +154,12 @@ func (h *AppHandler) applyComponentFunc(appParser *appfile.Parser, appRev *v1bet skipStandardWorkload := skipApplyWorkload(wl) if !skipStandardWorkload { - if err := h.Dispatch(context.TODO(), "", common.WorkflowResourceCreator, readyWorkload); err != nil { + if err := h.Dispatch(ctx, clusterName, common.WorkflowResourceCreator, readyWorkload); err != nil { return nil, nil, false, errors.WithMessage(err, "DispatchStandardWorkload") } } - if err := h.Dispatch(context.TODO(), "", common.WorkflowResourceCreator, readyTraits...); err != nil { + if err := h.Dispatch(ctx, clusterName, common.WorkflowResourceCreator, readyTraits...); err != nil { return nil, nil, false, errors.WithMessage(err, "DispatchTraits") } @@ -169,7 +171,7 @@ func (h *AppHandler) applyComponentFunc(appParser *appfile.Parser, appRev *v1bet if !isHealth { return nil, nil, false, nil } - workload, traits, err := getComponentResources(manifest, skipStandardWorkload, cli) + workload, traits, err := getComponentResources(ctx, manifest, skipStandardWorkload, cli) return workload, traits, true, err } } @@ -183,14 +185,14 @@ func skipApplyWorkload(wl *appfile.Workload) bool { return false } -func getComponentResources(manifest *types.ComponentManifest, skipStandardWorkload bool, cli client.Client) (*unstructured.Unstructured, []*unstructured.Unstructured, error) { +func getComponentResources(ctx context.Context, manifest *types.ComponentManifest, skipStandardWorkload bool, cli client.Client) (*unstructured.Unstructured, []*unstructured.Unstructured, error) { var ( workload *unstructured.Unstructured traits []*unstructured.Unstructured ) if !skipStandardWorkload { v := manifest.StandardWorkload.DeepCopy() - if err := cli.Get(context.Background(), client.ObjectKeyFromObject(manifest.StandardWorkload), v); err != nil { + if err := cli.Get(ctx, client.ObjectKeyFromObject(manifest.StandardWorkload), v); err != nil { return nil, nil, err } workload = v @@ -198,7 +200,7 @@ func getComponentResources(manifest *types.ComponentManifest, skipStandardWorklo for _, trait := range manifest.Traits { v := trait.DeepCopy() - if err := cli.Get(context.Background(), client.ObjectKeyFromObject(trait), v); err != nil { + if err := cli.Get(ctx, client.ObjectKeyFromObject(trait), v); err != nil { return workload, nil, err } traits = append(traits, v) diff --git a/pkg/controller/core.oam.dev/v1alpha2/application/revision.go b/pkg/controller/core.oam.dev/v1alpha2/application/revision.go index bf34d8793..9f6152b55 100644 --- a/pkg/controller/core.oam.dev/v1alpha2/application/revision.go +++ b/pkg/controller/core.oam.dev/v1alpha2/application/revision.go @@ -582,6 +582,27 @@ func ComputeComponentRevisionHash(comp *types.ComponentManifest) (string, error) return utils.ComputeSpecHash(&compRevisionHash) } +// createOrGetResourceTracker create or get a resource tracker to manage all componentRevisions +func (h *AppHandler) createOrGetResourceTracker(ctx context.Context) (*v1beta1.ResourceTracker, error) { + rt := &v1beta1.ResourceTracker{} + rtName := h.app.Name + "-" + h.app.Namespace + if err := h.r.Get(ctx, ktypes.NamespacedName{Name: rtName}, rt); err != nil { + if !apierrors.IsNotFound(err) { + return nil, err + } + rt.SetName(rtName) + rt.SetLabels(map[string]string{ + oam.LabelAppName: h.app.Name, + oam.LabelAppNamespace: h.app.Namespace, + }) + rt.SetAnnotations(map[string]string{oam.AnnotationResourceTrackerLifeLong: "true"}) + if err = h.r.Create(ctx, rt); err != nil { + return nil, err + } + } + return rt, nil +} + // createControllerRevision records snapshot of a component func (h *AppHandler) createControllerRevision(ctx context.Context, cm *types.ComponentManifest) error { comp, err := componentManifest2Component(cm) @@ -589,6 +610,10 @@ func (h *AppHandler) createControllerRevision(ctx context.Context, cm *types.Com return err } revision, _ := utils.ExtractRevision(cm.RevisionName) + rt, err := h.createOrGetResourceTracker(ctx) + if err != nil { + return err + } cr := &appsv1.ControllerRevision{ ObjectMeta: metav1.ObjectMeta{ Name: cm.RevisionName, @@ -596,9 +621,9 @@ func (h *AppHandler) createControllerRevision(ctx context.Context, cm *types.Com OwnerReferences: []metav1.OwnerReference{ { APIVersion: v1beta1.SchemeGroupVersion.String(), - Kind: v1beta1.ApplicationKind, - Name: h.app.Name, - UID: h.app.UID, + Kind: v1beta1.ResourceTrackerKind, + Name: rt.GetName(), + UID: rt.GetUID(), Controller: pointer.BoolPtr(true), }, }, @@ -757,6 +782,9 @@ func gatherUsingAppRevision(ctx context.Context, h *AppHandler) (map[string]bool return nil, err } for _, rt := range rtList.Items { + if dispatch.IsLifeLongResourceTracker(rt) { + continue + } appRev := dispatch.ExtractAppRevisionName(rt.Name, ns) usingRevision[appRev] = true } diff --git a/pkg/oam/labels.go b/pkg/oam/labels.go index e3e1b0404..856ea7597 100644 --- a/pkg/oam/labels.go +++ b/pkg/oam/labels.go @@ -117,6 +117,9 @@ const ( // AnnotationDefinitionRevisionName is used to specify the name of DefinitionRevision in component/trait definition AnnotationDefinitionRevisionName = "definitionrevision.oam.dev/name" + // AnnotationResourceTrackerLifeLong is used to identify life-long resourcetracker which should only be recycled when application is deleted + AnnotationResourceTrackerLifeLong = "resourcetracker.oam.dev/life-long" + // AnnotationAddonsName records the name of initializer stored in configMap AnnotationAddonsName = "addons.oam.dev/name" ) diff --git a/pkg/stdlib/op.cue b/pkg/stdlib/op.cue index 4d82eb54f..13e62aaa9 100644 --- a/pkg/stdlib/op.cue +++ b/pkg/stdlib/op.cue @@ -109,13 +109,14 @@ import ( } } @step(3) - target: yaml.Unmarshal(configMap.value.data["\(env)"]) - apply: #Steps & { - for key, val in target { - "\(key)": kube.#Apply & { - value: val - if val.metadata.labels != _|_ && val.metadata.labels["cluster.oam.dev/clusterName"] != _|_ { - cluster: val.metadata.labels["cluster.oam.dev/clusterName"] + patchedApp: yaml.Unmarshal(configMap.value.data["\(env)"])[context.name] + components: patchedApp.spec.components + apply: #Steps & { + for key, comp in components { + "\(key)": #ApplyComponent & { + value: comp + if patchedApp.metadata.labels != _|_ && patchedApp.metadata.labels["cluster.oam.dev/clusterName"] != _|_ { + cluster: patchedApp.metadata.labels["cluster.oam.dev/clusterName"] } } @step(4) } diff --git a/pkg/stdlib/pkgs/oam.cue b/pkg/stdlib/pkgs/oam.cue index 101e8c4c2..b30d87ecc 100644 --- a/pkg/stdlib/pkgs/oam.cue +++ b/pkg/stdlib/pkgs/oam.cue @@ -1,6 +1,7 @@ #ApplyComponent: { #provider: "oam" #do: "component-apply" + cluster: *"" | string value: {...} patch?: {...} ... diff --git a/pkg/workflow/providers/oam/apply.go b/pkg/workflow/providers/oam/apply.go index 99dfcfd98..b21617b30 100644 --- a/pkg/workflow/providers/oam/apply.go +++ b/pkg/workflow/providers/oam/apply.go @@ -39,7 +39,7 @@ const ( ) // ComponentApply apply oam component. -type ComponentApply func(comp common.ApplicationComponent, patcher *value.Value) (*unstructured.Unstructured, []*unstructured.Unstructured, bool, error) +type ComponentApply func(comp common.ApplicationComponent, patcher *value.Value, clusterName string) (*unstructured.Unstructured, []*unstructured.Unstructured, bool, error) type provider struct { apply ComponentApply @@ -58,7 +58,11 @@ func (p *provider) ApplyComponent(ctx wfContext.Context, v *value.Value, act wfT return err } patcher, _ := v.LookupValue("patch") - workload, traits, healthy, err := p.apply(comp, patcher) + clusterName, err := v.GetString("cluster") + if err != nil { + clusterName = "" + } + workload, traits, healthy, err := p.apply(comp, patcher, clusterName) if err != nil { return err } diff --git a/pkg/workflow/providers/oam/apply_test.go b/pkg/workflow/providers/oam/apply_test.go index 39e132819..65a87581b 100644 --- a/pkg/workflow/providers/oam/apply_test.go +++ b/pkg/workflow/providers/oam/apply_test.go @@ -115,7 +115,7 @@ func TestLoadComponent(t *testing.T) { var testHealthy bool -func simpleComponentApplyForTest(comp common.ApplicationComponent, _ *value.Value) (*unstructured.Unstructured, []*unstructured.Unstructured, bool, error) { +func simpleComponentApplyForTest(comp common.ApplicationComponent, _ *value.Value, _ string) (*unstructured.Unstructured, []*unstructured.Unstructured, bool, error) { workload := new(unstructured.Unstructured) workload.UnmarshalJSON([]byte(`{ "apiVersion": "v1", diff --git a/test/e2e-test/application_test.go b/test/e2e-test/application_test.go index abc7c9eb9..5efe61ca3 100644 --- a/test/e2e-test/application_test.go +++ b/test/e2e-test/application_test.go @@ -138,8 +138,8 @@ var _ = Describe("Application Normal tests", func() { return fmt.Errorf("expect revision %d != real %d", revisionNum, gotCR.Revision) } ctrlOwner := metav1.GetControllerOf(gotCR) - if ctrlOwner == nil || ctrlOwner.Kind != v1beta1.ApplicationKind { - return fmt.Errorf("expect ControllerRevision is control-owned by an Application") + if ctrlOwner == nil || ctrlOwner.Kind != v1beta1.ResourceTrackerKind { + return fmt.Errorf("expect ControllerRevision is control-owned by a ResourceTracker") } return nil },