Skip GC proces when revision is still used by route

cmd/controller/main.go

@@ -107,6 +107,7 @@ func main() {
 			opt,
 			configurationInformer,
 			revisionInformer,
+			routeInformer,
 		),
 		revision.NewController(
 			opt,

pkg/reconciler/configuration/configuration.go

@@ -48,6 +48,7 @@ type Reconciler struct {
 	// listers index properties about resources
 	configurationLister listers.ConfigurationLister
 	revisionLister      listers.RevisionLister
+	routeLister         listers.RouteLister
 
 	configStore configStore
 }
@@ -310,13 +311,26 @@ func (c *Reconciler) gcRevisions(ctx context.Context, config *v1alpha1.Configura
 		return nil
 	}
 
+	var route *v1alpha1.Route
+
+	routeName, ok := config.GetLabels()[serving.RouteLabelKey]
+	if !ok {
+		logger.Warnf("%s does not have %q label", config.Name, serving.RouteLabelKey)
+	} else {
+		route, err = c.routeLister.Routes(config.Namespace).Get(routeName)
+		if err != nil && !errors.IsNotFound(err) {
+			// Go ahead GC process if route not found.
+			return err
+		}
+	}
+
 	// Sort by creation timestamp descending
 	sort.Slice(revs, func(i, j int) bool {
 		return revs[j].CreationTimestamp.Before(&revs[i].CreationTimestamp)
 	})
 
 	for _, rev := range revs[gcSkipOffset:] {
-		if isRevisionStale(ctx, rev, config) {
+		if isRevisionStale(ctx, rev, config, route) {
 			err := c.ServingClientSet.ServingV1alpha1().Revisions(rev.Namespace).Delete(rev.Name, &metav1.DeleteOptions{})
 			if err != nil {
 				logger.Errorf("Failed to delete stale revision: %v", err)
@@ -327,7 +341,7 @@ func (c *Reconciler) gcRevisions(ctx context.Context, config *v1alpha1.Configura
 	return nil
 }
 
-func isRevisionStale(ctx context.Context, rev *v1alpha1.Revision, config *v1alpha1.Configuration) bool {
+func isRevisionStale(ctx context.Context, rev *v1alpha1.Revision, config *v1alpha1.Configuration, route *v1alpha1.Route) bool {
 	cfg := configns.FromContext(ctx).RevisionGC
 	logger := logging.FromContext(ctx)
 
@@ -341,6 +355,14 @@ func isRevisionStale(ctx context.Context, rev *v1alpha1.Revision, config *v1alph
 		return false
 	}
 
+	if route != nil {
+		for _, v := range route.Status.Traffic {
+			if v.RevisionName == rev.Name {
+				return false
+			}
+		}
+	}
+
 	lastPin, err := rev.GetLastPinned()
 	if err != nil {
 		if err.(v1alpha1.LastPinnedParseError).Type != v1alpha1.AnnotationParseErrorTypeMissing {

pkg/reconciler/configuration/configuration_test.go

@@ -399,6 +399,7 @@ func TestReconcile(t *testing.T) {
 			Base:                reconciler.NewBase(opt, controllerAgentName),
 			configurationLister: listers.GetConfigurationLister(),
 			revisionLister:      listers.GetRevisionLister(),
+			routeLister:         listers.GetRouteLister(),
 			configStore: &testConfigStore{
 				config: ReconcilerTestConfig(),
 			},
@@ -537,6 +538,7 @@ func TestGCReconcile(t *testing.T) {
 			Base:                reconciler.NewBase(opt, controllerAgentName),
 			configurationLister: listers.GetConfigurationLister(),
 			revisionLister:      listers.GetRevisionLister(),
+			routeLister:         listers.GetRouteLister(),
 			configStore: &testConfigStore{
 				config: &config.Config{
 					RevisionGC: &gc.Config{
@@ -608,6 +610,7 @@ func TestIsRevisionStale(t *testing.T) {
 		name      string
 		rev       *v1alpha1.Revision
 		latestRev string
+		targetRev string
 		want      bool
 	}{{
 		name: "fresh revision that was never pinned",
@@ -652,6 +655,19 @@ func TestIsRevisionStale(t *testing.T) {
 			},
 		},
 		want: true,
+	}, {
+		name: "stale revision that was still used by route",
+		rev: &v1alpha1.Revision{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:              "myrev",
+				CreationTimestamp: metav1.NewTime(staleTime),
+				Annotations: map[string]string{
+					"serving.knative.dev/lastPinned": fmt.Sprintf("%d", staleTime.Unix()),
+				},
+			},
+		},
+		targetRev: "myrev",
+		want:      false,
 	}, {
 		name: "stale revision that was previously pinned",
 		rev: &v1alpha1.Revision{
@@ -712,7 +728,18 @@ func TestIsRevisionStale(t *testing.T) {
 				},
 			}
 
-			got := isRevisionStale(ctx, test.rev, cfg)
+			route := &v1alpha1.Route{
+				Status: v1alpha1.RouteStatus{
+					RouteStatusFields: v1alpha1.RouteStatusFields{
+						Traffic: []v1alpha1.TrafficTarget{{
+							TrafficTarget: v1beta1.TrafficTarget{
+								RevisionName: test.targetRev,
+							}}},
+					},
+				},
+			}
+
+			got := isRevisionStale(ctx, test.rev, cfg, route)
 
 			if got != test.want {
 				t.Errorf("IsRevisionStale want %v got %v", test.want, got)

pkg/reconciler/configuration/controller.go

@@ -40,12 +40,14 @@ func NewController(
 	opt reconciler.Options,
 	configurationInformer servinginformers.ConfigurationInformer,
 	revisionInformer servinginformers.RevisionInformer,
+	routeInformer servinginformers.RouteInformer,
 ) *controller.Impl {
 
 	c := &Reconciler{
 		Base:                reconciler.NewBase(opt, controllerAgentName),
 		configurationLister: configurationInformer.Lister(),
 		revisionLister:      revisionInformer.Lister(),
+		routeLister:         routeInformer.Lister(),
 	}
 	impl := controller.NewImpl(c, c.Logger, "Configurations")
 

pkg/reconciler/configuration/queueing_test.go

@@ -135,6 +135,7 @@ func newTestController(t *testing.T, stopCh chan struct{}) (
 		},
 		servingInformer.Serving().V1alpha1().Configurations(),
 		servingInformer.Serving().V1alpha1().Revisions(),
+		servingInformer.Serving().V1alpha1().Routes(),
 	)
 
 	return