Repository: knative/serving
Pull request #10107 (2020-11-13 10:35:58)

Poll aggressively, but don't log aggressively
We poll [every 50ms](https://github.com/knative/serving/blob/master/pkg/queue/readiness/probe.go#L36) on startup which results in a _lot_ of repetitive errors being printed if the user container takes longer than that to start up (which is pretty common). In our environment these logs end up using lots of space in our log collection tool unnecessarily.

This just logs the last error seen before we gave up instead.

/assign @markusthoemmes @vagababov 



knative-prow-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/knative/serving/pull/10107#" title="Author self-approved">julz</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=knative%2Fserving).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[pkg/queue/OWNERS](https://github.com/knative/serving/blob/master/pkg/queue/OWNERS)~~ [julz]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
codecov[bot]: # [Codecov](https://codecov.io/gh/knative/serving/pull/10107?src=pr&el=h1) Report
> Merging [#10107](https://codecov.io/gh/knative/serving/pull/10107?src=pr&el=desc) (f191762) into [master](https://codecov.io/gh/knative/serving/commit/a750a8bf1c4058a06b8f35a669164e27865d86e9?el=desc) (a750a8b) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/knative/serving/pull/10107/graphs/tree.svg?width=650&height=150&src=pr&token=MPpDfAj4Ui)](https://codecov.io/gh/knative/serving/pull/10107?src=pr&el=tree)

```diff
@@           Coverage Diff           @@
##           master   #10107   +/-   ##
=======================================
  Coverage   88.01%   88.01%           
=======================================
  Files         185      185           
  Lines        8717     8721    +4     
=======================================
+ Hits         7672     7676    +4     
  Misses        795      795           
  Partials      250      250           
```


| [Impacted Files](https://codecov.io/gh/knative/serving/pull/10107?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pkg/queue/readiness/probe.go](https://codecov.io/gh/knative/serving/pull/10107/diff?src=pr&el=tree#diff-cGtnL3F1ZXVlL3JlYWRpbmVzcy9wcm9iZS5nbw==) | `100.00% <100.00%> (ø)` | |
| [pkg/reconciler/configuration/configuration.go](https://codecov.io/gh/knative/serving/pull/10107/diff?src=pr&el=tree#diff-cGtnL3JlY29uY2lsZXIvY29uZmlndXJhdGlvbi9jb25maWd1cmF0aW9uLmdv) | `86.71% <0.00%> (-1.57%)` | :arrow_down: |
| [pkg/reconciler/autoscaling/kpa/scaler.go](https://codecov.io/gh/knative/serving/pull/10107/diff?src=pr&el=tree#diff-cGtnL3JlY29uY2lsZXIvYXV0b3NjYWxpbmcva3BhL3NjYWxlci5nbw==) | `90.00% <0.00%> (+1.42%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/knative/serving/pull/10107?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/knative/serving/pull/10107?src=pr&el=footer). Last update [a750a8b...f191762](https://codecov.io/gh/knative/serving/pull/10107?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

knative-prow-robot: @julz: The following test **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pull-knative-serving-upgrade-tests | f191762b4e5c5601f0514c7b057775e6c2db860f | [link](https://prow.knative.dev/view/gs/knative-prow/pr-logs/pull/knative_serving/10107/pull-knative-serving-upgrade-tests/1327198548835438592) | `/test pull-knative-serving-upgrade-tests`

[Full PR test history](https://gubernator.knative.dev/pr/knative_serving/10107). [Your PR dashboard](https://gubernator.knative.dev/pr/julz).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
knative-prow-robot: @julz: The following tests **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pull-knative-serving-upgrade-tests | f191762b4e5c5601f0514c7b057775e6c2db860f | [link](https://prow.knative.dev/view/gs/knative-prow/pr-logs/pull/knative_serving/10107/pull-knative-serving-upgrade-tests/1327198548835438592) | `/test pull-knative-serving-upgrade-tests`
pull-knative-serving-istio-stable-no-mesh-tls | f191762b4e5c5601f0514c7b057775e6c2db860f | [link](https://prow.knative.dev/view/gs/knative-prow/pr-logs/pull/knative_serving/10107/pull-knative-serving-istio-stable-no-mesh-tls/1327198548873187328) | `/test pull-knative-serving-istio-stable-no-mesh-tls`

[Full PR test history](https://gubernator.knative.dev/pr/knative_serving/10107). [Your PR dashboard](https://gubernator.knative.dev/pr/julz).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
knative-prow-robot: @julz: The following tests **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pull-knative-serving-upgrade-tests | f191762b4e5c5601f0514c7b057775e6c2db860f | [link](https://prow.knative.dev/view/gs/knative-prow/pr-logs/pull/knative_serving/10107/pull-knative-serving-upgrade-tests/1327198548835438592) | `/test pull-knative-serving-upgrade-tests`
pull-knative-serving-istio-stable-no-mesh-tls | f191762b4e5c5601f0514c7b057775e6c2db860f | [link](https://prow.knative.dev/view/gs/knative-prow/pr-logs/pull/knative_serving/10107/pull-knative-serving-istio-stable-no-mesh-tls/1327198548873187328) | `/test pull-knative-serving-istio-stable-no-mesh-tls`
pull-knative-serving-istio-stable-no-mesh | f191762b4e5c5601f0514c7b057775e6c2db860f | [link](https://prow.knative.dev/view/gs/knative-prow/pr-logs/pull/knative_serving/10107/pull-knative-serving-istio-stable-no-mesh/1327198548856410112) | `/test pull-knative-serving-istio-stable-no-mesh`

[Full PR test history](https://gubernator.knative.dev/pr/knative_serving/10107). [Your PR dashboard](https://gubernator.knative.dev/pr/julz).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
julz: looks like prow may be having some trouble :(.

Patch:
diff --git a/docs/runtime-contract.md b/docs/runtime-contract.md
index edf550f12..7cc7b0ea7 100644
--- a/docs/runtime-contract.md
+++ b/docs/runtime-contract.md
@@ -504,7 +504,8 @@ changes.
 From the OCI spec:
 
 > `rootfsPropagation` (string, OPTIONAL) sets the rootfs's mount propagation.
-> Its value is either slave, private, shared or unbindable. The <!-- wokeignore:rule=slave -->
+> Its value is either slave,<!-- wokeignore:rule=slave --> private, shared or
+> unbindable. The
 > [Shared Subtrees](https://www.kernel.org/doc/Documentation/filesystems/sharedsubtree.txt)
 > article in the kernel documentation has more information about mount
 > propagation.
diff --git a/pkg/queue/readiness/probe.go b/pkg/queue/readiness/probe.go
index 6ce77fde2..ba7463877 100644
--- a/pkg/queue/readiness/probe.go
+++ b/pkg/queue/readiness/probe.go
@@ -143,24 +143,36 @@ func (p *Probe) probeContainerImpl() bool {
 }
 
 func (p *Probe) doProbe(probe func(time.Duration) error) error {
-	if p.IsAggressive() {
-		return wait.PollImmediate(retryInterval, p.pollTimeout, func() (bool, error) {
-			if err := probe(aggressiveProbeTimeout); err != nil {
-				fmt.Fprintln(p.out, "aggressive probe error: ", err)
-				// Reset count of consecutive successes to zero.
-				p.count = 0
-				return false, nil
-			}
-
-			p.count++
-
-			// Return success if count of consecutive successes is equal to or greater
-			// than the probe's SuccessThreshold.
-			return p.count >= p.SuccessThreshold, nil
-		})
+	if !p.IsAggressive() {
+		return probe(time.Duration(p.TimeoutSeconds) * time.Second)
 	}
 
-	return probe(time.Duration(p.TimeoutSeconds) * time.Second)
+	var failCount int
+	var lastProbeErr error
+	pollErr := wait.PollImmediate(retryInterval, p.pollTimeout, func() (bool, error) {
+		if err := probe(aggressiveProbeTimeout); err != nil {
+			// Reset count of consecutive successes to zero.
+			p.count = 0
+			// Don't log this now since we probe every 50ms and some failures are
+			// expected if the user container takes longer than that to start up.
+			// We'll log the lastProbeErr if we don't eventually succeed.
+			lastProbeErr = err
+			failCount++
+			return false, nil
+		}
+
+		p.count++
+
+		// Return success if count of consecutive successes is equal to or greater
+		// than the probe's SuccessThreshold.
+		return p.count >= p.SuccessThreshold, nil
+	})
+
+	if lastProbeErr != nil {
+		fmt.Fprintf(p.out, "aggressive probe error (failed %d times): %v\n", failCount, lastProbeErr)
+	}
+
+	return pollErr
 }
 
 // tcpProbe function executes TCP probe once if its standard probe
diff --git a/pkg/reconciler/route/reconcile_resources.go b/pkg/reconciler/route/reconcile_resources.go
index d8751ab3f..227ce7135 100644
--- a/pkg/reconciler/route/reconcile_resources.go
+++ b/pkg/reconciler/route/reconcile_resources.go
@@ -18,9 +18,11 @@ package route
 
 import (
 	"context"
+	"encoding/json"
 	"errors"
 	"fmt"
 
+	"github.com/davecgh/go-spew/spew"
 	"go.uber.org/zap"
 	"golang.org/x/sync/errgroup"
 	corev1 "k8s.io/api/core/v1"
@@ -30,9 +32,11 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/sets"
 
+	"knative.dev/networking/pkg/apis/networking"
 	netv1alpha1 "knative.dev/networking/pkg/apis/networking/v1alpha1"
 	"knative.dev/pkg/apis/duck"
 	"knative.dev/pkg/controller"
+	"knative.dev/pkg/kmeta"
 	"knative.dev/pkg/logging"
 	v1 "knative.dev/serving/pkg/apis/serving/v1"
 	"knative.dev/serving/pkg/reconciler/route/config"
@@ -40,10 +44,18 @@ import (
 	"knative.dev/serving/pkg/reconciler/route/traffic"
 )
 
-func (c *Reconciler) reconcileIngress(ctx context.Context, r *v1.Route, desired *netv1alpha1.Ingress) (*netv1alpha1.Ingress, error) {
+func (c *Reconciler) reconcileIngress(ctx context.Context, r *v1.Route, desired *netv1alpha1.Ingress, tc *traffic.Config) (*netv1alpha1.Ingress, error) {
 	recorder := controller.GetEventRecorder(ctx)
 	ingress, err := c.ingressLister.Ingresses(desired.Namespace).Get(desired.Name)
+
+	// Get the current rollout state as described by the traffic.
+	curRO := tc.BuildRollout()
+
 	if apierrs.IsNotFound(err) {
+		// If there is no exisiting Ingress, then current rollout is _the_ rollout.
+		desired.Annotations = kmeta.UnionMaps(desired.Annotations, map[string]string{
+			networking.RolloutAnnotationKey: serializeRollout(ctx, curRO),
+		})
 		ingress, err = c.netclient.NetworkingV1alpha1().Ingresses(desired.Namespace).Create(ctx, desired, metav1.CreateOptions{})
 		if err != nil {
 			recorder.Eventf(r, corev1.EventTypeWarning, "CreationFailed", "Failed to create Ingress: %v", err)
@@ -54,25 +66,34 @@ func (c *Reconciler) reconcileIngress(ctx context.Context, r *v1.Route, desired
 		return ingress, nil
 	} else if err != nil {
 		return nil, err
-	} else if !equality.Semantic.DeepEqual(ingress.Spec, desired.Spec) ||
-		!equality.Semantic.DeepEqual(ingress.Annotations, desired.Annotations) ||
-		!equality.Semantic.DeepEqual(ingress.Labels, desired.Labels) {
-		// It is notable that one reason for differences here may be defaulting.
-		// When that is the case, the Update will end up being a nop because the
-		// webhook will bring them into alignment and no new reconciliation will occur.
-		// Also, compare annotation and label in case ingress.Class or parent route's labels
-		// is updated.
-
-		// Don't modify the informers copy
-		origin := ingress.DeepCopy()
-		origin.Spec = desired.Spec
-		origin.Annotations = desired.Annotations
-		origin.Labels = desired.Labels
-		updated, err := c.netclient.NetworkingV1alpha1().Ingresses(origin.Namespace).Update(ctx, origin, metav1.UpdateOptions{})
-		if err != nil {
-			return nil, fmt.Errorf("failed to update Ingress: %w", err)
+	} else {
+		// Ingress exists. We need to compute the rollout spec diff.
+		prevRO := deserializeRollout(ctx, ingress.Annotations[networking.RolloutAnnotationKey])
+		effectiveRO := curRO.Step(prevRO)
+		// Update the annotation.
+		desired.Annotations[networking.RolloutAnnotationKey] = serializeRollout(ctx, effectiveRO)
+		// TODO(vagababov): apply the Rollout to the ingress spec here.
+		if !equality.Semantic.DeepEqual(ingress.Spec, desired.Spec) ||
+			!equality.Semantic.DeepEqual(ingress.Annotations, desired.Annotations) ||
+			!equality.Semantic.DeepEqual(ingress.Labels, desired.Labels) {
+			// It is notable that one reason for differences here may be defaulting.
+			// When that is the case, the Update will end up being a nop because the
+			// webhook will bring them into alignment and no new reconciliation will occur.
+			// Also, compare annotation and label in case ingress.Class or parent route's labels
+			// is updated.
+
+			// Don't modify the informers copy
+			origin := ingress.DeepCopy()
+			origin.Spec = desired.Spec
+			origin.Annotations = desired.Annotations
+			origin.Labels = desired.Labels
+			updated, err := c.netclient.NetworkingV1alpha1().Ingresses(origin.Namespace).Update(
+				ctx, origin, metav1.UpdateOptions{})
+			if err != nil {
+				return nil, fmt.Errorf("failed to update Ingress: %w", err)
+			}
+			return updated, nil
 		}
-		return updated, nil
 	}
 
 	return ingress, err
@@ -227,3 +248,30 @@ func (c *Reconciler) reconcileTargetRevisions(ctx context.Context, t *traffic.Co
 	}
 	return eg.Wait()
 }
+
+func serializeRollout(ctx context.Context, r *traffic.Rollout) string {
+	sr, err := json.Marshal(r)
+	if err != nil {
+		// This must never happen in the normal course of things.
+		logging.FromContext(ctx).Warnw("Error serializing Rollout: "+spew.Sprint(r),
+			zap.Error(err))
+		return ""
+	}
+	return string(sr)
+}
+
+func deserializeRollout(ctx context.Context, ro string) *traffic.Rollout {
+	if ro == "" {
+		return nil
+	}
+	r := &traffic.Rollout{}
+	// Failure can happen if users manually tweaked the
+	// annotation or there's etcd corruption. Just log, rollouts
+	// are not mission critical.
+	if err := json.Unmarshal([]byte(ro), r); err != nil {
+		logging.FromContext(ctx).Warnw("Error deserializing Rollout: "+ro,
+			zap.Error(err))
+		return nil
+	}
+	return r
+}
diff --git a/pkg/reconciler/route/resources/ingress.go b/pkg/reconciler/route/resources/ingress.go
index b73590396..129d66e9e 100644
--- a/pkg/reconciler/route/resources/ingress.go
+++ b/pkg/reconciler/route/resources/ingress.go
@@ -18,11 +18,8 @@ package resources
 
 import (
 	"context"
-	"encoding/json"
 	"sort"
 
-	"github.com/davecgh/go-spew/spew"
-	"go.uber.org/zap"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/intstr"
@@ -33,7 +30,6 @@ import (
 	netv1alpha1 "knative.dev/networking/pkg/apis/networking/v1alpha1"
 	ingress "knative.dev/networking/pkg/ingress"
 	"knative.dev/pkg/kmeta"
-	"knative.dev/pkg/logging"
 	"knative.dev/serving/pkg/activator"
 	apicfg "knative.dev/serving/pkg/apis/config"
 	"knative.dev/serving/pkg/apis/serving"
@@ -78,7 +74,6 @@ func MakeIngress(
 			}),
 			Annotations: kmeta.FilterMap(kmeta.UnionMaps(map[string]string{
 				networking.IngressClassAnnotationKey: ingressClass,
-				networking.RolloutAnnotationKey:      serializeRollout(ctx, tc.BuildRollout()),
 			}, r.GetAnnotations()), func(key string) bool {
 				return key == corev1.LastAppliedConfigAnnotation
 			}),
@@ -88,17 +83,6 @@ func MakeIngress(
 	}, nil
 }
 
-func serializeRollout(ctx context.Context, r *traffic.Rollout) string {
-	sr, err := json.Marshal(r)
-	if err != nil {
-		// This must not never happen in the normal course of things.
-		logging.FromContext(ctx).Warnw("Error serializing Rollout: "+spew.Sprint(r),
-			zap.Error(err))
-		return ""
-	}
-	return string(sr)
-}
-
 // makeIngressSpec builds a new IngressSpec from inputs.
 func makeIngressSpec(
 	ctx context.Context,
@@ -260,7 +244,7 @@ func makeBaseIngressPath(ns string, targets traffic.RevisionTargets) *netv1alpha
 	// Optimistically allocate |targets| elements.
 	splits := make([]netv1alpha1.IngressBackendSplit, 0, len(targets))
 	for _, t := range targets {
-		if t.Percent == nil || *t.Percent == 0 {
+		if *t.Percent == 0 {
 			continue
 		}
 
diff --git a/pkg/reconciler/route/route.go b/pkg/reconciler/route/route.go
index a6ddb89b1..b158b3d0e 100644
--- a/pkg/reconciler/route/route.go
+++ b/pkg/reconciler/route/route.go
@@ -183,7 +183,7 @@ func (c *Reconciler) reconcileIngressResources(ctx context.Context, r *v1.Route,
 		return nil, err
 	}
 
-	ingress, err := c.reconcileIngress(ctx, r, desired)
+	ingress, err := c.reconcileIngress(ctx, r, desired, tc)
 	if err != nil {
 		return nil, err
 	}
diff --git a/pkg/reconciler/route/traffic/traffic.go b/pkg/reconciler/route/traffic/traffic.go
index 05e6036b9..c9c233cce 100644
--- a/pkg/reconciler/route/traffic/traffic.go
+++ b/pkg/reconciler/route/traffic/traffic.go
@@ -194,12 +194,12 @@ func buildRolloutForTag(r *Rollout, tag string, rts RevisionTargets) {
 		r.Configurations = append(r.Configurations, ConfigurationRollout{
 			ConfigurationName: rt.ConfigurationName,
 			Tag:               tag,
-			Percent:           int(zeroIfNil(rt.Percent)),
+			Percent:           int(*rt.Percent),
 			Revisions: []RevisionRollout{{
 				RevisionName: rt.RevisionName,
 				// Note: this will match config value in steady state, but
 				// during rollout it will be overridden by the rollout logic.
-				Percent: int(zeroIfNil(rt.Percent)),
+				Percent: int(*rt.Percent),
 			}},
 		})
 	}
@@ -335,21 +335,13 @@ func (cb *configBuilder) addRevisionTarget(tt *v1.TrafficTarget) error {
 	return nil
 }
 
-// zeroIfNil returns `0` if `ptr==nil`, or `*ptr` otherwise.
-func zeroIfNil(ptr *int64) int64 {
-	if ptr == nil {
-		return 0
-	}
-	return *ptr
-}
-
 // This find the exact revision+tag pair and if so, just adds the percentages.
 // This expects single digit lists, so just does an O(N) search.
 func mergeIfNecessary(rts RevisionTargets, rt RevisionTarget) RevisionTargets {
 	for i := range rts {
 		if rts[i].Tag == rt.Tag && rts[i].RevisionName == rt.RevisionName &&
 			*rt.LatestRevision == *rts[i].LatestRevision {
-			rts[i].Percent = ptr.Int64(zeroIfNil(rts[i].Percent) + zeroIfNil(rt.Percent))
+			rts[i].Percent = ptr.Int64(*rts[i].Percent + *rt.Percent)
 			return rts
 		}
 	}


Test patch:
diff --git a/pkg/queue/readiness/probe_test.go b/pkg/queue/readiness/probe_test.go
index b747f7574..6a94eedbe 100644
--- a/pkg/queue/readiness/probe_test.go
+++ b/pkg/queue/readiness/probe_test.go
@@ -24,6 +24,7 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"net/url"
+	"strings"
 	"testing"
 	"time"
 
@@ -79,6 +80,29 @@ func TestTCPFailure(t *testing.T) {
 	}
 }
 
+func TestAggressiveFailureOnlyLogsOnce(t *testing.T) {
+	pb := NewProbe(&corev1.Probe{
+		PeriodSeconds:    0, // Aggressive probe.
+		TimeoutSeconds:   1,
+		SuccessThreshold: 1,
+		FailureThreshold: 1,
+		Handler: corev1.Handler{
+			TCPSocket: &corev1.TCPSocketAction{
+				Host: "127.0.0.1",
+				Port: intstr.FromInt(12345),
+			},
+		},
+	})
+
+	var buf bytes.Buffer
+	pb.out = &buf
+
+	pb.ProbeContainer()
+	if got := strings.Count(buf.String(), "aggressive probe error"); got != 1 {
+		t.Error("Expected exactly one instance of 'aggressive probe error' in the log, got", got)
+	}
+}
+
 func TestEmptyHandler(t *testing.T) {
 	pb := NewProbe(&corev1.Probe{
 		PeriodSeconds:    1,
@@ -637,7 +661,7 @@ func TestKnTCPProbeSuccessThresholdIncludesFailure(t *testing.T) {
 	}
 
 	if probeErr := <-errChan; !probeErr {
-		t.Error("Wanted ProbeContainer() successed but got error")
+		t.Error("Wanted ProbeContainer() to succeed, but got error")
 	}
 	if got := pb.count; got < successThreshold {
 		t.Errorf("Count = %d, want: %d", got, successThreshold)
diff --git a/pkg/reconciler/route/reconcile_resources_test.go b/pkg/reconciler/route/reconcile_resources_test.go
index 24fecf20a..b04be03e7 100644
--- a/pkg/reconciler/route/reconcile_resources_test.go
+++ b/pkg/reconciler/route/reconcile_resources_test.go
@@ -49,9 +49,9 @@ func TestReconcileIngressInsert(t *testing.T) {
 	defer cancel()
 
 	r := Route("test-ns", "test-route")
-	ci := newTestIngress(t, r)
+	ci, tc := newTestIngress(t, r)
 
-	if _, err := reconciler.reconcileIngress(ctx, r, ci); err != nil {
+	if _, err := reconciler.reconcileIngress(ctx, r, ci, tc); err != nil {
 		t.Error("Unexpected error:", err)
 	}
 }
@@ -65,15 +65,15 @@ func TestReconcileIngressUpdate(t *testing.T) {
 
 	r := Route("test-ns", "test-route")
 
-	ci := newTestIngress(t, r)
-	if _, err := reconciler.reconcileIngress(ctx, r, ci); err != nil {
+	ci, tc := newTestIngress(t, r)
+	if _, err := reconciler.reconcileIngress(ctx, r, ci, tc); err != nil {
 		t.Error("Unexpected error:", err)
 	}
 
 	updated := getRouteIngressFromClient(ctx, t, r)
 	fakeciinformer.Get(ctx).Informer().GetIndexer().Add(updated)
 
-	ci2 := newTestIngress(t, r, func(tc *traffic.Config) {
+	ci2, tc := newTestIngress(t, r, func(tc *traffic.Config) {
 		tc.Targets[traffic.DefaultTarget][0].TrafficTarget.Percent = ptr.Int64(50)
 		tc.Targets[traffic.DefaultTarget] = append(tc.Targets[traffic.DefaultTarget], traffic.RevisionTarget{
 			TrafficTarget: v1.TrafficTarget{
@@ -82,13 +82,13 @@ func TestReconcileIngressUpdate(t *testing.T) {
 			},
 		})
 	})
-	if _, err := reconciler.reconcileIngress(ctx, r, ci2); err != nil {
+	if _, err := reconciler.reconcileIngress(ctx, r, ci2, tc); err != nil {
 		t.Error("Unexpected error:", err)
 	}
 
 	updated = getRouteIngressFromClient(ctx, t, r)
 	if diff := cmp.Diff(ci2, updated); diff != "" {
-		t.Error("Unexpected diff (-want +got):", diff)
+		t.Errorf("Unexpected diff (-want +got):\n%s", diff)
 	}
 	if diff := cmp.Diff(ci, updated); diff == "" {
 		t.Error("Expected difference, but found none")
@@ -197,7 +197,7 @@ func getLastPinnedTimestamp(t *testing.T, rev *v1.Revision) (string, error) {
 	return lastPinnedTime, nil
 }
 
-func newTestIngress(t *testing.T, r *v1.Route, trafficOpts ...func(tc *traffic.Config)) *netv1alpha1.Ingress {
+func newTestIngress(t *testing.T, r *v1.Route, trafficOpts ...func(tc *traffic.Config)) (*netv1alpha1.Ingress, *traffic.Config) {
 	tc := &traffic.Config{Targets: map[string]traffic.RevisionTargets{
 		traffic.DefaultTarget: {{
 			TrafficTarget: v1.TrafficTarget{
@@ -219,7 +219,7 @@ func newTestIngress(t *testing.T, r *v1.Route, trafficOpts ...func(tc *traffic.C
 	if err != nil {
 		t.Error("Unexpected error:", err)
 	}
-	return ingress
+	return ingress, tc
 }
 
 func TestReconcileIngressClassAnnotation(t *testing.T) {
@@ -232,19 +232,19 @@ func TestReconcileIngressClassAnnotation(t *testing.T) {
 	const expClass = "foo.ingress.networking.knative.dev"
 
 	r := Route("test-ns", "test-route")
-	ci := newTestIngress(t, r)
-	if _, err := reconciler.reconcileIngress(ctx, r, ci); err != nil {
+	ci, tc := newTestIngress(t, r)
+	if _, err := reconciler.reconcileIngress(ctx, r, ci, tc); err != nil {
 		t.Error("Unexpected error:", err)
 	}
 
 	updated := getRouteIngressFromClient(ctx, t, r)
 	fakeciinformer.Get(ctx).Informer().GetIndexer().Add(updated)
 
-	ci2 := newTestIngress(t, r)
+	ci2, tc := newTestIngress(t, r)
 	// Add ingress.class annotation.
 	ci2.Annotations[networking.IngressClassAnnotationKey] = expClass
 
-	if _, err := reconciler.reconcileIngress(ctx, r, ci2); err != nil {
+	if _, err := reconciler.reconcileIngress(ctx, r, ci2, tc); err != nil {
 		t.Error("Unexpected error:", err)
 	}
 
diff --git a/pkg/reconciler/route/resources/ingress_test.go b/pkg/reconciler/route/resources/ingress_test.go
index 4c894bce3..c3e5753d5 100644
--- a/pkg/reconciler/route/resources/ingress_test.go
+++ b/pkg/reconciler/route/resources/ingress_test.go
@@ -51,8 +51,6 @@ const (
 	testRouteName       = "test-route"
 	testAnnotationValue = "test-annotation-value"
 	testIngressClass    = "test-ingress"
-
-	emptyRollout = "{}"
 )
 
 func TestMakeIngressCorrectMetadata(t *testing.T) {
@@ -81,7 +79,6 @@ func TestMakeIngressCorrectMetadata(t *testing.T) {
 			// Make sure to get passdownIngressClass instead of ingressClass
 			networking.IngressClassAnnotationKey: passdownIngressClass,
 			"test-annotation":                    "bar",
-			networking.RolloutAnnotationKey:      emptyRollout,
 		},
 		OwnerReferences: []metav1.OwnerReference{*kmeta.NewControllerRef(r)},
 	}
@@ -133,7 +130,6 @@ func TestMakeIngressWithRollout(t *testing.T) {
 			// Make sure to get passdownIngressClass instead of ingressClass
 			networking.IngressClassAnnotationKey: passdownIngressClass,
 			"test-annotation":                    "bar",
-			networking.RolloutAnnotationKey:      serializeRollout(context.Background(), cfg.BuildRollout()),
 		},
 		OwnerReferences: []metav1.OwnerReference{*kmeta.NewControllerRef(r)},
 	}
@@ -625,7 +621,7 @@ func TestMakeIngressRuleZeroPercentTarget(t *testing.T) {
 	}
 }
 
-// One active target and a target of nil (implied zero) percent.
+// One active target and a target of 0 percent (nil is now impossible with defaulting to 0).
 func TestMakeIngressRuleNilPercentTarget(t *testing.T) {
 	targets := []traffic.RevisionTarget{{
 		TrafficTarget: v1.TrafficTarget{
@@ -638,6 +634,7 @@ func TestMakeIngressRuleNilPercentTarget(t *testing.T) {
 		TrafficTarget: v1.TrafficTarget{
 			ConfigurationName: "new-config",
 			RevisionName:      "new-revision",
+			Percent:           ptr.Int64(0),
 		},
 	}}
 	domains := []string{"test.org"}
@@ -865,7 +862,7 @@ func TestMakeIngressRuleZeroPercentTargetInactive(t *testing.T) {
 	}
 }
 
-func TestMakeIngressRuleNilPercentTargetInactive(t *testing.T) {
+func TestMakeIngressRuleImplicit0PercentTargetInactive(t *testing.T) {
 	targets := []traffic.RevisionTarget{{
 		TrafficTarget: v1.TrafficTarget{
 			ConfigurationName: "config",
@@ -877,7 +874,7 @@ func TestMakeIngressRuleNilPercentTargetInactive(t *testing.T) {
 		TrafficTarget: v1.TrafficTarget{
 			ConfigurationName: "new-config",
 			RevisionName:      "new-revision",
-			Percent:           nil,
+			Percent:           ptr.Int64(0),
 		},
 	}}
 	domains := []string{"test.org"}
@@ -922,7 +919,6 @@ func TestMakeIngressWithTLS(t *testing.T) {
 			Namespace: ns,
 			Annotations: map[string]string{
 				networking.IngressClassAnnotationKey: ingressClass,
-				networking.RolloutAnnotationKey:      emptyRollout,
 			},
 			Labels: map[string]string{
 				serving.RouteLabelKey:          "test-route",
diff --git a/pkg/reconciler/route/table_test.go b/pkg/reconciler/route/table_test.go
index 95cbbe892..054439d1e 100644
--- a/pkg/reconciler/route/table_test.go
+++ b/pkg/reconciler/route/table_test.go
@@ -713,7 +713,11 @@ func TestReconcile(t *testing.T) {
 						}},
 					},
 				},
-			),
+				simpleRollout("config", []traffic.RevisionRollout{{
+					RevisionName: "config-00001", Percent: 99,
+				}, {
+					RevisionName: "config-00002", Percent: 1,
+				}})),
 		}},
 		WantStatusUpdates: []clientgotesting.UpdateActionImpl{{
 			Object: Route("default", "new-latest-ready", WithConfigTarget("config"),
@@ -913,7 +917,11 @@ func TestReconcile(t *testing.T) {
 						}},
 					},
 				},
-			),
+				simpleRollout("config", []traffic.RevisionRollout{{
+					RevisionName: "config-00001", Percent: 99,
+				}, {
+					RevisionName: "config-00002", Percent: 1,
+				}})),
 		}},
 		WantStatusUpdates: []clientgotesting.UpdateActionImpl{{
 			Object: Route("default", "update-ci-failure", WithConfigTarget("config"),
@@ -2555,6 +2563,12 @@ func ingressWithClass(r *v1.Route, tc *traffic.Config, class string, io ...Ingre
 func baseIngressWithClass(r *v1.Route, tc *traffic.Config, class string, io ...IngressOption) *netv1alpha1.Ingress {
 	ingress, _ := resources.MakeIngress(getContext(), r, tc, nil, class)
 
+	// By default attach current rollout.
+	ro := tc.BuildRollout()
+	ingress.Annotations = kmeta.UnionMaps(ingress.Annotations, map[string]string{
+		networking.RolloutAnnotationKey: serializeRollout(context.Background(), ro),
+	})
+
 	for _, opt := range io {
 		opt(ingress)
 	}
@@ -2569,6 +2583,12 @@ func ingressWithTLS(r *v1.Route, tc *traffic.Config, tls []netv1alpha1.IngressTL
 func baseIngressWithTLS(r *v1.Route, tc *traffic.Config, tls []netv1alpha1.IngressTLS, challenges []netv1alpha1.HTTP01Challenge, io ...IngressOption) *netv1alpha1.Ingress {
 	ingress, _ := resources.MakeIngress(getContext(), r, tc, tls, TestIngressClass, challenges...)
 
+	// By default attach current rollout.
+	ro := tc.BuildRollout()
+	ingress.Annotations = kmeta.UnionMaps(ingress.Annotations, map[string]string{
+		networking.RolloutAnnotationKey: serializeRollout(context.Background(), ro),
+	})
+
 	for _, opt := range io {
 		opt(ingress)
 	}
@@ -2724,3 +2744,17 @@ func setResponsiveGCFeature(ctx context.Context, flag cfgmap.Flag) context.Conte
 	c.Features.ResponsiveRevisionGC = flag
 	return cfgmap.ToContext(ctx, c)
 }
+
+func simpleRollout(cfg string, revs []traffic.RevisionRollout) IngressOption {
+	return func(i *netv1alpha1.Ingress) {
+		r := &traffic.Rollout{
+			Configurations: []traffic.ConfigurationRollout{{
+				ConfigurationName: cfg,
+				Percent:           100,
+				Revisions:         revs,
+			}},
+		}
+		ro := serializeRollout(context.Background(), r)
+		i.Annotations[networking.RolloutAnnotationKey] = ro
+	}
+}
diff --git a/test/e2e-networking-library.sh b/test/e2e-networking-library.sh
index c9cec53ca..a95c775f8 100644
--- a/test/e2e-networking-library.sh
+++ b/test/e2e-networking-library.sh
@@ -157,7 +157,7 @@ function install_contour() {
 function wait_until_ingress_running() {
   if [[ -n "${ISTIO_VERSION}" ]]; then
     wait_until_pods_running istio-system || return 1
-    wait_until_service_has_external_http_address istio-system istio-ingressgateway
+    wait_until_service_has_external_http_address istio-system istio-ingressgateway || return 1
   fi
   if [[ -n "${GLOO_VERSION}" ]]; then
     # we must set these override values to allow the test spoofing client to work with Gloo