diff --git a/pkg/reconciler/ingress/fixtures_test.go b/pkg/reconciler/ingress/fixtures_test.go index 1a36b5bfd..25d226304 100644 --- a/pkg/reconciler/ingress/fixtures_test.go +++ b/pkg/reconciler/ingress/fixtures_test.go @@ -36,6 +36,7 @@ type HTTPRoute struct { Hostname string Rules []RuleBuilder StatusConditions []metav1.Condition + ClusterLocal bool } func (r HTTPRoute) Build() *gatewayapi.HTTPRoute { @@ -79,6 +80,11 @@ func (r HTTPRoute) Build() *gatewayapi.HTTPRoute { }, } + if r.ClusterLocal { + route.Labels[networking.VisibilityLabelKey] = "cluster-local" + route.Spec.CommonRouteSpec.ParentRefs[0].Name = gatewayapi.ObjectName(privateName) + } + for _, hostname := range hostnames { route.Spec.Hostnames = append( route.Spec.Hostnames, diff --git a/pkg/reconciler/ingress/ingress.go b/pkg/reconciler/ingress/ingress.go index 71141efe5..916dbbbed 100644 --- a/pkg/reconciler/ingress/ingress.go +++ b/pkg/reconciler/ingress/ingress.go @@ -92,30 +92,35 @@ func (c *Reconciler) reconcileIngress(ctx context.Context, ing *v1alpha1.Ingress ing.Status.InitializeConditions() var ( - hash string - err error + ingressHash string + err error + probeKey = types.NamespacedName{ + Name: ing.Name, + Namespace: ing.Namespace, + } ) - if hash, err = ingress.InsertProbe(ing); err != nil { + if ingressHash, err = ingress.InsertProbe(ing); err != nil { return fmt.Errorf("failed to add knative probe header: %w", err) } backends := status.Backends{ - Version: hash, - Key: types.NamespacedName{ - Name: ing.Name, - Namespace: ing.Namespace, - }, + Version: ingressHash, + Key: probeKey, } + probe, _ := c.statusManager.IsProbeActive(probeKey) + for _, rule := range ing.Spec.Rules { rule := rule - httproute, err := c.reconcileHTTPRoute(ctx, &hash, ing, &rule) + httproute, routeHash, err := c.reconcileHTTPRoute(ctx, probe, ingressHash, ing, &rule) if err != nil { return err } + backends.Version = routeHash + if isHTTPRouteReady(httproute) { ing.Status.MarkNetworkConfigured() gatherProbes(&backends, httproute, rule.Visibility) @@ -124,9 +129,6 @@ func (c *Reconciler) reconcileIngress(ctx context.Context, ing *v1alpha1.Ingress } } - // Hash might have changed depending on HTTPRoute reconciliation - backends.Version = hash - externalIngressTLS := ing.GetIngressTLSForVisibility(v1alpha1.IngressVisibilityExternalIP) listeners := make([]*gatewayapi.Listener, 0, len(externalIngressTLS)) for _, tls := range externalIngressTLS { diff --git a/pkg/reconciler/ingress/ingress_test.go b/pkg/reconciler/ingress/ingress_test.go index 7ec1f7514..cda83f816 100644 --- a/pkg/reconciler/ingress/ingress_test.go +++ b/pkg/reconciler/ingress/ingress_test.go @@ -394,6 +394,9 @@ func TestReconcileProbing(t *testing.T) { Name: "first reconciler probe returns false", Key: "ns/name", Ctx: withStatusManager(&fakeStatusManager{ + FakeIsProbeActive: func(types.NamespacedName) (status.ProbeState, bool) { + return status.ProbeState{Ready: false}, false + }, FakeDoProbes: func(context.Context, status.Backends) (status.ProbeState, error) { return status.ProbeState{Ready: false}, nil }, @@ -423,6 +426,9 @@ func TestReconcileProbing(t *testing.T) { Name: "first reconcile probe returns an error", Key: "ns/name", Ctx: withStatusManager(&fakeStatusManager{ + FakeIsProbeActive: func(types.NamespacedName) (status.ProbeState, bool) { + return status.ProbeState{Ready: false}, false + }, FakeDoProbes: func(context.Context, status.Backends) (status.ProbeState, error) { return status.ProbeState{Ready: false}, errors.New("this is the error") }, @@ -472,7 +478,7 @@ func TestReconcileProbing(t *testing.T) { Name: "updated ingress - new backends used for endpoint probing", Key: "ns/name", Objects: append([]runtime.Object{ - ing(withSecondRevisionSpec, withGatewayAPIclass, withFinalizer, makeItReady), + ing(withBasicSpec, withSecondRevisionSpec, withGatewayAPIclass, withFinalizer, makeItReady), httpRoute(t, ing(withBasicSpec, withGatewayAPIclass), httpRouteReady), }, servicesAndEndpoints...), Ctx: withStatusManager(&fakeStatusManager{ @@ -484,7 +490,9 @@ func TestReconcileProbing(t *testing.T) { }, }), WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ - Object: ing(withSecondRevisionSpec, + Object: ing( + withBasicSpec, + withSecondRevisionSpec, withGatewayAPIclass, withFinalizer, makeItReady, @@ -545,7 +553,8 @@ func TestReconcileProbing(t *testing.T) { }, }), Objects: append([]runtime.Object{ - ing(withSecondRevisionSpec, + ing(withBasicSpec, + withSecondRevisionSpec, withGatewayAPIclass, withFinalizer, makeItReady, @@ -602,7 +611,8 @@ func TestReconcileProbing(t *testing.T) { }, }), Objects: append([]runtime.Object{ - ing(withSecondRevisionSpec, + ing(withBasicSpec, + withSecondRevisionSpec, withGatewayAPIclass, withFinalizer, makeItReady, @@ -698,7 +708,8 @@ func TestReconcileProbing(t *testing.T) { }, }), Objects: append([]runtime.Object{ - ing(withSecondRevisionSpec, + ing(withBasicSpec, + withSecondRevisionSpec, withGatewayAPIclass, withFinalizer, makeItReady, @@ -910,7 +921,7 @@ func TestReconcileProbing(t *testing.T) { }, servicesAndEndpoints...), }, { - Name: "transition probe complete - drop endpoint probes", + Name: "transition probe complete - drop probes", Key: "ns/name", Ctx: withStatusManager(&fakeStatusManager{ FakeIsProbeActive: func(types.NamespacedName) (status.ProbeState, bool) { @@ -922,7 +933,8 @@ func TestReconcileProbing(t *testing.T) { }, }), Objects: append([]runtime.Object{ - ing(withSecondRevisionSpec, + ing(withBasicSpec, + withSecondRevisionSpec, withGatewayAPIclass, withFinalizer, makeItReady, @@ -986,7 +998,7 @@ func TestReconcileProbing(t *testing.T) { }.Build(), }}, }, { - Name: "dropping endpoint probes complete - mark ingress ready", + Name: "dropping probes complete - mark ingress ready", Key: "ns/name", Ctx: withStatusManager(&fakeStatusManager{ FakeIsProbeActive: func(types.NamespacedName) (status.ProbeState, bool) { @@ -998,14 +1010,16 @@ func TestReconcileProbing(t *testing.T) { }, }), WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ - Object: ing(withSecondRevisionSpec, + Object: ing(withBasicSpec, + withSecondRevisionSpec, withGatewayAPIclass, withFinalizer, makeItReady, ), }}, Objects: append([]runtime.Object{ - ing(withSecondRevisionSpec, + ing(withBasicSpec, + withSecondRevisionSpec, withGatewayAPIclass, withFinalizer, makeItReady, @@ -1051,7 +1065,8 @@ func TestReconcileProbing(t *testing.T) { }, }), Objects: append([]runtime.Object{ - ing(withSecondRevisionSpec, + ing(withBasicSpec, + withSecondRevisionSpec, withGatewayAPIclass, withFinalizer, makeItReady, @@ -1109,7 +1124,13 @@ func TestReconcileProbing(t *testing.T) { }, }), Objects: append([]runtime.Object{ - ing(withThirdRevisionSpec, withGatewayAPIclass, withFinalizer, makeItReady, makeLoadBalancerNotReady), + ing( + withBasicSpec, + withThirdRevisionSpec, + withGatewayAPIclass, + withFinalizer, + makeItReady, + makeLoadBalancerNotReady), HTTPRoute{ Name: "example.com", Namespace: "ns", @@ -1238,24 +1259,892 @@ func TestReconcileProbing(t *testing.T) { }}, }.Build(), }}, - }} - - table.Test(t, MakeFactory(func(ctx context.Context, listers *Listers, cmw configmap.Watcher) controller.Reconciler { - statusManager := ctx.Value(fakeStatusKey).(status.Manager) - r := &Reconciler{ - gwapiclient: fakegwapiclientset.Get(ctx), - // Listers index properties about resources - httprouteLister: listers.GetHTTPRouteLister(), - gatewayLister: listers.GetGatewayLister(), - statusManager: statusManager, - } - return ingressreconciler.NewReconciler(ctx, logging.FromContext(ctx), fakeingressclient.Get(ctx), - listers.GetIngressLister(), controller.GetEventRecorder(ctx), r, gatewayAPIIngressClassName, - controller.Options{ - ConfigStore: &testConfigStore{ - config: defaultConfig, - }}) - })) + }, { + Name: "multiple visibility - updated ingress - new backends used for endpoint probing", + Key: "ns/name", + Objects: append([]runtime.Object{ + ing( + withBasicSpec, + withInternalSpec, + withSecondRevisionSpec, + withGatewayAPIclass, + withFinalizer, + makeItReady), + HTTPRoute{ + Name: "example.com", + Namespace: "ns", + Hostname: "example.com", + Rules: []RuleBuilder{ + EndpointProbeRule{ + Namespace: "ns", + Name: "goo", + Hash: "first-hash", + Port: 123, + }, + NormalRule{ + Namespace: "ns", + Name: "goo", + Port: 123, + Weight: 100, + }, + }, + StatusConditions: []metav1.Condition{{ + Type: string(gatewayapi.RouteConditionAccepted), + Status: metav1.ConditionTrue, + }}, + }.Build(), + HTTPRoute{ + Name: "foo.svc.cluster.local", + Namespace: "ns", + Hostnames: []string{"foo.svc", "foo.svc.cluster.local"}, + ClusterLocal: true, + Rules: []RuleBuilder{ + EndpointProbeRule{ + Namespace: "ns", + Name: "goo", + Hash: "first-hash", + Port: 124, + }, + NormalRule{ + Namespace: "ns", + Name: "goo", + Port: 124, + Weight: 100, + }, + }, + StatusConditions: []metav1.Condition{{ + Type: string(gatewayapi.RouteConditionAccepted), + Status: metav1.ConditionTrue, + }}, + }.Build(), + }, servicesAndEndpoints...), + Ctx: withStatusManager(&fakeStatusManager{ + FakeIsProbeActive: func(types.NamespacedName) (status.ProbeState, bool) { + return status.ProbeState{Ready: true, Version: "previous"}, true + }, + FakeDoProbes: func(context.Context, status.Backends) (status.ProbeState, error) { + return status.ProbeState{Ready: false}, nil + }, + }), + WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ + Object: ing( + withBasicSpec, + withInternalSpec, + withSecondRevisionSpec, + withGatewayAPIclass, + withFinalizer, + makeItReady, + makeLoadBalancerNotReady, + ), + }}, + WantUpdates: []clientgotesting.UpdateActionImpl{{ + Object: HTTPRoute{ + Name: "example.com", + Namespace: "ns", + Hostname: "example.com", + Rules: []RuleBuilder{ + EndpointProbeRule{ + Namespace: "ns", + Name: "goo", + Hash: "ep-ff3cee4d49fbd4547b85c63d56e88eb866d4043951761f069d6afe14a2e61970", + Port: 123, + }, + NormalRule{ + Namespace: "ns", + Name: "goo", + Port: 123, + Weight: 100, + }, + EndpointProbeRule{ + Namespace: "ns", + Name: "second-revision", + Path: "/.well-known/knative/revision/ns/second-revision", + Hash: "ep-ff3cee4d49fbd4547b85c63d56e88eb866d4043951761f069d6afe14a2e61970", + Port: 123, + }, + EndpointProbeRule{ + Namespace: "ns", + Name: "goo", + Path: "/.well-known/knative/revision/ns/goo", + Hash: "ep-ff3cee4d49fbd4547b85c63d56e88eb866d4043951761f069d6afe14a2e61970", + Port: 123, + }, + }, + StatusConditions: []metav1.Condition{{ + Type: string(gatewayapi.RouteConditionAccepted), + Status: metav1.ConditionTrue, + }}, + }.Build(), + }, { + Object: HTTPRoute{ + Name: "foo.svc.cluster.local", + Namespace: "ns", + Hostnames: []string{"foo.svc", "foo.svc.cluster.local"}, + ClusterLocal: true, + Rules: []RuleBuilder{ + EndpointProbeRule{ + Namespace: "ns", + Name: "goo", + Hash: "ep-ff3cee4d49fbd4547b85c63d56e88eb866d4043951761f069d6afe14a2e61970", + Port: 124, + }, + NormalRule{ + Namespace: "ns", + Name: "goo", + Port: 124, + Weight: 100, + }, + EndpointProbeRule{ + Namespace: "ns", + Name: "second-revision", + Path: "/.well-known/knative/revision/ns/second-revision", + Hash: "ep-ff3cee4d49fbd4547b85c63d56e88eb866d4043951761f069d6afe14a2e61970", + Port: 124, + }, + EndpointProbeRule{ + Namespace: "ns", + Name: "goo", + Path: "/.well-known/knative/revision/ns/goo", + Hash: "ep-ff3cee4d49fbd4547b85c63d56e88eb866d4043951761f069d6afe14a2e61970", + Port: 124, + }, + }, + StatusConditions: []metav1.Condition{{ + Type: string(gatewayapi.RouteConditionAccepted), + Status: metav1.ConditionTrue, + }}, + }.Build(), + }}, + }, { + Name: "multiple visibility - steady state ingress - endpoint probing still not ready", + Key: "ns/name", + Ctx: withStatusManager(&fakeStatusManager{ + FakeIsProbeActive: func(types.NamespacedName) (status.ProbeState, bool) { + return status.ProbeState{ + Ready: false, + Version: "ep-9333a9a68409bb44f2a5f538d2d7c617e5338b6b6c1ebc5e00a19612a5c962c2", + }, true + }, + FakeDoProbes: func(context.Context, status.Backends) (status.ProbeState, error) { + return status.ProbeState{Ready: false}, nil + }, + }), + Objects: append([]runtime.Object{ + ing(withBasicSpec, + withInternalSpec, + withSecondRevisionSpec, + withGatewayAPIclass, + withFinalizer, + makeItReady, + makeLoadBalancerNotReady, + ), + HTTPRoute{ + Name: "example.com", + Namespace: "ns", + Hostname: "example.com", + Rules: []RuleBuilder{ + EndpointProbeRule{ + Namespace: "ns", + Name: "goo", + Hash: "ep-ff3cee4d49fbd4547b85c63d56e88eb866d4043951761f069d6afe14a2e61970", + Port: 123, + }, + NormalRule{ + Namespace: "ns", + Name: "goo", + Port: 123, + Weight: 100, + }, + EndpointProbeRule{ + Namespace: "ns", + Name: "second-revision", + Path: "/.well-known/knative/revision/ns/second-revision", + Hash: "ep-ff3cee4d49fbd4547b85c63d56e88eb866d4043951761f069d6afe14a2e61970", + Port: 123, + }, + EndpointProbeRule{ + Namespace: "ns", + Name: "goo", + Path: "/.well-known/knative/revision/ns/goo", + Hash: "ep-ff3cee4d49fbd4547b85c63d56e88eb866d4043951761f069d6afe14a2e61970", + Port: 123, + }, + }, + StatusConditions: []metav1.Condition{{ + Type: string(gatewayapi.RouteConditionAccepted), + Status: metav1.ConditionTrue, + }}, + }.Build(), + HTTPRoute{ + Name: "foo.svc.cluster.local", + Namespace: "ns", + Hostnames: []string{"foo.svc", "foo.svc.cluster.local"}, + ClusterLocal: true, + Rules: []RuleBuilder{ + EndpointProbeRule{ + Namespace: "ns", + Name: "goo", + Hash: "ep-ff3cee4d49fbd4547b85c63d56e88eb866d4043951761f069d6afe14a2e61970", + Port: 124, + }, + NormalRule{ + Namespace: "ns", + Name: "goo", + Port: 124, + Weight: 100, + }, + EndpointProbeRule{ + Namespace: "ns", + Name: "second-revision", + Path: "/.well-known/knative/revision/ns/second-revision", + Hash: "ep-ff3cee4d49fbd4547b85c63d56e88eb866d4043951761f069d6afe14a2e61970", + Port: 124, + }, + EndpointProbeRule{ + Namespace: "ns", + Name: "goo", + Path: "/.well-known/knative/revision/ns/goo", + Hash: "ep-ff3cee4d49fbd4547b85c63d56e88eb866d4043951761f069d6afe14a2e61970", + Port: 124, + }, + }, + StatusConditions: []metav1.Condition{{ + Type: string(gatewayapi.RouteConditionAccepted), + Status: metav1.ConditionTrue, + }}, + }.Build(), + }, servicesAndEndpoints...), + }, { + Name: "multiple visibility - endpoints are ready - transition to new backends", + Key: "ns/name", + Ctx: withStatusManager(&fakeStatusManager{ + FakeIsProbeActive: func(types.NamespacedName) (status.ProbeState, bool) { + state := status.ProbeState{Ready: true, Version: "ep-ff3cee4d49fbd4547b85c63d56e88eb866d4043951761f069d6afe14a2e61970"} + return state, true + }, + FakeDoProbes: func(context.Context, status.Backends) (status.ProbeState, error) { + return status.ProbeState{Ready: false}, nil + }, + }), + Objects: append([]runtime.Object{ + ing( + withBasicSpec, + withInternalSpec, + withSecondRevisionSpec, + withGatewayAPIclass, + withFinalizer, + makeItReady, + makeLoadBalancerNotReady, + ), + HTTPRoute{ + Name: "example.com", + Namespace: "ns", + Hostname: "example.com", + Rules: []RuleBuilder{ + EndpointProbeRule{ + Namespace: "ns", + Name: "goo", + Hash: "ep-ff3cee4d49fbd4547b85c63d56e88eb866d4043951761f069d6afe14a2e61970", + Port: 123, + }, + NormalRule{ + Namespace: "ns", + Name: "goo", + Port: 123, + Weight: 100, + }, + EndpointProbeRule{ + Namespace: "ns", + Name: "second-revision", + Path: "/.well-known/knative/revision/ns/second-revision", + Hash: "ep-ff3cee4d49fbd4547b85c63d56e88eb866d4043951761f069d6afe14a2e61970", + Port: 123, + }, + EndpointProbeRule{ + Namespace: "ns", + Name: "goo", + Path: "/.well-known/knative/revision/ns/goo", + Hash: "ep-ff3cee4d49fbd4547b85c63d56e88eb866d4043951761f069d6afe14a2e61970", + Port: 123, + }, + }, + StatusConditions: []metav1.Condition{{ + Type: string(gatewayapi.RouteConditionAccepted), + Status: metav1.ConditionTrue, + }}, + }.Build(), + HTTPRoute{ + Name: "foo.svc.cluster.local", + Namespace: "ns", + Hostnames: []string{"foo.svc", "foo.svc.cluster.local"}, + ClusterLocal: true, + Rules: []RuleBuilder{ + EndpointProbeRule{ + Namespace: "ns", + Name: "goo", + Hash: "ep-ff3cee4d49fbd4547b85c63d56e88eb866d4043951761f069d6afe14a2e61970", + Port: 124, + }, + NormalRule{ + Namespace: "ns", + Name: "goo", + Port: 124, + Weight: 100, + }, + EndpointProbeRule{ + Namespace: "ns", + Name: "second-revision", + Path: "/.well-known/knative/revision/ns/second-revision", + Hash: "ep-ff3cee4d49fbd4547b85c63d56e88eb866d4043951761f069d6afe14a2e61970", + Port: 124, + }, + EndpointProbeRule{ + Namespace: "ns", + Name: "goo", + Path: "/.well-known/knative/revision/ns/goo", + Hash: "ep-ff3cee4d49fbd4547b85c63d56e88eb866d4043951761f069d6afe14a2e61970", + Port: 124, + }, + }, + StatusConditions: []metav1.Condition{{ + Type: string(gatewayapi.RouteConditionAccepted), + Status: metav1.ConditionTrue, + }}, + }.Build(), + }, servicesAndEndpoints...), + WantUpdates: []clientgotesting.UpdateActionImpl{{ + Object: HTTPRoute{ + Name: "example.com", + Namespace: "ns", + Hostname: "example.com", + Rules: []RuleBuilder{ + EndpointProbeRule{ + Namespace: "ns", + Name: "second-revision", + Hash: "tr-ff3cee4d49fbd4547b85c63d56e88eb866d4043951761f069d6afe14a2e61970", + Port: 123, + }, + NormalRule{ + Namespace: "ns", + Name: "second-revision", + Port: 123, + Weight: 100, + }, + EndpointProbeRule{ + Namespace: "ns", + Name: "second-revision", + Path: "/.well-known/knative/revision/ns/second-revision", + Hash: "tr-ff3cee4d49fbd4547b85c63d56e88eb866d4043951761f069d6afe14a2e61970", + Port: 123, + }, + EndpointProbeRule{ + Namespace: "ns", + Name: "goo", + Path: "/.well-known/knative/revision/ns/goo", + Hash: "tr-ff3cee4d49fbd4547b85c63d56e88eb866d4043951761f069d6afe14a2e61970", + Port: 123, + }, + }, + StatusConditions: []metav1.Condition{{ + Type: string(gatewayapi.RouteConditionAccepted), + Status: metav1.ConditionTrue, + }}, + }.Build(), + }, { + Object: HTTPRoute{ + Name: "foo.svc.cluster.local", + Namespace: "ns", + Hostnames: []string{"foo.svc", "foo.svc.cluster.local"}, + ClusterLocal: true, + Rules: []RuleBuilder{ + EndpointProbeRule{ + Namespace: "ns", + Name: "second-revision", + Hash: "tr-ff3cee4d49fbd4547b85c63d56e88eb866d4043951761f069d6afe14a2e61970", + Port: 124, + }, + NormalRule{ + Namespace: "ns", + Name: "second-revision", + Port: 124, + Weight: 100, + }, + EndpointProbeRule{ + Namespace: "ns", + Name: "second-revision", + Path: "/.well-known/knative/revision/ns/second-revision", + Hash: "tr-ff3cee4d49fbd4547b85c63d56e88eb866d4043951761f069d6afe14a2e61970", + Port: 124, + }, + EndpointProbeRule{ + Namespace: "ns", + Name: "goo", + Path: "/.well-known/knative/revision/ns/goo", + Hash: "tr-ff3cee4d49fbd4547b85c63d56e88eb866d4043951761f069d6afe14a2e61970", + Port: 124, + }, + }, + StatusConditions: []metav1.Condition{{ + Type: string(gatewayapi.RouteConditionAccepted), + Status: metav1.ConditionTrue, + }}, + }.Build(), + }}, + }, { + Name: "multiple visibility - steady state - transition probing still not ready", + Key: "ns/name", + Ctx: withStatusManager(&fakeStatusManager{ + FakeIsProbeActive: func(types.NamespacedName) (status.ProbeState, bool) { + state := status.ProbeState{Ready: false, Version: "tr-ff3cee4d49fbd4547b85c63d56e88eb866d4043951761f069d6afe14a2e61970"} + return state, true + }, + FakeDoProbes: func(context.Context, status.Backends) (status.ProbeState, error) { + return status.ProbeState{Ready: false}, nil + }, + }), + Objects: append([]runtime.Object{ + ing( + withBasicSpec, + withInternalSpec, + withSecondRevisionSpec, + withGatewayAPIclass, + withFinalizer, + makeItReady, + makeLoadBalancerNotReady, + ), + HTTPRoute{ + Name: "example.com", + Namespace: "ns", + Hostname: "example.com", + Rules: []RuleBuilder{ + EndpointProbeRule{ + Namespace: "ns", + Name: "second-revision", + Hash: "tr-ff3cee4d49fbd4547b85c63d56e88eb866d4043951761f069d6afe14a2e61970", + Port: 123, + }, + NormalRule{ + Namespace: "ns", + Name: "second-revision", + Port: 123, + Weight: 100, + }, + EndpointProbeRule{ + Namespace: "ns", + Name: "second-revision", + Path: "/.well-known/knative/revision/ns/second-revision", + Hash: "tr-ff3cee4d49fbd4547b85c63d56e88eb866d4043951761f069d6afe14a2e61970", + Port: 123, + }, + EndpointProbeRule{ + Namespace: "ns", + Name: "goo", + Path: "/.well-known/knative/revision/ns/goo", + Hash: "tr-ff3cee4d49fbd4547b85c63d56e88eb866d4043951761f069d6afe14a2e61970", + Port: 123, + }, + }, + StatusConditions: []metav1.Condition{{ + Type: string(gatewayapi.RouteConditionAccepted), + Status: metav1.ConditionTrue, + }}, + }.Build(), + HTTPRoute{ + Name: "foo.svc.cluster.local", + Namespace: "ns", + Hostnames: []string{"foo.svc", "foo.svc.cluster.local"}, + ClusterLocal: true, + Rules: []RuleBuilder{ + EndpointProbeRule{ + Namespace: "ns", + Name: "second-revision", + Hash: "tr-ff3cee4d49fbd4547b85c63d56e88eb866d4043951761f069d6afe14a2e61970", + Port: 124, + }, + NormalRule{ + Namespace: "ns", + Name: "second-revision", + Port: 124, + Weight: 100, + }, + EndpointProbeRule{ + Namespace: "ns", + Name: "second-revision", + Path: "/.well-known/knative/revision/ns/second-revision", + Hash: "tr-ff3cee4d49fbd4547b85c63d56e88eb866d4043951761f069d6afe14a2e61970", + Port: 124, + }, + EndpointProbeRule{ + Namespace: "ns", + Name: "goo", + Path: "/.well-known/knative/revision/ns/goo", + Hash: "tr-ff3cee4d49fbd4547b85c63d56e88eb866d4043951761f069d6afe14a2e61970", + Port: 124, + }, + }, + StatusConditions: []metav1.Condition{{ + Type: string(gatewayapi.RouteConditionAccepted), + Status: metav1.ConditionTrue, + }}, + }.Build(), + }, servicesAndEndpoints...), + }, { + Name: "multiple visibility - transition complete - drop probes", + Key: "ns/name", + Ctx: withStatusManager(&fakeStatusManager{ + FakeIsProbeActive: func(types.NamespacedName) (status.ProbeState, bool) { + state := status.ProbeState{Ready: true, Version: "tr-ff3cee4d49fbd4547b85c63d56e88eb866d4043951761f069d6afe14a2e61970"} + return state, true + }, + FakeDoProbes: func(context.Context, status.Backends) (status.ProbeState, error) { + return status.ProbeState{Ready: false}, nil + }, + }), + Objects: append([]runtime.Object{ + ing( + withBasicSpec, + withInternalSpec, + withSecondRevisionSpec, + withGatewayAPIclass, + withFinalizer, + makeItReady, + makeLoadBalancerNotReady, + ), + HTTPRoute{ + Name: "example.com", + Namespace: "ns", + Hostname: "example.com", + Rules: []RuleBuilder{ + EndpointProbeRule{ + Namespace: "ns", + Name: "second-revision", + Hash: "tr-ff3cee4d49fbd4547b85c63d56e88eb866d4043951761f069d6afe14a2e61970", + Port: 123, + }, + NormalRule{ + Namespace: "ns", + Name: "second-revision", + Port: 123, + Weight: 100, + }, + EndpointProbeRule{ + Namespace: "ns", + Name: "second-revision", + Path: "/.well-known/knative/revision/ns/second-revision", + Hash: "tr-ff3cee4d49fbd4547b85c63d56e88eb866d4043951761f069d6afe14a2e61970", + Port: 123, + }, + EndpointProbeRule{ + Namespace: "ns", + Name: "goo", + Path: "/.well-known/knative/revision/ns/goo", + Hash: "tr-ff3cee4d49fbd4547b85c63d56e88eb866d4043951761f069d6afe14a2e61970", + Port: 123, + }, + }, + StatusConditions: []metav1.Condition{{ + Type: string(gatewayapi.RouteConditionAccepted), + Status: metav1.ConditionTrue, + }}, + }.Build(), + HTTPRoute{ + Name: "foo.svc.cluster.local", + Namespace: "ns", + Hostnames: []string{"foo.svc", "foo.svc.cluster.local"}, + ClusterLocal: true, + Rules: []RuleBuilder{ + EndpointProbeRule{ + Namespace: "ns", + Name: "second-revision", + Hash: "tr-ff3cee4d49fbd4547b85c63d56e88eb866d4043951761f069d6afe14a2e61970", + Port: 124, + }, + NormalRule{ + Namespace: "ns", + Name: "second-revision", + Port: 124, + Weight: 100, + }, + EndpointProbeRule{ + Namespace: "ns", + Name: "second-revision", + Path: "/.well-known/knative/revision/ns/second-revision", + Hash: "tr-ff3cee4d49fbd4547b85c63d56e88eb866d4043951761f069d6afe14a2e61970", + Port: 124, + }, + EndpointProbeRule{ + Namespace: "ns", + Name: "goo", + Path: "/.well-known/knative/revision/ns/goo", + Hash: "tr-ff3cee4d49fbd4547b85c63d56e88eb866d4043951761f069d6afe14a2e61970", + Port: 124, + }, + }, + StatusConditions: []metav1.Condition{{ + Type: string(gatewayapi.RouteConditionAccepted), + Status: metav1.ConditionTrue, + }}, + }.Build(), + }, servicesAndEndpoints...), + WantUpdates: []clientgotesting.UpdateActionImpl{{ + Object: HTTPRoute{ + Name: "example.com", + Namespace: "ns", + Hostname: "example.com", + Rules: []RuleBuilder{ + EndpointProbeRule{ + Namespace: "ns", + Name: "second-revision", + Hash: "ff3cee4d49fbd4547b85c63d56e88eb866d4043951761f069d6afe14a2e61970", + Port: 123, + }, + NormalRule{ + Namespace: "ns", + Name: "second-revision", + Port: 123, + Weight: 100, + }, + }, + StatusConditions: []metav1.Condition{{ + Type: string(gatewayapi.RouteConditionAccepted), + Status: metav1.ConditionTrue, + }}, + }.Build(), + }, { + Object: HTTPRoute{ + Name: "foo.svc.cluster.local", + Namespace: "ns", + Hostnames: []string{"foo.svc", "foo.svc.cluster.local"}, + ClusterLocal: true, + Rules: []RuleBuilder{ + EndpointProbeRule{ + Namespace: "ns", + Name: "second-revision", + Hash: "ff3cee4d49fbd4547b85c63d56e88eb866d4043951761f069d6afe14a2e61970", + Port: 124, + }, + NormalRule{ + Namespace: "ns", + Name: "second-revision", + Port: 124, + Weight: 100, + }, + }, + StatusConditions: []metav1.Condition{{ + Type: string(gatewayapi.RouteConditionAccepted), + Status: metav1.ConditionTrue, + }}, + }.Build(), + }}, + }, { + Name: "multiple visibility - dropping probes complete - mark ingress ready", + Key: "ns/name", + Ctx: withStatusManager(&fakeStatusManager{ + FakeIsProbeActive: func(types.NamespacedName) (status.ProbeState, bool) { + state := status.ProbeState{Ready: true, Version: "ff3cee4d49fbd4547b85c63d56e88eb866d4043951761f069d6afe14a2e61970"} + return state, true + }, + FakeDoProbes: func(context.Context, status.Backends) (status.ProbeState, error) { + return status.ProbeState{Ready: true}, nil + }, + }), + Objects: append([]runtime.Object{ + ing( + withBasicSpec, + withInternalSpec, + withSecondRevisionSpec, + withGatewayAPIclass, + withFinalizer, + makeItReady, + makeLoadBalancerNotReady, + ), + HTTPRoute{ + Name: "example.com", + Namespace: "ns", + Hostname: "example.com", + Rules: []RuleBuilder{ + EndpointProbeRule{ + Namespace: "ns", + Name: "second-revision", + Hash: "ff3cee4d49fbd4547b85c63d56e88eb866d4043951761f069d6afe14a2e61970", + Port: 123, + }, + NormalRule{ + Namespace: "ns", + Name: "second-revision", + Port: 123, + Weight: 100, + }, + }, + StatusConditions: []metav1.Condition{{ + Type: string(gatewayapi.RouteConditionAccepted), + Status: metav1.ConditionTrue, + }}, + }.Build(), + HTTPRoute{ + Name: "foo.svc.cluster.local", + Namespace: "ns", + Hostnames: []string{"foo.svc", "foo.svc.cluster.local"}, + ClusterLocal: true, + Rules: []RuleBuilder{ + EndpointProbeRule{ + Namespace: "ns", + Name: "second-revision", + Hash: "ff3cee4d49fbd4547b85c63d56e88eb866d4043951761f069d6afe14a2e61970", + Port: 124, + }, + NormalRule{ + Namespace: "ns", + Name: "second-revision", + Port: 124, + Weight: 100, + }, + }, + StatusConditions: []metav1.Condition{{ + Type: string(gatewayapi.RouteConditionAccepted), + Status: metav1.ConditionTrue, + }}, + }.Build(), + }, servicesAndEndpoints...), + WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ + Object: ing( + withBasicSpec, + withInternalSpec, + withSecondRevisionSpec, + withGatewayAPIclass, + withFinalizer, + makeItReady, + ), + }}, + }, { + Name: "multiple visibility - steady state ingress - probe state flips while reconciliing", + // HTTPRoutes should flip states together + Key: "ns/name", + Ctx: withStatusManager(&fakeStatusManager{ + FakeIsProbeActive: ProbeIsReadyAfter{ + Attempts: 1, + Hash: "ep-ff3cee4d49fbd4547b85c63d56e88eb866d4043951761f069d6afe14a2e61970", + }.Build(), + FakeDoProbes: func(context.Context, status.Backends) (status.ProbeState, error) { + return status.ProbeState{Ready: false}, nil + }, + }), + Objects: append([]runtime.Object{ + ing(withBasicSpec, + withInternalSpec, + withSecondRevisionSpec, + withGatewayAPIclass, + withFinalizer, + makeItReady, + makeLoadBalancerNotReady, + ), + HTTPRoute{ + Name: "example.com", + Namespace: "ns", + Hostname: "example.com", + Rules: []RuleBuilder{ + EndpointProbeRule{ + Namespace: "ns", + Name: "goo", + Hash: "ep-ff3cee4d49fbd4547b85c63d56e88eb866d4043951761f069d6afe14a2e61970", + Port: 123, + }, + NormalRule{ + Namespace: "ns", + Name: "goo", + Port: 123, + Weight: 100, + }, + EndpointProbeRule{ + Namespace: "ns", + Name: "second-revision", + Path: "/.well-known/knative/revision/ns/second-revision", + Hash: "ep-ff3cee4d49fbd4547b85c63d56e88eb866d4043951761f069d6afe14a2e61970", + Port: 123, + }, + EndpointProbeRule{ + Namespace: "ns", + Name: "goo", + Path: "/.well-known/knative/revision/ns/goo", + Hash: "ep-ff3cee4d49fbd4547b85c63d56e88eb866d4043951761f069d6afe14a2e61970", + Port: 123, + }, + }, + StatusConditions: []metav1.Condition{{ + Type: string(gatewayapi.RouteConditionAccepted), + Status: metav1.ConditionTrue, + }}, + }.Build(), + HTTPRoute{ + Name: "foo.svc.cluster.local", + Namespace: "ns", + Hostnames: []string{"foo.svc", "foo.svc.cluster.local"}, + ClusterLocal: true, + Rules: []RuleBuilder{ + EndpointProbeRule{ + Namespace: "ns", + Name: "goo", + Hash: "ep-ff3cee4d49fbd4547b85c63d56e88eb866d4043951761f069d6afe14a2e61970", + Port: 124, + }, + NormalRule{ + Namespace: "ns", + Name: "goo", + Port: 124, + Weight: 100, + }, + EndpointProbeRule{ + Namespace: "ns", + Name: "second-revision", + Path: "/.well-known/knative/revision/ns/second-revision", + Hash: "ep-ff3cee4d49fbd4547b85c63d56e88eb866d4043951761f069d6afe14a2e61970", + Port: 124, + }, + EndpointProbeRule{ + Namespace: "ns", + Name: "goo", + Path: "/.well-known/knative/revision/ns/goo", + Hash: "ep-ff3cee4d49fbd4547b85c63d56e88eb866d4043951761f069d6afe14a2e61970", + Port: 124, + }, + }, + StatusConditions: []metav1.Condition{{ + Type: string(gatewayapi.RouteConditionAccepted), + Status: metav1.ConditionTrue, + }}, + }.Build(), + }, servicesAndEndpoints...), + }} + + table.Test(t, MakeFactory(func(ctx context.Context, listers *Listers, cmw configmap.Watcher) controller.Reconciler { + statusManager := ctx.Value(fakeStatusKey).(status.Manager) + r := &Reconciler{ + gwapiclient: fakegwapiclientset.Get(ctx), + // Listers index properties about resources + httprouteLister: listers.GetHTTPRouteLister(), + gatewayLister: listers.GetGatewayLister(), + statusManager: statusManager, + } + return ingressreconciler.NewReconciler(ctx, logging.FromContext(ctx), fakeingressclient.Get(ctx), + listers.GetIngressLister(), controller.GetEventRecorder(ctx), r, gatewayAPIIngressClassName, + controller.Options{ + ConfigStore: &testConfigStore{ + config: defaultConfig, + }}) + })) +} + +type ProbeIsReadyAfter struct { + Attempts int + Hash string +} + +func (p ProbeIsReadyAfter) Build() func(types.NamespacedName) (status.ProbeState, bool) { + return func(types.NamespacedName) (status.ProbeState, bool) { + ready := p.Attempts <= 0 + p.Attempts-- + return status.ProbeState{Ready: ready, Version: p.Hash}, true + } } func makeLoadBalancerNotReady(i *v1alpha1.Ingress) { diff --git a/pkg/reconciler/ingress/lister_test.go b/pkg/reconciler/ingress/lister_test.go index 1f3b5cec6..e8707a2c3 100644 --- a/pkg/reconciler/ingress/lister_test.go +++ b/pkg/reconciler/ingress/lister_test.go @@ -431,7 +431,7 @@ var ( func withBasicSpec(i *v1alpha1.Ingress) { i.Spec.HTTPOption = v1alpha1.HTTPOptionEnabled - i.Spec.Rules = append(i.Spec.Rules, v1alpha1.IngressRule{ + i.Spec.Rules = []v1alpha1.IngressRule{{ Hosts: []string{"example.com"}, Visibility: v1alpha1.IngressVisibilityExternalIP, HTTP: &v1alpha1.HTTPIngressRuleValue{ @@ -450,17 +450,17 @@ func withBasicSpec(i *v1alpha1.Ingress) { }}, }}, }, - }) + }} } func withSecondRevisionSpec(i *v1alpha1.Ingress) { - withBasicSpec(i) - i.Spec.Rules[0].HTTP.Paths[0].Splits[0].ServiceName = "second-revision" - i.Spec.Rules[0].HTTP.Paths[0].Splits[0].AppendHeaders["K-Serving-Revision"] = "second-revision" + for idx := range i.Spec.Rules { + i.Spec.Rules[idx].HTTP.Paths[0].Splits[0].ServiceName = "second-revision" + i.Spec.Rules[idx].HTTP.Paths[0].Splits[0].AppendHeaders["K-Serving-Revision"] = "second-revision" + } } func withThirdRevisionSpec(i *v1alpha1.Ingress) { - withBasicSpec(i) i.Spec.Rules[0].HTTP.Paths[0].Splits[0].ServiceName = "third-revision" i.Spec.Rules[0].HTTP.Paths[0].Splits[0].AppendHeaders["K-Serving-Revision"] = "third-revision" } diff --git a/pkg/reconciler/ingress/reconcile_resources.go b/pkg/reconciler/ingress/reconcile_resources.go index 485849878..0adf5d2e9 100644 --- a/pkg/reconciler/ingress/reconcile_resources.go +++ b/pkg/reconciler/ingress/reconcile_resources.go @@ -34,6 +34,7 @@ import ( "knative.dev/net-gateway-api/pkg/reconciler/ingress/config" "knative.dev/net-gateway-api/pkg/reconciler/ingress/resources" + "knative.dev/net-gateway-api/pkg/status" netv1alpha1 "knative.dev/networking/pkg/apis/networking/v1alpha1" "knative.dev/networking/pkg/http/header" "knative.dev/pkg/controller" @@ -44,40 +45,42 @@ const listenerPrefix = "kni-" // reconcileHTTPRoute reconciles HTTPRoute. func (c *Reconciler) reconcileHTTPRoute( ctx context.Context, - hash *string, + probe status.ProbeState, + hash string, ing *netv1alpha1.Ingress, rule *netv1alpha1.IngressRule, -) (*gatewayapi.HTTPRoute, error) { +) (*gatewayapi.HTTPRoute, string, error) { recorder := controller.GetEventRecorder(ctx) httproute, err := c.httprouteLister.HTTPRoutes(ing.Namespace).Get(resources.LongestHost(rule.Hosts)) if apierrs.IsNotFound(err) { desired, err := resources.MakeHTTPRoute(ctx, ing, rule) if err != nil { - return nil, err + return nil, probe.Version, err } httproute, err = c.gwapiclient.GatewayV1beta1().HTTPRoutes(desired.Namespace).Create(ctx, desired, metav1.CreateOptions{}) if err != nil { recorder.Eventf(ing, corev1.EventTypeWarning, "CreationFailed", "Failed to create HTTPRoute: %v", err) - return nil, fmt.Errorf("failed to create HTTPRoute: %w", err) + return nil, probe.Version, fmt.Errorf("failed to create HTTPRoute: %w", err) } recorder.Eventf(ing, corev1.EventTypeNormal, "Created", "Created HTTPRoute %q", httproute.GetName()) - return httproute, nil + return httproute, probe.Version, nil } else if err != nil { - return nil, err + return nil, probe.Version, err } - return c.reconcileHTTPRouteUpdate(ctx, hash, ing, rule, httproute.DeepCopy()) + return c.reconcileHTTPRouteUpdate(ctx, probe, hash, ing, rule, httproute.DeepCopy()) } func (c *Reconciler) reconcileHTTPRouteUpdate( ctx context.Context, - hash *string, + probe status.ProbeState, + hash string, ing *netv1alpha1.Ingress, rule *netv1alpha1.IngressRule, httproute *gatewayapi.HTTPRoute, -) (*gatewayapi.HTTPRoute, error) { +) (*gatewayapi.HTTPRoute, string, error) { const ( endpointPrefix = "ep-" @@ -85,18 +88,14 @@ func (c *Reconciler) reconcileHTTPRouteUpdate( ) var ( - probeKey = types.NamespacedName{ - Name: ing.Name, - Namespace: ing.Namespace, - } original = httproute.DeepCopy() recorder = controller.GetEventRecorder(ctx) - desired *gatewayapi.HTTPRoute - err error - probe, _ = c.statusManager.IsProbeActive(probeKey) wasEndpointProbe = strings.HasPrefix(probe.Version, endpointPrefix) wasTransitionProbe = strings.HasPrefix(probe.Version, transitionPrefix) + + desired *gatewayapi.HTTPRoute + err error ) probeHash := strings.TrimPrefix(probe.Version, endpointPrefix) @@ -104,45 +103,45 @@ func (c *Reconciler) reconcileHTTPRouteUpdate( newBackends, oldBackends := computeBackends(httproute, rule) - if wasTransitionProbe && probeHash == *hash && probe.Ready { + if wasTransitionProbe && probeHash == hash && probe.Ready { desired, err = resources.MakeHTTPRoute(ctx, ing, rule) - } else if wasEndpointProbe && probeHash == *hash && probe.Ready { - *hash = transitionPrefix + *hash + } else if wasEndpointProbe && probeHash == hash && probe.Ready { + hash = transitionPrefix + hash desired, err = resources.MakeHTTPRoute(ctx, ing, rule) - resources.UpdateProbeHash(desired, *hash) + resources.UpdateProbeHash(desired, hash) resources.RemoveEndpointProbes(httproute) for _, backend := range newBackends { - resources.AddEndpointProbe(desired, *hash, backend) + resources.AddEndpointProbe(desired, hash, backend) } for _, backend := range oldBackends { - resources.AddOldBackend(desired, *hash, backend) + resources.AddOldBackend(desired, hash, backend) } } else if len(newBackends) > 0 { - *hash = endpointPrefix + *hash + hash = endpointPrefix + hash desired = httproute.DeepCopy() - resources.UpdateProbeHash(desired, *hash) + resources.UpdateProbeHash(desired, hash) resources.RemoveEndpointProbes(desired) for _, backend := range newBackends { - resources.AddEndpointProbe(desired, *hash, backend) + resources.AddEndpointProbe(desired, hash, backend) } for _, backend := range oldBackends { - resources.AddOldBackend(desired, *hash, backend) + resources.AddOldBackend(desired, hash, backend) } - } else if probeHash != *hash { + } else if probeHash != hash { desired, err = resources.MakeHTTPRoute(ctx, ing, rule) } else { // Noop if probe.Version != "" { - *hash = probe.Version + hash = probe.Version } // desired, err = resources.MakeHTTPRoute(ctx, ing, rule) - return httproute, nil + return httproute, hash, nil } if err != nil { - return nil, err + return nil, hash, err } if !equality.Semantic.DeepEqual(original.Spec, desired.Spec) || @@ -159,12 +158,12 @@ func (c *Reconciler) reconcileHTTPRouteUpdate( if err != nil { recorder.Eventf(ing, corev1.EventTypeWarning, "UpdateFailed", "Failed to update HTTPRoute: %v", err) - return nil, fmt.Errorf("failed to update HTTPRoute: %w", err) + return nil, hash, fmt.Errorf("failed to update HTTPRoute: %w", err) } - return updated, nil + return updated, hash, nil } - return httproute, nil + return httproute, hash, nil } func (c *Reconciler) reconcileTLS(