Feat: support change resource gc policy from onAppUpdate to Never (#4530)

Signed-off-by: Somefive <yd219913@alibaba-inc.com>
This commit is contained in:
Somefive
2022-08-02 20:17:47 +08:00
committed by GitHub
parent 233fe5e7a7
commit 53e5a3ff2d
19 changed files with 170 additions and 59 deletions

View File

@@ -62,12 +62,12 @@ type GarbageCollectPolicyRule struct {
// if one resource is specified with conflict strategies, strategy as component go first.
// 2) for ApplyOncePolicyRule only CompNames and ResourceTypes are used
type ResourcePolicyRuleSelector struct {
CompNames []string `json:"componentNames"`
CompTypes []string `json:"componentTypes"`
OAMResourceTypes []string `json:"oamTypes"`
TraitTypes []string `json:"traitTypes"`
ResourceTypes []string `json:"resourceTypes"`
ResourceNames []string `json:"resourceNames"`
CompNames []string `json:"componentNames,omitempty"`
CompTypes []string `json:"componentTypes,omitempty"`
OAMResourceTypes []string `json:"oamTypes,omitempty"`
TraitTypes []string `json:"traitTypes,omitempty"`
ResourceTypes []string `json:"resourceTypes,omitempty"`
ResourceNames []string `json:"resourceNames,omitempty"`
}
// Match check if current rule selector match the target resource

View File

@@ -82,6 +82,8 @@ type ManagedResource struct {
Data *runtime.RawExtension `json:"raw,omitempty"`
// Deleted marks the resource to be deleted
Deleted bool `json:"deleted,omitempty"`
// SkipGC marks the resource to skip gc
SkipGC bool `json:"skipGC,omitempty"`
}
// Equal check if two managed resource equals
@@ -215,8 +217,9 @@ func (in *ResourceTracker) ContainsManagedResource(rsc client.Object) bool {
}
// AddManagedResource add object to managed resources, if exists, update
func (in *ResourceTracker) AddManagedResource(rsc client.Object, metaOnly bool, creator common.ResourceCreatorRole) (updated bool) {
func (in *ResourceTracker) AddManagedResource(rsc client.Object, metaOnly bool, skipGC bool, creator common.ResourceCreatorRole) (updated bool) {
mr := newManagedResourceFromResource(rsc)
mr.SkipGC = skipGC
if !metaOnly {
mr.Data = &runtime.RawExtension{Object: rsc}
}

View File

@@ -156,16 +156,16 @@ func TestResourceTracker_ManagedResource(t *testing.T) {
r := require.New(t)
input := &ResourceTracker{}
deploy1 := v12.Deployment{ObjectMeta: v13.ObjectMeta{Name: "deploy1"}}
input.AddManagedResource(&deploy1, true, "")
input.AddManagedResource(&deploy1, true, false, "")
r.Equal(1, len(input.Spec.ManagedResources))
cm2 := v1.ConfigMap{ObjectMeta: v13.ObjectMeta{Name: "cm2"}}
input.AddManagedResource(&cm2, false, "")
input.AddManagedResource(&cm2, false, false, "")
r.Equal(2, len(input.Spec.ManagedResources))
pod3 := v1.Pod{ObjectMeta: v13.ObjectMeta{Name: "pod3"}}
input.AddManagedResource(&pod3, false, "")
input.AddManagedResource(&pod3, false, false, "")
r.Equal(3, len(input.Spec.ManagedResources))
deploy1.Spec.Replicas = pointer.Int32(5)
input.AddManagedResource(&deploy1, false, "")
input.AddManagedResource(&deploy1, false, false, "")
r.Equal(3, len(input.Spec.ManagedResources))
input.DeleteManagedResource(&cm2, false)
r.Equal(3, len(input.Spec.ManagedResources))

View File

@@ -105,6 +105,9 @@ spec:
description: 'Specific resourceVersion to which this reference
is made, if any. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#concurrency-control-and-consistency'
type: string
skipGC:
description: SkipGC marks the resource to skip gc
type: boolean
trait:
type: string
uid:

View File

@@ -105,6 +105,9 @@ spec:
description: 'Specific resourceVersion to which this reference
is made, if any. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#concurrency-control-and-consistency'
type: string
skipGC:
description: SkipGC marks the resource to skip gc
type: boolean
trait:
type: string
uid:

View File

@@ -105,6 +105,9 @@ spec:
description: 'Specific resourceVersion to which this reference
is made, if any. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#concurrency-control-and-consistency'
type: string
skipGC:
description: SkipGC marks the resource to skip gc
type: boolean
trait:
type: string
uid:

View File

@@ -43,7 +43,7 @@ func (h *resourceKeeper) DispatchComponentRevision(ctx context.Context, cr *v1.C
obj.SetName(cr.Name)
obj.SetNamespace(cr.Namespace)
obj.SetLabels(cr.Labels)
if err = resourcetracker.RecordManifestsInResourceTracker(multicluster.ContextInLocalCluster(ctx), h.Client, rt, []*unstructured.Unstructured{obj}, true, common.WorkflowResourceCreator); err != nil {
if err = resourcetracker.RecordManifestsInResourceTracker(multicluster.ContextInLocalCluster(ctx), h.Client, rt, []*unstructured.Unstructured{obj}, true, false, common.WorkflowResourceCreator); err != nil {
return errors.Wrapf(err, "failed to record componentrevision %s/%s/%s", oam.GetCluster(cr), cr.Namespace, cr.Name)
}
if err = h.Client.Create(auth.ContextWithUserInfo(multicluster.ContextWithClusterName(ctx, oam.GetCluster(cr)), h.app), cr); err != nil {

View File

@@ -72,19 +72,17 @@ func (h *resourceKeeper) Delete(ctx context.Context, manifests []*unstructured.U
func (h *resourceKeeper) delete(ctx context.Context, manifest *unstructured.Unstructured, cfg *deleteConfig) (err error) {
// 1. mark manifests as deleted in resourcetracker
if !cfg.skipRT {
var rt *v1beta1.ResourceTracker
if cfg.useRoot {
rt, err = h.getRootRT(ctx)
} else {
rt, err = h.getCurrentRT(ctx)
}
if err != nil {
return errors.Wrapf(err, "failed to get resourcetracker")
}
if err = resourcetracker.DeletedManifestInResourceTracker(multicluster.ContextInLocalCluster(ctx), h.Client, rt, manifest, false); err != nil {
return errors.Wrapf(err, "failed to delete resources in resourcetracker")
}
var rt *v1beta1.ResourceTracker
if cfg.useRoot || cfg.skipGC {
rt, err = h.getRootRT(ctx)
} else {
rt, err = h.getCurrentRT(ctx)
}
if err != nil {
return errors.Wrapf(err, "failed to get resourcetracker")
}
if err = resourcetracker.DeletedManifestInResourceTracker(multicluster.ContextInLocalCluster(ctx), h.Client, rt, manifest, false); err != nil {
return errors.Wrapf(err, "failed to delete resources in resourcetracker")
}
// 2. delete manifests
deleteCtx := multicluster.ContextWithClusterName(ctx, oam.GetCluster(manifest))

View File

@@ -82,6 +82,7 @@ func (h *resourceKeeper) Dispatch(ctx context.Context, manifests []*unstructured
func (h *resourceKeeper) record(ctx context.Context, manifests []*unstructured.Unstructured, options ...DispatchOption) error {
h.mu.Lock()
defer h.mu.Unlock()
var skipGCManifests []*unstructured.Unstructured
var rootManifests []*unstructured.Unstructured
var versionManifests []*unstructured.Unstructured
@@ -94,33 +95,37 @@ func (h *resourceKeeper) record(ctx context.Context, manifests []*unstructured.U
}
}
cfg := newDispatchConfig(_options...)
if !cfg.skipRT {
if cfg.useRoot {
rootManifests = append(rootManifests, manifest)
} else {
versionManifests = append(versionManifests, manifest)
}
switch {
case cfg.skipGC:
skipGCManifests = append(skipGCManifests, manifest)
case cfg.useRoot:
rootManifests = append(rootManifests, manifest)
default:
versionManifests = append(versionManifests, manifest)
}
}
}
cfg := newDispatchConfig(options...)
ctx = auth.ContextClearUserInfo(ctx)
if len(rootManifests) != 0 {
if len(rootManifests)+len(skipGCManifests) != 0 {
rt, err := h.getRootRT(ctx)
if err != nil {
return errors.Wrapf(err, "failed to get resourcetracker")
}
if err = resourcetracker.RecordManifestsInResourceTracker(multicluster.ContextInLocalCluster(ctx), h.Client, rt, rootManifests, cfg.metaOnly, cfg.creator); err != nil {
if err = resourcetracker.RecordManifestsInResourceTracker(multicluster.ContextInLocalCluster(ctx), h.Client, rt, rootManifests, cfg.metaOnly, false, cfg.creator); err != nil {
return errors.Wrapf(err, "failed to record resources in resourcetracker %s", rt.Name)
}
if err = resourcetracker.RecordManifestsInResourceTracker(multicluster.ContextInLocalCluster(ctx), h.Client, rt, skipGCManifests, cfg.metaOnly, true, cfg.creator); err != nil {
return errors.Wrapf(err, "failed to record resources (skip-gc) in resourcetracker %s", rt.Name)
}
}
rt, err := h.getCurrentRT(ctx)
if err != nil {
return errors.Wrapf(err, "failed to get resourcetracker")
}
if err = resourcetracker.RecordManifestsInResourceTracker(multicluster.ContextInLocalCluster(ctx), h.Client, rt, versionManifests, cfg.metaOnly, cfg.creator); err != nil {
if err = resourcetracker.RecordManifestsInResourceTracker(multicluster.ContextInLocalCluster(ctx), h.Client, rt, versionManifests, cfg.metaOnly, false, cfg.creator); err != nil {
return errors.Wrapf(err, "failed to record resources in resourcetracker %s", rt.Name)
}
return nil

View File

@@ -69,10 +69,10 @@ func TestResourceKeeperDispatchAndDelete(t *testing.T) {
r.NoError(rk.Dispatch(context.Background(), []*unstructured.Unstructured{cm1, cm2, cm3}, nil))
r.NotNil(rk._rootRT)
r.NotNil(rk._currentRT)
r.Equal(1, len(rk._rootRT.Spec.ManagedResources))
r.Equal(2, len(rk._rootRT.Spec.ManagedResources))
r.Equal(1, len(rk._currentRT.Spec.ManagedResources))
r.NoError(rk.Delete(context.Background(), []*unstructured.Unstructured{cm1, cm2, cm3}))
r.Equal(1, len(rk._rootRT.Spec.ManagedResources))
r.Equal(2, len(rk._rootRT.Spec.ManagedResources))
r.Equal(1, len(rk._currentRT.Spec.ManagedResources))
}

View File

@@ -346,6 +346,14 @@ func (h *gcHandler) deleteManagedResource(ctx context.Context, mr v1beta1.Manage
return nil
}
}
if mr.SkipGC {
if labels := entry.obj.GetLabels(); labels != nil {
delete(labels, oam.LabelAppName)
delete(labels, oam.LabelAppNamespace)
entry.obj.SetLabels(labels)
}
return errors.Wrapf(h.Client.Update(ctx, entry.obj), "failed to remove owner labels for resource while skipping gc")
}
if err := h.Client.Delete(_ctx, entry.obj); err != nil && !kerrors.IsNotFound(err) {
return errors.Wrapf(err, "failed to delete resource %s", mr.ResourceKey())
}

View File

@@ -189,7 +189,7 @@ var _ = Describe("Test ResourceKeeper garbage collection", func() {
}
By("Test delete normal resource")
o1 := createResource("o1", "app", namespace, "")
h._currentRT.AddManagedResource(o1, false, "test")
h._currentRT.AddManagedResource(o1, false, false, "test")
Expect(cli.Create(ctx, o1)).Should(Succeed())
h.cache.registerResourceTrackers(h._currentRT)
Expect(h.Finalize(ctx)).Should(Succeed())
@@ -199,7 +199,7 @@ var _ = Describe("Test ResourceKeeper garbage collection", func() {
By("Test delete resource shared by others")
o2 := createResource("o2", "app", namespace, fmt.Sprintf("%s/app,x/y", namespace))
h._currentRT.AddManagedResource(o2, false, "test")
h._currentRT.AddManagedResource(o2, false, false, "test")
Expect(cli.Create(ctx, o2)).Should(Succeed())
h.cache.registerResourceTrackers(h._currentRT)
Expect(h.Finalize(ctx)).Should(Succeed())
@@ -212,7 +212,7 @@ var _ = Describe("Test ResourceKeeper garbage collection", func() {
By("Test delete resource shared by self")
o3 := createResource("o3", "app", namespace, fmt.Sprintf("%s/app", namespace))
h._currentRT.AddManagedResource(o3, false, "test")
h._currentRT.AddManagedResource(o3, false, false, "test")
Expect(cli.Create(ctx, o3)).Should(Succeed())
h.cache.registerResourceTrackers(h._currentRT)
Expect(h.Finalize(ctx)).Should(Succeed())

View File

@@ -104,9 +104,9 @@ func TestResourceKeeperGarbageCollect(t *testing.T) {
obj.SetName(cr.GetName())
obj.SetNamespace(cr.GetNamespace())
obj.SetLabels(cr.GetLabels())
r.NoError(resourcetracker.RecordManifestsInResourceTracker(ctx, cli, crRT, []*unstructured.Unstructured{obj}, true, ""))
r.NoError(resourcetracker.RecordManifestsInResourceTracker(ctx, cli, crRT, []*unstructured.Unstructured{obj}, true, false, ""))
}
r.NoError(resourcetracker.RecordManifestsInResourceTracker(ctx, cli, _rt, []*unstructured.Unstructured{cmMaps[i]}, true, ""))
r.NoError(resourcetracker.RecordManifestsInResourceTracker(ctx, cli, _rt, []*unstructured.Unstructured{cmMaps[i]}, true, false, ""))
}
checkCount := func(cmCount, rtCount int, crCount int) {

View File

@@ -23,7 +23,7 @@ import (
type rtConfig struct {
useRoot bool
skipRT bool
skipGC bool
}
// MetaOnlyOption record only meta part in resourcetracker, which disables the configuration-drift-prevention
@@ -40,15 +40,14 @@ type CreatorOption struct {
// ApplyToDispatchConfig apply change to dispatch config
func (option CreatorOption) ApplyToDispatchConfig(cfg *dispatchConfig) { cfg.creator = option.Creator }
// SkipRTOption skip the rt recording during dispatch/delete resources, which means the resource will not be controlled
// by application resourcetracker
type SkipRTOption struct{}
// SkipGCOption marks the recorded resource to skip gc
type SkipGCOption struct{}
// ApplyToDispatchConfig apply change to dispatch config
func (option SkipRTOption) ApplyToDispatchConfig(cfg *dispatchConfig) { cfg.skipRT = true }
func (option SkipGCOption) ApplyToDispatchConfig(cfg *dispatchConfig) { cfg.skipGC = true }
// ApplyToDeleteConfig apply change to delete config
func (option SkipRTOption) ApplyToDeleteConfig(cfg *deleteConfig) { cfg.skipRT = true }
func (option SkipGCOption) ApplyToDeleteConfig(cfg *deleteConfig) { cfg.skipGC = true }
// UseRootOption let the recording and management of target resource belongs to the RootRT instead of VersionedRT. This
// will let the resource be alive as long as the application is still alive.
@@ -106,13 +105,13 @@ type GarbageCollectStrategyOption v1alpha1.GarbageCollectStrategy
func (option GarbageCollectStrategyOption) applyToRTConfig(cfg *rtConfig) {
switch v1alpha1.GarbageCollectStrategy(option) {
case v1alpha1.GarbageCollectStrategyOnAppUpdate:
cfg.skipRT = false
cfg.skipGC = false
cfg.useRoot = false
case v1alpha1.GarbageCollectStrategyOnAppDelete:
cfg.skipRT = false
cfg.skipGC = false
cfg.useRoot = true
case v1alpha1.GarbageCollectStrategyNever:
cfg.skipRT = true
cfg.skipGC = true
}
}

View File

@@ -31,9 +31,9 @@ func TestDispatchOptions(t *testing.T) {
option: MetaOnlyOption{},
cfg: dispatchConfig{metaOnly: true},
},
"skip-rt": {
option: SkipRTOption{},
cfg: dispatchConfig{rtConfig: rtConfig{skipRT: true}},
"skip-gc": {
option: SkipGCOption{},
cfg: dispatchConfig{rtConfig: rtConfig{skipGC: true}},
},
"use-root": {
option: UseRootOption{},
@@ -52,9 +52,9 @@ func TestDeleteOptions(t *testing.T) {
option DeleteOption
cfg deleteConfig
}{
"skip-rt": {
option: SkipRTOption{},
cfg: deleteConfig{rtConfig: rtConfig{skipRT: true}},
"skip-gc": {
option: SkipGCOption{},
cfg: deleteConfig{rtConfig: rtConfig{skipGC: true}},
},
"use-root": {
option: UseRootOption{},

View File

@@ -213,12 +213,16 @@ func RecordManifestsInResourceTracker(
rt *v1beta1.ResourceTracker,
manifests []*unstructured.Unstructured,
metaOnly bool,
skipGC bool,
creator common.ResourceCreatorRole) error {
if len(manifests) != 0 {
updated := false
for _, manifest := range manifests {
rt.AddManagedResource(manifest, metaOnly, creator)
updated = rt.AddManagedResource(manifest, metaOnly, skipGC, creator) || updated
}
if updated {
return cli.Update(ctx, rt)
}
return cli.Update(ctx, rt)
}
return nil
}

View File

@@ -100,7 +100,7 @@ func TestRecordAndDeleteManifestsInResourceTracker(t *testing.T) {
obj := &unstructured.Unstructured{}
obj.SetName(fmt.Sprintf("workload-%d", i))
objs = append(objs, obj)
r.NoError(RecordManifestsInResourceTracker(context.Background(), cli, rt, []*unstructured.Unstructured{obj}, rand.Int()%2 == 0, ""))
r.NoError(RecordManifestsInResourceTracker(context.Background(), cli, rt, []*unstructured.Unstructured{obj}, rand.Int()%2 == 0, false, ""))
}
rand.Shuffle(len(objs), func(i, j int) { objs[i], objs[j] = objs[j], objs[i] })
for i := 0; i < n; i++ {

View File

@@ -603,5 +603,61 @@ var _ = Describe("Test multicluster scenario", func() {
g.Expect(app.Status.Phase).Should(Equal(common.ApplicationRunning))
}, 20*time.Second).Should(Succeed())
})
It("Test applications with gc policy change (onAppUpdate -> never)", func() {
bs, err := ioutil.ReadFile("./testdata/app/app-gc-policy-change.yaml")
Expect(err).Should(Succeed())
appYaml := strings.ReplaceAll(string(bs), "TEST_NAMESPACE", testNamespace)
app := &v1beta1.Application{}
Expect(yaml.Unmarshal([]byte(appYaml), app)).Should(Succeed())
Expect(k8sClient.Create(hubCtx, 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.ApplicationRunning))
}, 20*time.Second).Should(Succeed())
By("update gc policy to never")
Eventually(func(g Gomega) {
g.Expect(k8sClient.Get(hubCtx, client.ObjectKeyFromObject(app), app))
gcPolicy := &v1alpha1.GarbageCollectPolicySpec{}
g.Expect(json.Unmarshal(app.Spec.Policies[0].Properties.Raw, gcPolicy)).Should(Succeed())
gcPolicy.Rules[0].Strategy = v1alpha1.GarbageCollectStrategyNever
bs, err = json.Marshal(gcPolicy)
g.Expect(err).Should(Succeed())
app.Spec.Policies[0].Properties = &runtime.RawExtension{Raw: bs}
g.Expect(k8sClient.Update(hubCtx, app)).Should(Succeed())
}, 10*time.Second).Should(Succeed())
By("check app updated and resource still exists")
Eventually(func(g Gomega) {
g.Expect(k8sClient.Get(hubCtx, client.ObjectKeyFromObject(app), app)).Should(Succeed())
g.Expect(app.Status.ObservedGeneration).Should(Equal(int64(2)))
g.Expect(app.Status.Phase).Should(Equal(common.ApplicationRunning))
g.Expect(k8sClient.Get(hubCtx, types.NamespacedName{Namespace: testNamespace, Name: "gc-policy-test"}, &corev1.ConfigMap{})).Should(Succeed())
}, 20*time.Second).Should(Succeed())
By("update app to new object")
Eventually(func(g Gomega) {
g.Expect(k8sClient.Get(hubCtx, client.ObjectKeyFromObject(app), app))
app.Spec.Components[0].Properties = &runtime.RawExtension{Raw: []byte(`{"objects":[{"apiVersion":"v1","kind":"ConfigMap","metadata":{"name":"another"},"data":{"key":"new-val"}}]}`)}
g.Expect(k8sClient.Update(hubCtx, app)).Should(Succeed())
}).Should(Succeed())
By("check app updated and resource still exists")
Eventually(func(g Gomega) {
g.Expect(k8sClient.Get(hubCtx, client.ObjectKeyFromObject(app), app)).Should(Succeed())
g.Expect(app.Status.ObservedGeneration).Should(Equal(int64(3)))
g.Expect(app.Status.Phase).Should(Equal(common.ApplicationRunning))
g.Expect(k8sClient.Get(hubCtx, types.NamespacedName{Namespace: testNamespace, Name: "gc-policy-test"}, &corev1.ConfigMap{})).Should(Succeed())
g.Expect(k8sClient.Get(hubCtx, types.NamespacedName{Namespace: testNamespace, Name: "another"}, &corev1.ConfigMap{})).Should(Succeed())
}, 20*time.Second).Should(Succeed())
By("delete app and check resource")
Eventually(func(g Gomega) {
g.Expect(client.IgnoreNotFound(k8sClient.Delete(hubCtx, app))).Should(Succeed())
g.Expect(k8sClient.Get(hubCtx, types.NamespacedName{Namespace: testNamespace, Name: "gc-policy-test"}, &corev1.ConfigMap{})).Should(Succeed())
g.Expect(k8sClient.Get(hubCtx, types.NamespacedName{Namespace: testNamespace, Name: "another"}, &corev1.ConfigMap{})).Should(Satisfy(kerrors.IsNotFound))
})
})
})
})

View File

@@ -0,0 +1,29 @@
apiVersion: core.oam.dev/v1beta1
kind: Application
metadata:
name: app-gc-policy-change-test
namespace: TEST_NAMESPACE
spec:
components:
- name: gc-policy-test
properties:
objects:
- apiVersion: v1
kind: ConfigMap
metadata:
name: gc-policy-test
data:
key: value
type: k8s-objects
policies:
- name: gc-policy
type: garbage-collect
properties:
rules:
- selector:
resourceTypes: ["ConfigMap"]
strategy: onAppUpdate
- name: topology
type: topology
properties:
clusters: ["local"]