From 3e7900e9cdc12b9ca191212c1bb1fbec30b83f45 Mon Sep 17 00:00:00 2001 From: Julian Friedman Date: Tue, 17 Aug 2021 13:42:16 +0100 Subject: [PATCH] Remove deprecated ServiceName field (#11817) * Remove deprecated ServiceName field We said we'd remove this in 0.25, the next version is now 0.26. * codegen --- config/core/300-resources/revision.yaml | 3 - docs/serving-api.md | 17 ---- pkg/apis/serving/v1/revision_types.go | 9 --- .../revision/reconcile_resources.go | 4 - pkg/reconciler/revision/table_test.go | 80 +++++++++---------- pkg/reconciler/route/queueing_test.go | 2 +- pkg/reconciler/route/route_test.go | 8 +- pkg/reconciler/route/table_test.go | 61 +++++++------- pkg/testing/v1/revision.go | 5 -- 9 files changed, 75 insertions(+), 114 deletions(-) diff --git a/config/core/300-resources/revision.yaml b/config/core/300-resources/revision.yaml index a848bccb4e..b5b4357473 100644 --- a/config/core/300-resources/revision.yaml +++ b/config/core/300-resources/revision.yaml @@ -698,6 +698,3 @@ spec: description: ObservedGeneration is the 'Generation' of the Service that was last processed by the controller. type: integer format: int64 - serviceName: - description: 'ServiceName holds the name of a core Kubernetes Service resource that load balances over the pods backing this Revision. Deprecated: revision service name is effectively equal to the revision name, as per #10540. 0.23 — stop populating 0.25 — remove.' - type: string diff --git a/docs/serving-api.md b/docs/serving-api.md index 7ba292f9de..abe8e3f3d7 100644 --- a/docs/serving-api.md +++ b/docs/serving-api.md @@ -1417,23 +1417,6 @@ knative.dev/pkg/apis/duck/v1.Status -serviceName
- -string - - - -(Optional) -

ServiceName holds the name of a core Kubernetes Service resource that -load balances over the pods backing this Revision. -Deprecated: revision service name is effectively equal to the revision name, -as per #10540. -0.23 — stop populating -0.25 — remove.

- - - - logUrl
string diff --git a/pkg/apis/serving/v1/revision_types.go b/pkg/apis/serving/v1/revision_types.go index 2a54942fdc..3e220dade9 100644 --- a/pkg/apis/serving/v1/revision_types.go +++ b/pkg/apis/serving/v1/revision_types.go @@ -123,15 +123,6 @@ func IsRevisionCondition(t apis.ConditionType) bool { type RevisionStatus struct { duckv1.Status `json:",inline"` - // ServiceName holds the name of a core Kubernetes Service resource that - // load balances over the pods backing this Revision. - // Deprecated: revision service name is effectively equal to the revision name, - // as per #10540. - // 0.23 — stop populating - // 0.25 — remove. - // +optional - ServiceName string `json:"serviceName,omitempty"` - // LogURL specifies the generated logging url for this particular revision // based on the revision url template specified in the controller's config. // +optional diff --git a/pkg/reconciler/revision/reconcile_resources.go b/pkg/reconciler/revision/reconcile_resources.go index 8f7e1ed2ef..d1fd92e837 100644 --- a/pkg/reconciler/revision/reconcile_resources.go +++ b/pkg/reconciler/revision/reconcile_resources.go @@ -172,10 +172,6 @@ func (c *Reconciler) reconcilePA(ctx context.Context, rev *v1.Revision) error { } } - // The public service name is always equal to the revision name itself. - // Historically it's been acquired from the PA object, so the assignment is here. - rev.Status.ServiceName = rev.Name - logger.Debugf("Observed PA Status=%#v", pa.Status) rev.Status.PropagateAutoscalerStatus(&pa.Status) return nil diff --git a/pkg/reconciler/revision/table_test.go b/pkg/reconciler/revision/table_test.go index 6e618c877a..65297fe060 100644 --- a/pkg/reconciler/revision/table_test.go +++ b/pkg/reconciler/revision/table_test.go @@ -91,7 +91,7 @@ func TestReconcile(t *testing.T) { WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ Object: Revision("foo", "first-reconcile", // The first reconciliation Populates the following status properties. - WithLogURL, allUnknownConditions, MarkDeploying("Deploying"), WithK8sServiceName, + WithLogURL, allUnknownConditions, MarkDeploying("Deploying"), withDefaultContainerStatuses(), WithRevisionObservedGeneration(1)), }}, Key: "foo/first-reconcile", @@ -115,7 +115,7 @@ func TestReconcile(t *testing.T) { WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ Object: Revision("foo", "update-status-failure", // Despite failure, the following status properties are set. - WithLogURL, allUnknownConditions, MarkDeploying("Deploying"), WithK8sServiceName, + WithLogURL, allUnknownConditions, MarkDeploying("Deploying"), withDefaultContainerStatuses(), WithRevisionObservedGeneration(1)), }}, WantEvents: []string{ @@ -185,7 +185,7 @@ func TestReconcile(t *testing.T) { // are necessary. Objects: []runtime.Object{ Revision("foo", "stable-reconcile", WithLogURL, allUnknownConditions, - WithK8sServiceName, withDefaultContainerStatuses(), WithRevisionObservedGeneration(1)), + withDefaultContainerStatuses(), WithRevisionObservedGeneration(1)), pa("foo", "stable-reconcile", WithReachabilityUnknown), deploy(t, "foo", "stable-reconcile"), @@ -198,7 +198,7 @@ func TestReconcile(t *testing.T) { // Test that we update a deployment with new containers when they disagree // with our desired spec. Objects: []runtime.Object{ - Revision("foo", "fix-containers", WithK8sServiceName, + Revision("foo", "fix-containers", WithLogURL, allUnknownConditions, withDefaultContainerStatuses(), WithRevisionObservedGeneration(1)), pa("foo", "fix-containers", WithReachabilityUnknown), changeContainers(deploy(t, "foo", "fix-containers")), @@ -217,7 +217,7 @@ func TestReconcile(t *testing.T) { }, Objects: []runtime.Object{ Revision("foo", "failure-update-deploy", - WithK8sServiceName, WithLogURL, allUnknownConditions, + WithLogURL, allUnknownConditions, withDefaultContainerStatuses(), WithRevisionObservedGeneration(1)), pa("foo", "failure-update-deploy"), changeContainers(deploy(t, "foo", "failure-update-deploy")), @@ -238,7 +238,7 @@ func TestReconcile(t *testing.T) { // state (port-Reserve), and verify that no changes are necessary. Objects: []runtime.Object{ Revision("foo", "stable-deactivation", - WithLogURL, MarkRevisionReady, WithK8sServiceName, + WithLogURL, MarkRevisionReady, MarkInactive("NoTraffic", "This thing is inactive."), withDefaultContainerStatuses(), WithRevisionObservedGeneration(1)), pa("foo", "stable-deactivation", @@ -252,14 +252,14 @@ func TestReconcile(t *testing.T) { Name: "pa is ready", Objects: []runtime.Object{ Revision("foo", "pa-ready", - WithK8sServiceName, WithLogURL, allUnknownConditions), + WithLogURL, allUnknownConditions), pa("foo", "pa-ready", WithPASKSReady, WithTraffic, WithScaleTargetInitialized, WithPAStatusService("new-stuff"), WithReachabilityUnknown), deploy(t, "foo", "pa-ready"), image("foo", "pa-ready"), }, WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ - Object: Revision("foo", "pa-ready", WithK8sServiceName, + Object: Revision("foo", "pa-ready", WithLogURL, // When the endpoint and pa are ready, then we will see the // Revision become ready. @@ -274,7 +274,7 @@ func TestReconcile(t *testing.T) { // Test propagating the pa not ready status to the Revision. Objects: []runtime.Object{ Revision("foo", "pa-not-ready", - WithK8sServiceName, WithLogURL, + WithLogURL, MarkRevisionReady, WithRevisionObservedGeneration(1)), pa("foo", "pa-not-ready", WithPAStatusService("its-not-confidential"), @@ -286,7 +286,7 @@ func TestReconcile(t *testing.T) { WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ Object: Revision("foo", "pa-not-ready", WithLogURL, MarkRevisionReady, withDefaultContainerStatuses(), - WithK8sServiceName, + // When we reconcile a ready state and our pa is in an activating // state, we should see the following mutation. MarkActivating("Queued", "Requests to the target are being buffered as resources are provisioned."), @@ -299,7 +299,7 @@ func TestReconcile(t *testing.T) { // Test propagating the inactivity signal from the pa to the Revision. Objects: []runtime.Object{ Revision("foo", "pa-inactive", - WithK8sServiceName, WithLogURL, + WithLogURL, MarkRevisionReady, WithRevisionObservedGeneration(1)), pa("foo", "pa-inactive", WithNoTraffic("NoTraffic", "This thing is inactive."), @@ -311,7 +311,7 @@ func TestReconcile(t *testing.T) { WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ Object: Revision("foo", "pa-inactive", WithLogURL, MarkRevisionReady, withDefaultContainerStatuses(), - WithK8sServiceName, + // When we reconcile an "all ready" revision when the PA // is inactive, we should see the following change. MarkInactive("NoTraffic", "This thing is inactive."), WithRevisionObservedGeneration(1)), @@ -322,7 +322,7 @@ func TestReconcile(t *testing.T) { // Test propagating the inactivity signal from the pa to the Revision. Objects: []runtime.Object{ Revision("foo", "pa-inactive", - WithK8sServiceName, WithLogURL, + WithLogURL, WithRevisionObservedGeneration(1)), pa("foo", "pa-inactive", WithNoTraffic("NoTraffic", "This thing is inactive."), @@ -337,14 +337,14 @@ func TestReconcile(t *testing.T) { // is inactive, we should see the following change. MarkInactive("NoTraffic", "This thing is inactive."), WithRevisionObservedGeneration(1), MarkResourcesUnavailable(v1.ReasonProgressDeadlineExceeded, - "Initial scale was never achieved"), WithK8sServiceName), + "Initial scale was never achieved")), }}, Key: "foo/pa-inactive", }, { Name: "pa is not ready with initial scale zero, but ServiceName still empty, so not marking resources available false", Objects: []runtime.Object{ Revision("foo", "pa-inactive", allUnknownConditions, - WithK8sServiceName, WithLogURL, + WithLogURL, WithRevisionObservedGeneration(1)), pa("foo", "pa-inactive", WithNoTraffic("NoTraffic", "This thing is inactive."), WithPAStatusService("")), @@ -355,7 +355,7 @@ func TestReconcile(t *testing.T) { // We should not mark resources unavailable if ServiceName is empty Object: Revision("foo", "pa-inactive", WithLogURL, withDefaultContainerStatuses(), allUnknownConditions, - WithK8sServiceName, MarkInactive("NoTraffic", "This thing is inactive."), + MarkInactive("NoTraffic", "This thing is inactive."), WithRevisionObservedGeneration(1)), }}, Key: "foo/pa-inactive", @@ -365,7 +365,7 @@ func TestReconcile(t *testing.T) { // But propagate the service name. Objects: []runtime.Object{ Revision("foo", "pa-inactive", - WithK8sServiceName, WithLogURL, MarkRevisionReady, + WithLogURL, MarkRevisionReady, WithRevisionObservedGeneration(1)), pa("foo", "pa-inactive", WithNoTraffic("NoTraffic", "This thing is inactive."), @@ -377,7 +377,7 @@ func TestReconcile(t *testing.T) { }, WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ Object: Revision("foo", "pa-inactive", - WithLogURL, MarkRevisionReady, WithK8sServiceName, + WithLogURL, MarkRevisionReady, // When we reconcile an "all ready" revision when the PA // is inactive, we should see the following change. MarkInactive("NoTraffic", "This thing is inactive."), @@ -391,7 +391,7 @@ func TestReconcile(t *testing.T) { // Protocol type is the only thing that can be changed on PA Objects: []runtime.Object{ Revision("foo", "fix-mutated-pa", - WithK8sServiceName, WithLogURL, MarkRevisionReady, + WithLogURL, MarkRevisionReady, WithRoutingState(v1.RoutingStateActive, fc)), pa("foo", "fix-mutated-pa", WithProtocolType(networking.ProtocolH2C), WithTraffic, WithPASKSReady, WithScaleTargetInitialized, WithReachabilityReachable, @@ -404,7 +404,7 @@ func TestReconcile(t *testing.T) { WithLogURL, allUnknownConditions, // When our reconciliation has to change the service // we should see the following mutations to status. - WithK8sServiceName, + WithRoutingState(v1.RoutingStateActive, fc), WithLogURL, MarkRevisionReady, withDefaultContainerStatuses()), }}, @@ -419,7 +419,7 @@ func TestReconcile(t *testing.T) { // Same as above, but will fail during the update. Objects: []runtime.Object{ Revision("foo", "fix-mutated-pa-fail", - WithK8sServiceName, WithLogURL, allUnknownConditions, + WithLogURL, allUnknownConditions, withDefaultContainerStatuses(), WithRevisionObservedGeneration(1)), pa("foo", "fix-mutated-pa-fail", WithProtocolType(networking.ProtocolH2C), WithReachabilityUnknown), deploy(t, "foo", "fix-mutated-pa-fail"), @@ -445,14 +445,14 @@ func TestReconcile(t *testing.T) { // status of the Revision. Objects: []runtime.Object{ Revision("foo", "deploy-timeout", - WithK8sServiceName, WithLogURL, MarkActive), + WithLogURL, MarkActive), pa("foo", "deploy-timeout"), // pa can't be ready since deployment times out. timeoutDeploy(deploy(t, "foo", "deploy-timeout"), "I timed out!"), image("foo", "deploy-timeout"), }, WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ Object: Revision("foo", "deploy-timeout", - WithLogURL, allUnknownConditions, WithK8sServiceName, + WithLogURL, allUnknownConditions, // When the revision is reconciled after a Deployment has // timed out, we should see it marked with the PDE state. MarkProgressDeadlineExceeded("I timed out!"), withDefaultContainerStatuses(), @@ -470,14 +470,14 @@ func TestReconcile(t *testing.T) { // It then verifies that Reconcile propagates this into the status of the Revision. Objects: []runtime.Object{ Revision("foo", "deploy-replica-failure", - WithK8sServiceName, WithLogURL, MarkActive), + WithLogURL, MarkActive), pa("foo", "deploy-replica-failure"), replicaFailureDeploy(deploy(t, "foo", "deploy-replica-failure"), "I replica failed!"), image("foo", "deploy-replica-failure"), }, WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ Object: Revision("foo", "deploy-replica-failure", - WithLogURL, allUnknownConditions, WithK8sServiceName, + WithLogURL, allUnknownConditions, // When the revision is reconciled after a Deployment has // timed out, we should see it marked with the FailedCreate state. MarkResourcesUnavailable("FailedCreate", "I replica failed!"), @@ -492,7 +492,7 @@ func TestReconcile(t *testing.T) { // Test the propagation of ImagePullBackoff from user container. Objects: []runtime.Object{ Revision("foo", "pull-backoff", - WithK8sServiceName, WithLogURL, MarkActivating("Deploying", "")), + WithLogURL, MarkActivating("Deploying", "")), pa("foo", "pull-backoff"), // pa can't be ready since deployment times out. pod(t, "foo", "pull-backoff", WithWaitingContainer("pull-backoff", "ImagePullBackoff", "can't pull it")), timeoutDeploy(deploy(t, "foo", "pull-backoff"), "Timed out!"), @@ -500,7 +500,7 @@ func TestReconcile(t *testing.T) { }, WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ Object: Revision("foo", "pull-backoff", - WithLogURL, allUnknownConditions, WithK8sServiceName, + WithLogURL, allUnknownConditions, MarkResourcesUnavailable("ImagePullBackoff", "can't pull it"), withDefaultContainerStatuses(), WithRevisionObservedGeneration(1)), }}, WantUpdates: []clientgotesting.UpdateActionImpl{{ @@ -515,14 +515,14 @@ func TestReconcile(t *testing.T) { // that Reconcile propagates this into the status of the Revision. Objects: []runtime.Object{ Revision("foo", "pod-error", - WithK8sServiceName, WithLogURL, allUnknownConditions, MarkActive), + WithLogURL, allUnknownConditions, MarkActive), pa("foo", "pod-error"), // PA can't be ready, since no traffic. pod(t, "foo", "pod-error", WithFailingContainer("pod-error", 5, "I failed man!")), deploy(t, "foo", "pod-error"), image("foo", "pod-error"), }, WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ - Object: Revision("foo", "pod-error", WithK8sServiceName, + Object: Revision("foo", "pod-error", WithLogURL, allUnknownConditions, MarkContainerExiting(5, v1.RevisionContainerExitingMessage("I failed man!")), withDefaultContainerStatuses(), WithRevisionObservedGeneration(1)), }}, @@ -537,14 +537,14 @@ func TestReconcile(t *testing.T) { // that Reconcile propagates this into the status of the Revision. Objects: []runtime.Object{ Revision("foo", "pod-schedule-error", - WithK8sServiceName, WithLogURL, allUnknownConditions, MarkActive), + WithLogURL, allUnknownConditions, MarkActive), pa("foo", "pod-schedule-error"), // PA can't be ready, since no traffic. pod(t, "foo", "pod-schedule-error", WithUnschedulableContainer("Insufficient energy", "Unschedulable")), deploy(t, "foo", "pod-schedule-error"), image("foo", "pod-schedule-error"), }, WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ - Object: Revision("foo", "pod-schedule-error", WithK8sServiceName, + Object: Revision("foo", "pod-schedule-error", WithLogURL, allUnknownConditions, MarkResourcesUnavailable("Insufficient energy", "Unschedulable"), withDefaultContainerStatuses(), WithRevisionObservedGeneration(1)), }}, @@ -560,14 +560,14 @@ func TestReconcile(t *testing.T) { // Revision. It then creates an Endpoints resource with active subsets. // This signal should make our Reconcile mark the Revision as Ready. Objects: []runtime.Object{ - Revision("foo", "steady-ready", WithK8sServiceName, WithLogURL), + Revision("foo", "steady-ready", WithLogURL), pa("foo", "steady-ready", WithPASKSReady, WithTraffic, WithScaleTargetInitialized, WithPAStatusService("steadier-even")), deploy(t, "foo", "steady-ready"), image("foo", "steady-ready"), }, WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ - Object: Revision("foo", "steady-ready", WithK8sServiceName, WithLogURL, + Object: Revision("foo", "steady-ready", WithLogURL, // All resources are ready to go, we should see the revision being // marked ready MarkRevisionReady, withDefaultContainerStatuses(), WithRevisionObservedGeneration(1)), @@ -580,14 +580,14 @@ func TestReconcile(t *testing.T) { Name: "lost pa owner ref", WantErr: true, Objects: []runtime.Object{ - Revision("foo", "missing-owners", WithK8sServiceName, WithLogURL, + Revision("foo", "missing-owners", WithLogURL, MarkRevisionReady), pa("foo", "missing-owners", WithTraffic, WithPodAutoscalerOwnersRemoved), deploy(t, "foo", "missing-owners"), image("foo", "missing-owners"), }, WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ - Object: Revision("foo", "missing-owners", WithK8sServiceName, WithLogURL, + Object: Revision("foo", "missing-owners", WithLogURL, MarkRevisionReady, // When we're missing the OwnerRef for PodAutoscaler we see this update. MarkResourceNotOwned("PodAutoscaler", "missing-owners"), withDefaultContainerStatuses(), WithRevisionObservedGeneration(1)), @@ -600,14 +600,14 @@ func TestReconcile(t *testing.T) { Name: "lost deployment owner ref", WantErr: true, Objects: []runtime.Object{ - Revision("foo", "missing-owners", WithK8sServiceName, WithLogURL, + Revision("foo", "missing-owners", WithLogURL, MarkRevisionReady), pa("foo", "missing-owners", WithTraffic), noOwner(deploy(t, "foo", "missing-owners")), image("foo", "missing-owners"), }, WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ - Object: Revision("foo", "missing-owners", WithK8sServiceName, WithLogURL, + Object: Revision("foo", "missing-owners", WithLogURL, MarkRevisionReady, // When we're missing the OwnerRef for Deployment we see this update. MarkResourceNotOwned("Deployment", "missing-owners-deployment"), withDefaultContainerStatuses(), WithRevisionObservedGeneration(1)), @@ -620,7 +620,7 @@ func TestReconcile(t *testing.T) { Name: "image pull secrets", // This test case tests that the image pull secrets from revision propagate to deployment and image Objects: []runtime.Object{ - Revision("foo", "image-pull-secrets", WithImagePullSecrets("foo-secret"), WithK8sServiceName), + Revision("foo", "image-pull-secrets", WithImagePullSecrets("foo-secret")), }, WantCreates: []runtime.Object{ pa("foo", "image-pull-secrets"), @@ -629,13 +629,13 @@ func TestReconcile(t *testing.T) { }, WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ Object: Revision("foo", "image-pull-secrets", - WithImagePullSecrets("foo-secret"), WithK8sServiceName, + WithImagePullSecrets("foo-secret"), WithLogURL, allUnknownConditions, MarkDeploying("Deploying"), withDefaultContainerStatuses(), WithRevisionObservedGeneration(1)), }}, Key: "foo/image-pull-secrets", }} - table.Test(t, MakeFactory(func(ctx context.Context, listers *Listers, cmw configmap.Watcher) controller.Reconciler { + table.Test(t, MakeFactory(func(ctx context.Context, listers *Listers, _ configmap.Watcher) controller.Reconciler { r := &Reconciler{ kubeclient: kubeclient.Get(ctx), client: servingclient.Get(ctx), diff --git a/pkg/reconciler/route/queueing_test.go b/pkg/reconciler/route/queueing_test.go index 1bb3b648ae..9c1446e895 100644 --- a/pkg/reconciler/route/queueing_test.go +++ b/pkg/reconciler/route/queueing_test.go @@ -48,7 +48,7 @@ func TestNewRouteCallsSyncHandler(t *testing.T) { ctx, cancel, informers := SetupFakeContextWithCancel(t) // A standalone revision - rev := Revision(testNamespace, "test-rev", MarkRevisionReady, WithK8sServiceName) + rev := Revision(testNamespace, "test-rev", MarkRevisionReady) // A route targeting the revision route := Route(testNamespace, "test-route", WithSpecTraffic( v1.TrafficTarget{ diff --git a/pkg/reconciler/route/route_test.go b/pkg/reconciler/route/route_test.go index d86e7d966b..9351cdb360 100644 --- a/pkg/reconciler/route/route_test.go +++ b/pkg/reconciler/route/route_test.go @@ -85,7 +85,7 @@ func testConfiguration() *v1.Configuration { func testRevision() *v1.Revision { return Revision(testNamespace, "p-deadbeef", func(r *v1.Revision) { r.Spec = *(&v1.ConfigurationSpec{}).GetTemplate().Spec.DeepCopy() - }, MarkRevisionReady, WithK8sServiceName) + }, MarkRevisionReady) } func newTestSetup(t *testing.T, opts ...reconcilerOption) ( @@ -541,7 +541,7 @@ func TestCreateRouteWithDuplicateTargets(t *testing.T) { defer cf() // A standalone revision - rev := Revision(testNamespace, "test-rev", MarkRevisionReady, WithK8sServiceName) + rev := Revision(testNamespace, "test-rev", MarkRevisionReady) fakeservingclient.Get(ctx).ServingV1().Revisions(testNamespace).Create(ctx, rev, metav1.CreateOptions{}) fakerevisioninformer.Get(ctx).Informer().GetIndexer().Add(rev) @@ -761,7 +761,7 @@ func TestCreateRouteWithNamedTargets(t *testing.T) { ctx, _, ctl, _, cf := newTestSetup(t) defer cf() // A standalone revision - rev := Revision(testNamespace, "test-rev", MarkRevisionReady, WithK8sServiceName) + rev := Revision(testNamespace, "test-rev", MarkRevisionReady) fakeservingclient.Get(ctx).ServingV1().Revisions(testNamespace).Create(ctx, rev, metav1.CreateOptions{}) fakerevisioninformer.Get(ctx).Informer().GetIndexer().Add(rev) @@ -971,7 +971,7 @@ func TestCreateRouteWithNamedTargetsAndTagBasedRouting(t *testing.T) { }, }) // A standalone revision - rev := Revision(testNamespace, "test-rev", MarkRevisionReady, WithK8sServiceName) + rev := Revision(testNamespace, "test-rev", MarkRevisionReady) fakeservingclient.Get(ctx).ServingV1().Revisions(testNamespace).Create(ctx, rev, metav1.CreateOptions{}) fakerevisioninformer.Get(ctx).Informer().GetIndexer().Add(rev) diff --git a/pkg/reconciler/route/table_test.go b/pkg/reconciler/route/table_test.go index 0ed6834bd7..a92dff6f1f 100644 --- a/pkg/reconciler/route/table_test.go +++ b/pkg/reconciler/route/table_test.go @@ -146,7 +146,7 @@ func TestReconcile(t *testing.T) { cfg("default", "ing-unknown", WithConfigGeneration(1), WithLatestCreated("ing-unknown-00001"), WithLatestReady("ing-unknown-00001")), - rev("default", "ing-unknown", 1, MarkRevisionReady, WithRevName("ing-unknown-00001"), WithK8sServiceName), + rev("default", "ing-unknown", 1, MarkRevisionReady, WithRevName("ing-unknown-00001")), }, WantCreates: []runtime.Object{ simpleIngress( @@ -194,7 +194,7 @@ func TestReconcile(t *testing.T) { Route("default", "ingress-failed", WithConfigTarget("config"), WithRouteUID("12-34"), WithRouteGeneration(1)), cfg("default", "config", WithConfigGeneration(1), WithLatestCreated("config-00001"), WithLatestReady("config-00001")), - rev("default", "config", 1, MarkRevisionReady, WithRevName("config-00001"), WithK8sServiceName), + rev("default", "config", 1, MarkRevisionReady, WithRevName("config-00001")), simpleIngress( Route("default", "ingress-failed", WithConfigTarget("config"), WithURL, WithRouteUID("12-34")), @@ -241,7 +241,7 @@ func TestReconcile(t *testing.T) { WithRouteGeneration(1)), cfg("default", "config", WithConfigGeneration(1), WithLatestCreated("config-00001"), WithLatestReady("config-00001")), - rev("default", "config", 1, MarkRevisionReady, WithRevName("config-00001"), WithK8sServiceName), + rev("default", "config", 1, MarkRevisionReady, WithRevName("config-00001")), }, WantCreates: []runtime.Object{ simpleIngress( @@ -292,7 +292,7 @@ func TestReconcile(t *testing.T) { WithRouteUID("65-23"), WithRouteGeneration(1)), cfg("default", "config", WithConfigGeneration(1), WithLatestCreated("config-00001"), WithLatestReady("config-00001")), - rev("default", "config", 1, MarkRevisionReady, WithRevName("config-00001"), WithK8sServiceName), + rev("default", "config", 1, MarkRevisionReady, WithRevName("config-00001")), }, WantCreates: []runtime.Object{ simpleIngress( @@ -499,7 +499,7 @@ func TestReconcile(t *testing.T) { WithRouteGeneration(2009), MarkInRollout), cfg("default", "config", WithConfigGeneration(1), WithLatestCreated("config-00001"), WithLatestReady("config-00001")), - rev("default", "config", 1, MarkRevisionReady, WithRevName("config-00001"), WithK8sServiceName), + rev("default", "config", 1, MarkRevisionReady, WithRevName("config-00001")), simpleIngress( Route("default", "becomes-ready", WithConfigTarget("config"), WithURL), &traffic.Config{ @@ -621,7 +621,7 @@ func TestReconcile(t *testing.T) { Route("default", "ingress-create-failure", WithConfigTarget("config"), WithRouteFinalizer, WithRouteGeneration(1)), cfg("default", "config", WithConfigGeneration(1), WithLatestCreated("config-00001"), WithLatestReady("config-00001")), - rev("default", "config", 1, MarkRevisionReady, WithRevName("config-00001"), WithK8sServiceName), + rev("default", "config", 1, MarkRevisionReady, WithRevName("config-00001")), }, // We induce a failure creating the Ingress. WantErr: true, @@ -766,7 +766,7 @@ func TestReconcile(t *testing.T) { // The Route controller attaches our label to this Configuration. WithConfigLabel("serving.knative.dev/route", "different-domain"), ), - rev("default", "config", 1, MarkRevisionReady, WithRevName("config-00001"), WithK8sServiceName), + rev("default", "config", 1, MarkRevisionReady, WithRevName("config-00001")), simpleIngress( Route("default", "different-domain", WithConfigTarget("config"), WithAnotherDomain), @@ -822,7 +822,7 @@ func TestReconcile(t *testing.T) { WithConfigGeneration(2), WithLatestReady("config-00001"), WithLatestCreated("config-00002"), WithConfigLabel("serving.knative.dev/route", "new-latest-created"), ), - rev("default", "config", 1, MarkRevisionReady, WithRevName("config-00001"), WithK8sServiceName), + rev("default", "config", 1, MarkRevisionReady, WithRevName("config-00001")), // This is the name of the new revision we're referencing above. rev("default", "config", 2, WithInitRevConditions, WithRevName("config-00002")), simpleIngress( @@ -861,9 +861,9 @@ func TestReconcile(t *testing.T) { // The Route controller attaches our label to this Configuration. WithConfigLabel("serving.knative.dev/route", "new-latest-ready"), ), - rev("default", "config", 1, MarkRevisionReady, WithRevName("config-00001"), WithK8sServiceName), + rev("default", "config", 1, MarkRevisionReady, WithRevName("config-00001")), // This is the name of the new revision we're referencing above. - rev("default", "config", 2, MarkRevisionReady, WithRevName("config-00002"), WithK8sServiceName), + rev("default", "config", 2, MarkRevisionReady, WithRevName("config-00002")), simpleIngress( Route("default", "new-latest-ready", WithConfigTarget("config"), WithURL), &traffic.Config{ @@ -950,9 +950,9 @@ func TestReconcile(t *testing.T) { // The Route controller attaches our label to this Configuration. WithConfigLabel("serving.knative.dev/route", "new-latest-ready"), ), - rev("default", "config", 1, MarkRevisionReady, WithRevName("config-00001"), WithK8sServiceName), + rev("default", "config", 1, MarkRevisionReady, WithRevName("config-00001")), // This is the name of the new revision we're referencing above. - rev("default", "config", 2, MarkRevisionReady, WithRevName("config-00002"), WithK8sServiceName), + rev("default", "config", 2, MarkRevisionReady, WithRevName("config-00002")), simpleIngress( Route("default", "new-latest-ready", WithConfigTarget("config"), WithURL), &traffic.Config{ @@ -1010,7 +1010,7 @@ func TestReconcile(t *testing.T) { WithRouteUID("65-23")), cfg("default", "config", WithConfigGeneration(1), WithLatestCreated("config-00001"), WithLatestReady("config-00001")), - rev("default", "config", 1, MarkRevisionReady, WithRevName("config-00001"), WithK8sServiceName), + rev("default", "config", 1, MarkRevisionReady, WithRevName("config-00001")), simpleIngress( Route("default", "becomes-local", WithConfigTarget("config"), WithRouteUID("65-23")), &traffic.Config{ @@ -1073,7 +1073,7 @@ func TestReconcile(t *testing.T) { WithRouteUID("65-23"), WithRouteGeneration(1)), cfg("default", "config", WithConfigGeneration(1), WithLatestCreated("config-00001"), WithLatestReady("config-00001")), - rev("default", "config", 1, MarkRevisionReady, WithRevName("config-00001"), WithK8sServiceName), + rev("default", "config", 1, MarkRevisionReady, WithRevName("config-00001")), simpleIngress( Route("default", "becomes-public", WithConfigTarget("config"), WithRouteUID("65-23"), WithRouteLabel(map[string]string{network.VisibilityLabelKey: "cluster-local"})), @@ -1147,9 +1147,9 @@ func TestReconcile(t *testing.T) { // The Route controller attaches our label to this Configuration. WithConfigLabel("serving.knative.dev/route", "update-ci-failure"), ), - rev("default", "config", 1, MarkRevisionReady, WithRevName("config-00001"), WithK8sServiceName), + rev("default", "config", 1, MarkRevisionReady, WithRevName("config-00001")), // This is the name of the new revision we're referencing above. - rev("default", "config", 2, MarkRevisionReady, WithRevName("config-00002"), WithK8sServiceName), + rev("default", "config", 2, MarkRevisionReady, WithRevName("config-00002")), simpleIngress( Route("default", "update-ci-failure", WithConfigTarget("config"), WithURL), &traffic.Config{ @@ -1389,7 +1389,7 @@ func TestReconcile(t *testing.T) { // The Route controller attaches our label to this Configuration. WithConfigLabel("serving.knative.dev/route", "ingress-mutation"), ), - rev("default", "config", 1, MarkRevisionReady, WithRevName("config-00001"), WithK8sServiceName), + rev("default", "config", 1, MarkRevisionReady, WithRevName("config-00001")), mutateIngress(simpleIngress( Route("default", "ingress-mutation", WithConfigTarget("config"), WithURL), &traffic.Config{ @@ -1525,8 +1525,8 @@ func TestReconcile(t *testing.T) { WithConfigGeneration(1), WithLatestCreated("blue-00001"), WithLatestReady("blue-00001")), cfg("default", "green", WithConfigGeneration(1), WithLatestCreated("green-00001"), WithLatestReady("green-00001")), - rev("default", "blue", 1, MarkRevisionReady, WithRevName("blue-00001"), WithK8sServiceName), - rev("default", "green", 1, MarkRevisionReady, WithRevName("green-00001"), WithK8sServiceName), + rev("default", "blue", 1, MarkRevisionReady, WithRevName("blue-00001")), + rev("default", "green", 1, MarkRevisionReady, WithRevName("green-00001")), }, WantCreates: []runtime.Object{ simpleIngress( @@ -1616,8 +1616,7 @@ func TestReconcile(t *testing.T) { }), WithRouteUID("1-2"), WithRouteFinalizer), cfg("default", "gray", WithConfigGeneration(1), WithLatestCreated("gray-00001"), WithLatestReady("gray-00001")), - rev("default", "gray", 1, MarkRevisionReady, WithRevName("gray-00001"), - WithK8sServiceName), + rev("default", "gray", 1, MarkRevisionReady, WithRevName("gray-00001")), }, WantCreates: []runtime.Object{ simpleIngress( @@ -1772,8 +1771,8 @@ func TestReconcile(t *testing.T) { ), cfg("default", "green", WithConfigGeneration(2020), WithLatestCreated("green-02021"), WithLatestReady("green-02020")), - rev("default", "blue", 1, MarkRevisionReady, WithRevName("blue-00001"), WithK8sServiceName), - rev("default", "green", 2020, MarkRevisionReady, WithRevName("green-02020"), WithK8sServiceName), + rev("default", "blue", 1, MarkRevisionReady, WithRevName("blue-00001")), + rev("default", "green", 2020, MarkRevisionReady, WithRevName("green-02020")), simpleIngress( Route("default", "switch-configs", WithConfigTarget("blue"), WithURL), &traffic.Config{ @@ -2120,7 +2119,7 @@ func TestReconcileEnableAutoTLS(t *testing.T) { WithRouteUID("12-34")), cfg("default", "config", WithConfigGeneration(1), WithLatestCreated("config-00001"), WithLatestReady("config-00001")), - rev("default", "config", 1, MarkRevisionReady, WithRevName("config-00001"), WithK8sServiceName), + rev("default", "config", 1, MarkRevisionReady, WithRevName("config-00001")), }, WantCreates: []runtime.Object{ ingressWithTLS( @@ -2173,7 +2172,7 @@ func TestReconcileEnableAutoTLS(t *testing.T) { Route("default", "becomes-ready", WithConfigTarget("config"), WithRouteUID("12-34")), cfg("default", "config", WithConfigGeneration(1), WithLatestCreated("config-00001"), WithLatestReady("config-00001")), - rev("default", "config", 1, MarkRevisionReady, WithRevName("config-00001"), WithK8sServiceName), + rev("default", "config", 1, MarkRevisionReady, WithRevName("config-00001")), }, WantCreates: []runtime.Object{ resources.MakeCertificates(Route("default", "becomes-ready", WithConfigTarget("config"), WithURL, WithRouteUID("12-34")), @@ -2225,7 +2224,7 @@ func TestReconcileEnableAutoTLS(t *testing.T) { Route("default", "becomes-ready", WithConfigTarget("config"), WithRouteUID("12-34")), cfg("default", "config", WithConfigGeneration(1), WithLatestCreated("config-00001"), WithLatestReady("config-00001")), - rev("default", "config", 1, MarkRevisionReady, WithRevName("config-00001"), WithK8sServiceName), + rev("default", "config", 1, MarkRevisionReady, WithRevName("config-00001")), certificateWithStatus(resources.MakeCertificates(Route("default", "becomes-ready", WithConfigTarget("config"), WithURL, WithRouteUID("12-34")), map[string]string{"becomes-ready.default.example.com": ""}, network.CertManagerCertificateClassName)[0], readyCertStatus()), }, @@ -2282,7 +2281,7 @@ func TestReconcileEnableAutoTLS(t *testing.T) { Route("default", "becomes-ready", WithConfigTarget("config"), WithRouteUID("12-34")), cfg("default", "config", WithConfigGeneration(1), WithLatestCreated("config-00001"), WithLatestReady("config-00001")), - rev("default", "config", 1, MarkRevisionReady, WithRevName("config-00001"), WithK8sServiceName), + rev("default", "config", 1, MarkRevisionReady, WithRevName("config-00001")), // MakeCertificates will create a certificate with DNS name "*.test-ns.example.com" which is not the host name // needed by the input Route. &netv1alpha1.Certificate{ @@ -2362,7 +2361,7 @@ func TestReconcileEnableAutoTLS(t *testing.T) { Route("default", "becomes-ready", WithConfigTarget("config"), WithRouteUID("12-34")), cfg("default", "config", WithConfigGeneration(1), WithLatestCreated("config-00001"), WithLatestReady("config-00001")), - rev("default", "config", 1, MarkRevisionReady, WithRevName("config-00001"), WithK8sServiceName), + rev("default", "config", 1, MarkRevisionReady, WithRevName("config-00001")), &netv1alpha1.Certificate{ ObjectMeta: metav1.ObjectMeta{ Name: "route-12-34", @@ -2456,7 +2455,7 @@ func TestReconcileEnableAutoTLS(t *testing.T) { Route("default", "becomes-ready", WithConfigTarget("config"), WithRouteUID("12-34"), WithRouteGeneration(1)), cfg("default", "config", WithConfigGeneration(1), WithLatestCreated("config-00001"), WithLatestReady("config-00001")), - rev("default", "config", 1, MarkRevisionReady, WithRevName("config-00001"), WithK8sServiceName), + rev("default", "config", 1, MarkRevisionReady, WithRevName("config-00001")), &netv1alpha1.Certificate{ ObjectMeta: metav1.ObjectMeta{ Name: "route-12-34", @@ -2502,7 +2501,7 @@ func TestReconcileEnableAutoTLS(t *testing.T) { Route("default", "becomes-ready", WithConfigTarget("config"), WithRouteUID("12-34"), WithRouteGeneration(1)), cfg("default", "config", WithConfigGeneration(1), WithLatestCreated("config-00001"), WithLatestReady("config-00001")), - rev("default", "config", 1, MarkRevisionReady, WithRevName("config-00001"), WithK8sServiceName), + rev("default", "config", 1, MarkRevisionReady, WithRevName("config-00001")), // MakeCertificates will create a certificate with DNS name "*.test-ns.example.com" which is not the host name // needed by the input Route. &netv1alpha1.Certificate{ @@ -2578,7 +2577,7 @@ func TestReconcileEnableAutoTLS(t *testing.T) { WithRouteUID("65-23")), cfg("default", "config", WithConfigGeneration(1), WithLatestCreated("config-00001"), WithLatestReady("config-00001")), - rev("default", "config", 1, MarkRevisionReady, WithRevName("config-00001"), WithK8sServiceName), + rev("default", "config", 1, MarkRevisionReady, WithRevName("config-00001")), simpleIngress( Route("default", "becomes-local", WithConfigTarget("config"), WithRouteUID("65-23")), &traffic.Config{ diff --git a/pkg/testing/v1/revision.go b/pkg/testing/v1/revision.go index ec4b009df2..77cce9b518 100644 --- a/pkg/testing/v1/revision.go +++ b/pkg/testing/v1/revision.go @@ -130,11 +130,6 @@ func MarkActive(r *v1.Revision) { r.Status.MarkActiveTrue() } -// WithK8sServiceName applies sn to the revision status field. -func WithK8sServiceName(r *v1.Revision) { - r.Status.ServiceName = r.Name -} - // MarkInactive calls .Status.MarkInactive on the Revision. func MarkInactive(reason, message string) RevisionOption { return func(r *v1.Revision) {