diff --git a/pkg/reconciler/route/resources/service.go b/pkg/reconciler/route/resources/service.go index 54aa9efa0ead..dcda5833c3e6 100644 --- a/pkg/reconciler/route/resources/service.go +++ b/pkg/reconciler/route/resources/service.go @@ -31,7 +31,6 @@ import ( netv1alpha1 "knative.dev/serving/pkg/apis/networking/v1alpha1" "knative.dev/serving/pkg/apis/serving" "knative.dev/serving/pkg/apis/serving/v1alpha1" - "knative.dev/serving/pkg/reconciler/route/config" "knative.dev/serving/pkg/reconciler/route/domains" ) @@ -109,10 +108,6 @@ func makeK8sService(ctx context.Context, route *v1alpha1.Route, targetName strin serving.RouteLabelKey: route.Name, } - if visibility, ok := route.Labels[config.VisibilityLabelKey]; ok { - svcLabels[config.VisibilityLabelKey] = visibility - } - return &corev1.Service{ ObjectMeta: metav1.ObjectMeta{ Name: hostname, diff --git a/pkg/reconciler/route/resources/service_test.go b/pkg/reconciler/route/resources/service_test.go index c5558b7b0a6e..2201813bd3d8 100644 --- a/pkg/reconciler/route/resources/service_test.go +++ b/pkg/reconciler/route/resources/service_test.go @@ -287,8 +287,7 @@ func TestMakeK8sPlaceholderService(t *testing.T) { SessionAffinity: corev1.ServiceAffinityNone, }, expectedLabels: map[string]string{ - serving.RouteLabelKey: "test-route", - config.VisibilityLabelKey: config.VisibilityClusterLocal, + serving.RouteLabelKey: "test-route", }, }} for _, tt := range tests { diff --git a/pkg/reconciler/route/route.go b/pkg/reconciler/route/route.go index f4c1df669e59..7a8db254ec80 100644 --- a/pkg/reconciler/route/route.go +++ b/pkg/reconciler/route/route.go @@ -19,6 +19,7 @@ package route import ( "context" "encoding/json" + "fmt" "go.uber.org/zap" corev1 "k8s.io/api/core/v1" @@ -191,6 +192,7 @@ func (c *Reconciler) reconcile(ctx context.Context, r *v1alpha1.Route) error { logger.Infof("Reconciling route: %#v", r) serviceNames, err := c.getServiceNames(ctx, r) + fmt.Printf("serviceNames = %#v\n", serviceNames) if err != nil { return err } @@ -485,17 +487,20 @@ func (c *Reconciler) getServiceNames(ctx context.Context, route *v1alpha1.Route) if err != nil { return nil, err } + if labels.IsObjectLocalVisibility(route.ObjectMeta) { + return &serviceNames{ + existingPublicServiceNames: existingPublicServiceNames, + existingClusterLocalServiceNames: existingClusterLocalServiceNames, + desiredPublicServiceNames: sets.NewString(), + desiredClusterLocalServiceNames: desiredServiceNames, + }, nil + } desiredPublicServiceNames := desiredServiceNames.Intersection(existingPublicServiceNames) desiredClusterLocalServiceNames := desiredServiceNames.Intersection(existingClusterLocalServiceNames) - // Any new desired services will follow the default route visibility. We only need to consider the case where it is - // "cluster-local". The alternative visibility means it should not be cluster local. + // Any new desired services will follow the default route visibility, which is public. serviceWithDefaultVisibility := desiredServiceNames.Difference(existingServiceNames) - if labels.IsObjectLocalVisibility(route.ObjectMeta) { - desiredClusterLocalServiceNames = desiredClusterLocalServiceNames.Union(serviceWithDefaultVisibility) - } else { - desiredPublicServiceNames = desiredPublicServiceNames.Union(serviceWithDefaultVisibility) - } + desiredPublicServiceNames = desiredPublicServiceNames.Union(serviceWithDefaultVisibility) return &serviceNames{ existingPublicServiceNames: existingPublicServiceNames, diff --git a/pkg/reconciler/route/table_test.go b/pkg/reconciler/route/table_test.go index ec5454021d71..c0aa978641b2 100644 --- a/pkg/reconciler/route/table_test.go +++ b/pkg/reconciler/route/table_test.go @@ -830,6 +830,159 @@ func TestReconcile(t *testing.T) { }}, Key: "default/new-latest-ready", SkipNamespaceValidation: true, + }, { + Name: "public becomes cluster local", + Objects: []runtime.Object{ + route("default", "becomes-local", WithConfigTarget("config"), + WithRouteLabel("serving.knative.dev/visibility", "cluster-local"), + WithRouteUID("65-23")), + cfg("default", "config", + WithGeneration(1), WithLatestCreated("config-00001"), WithLatestReady("config-00001")), + rev("default", "config", 1, MarkRevisionReady, WithRevName("config-00001"), WithServiceName("tb")), + simpleIngress( + route("default", "becomes-local", WithConfigTarget("config"), WithRouteUID("65-23")), + &traffic.Config{ + Targets: map[string]traffic.RevisionTargets{ + traffic.DefaultTarget: {{ + TrafficTarget: v1beta1.TrafficTarget{ + // Use the Revision name from the config. + RevisionName: "config-00001", + Percent: ptr.Int64(100), + }, + ServiceName: "tb", + Active: true, + }}, + }, + }, + ), + simpleK8sService(route("default", "becomes-local", WithConfigTarget("config"), + WithRouteLabel("serving.knative.dev/visibility", "cluster-local"), + WithRouteUID("65-23"))), + }, + WantUpdates: []clientgotesting.UpdateActionImpl{{ + Object: simpleIngressWithVisibility( + route("default", "becomes-local", WithConfigTarget("config"), + WithRouteUID("65-23"), + WithRouteLabel("serving.knative.dev/visibility", "cluster-local")), + &traffic.Config{ + Targets: map[string]traffic.RevisionTargets{ + traffic.DefaultTarget: {{ + TrafficTarget: v1beta1.TrafficTarget{ + // Use the Revision name from the config. + RevisionName: "config-00001", + Percent: ptr.Int64(100), + }, + ServiceName: "tb", + Active: true, + }}, + }, + }, + sets.NewString("becomes-local"), + ), + }}, + WantPatches: []clientgotesting.PatchActionImpl{ + patchFinalizers("default", "becomes-local"), + }, + WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ + Object: route("default", "becomes-local", WithConfigTarget("config"), + WithRouteUID("65-23"), + MarkTrafficAssigned, MarkIngressNotConfigured, + WithLocalDomain, WithAddress, WithInitRouteConditions, + WithRouteLabel("serving.knative.dev/visibility", "cluster-local"), + WithStatusTraffic(v1alpha1.TrafficTarget{ + TrafficTarget: v1beta1.TrafficTarget{ + RevisionName: "config-00001", + Percent: ptr.Int64(100), + LatestRevision: ptr.Bool(true), + }, + })), + }}, + WantDeleteCollections: []clientgotesting.DeleteCollectionActionImpl{{ + ListRestrictions: clientgotesting.ListRestrictions{ + Labels: labels.Set(map[string]string{ + serving.RouteLabelKey: "becomes-local", + serving.RouteNamespaceLabelKey: "default", + }).AsSelector(), + Fields: fields.Nothing(), + }, + }}, + Key: "default/becomes-local", + SkipNamespaceValidation: true, + }, { + Name: "cluster local becomes public", + Objects: []runtime.Object{ + route("default", "becomes-public", WithConfigTarget("config"), + WithRouteUID("65-23")), + cfg("default", "config", + WithGeneration(1), WithLatestCreated("config-00001"), WithLatestReady("config-00001")), + rev("default", "config", 1, MarkRevisionReady, WithRevName("config-00001"), WithServiceName("tb")), + simpleIngressWithVisibility( + route("default", "becomes-public", WithConfigTarget("config"), WithRouteUID("65-23"), + WithRouteLabel("serving.knative.dev/visibility", "cluster-local")), + &traffic.Config{ + Targets: map[string]traffic.RevisionTargets{ + traffic.DefaultTarget: {{ + TrafficTarget: v1beta1.TrafficTarget{ + // Use the Revision name from the config. + RevisionName: "config-00001", + Percent: ptr.Int64(100), + }, + ServiceName: "tb", + Active: true, + }}, + }, + }, + sets.NewString("becomes-public"), + ), + simpleK8sService(route("default", "becomes-public", WithConfigTarget("config"), + WithRouteUID("65-23"))), + }, + WantUpdates: []clientgotesting.UpdateActionImpl{{ + Object: simpleIngress( + route("default", "becomes-public", WithConfigTarget("config"), + WithRouteUID("65-23")), + &traffic.Config{ + Targets: map[string]traffic.RevisionTargets{ + traffic.DefaultTarget: {{ + TrafficTarget: v1beta1.TrafficTarget{ + // Use the Revision name from the config. + RevisionName: "config-00001", + Percent: ptr.Int64(100), + }, + ServiceName: "tb", + Active: true, + }}, + }, + }, + ), + }}, + WantPatches: []clientgotesting.PatchActionImpl{ + patchFinalizers("default", "becomes-public"), + }, + WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ + Object: route("default", "becomes-public", WithConfigTarget("config"), + WithRouteUID("65-23"), + MarkTrafficAssigned, MarkIngressNotConfigured, + WithAddress, WithInitRouteConditions, WithURL, + WithStatusTraffic(v1alpha1.TrafficTarget{ + TrafficTarget: v1beta1.TrafficTarget{ + RevisionName: "config-00001", + Percent: ptr.Int64(100), + LatestRevision: ptr.Bool(true), + }, + })), + }}, + WantDeleteCollections: []clientgotesting.DeleteCollectionActionImpl{{ + ListRestrictions: clientgotesting.ListRestrictions{ + Labels: labels.Set(map[string]string{ + serving.RouteLabelKey: "becomes-public", + serving.RouteNamespaceLabelKey: "default", + }).AsSelector(), + Fields: fields.Nothing(), + }, + }}, + Key: "default/becomes-public", + SkipNamespaceValidation: true, }, { Name: "failure updating cluster ingress", // Starting from the new latest ready, induce a failure updating the cluster ingress.