diff --git a/pkg/reconciler/ingress/status.go b/pkg/reconciler/ingress/status.go index 0a58d7410446..f99bd65de831 100644 --- a/pkg/reconciler/ingress/status.go +++ b/pkg/reconciler/ingress/status.go @@ -349,8 +349,7 @@ func (m *StatusProber) processWorkItem() bool { transport, item.url, prober.WithHeader(network.ProbeHeaderName, network.ProbeHeaderValue), - prober.ExpectsStatusCodes([]int{http.StatusOK}), - prober.ExpectsHeader(network.HashHeaderName, item.ingressState.hash)) + m.probeVerifier(item)) // In case of cancellation, drop the work item select { @@ -370,6 +369,40 @@ func (m *StatusProber) processWorkItem() bool { return true } +func (m *StatusProber) probeVerifier(item *workItem) prober.Verifier { + return func(r *http.Response, _ []byte) (bool, error) { + // In the happy path, the probe request is forwarded to Activator or Queue-Proxy and the response (HTTP 200) + // contains the "K-Network-Hash" header that can be compared with the expected hash. If the hashes match, + // probing is successful, if they don't match, a new probe will be sent later. + // An HTTP 404/503 is expected in the case of the creation of a new Knative service because the rules will + // not be present in the Envoy config until the new VirtualService is applied. + // No information can be extracted from any other scenario (e.g. HTTP 302), therefore in that case, + // probing is assumed to be successful because it is better to say that an Ingress is Ready before it + // actually is Ready than never marking it as Ready. It is best effort. + switch r.StatusCode { + case http.StatusOK: + hash := r.Header.Get(network.HashHeaderName) + switch hash { + case "": + m.logger.Errorf("Probing of %s abandoned, IP: %s:%s: the response doesn't contain the %q header", + item.url, item.podIP, item.podPort, network.HashHeaderName) + return true, nil + case item.ingressState.hash: + return true, nil + default: + return false, fmt.Errorf("unexpected hash: want %q, got %q", item.ingressState.hash, hash) + } + return false, fmt.Errorf("unexpected hash: want %q, got %q", item.ingressState.hash, hash) + case http.StatusNotFound, http.StatusServiceUnavailable: + return false, fmt.Errorf("unexpected status code: want %v, got %v", http.StatusOK, http.StatusNotFound) + default: + m.logger.Errorf("Probing of %s abandoned, IP: %s:%s: the response status is %v, expected 200 or 404", + item.url, item.podIP, item.podPort, r.StatusCode) + return true, nil + } + } +} + func (m *StatusProber) updateStates(ingressState *ingressState, podState *podState) { if atomic.AddInt32(&podState.successCount, 1) == 1 { // This is the first successful probe call for the pod, cancel all other work items for this pod diff --git a/pkg/reconciler/ingress/status_test.go b/pkg/reconciler/ingress/status_test.go index 7e8ad6abe366..47efa384b80f 100644 --- a/pkg/reconciler/ingress/status_test.go +++ b/pkg/reconciler/ingress/status_test.go @@ -668,6 +668,83 @@ func TestCancellation(t *testing.T) { } } +func TestProbeVerifier(t *testing.T) { + const hash = "Hi! I am hash!" + prober := NewStatusProber(zaptest.NewLogger(t).Sugar(), nil, nil, nil, nil) + verifier := prober.probeVerifier(&workItem{ + ingressState: &ingressState{ + hash: hash, + }, + podState: nil, + podIP: "", + podPort: "", + }) + cases := []struct { + name string + resp *http.Response + want bool + }{{ + name: "HTTP 200 matching hash", + resp: &http.Response{ + StatusCode: http.StatusOK, + Header: http.Header{network.HashHeaderName: []string{hash}}, + }, + want: true, + }, { + name: "HTTP 200 mismatching hash", + resp: &http.Response{ + StatusCode: http.StatusOK, + Header: http.Header{network.HashHeaderName: []string{"nope"}}, + }, + want: false, + }, { + name: "HTTP 200 missing header", + resp: &http.Response{ + StatusCode: http.StatusOK, + }, + want: true, + }, { + name: "HTTP 404", + resp: &http.Response{ + StatusCode: http.StatusNotFound, + }, + want: false, + }, { + name: "HTTP 503", + resp: &http.Response{ + StatusCode: http.StatusServiceUnavailable, + }, + want: false, + }, { + name: "HTTP 403", + resp: &http.Response{ + StatusCode: http.StatusForbidden, + }, + want: true, + }, { + name: "HTTP 503", + resp: &http.Response{ + StatusCode: http.StatusServiceUnavailable, + }, + want: false, + }, { + name: "HTTP 302", + resp: &http.Response{ + StatusCode: http.StatusFound, + }, + want: true, + }} + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + got, _ := verifier(c.resp, nil) + if got != c.want { + t.Errorf("got: %v, want: %v", got, c.want) + } + }) + } +} + type fakeGatewayLister struct { gateways []*v1alpha3.Gateway fails bool