From 188c550b2c9efe0a354a78e84ecb0187dd347628 Mon Sep 17 00:00:00 2001 From: Lvjiawei Date: Mon, 13 Jul 2020 23:05:59 +0800 Subject: [PATCH] Preserve Annotation to Prevent GC Fixes #8571 * Add preserve annotation to prevent GC from automatically deleting the revision. --- pkg/apis/serving/metadata_validation.go | 1 + pkg/apis/serving/metadata_validation_test.go | 8 +++++ pkg/apis/serving/register.go | 4 +++ pkg/reconciler/gc/v2/gc.go | 4 +++ pkg/reconciler/gc/v2/gc_test.go | 37 ++++++++++++++++++++ pkg/testing/v1/revision.go | 10 ++++++ 6 files changed, 64 insertions(+) diff --git a/pkg/apis/serving/metadata_validation.go b/pkg/apis/serving/metadata_validation.go index 0aede7a3be66..a4eef1b33069 100644 --- a/pkg/apis/serving/metadata_validation.go +++ b/pkg/apis/serving/metadata_validation.go @@ -39,6 +39,7 @@ var ( RevisionLastPinnedAnnotationKey, RoutingStateModifiedAnnotationKey, GroupNamePrefix+"forceUpgrade", + RevisionPreservedAnnotationKey, ) ) diff --git a/pkg/apis/serving/metadata_validation_test.go b/pkg/apis/serving/metadata_validation_test.go index 2c862ced811a..84d1e122b69c 100644 --- a/pkg/apis/serving/metadata_validation_test.go +++ b/pkg/apis/serving/metadata_validation_test.go @@ -133,6 +133,14 @@ func TestValidateObjectMetadata(t *testing.T) { RevisionLastPinnedAnnotationKey: "pinned-val", }, }, + }, { + name: "valid preserve annotation label", + objectMeta: &metav1.ObjectMeta{ + GenerateName: "some-name", + Annotations: map[string]string{ + RevisionLastPinnedAnnotationKey: "true", + }, + }, }, { name: "invalid knative prefix annotation", objectMeta: &metav1.ObjectMeta{ diff --git a/pkg/apis/serving/register.go b/pkg/apis/serving/register.go index 19d203f0c33a..48120c817474 100644 --- a/pkg/apis/serving/register.go +++ b/pkg/apis/serving/register.go @@ -33,6 +33,10 @@ const ( // pinned a revision RevisionLastPinnedAnnotationKey = GroupName + "/lastPinned" + // RevisionPreservedAnnotationKey is the annotation key used for preventing garbage collector + // from automatically deleting the revision. + RevisionPreservedAnnotationKey = GroupName + "/no-gc" + // RouteLabelKey is the label key attached to a Configuration indicating by // which Route it is configured as traffic target. // The key is also attached to Revision resources to indicate they are directly diff --git a/pkg/reconciler/gc/v2/gc.go b/pkg/reconciler/gc/v2/gc.go index 1fbfaf1b5a9e..2d27262ab5a1 100644 --- a/pkg/reconciler/gc/v2/gc.go +++ b/pkg/reconciler/gc/v2/gc.go @@ -19,6 +19,7 @@ package v2 import ( "context" "sort" + "strings" "time" "go.uber.org/zap" @@ -61,6 +62,9 @@ func Collect( }) for _, rev := range revs[gcSkipOffset:] { + if strings.EqualFold(rev.ObjectMeta.Annotations[serving.RevisionPreservedAnnotationKey], "true") { + continue + } if isRevisionStale(ctx, rev, config) { err := client.ServingV1().Revisions(rev.Namespace).Delete(rev.Name, &metav1.DeleteOptions{}) if err != nil { diff --git a/pkg/reconciler/gc/v2/gc_test.go b/pkg/reconciler/gc/v2/gc_test.go index e519ca3c70b3..2324730a2c55 100644 --- a/pkg/reconciler/gc/v2/gc_test.go +++ b/pkg/reconciler/gc/v2/gc_test.go @@ -208,6 +208,43 @@ func TestCollect(t *testing.T) { WithCreationTimestamp(oldest), WithLastPinned(old)), }, + }, { + name: "keep oldest because of the preserve annotation", + cfg: cfg("keep-oldest", "foo", 5556, + WithLatestCreated("5556"), + WithLatestReady("5556"), + WithConfigObservedGen), + revs: []*v1.Revision{ + rev("keep-oldest", "foo", 5554, MarkRevisionReady, + WithRevName("5554"), + WithCreationTimestamp(oldest), + WithLastPinned(oldest), + WithRevisionPreserveAnnotation()), + rev("keep-oldest", "foo", 5555, MarkRevisionReady, + WithRevName("5555"), + WithCreationTimestamp(older), + WithLastPinned(older)), + rev("keep-oldest", "foo", 5556, MarkRevisionReady, + WithRevName("5556"), + WithCreationTimestamp(old), + WithLastPinned(old)), + rev("keep-oldest", "foo", 5557, MarkRevisionReady, + WithRevName("5557"), + WithCreationTimestamp(tenMinutesAgo), + WithLastPinned(tenMinutesAgo)), + }, + wantDeletes: []clientgotesting.DeleteActionImpl{{ + ActionImpl: clientgotesting.ActionImpl{ + Namespace: "foo", + Verb: "delete", + Resource: schema.GroupVersionResource{ + Group: "serving.knative.dev", + Version: "v1", + Resource: "revisions", + }, + }, + Name: "5555", + }}, }} cfgMap := &config.Config{ diff --git a/pkg/testing/v1/revision.go b/pkg/testing/v1/revision.go index 646ac415236b..801d108e6337 100644 --- a/pkg/testing/v1/revision.go +++ b/pkg/testing/v1/revision.go @@ -92,6 +92,16 @@ func WithLastPinned(t time.Time) RevisionOption { } } +// WithRevisionPreserveAnnotation updates the annotation with preserve key. +func WithRevisionPreserveAnnotation() RevisionOption { + return func(rev *v1.Revision) { + rev.Annotations = kmeta.UnionMaps(rev.Annotations, + map[string]string{ + serving.RevisionPreservedAnnotationKey: "true", + }) + } +} + // WithRoutingStateModified updates the annotation to the provided timestamp. func WithRoutingStateModified(t time.Time) RevisionOption { return func(rev *v1.Revision) {