Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 35 additions & 2 deletions pkg/reconciler/ingress/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
Expand Down
77 changes: 77 additions & 0 deletions pkg/reconciler/ingress/status_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down