Repository: knative/serving
Pull request #10114 (2020-11-16 04:17:08)

Percent can no longer be nil
so remove the checks and update the tests.

/assign @tcnghia 

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

This pull-request has been approved by: *<a href="https://github.com/knative/serving/pull/10114#" title="Author self-approved">vagababov</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/reconciler/route/OWNERS](https://github.com/knative/serving/blob/master/pkg/reconciler/route/OWNERS)~~ [vagababov]

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":[]} -->
knative-prow-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/knative/serving/pull/10114#pullrequestreview-530910268" title="LGTM">tcnghia</a>*, *<a href="https://github.com/knative/serving/pull/10114#" title="Author self-approved">vagababov</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/reconciler/route/OWNERS](https://github.com/knative/serving/blob/master/pkg/reconciler/route/OWNERS)~~ [tcnghia,vagababov]

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":[]} -->
knative-prow-robot: @vagababov: The following test **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pull-knative-serving-unit-tests | 549bdeb44b9a430a772a7415c408d0704567043e | [link](https://prow.knative.dev/view/gs/knative-prow/pr-logs/pull/knative_serving/10114/pull-knative-serving-unit-tests/1328190367253663744) | `/test pull-knative-serving-unit-tests`

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

<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: New changes are detected. LGTM label has been removed.
codecov[bot]: # [Codecov](https://codecov.io/gh/knative/serving/pull/10114?src=pr&el=h1) Report
> Merging [#10114](https://codecov.io/gh/knative/serving/pull/10114?src=pr&el=desc) (9e76e9f) into [master](https://codecov.io/gh/knative/serving/commit/a750a8bf1c4058a06b8f35a669164e27865d86e9?el=desc) (a750a8b) will **increase** coverage by `0.06%`.
> The diff coverage is `100.00%`.

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

```diff
@@            Coverage Diff             @@
##           master   #10114      +/-   ##
==========================================
+ Coverage   88.01%   88.07%   +0.06%     
==========================================
  Files         185      185              
  Lines        8717     8714       -3     
==========================================
+ Hits         7672     7675       +3     
+ Misses        795      792       -3     
+ Partials      250      247       -3     
```


| [Impacted Files](https://codecov.io/gh/knative/serving/pull/10114?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pkg/reconciler/route/resources/ingress.go](https://codecov.io/gh/knative/serving/pull/10114/diff?src=pr&el=tree#diff-cGtnL3JlY29uY2lsZXIvcm91dGUvcmVzb3VyY2VzL2luZ3Jlc3MuZ28=) | `96.74% <100.00%> (ø)` | |
| [pkg/reconciler/route/traffic/traffic.go](https://codecov.io/gh/knative/serving/pull/10114/diff?src=pr&el=tree#diff-cGtnL3JlY29uY2lsZXIvcm91dGUvdHJhZmZpYy90cmFmZmljLmdv) | `94.15% <100.00%> (+1.04%)` | :arrow_up: |
| [pkg/autoscaler/statforwarder/forwarder.go](https://codecov.io/gh/knative/serving/pull/10114/diff?src=pr&el=tree#diff-cGtnL2F1dG9zY2FsZXIvc3RhdGZvcndhcmRlci9mb3J3YXJkZXIuZ28=) | `82.44% <0.00%> (+1.06%)` | :arrow_up: |
| [pkg/reconciler/autoscaling/kpa/scaler.go](https://codecov.io/gh/knative/serving/pull/10114/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/10114?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/10114?src=pr&el=footer). Last update [a750a8b...9e76e9f](https://codecov.io/gh/knative/serving/pull/10114?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

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

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

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

<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: @vagababov: The following tests **failed**, say `/retest` to rerun all failed tests:

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

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

<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: @vagababov: The following tests **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pull-knative-serving-upgrade-tests | 9e76e9f67ee75e9b68ec83c690061f135d3a6d6a | [link](https://prow.knative.dev/view/gs/knative-prow/pr-logs/pull/knative_serving/10114/pull-knative-serving-upgrade-tests/1328194856891191296) | `/test pull-knative-serving-upgrade-tests`
pull-knative-serving-istio-stable-no-mesh-tls | 9e76e9f67ee75e9b68ec83c690061f135d3a6d6a | [link](https://prow.knative.dev/view/gs/knative-prow/pr-logs/pull/knative_serving/10114/pull-knative-serving-istio-stable-no-mesh-tls/1328194857008631808) | `/test pull-knative-serving-istio-stable-no-mesh-tls`
pull-knative-serving-istio-stable-no-mesh | 9e76e9f67ee75e9b68ec83c690061f135d3a6d6a | [link](https://prow.knative.dev/view/gs/knative-prow/pr-logs/pull/knative_serving/10114/pull-knative-serving-istio-stable-no-mesh/1328194856966688768) | `/test pull-knative-serving-istio-stable-no-mesh`

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

<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 -->

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/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/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