HTTPS tests are flakey - Githubissues

dprotaso commented 1 year ago

https://testgrid.k8s.io/r/knative-own-testgrid/serving#https

github-actions[bot] commented 8 months ago

This issue is stale because it has been open for 90 days with no activity. It will automatically close after 30 more days of inactivity. Reopen the issue with /reopen. Mark the issue as fresh by adding the comment /remove-lifecycle stale.

ReToCode commented 5 months ago

I was able to find the root cause and was able to make the tests pass again here on a hacky PR.

Summary

There is a timing issue with how we handle external-domain-tls
A Knative Service is created first without the tls block in the KnativeIngress object as long as the secret is not there, we even have code to downgrade to http only if this happens, as the http path might already work
net-istio will thus configure a http route for the services external domain on the shared gateway
Our golang tests now see the Knative Service as ready=true and the first check calls out using http which also passes
If a certificate is now created, the Service/Route/Ingress are reconciled which causes net-istio to create a dedicated gateway for https while removing that domain from the shared gateway. It seems that that process is not seamless when there are already open connections on the "old" http configuration. This causes vegeta and other tests to fail.
This only happens when net-certmanager + certmanager have a backlog, meaning on parallel tests or on creation of a lot of services simultaneously.

Possible solutions

a) Our current definition says, a Knative Service can be ready if just http works. The certificate might never be provisioned, the Knative Service will still stay ready=true and http will work. We could ask if this is correct. If a user ask for external-domain-tls=enabled, why is a service that never once served https ready?

b) we could extend our tests to make sure they wait for the protocol to be ready that they test for. E.g. extend https://github.com/knative/serving/blob/bb9c8158a34a155ecf95ac0c4094bea600545b74/vendor/knative.dev/pkg/test/request.go#L147 to know when we except https (https://github.com/knative/serving/blob/3ac6d4a5e66003171fafcf8ac3dc3e365b6f2c77/test/e2e_flags.go#L38) and wait for the https route to be fully ready before doing our actual test cases.

c) other ideas?

Please add your ideas/opinions: @dprotaso @skonto @izabelacg

skonto commented 4 months ago

@ReToCode I think b) is less intrusive to fix. Maybe try that for now?

knative / serving

HTTPS tests are flakey #14435