-
Notifications
You must be signed in to change notification settings - Fork 261
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Flaky E2E test: e2e/TestDomain - create domain with TLS #1873
Comments
/assign |
The original issue is not fixed. We need to further investigate & verify the wait loop is working as expected and maybe fix ti. @xiangpingjiang I'll remove your previous assignment. But feel free to assign again and do a second pass if you are still interested. |
@dsimansk |
|
/assign |
The suspicious part in test log is the following. See There's still possibility to a short timeout (e.g. 3 - 5s) between create and verify operations. But I'd rather check first if the wait loop is working as expected.
|
It could be a timing related issue. This may work better for the for loop func domainDescribe(r *test.KnRunResultCollector, domainName string, tls bool) {
k := test.NewKubectl(r.KnTest().Kn().Namespace())
// Wait for Domain Mapping URL to be populated
var url string
timeout := time.After(30 * time.Second) // Adjust the timeout as needed
for {
select {
case <-time.After(time.Second):
out, err := k.Run("get", "domainmapping", domainName, "-o=jsonpath='{.status.url}'")
assert.NilError(r.T(), err)
if len(out) > 0 {
url = out
break
}
case <-timeout:
assert.Fail(r.T(), "Timed out waiting for the domain mapping URL to be populated")
return
}
}
out := r.KnTest().Kn().Run("domain", "describe", domainName)
r.AssertNoError(out)
if tls {
url = "https://" + domainName
} else {
url = "http://" + domainName
}
assert.Assert(r.T(), util.ContainsAll(out.Stdout, "Name", "Namespace", "URL", "Service", url))
} By introducing this more robust waiting mechanism, you can mitigate timing issues and reduce the likelihood of intermittent test failures. |
Thanks @Ipppoooo , i think its worth a try. Maybe don't retry every second as this will cause 30 calls if this is a non-recoverable error. Maybe use e.g. 5seconds or an exponential backoff. Fancy to send in a PR ? |
hello @rhuss It's different conditions. What do you think? |
The e2e tests are intermittently failing on e2e/TestDomain. In a specific
https
test case. When the URL should containhttps
but is empty. It's most likely a timing issue with empty value being checked prematurely.There's a wait loop to wait for URL to be populated before asserting it. It might be not working as expected.
https://github.com/knative/client/blob/2d0415a7951b99130ccb514fa0e3c5343338225c/test/e2e/domain_mapping_test.go#L136C1-L144C3
Error log:
https://prow.knative.dev/view/gs/knative-prow/logs/continuous_client_main_periodic/1710944161479266304#
/kind good-first-issue
The text was updated successfully, but these errors were encountered: