All controller unit tests should not cancel context until test is over

All controller unit tests were accidentally using a timeout context
for the informers, instead of a cancel context which stays alive until
each test is completely finished. There is no reason to risk
unpredictable behavior of a timeout being reached during an individual
test, even though with the previous 3 second timeout it could only be
reached on a machine which is running orders of magnitude slower than
usual, since each test usually runs in about 100-300 ms. Unfortunately,
sometimes our CI workers might get that slow.

This sparked a review of other usages of timeout contexts in other
tests, and all of them were increased to a minimum value of 1 minute,
under the rule of thumb that our tests will be more reliable on slow
machines if they "pass fast and fail slow".
This commit is contained in:
Ryan Richard
2021-03-04 17:26:01 -08:00
parent b102aa8991
commit d8c6894cbc
39 changed files with 182 additions and 182 deletions
+1 -1
View File
@@ -38,7 +38,7 @@ func TestCLIGetKubeconfigStaticToken(t *testing.T) {
library.AssertNoRestartsDuringTest(t, env.ConciergeNamespace, "")
// Create a test webhook configuration to use with the CLI.
ctx, cancelFunc := context.WithTimeout(context.Background(), 4*time.Minute)
ctx, cancelFunc := context.WithTimeout(context.Background(), 5*time.Minute)
defer cancelFunc()
authenticator := library.CreateTestWebhookAuthenticator(ctx, t)
@@ -78,7 +78,7 @@ func TestAPIServingCertificateAutoCreationAndRotation(t *testing.T) {
kubeClient := library.NewKubernetesClientset(t)
aggregatedClient := library.NewAggregatedClientset(t)
conciergeClient := library.NewConciergeClientset(t)
ctx, cancel := context.WithTimeout(context.Background(), 3*time.Minute)
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute)
defer cancel()
apiServiceName := "v1alpha1.login.concierge." + env.APIGroupSuffix
@@ -20,7 +20,7 @@ func TestGetDeployment(t *testing.T) {
env := library.IntegrationEnv(t)
client := library.NewKubernetesClientset(t)
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
defer cancel()
appDeployment, err := client.AppsV1().Deployments(env.ConciergeNamespace).Get(ctx, env.ConciergeAppName, metav1.GetOptions{})
+1 -1
View File
@@ -59,7 +59,7 @@ func TestClient(t *testing.T) {
library.AssertNoRestartsDuringTest(t, env.ConciergeNamespace, "")
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
defer cancel()
webhook := library.CreateTestWebhookAuthenticator(ctx, t)
@@ -25,7 +25,7 @@ func TestCredentialIssuer(t *testing.T) {
library.AssertNoRestartsDuringTest(t, env.ConciergeNamespace, "")
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
defer cancel()
t.Run("test successful CredentialIssuer", func(t *testing.T) {
@@ -27,7 +27,7 @@ func TestUnsuccessfulCredentialRequest(t *testing.T) {
library.AssertNoRestartsDuringTest(t, env.ConciergeNamespace, "")
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
defer cancel()
response, err := makeRequest(ctx, t, validCredentialRequestSpecWithRealToken(t, corev1.TypedLocalObjectReference{
@@ -137,7 +137,7 @@ func TestFailedCredentialRequestWhenTheRequestIsValidButTheTokenDoesNotAuthentic
// Create a testWebhook so we have a legitimate authenticator to pass to the
// TokenCredentialRequest API.
ctx, cancel := context.WithTimeout(context.Background(), time.Second*30)
ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
defer cancel()
testWebhook := library.CreateTestWebhookAuthenticator(ctx, t)
@@ -160,7 +160,7 @@ func TestCredentialRequest_ShouldFailWhenRequestDoesNotIncludeToken(t *testing.T
// Create a testWebhook so we have a legitimate authenticator to pass to the
// TokenCredentialRequest API.
ctx, cancel := context.WithTimeout(context.Background(), time.Second*30)
ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
defer cancel()
testWebhook := library.CreateTestWebhookAuthenticator(ctx, t)
@@ -188,7 +188,7 @@ func TestCredentialRequest_OtherwiseValidRequestWithRealTokenShouldFailWhenTheCl
library.AssertNoRestartsDuringTest(t, env.ConciergeNamespace, "")
ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second)
ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
defer cancel()
testWebhook := library.CreateTestWebhookAuthenticator(ctx, t)
@@ -208,7 +208,7 @@ func makeRequest(ctx context.Context, t *testing.T, spec loginv1alpha1.TokenCred
client := library.NewAnonymousConciergeClientset(t)
ctx, cancel := context.WithTimeout(ctx, 10*time.Second)
ctx, cancel := context.WithTimeout(ctx, time.Minute)
defer cancel()
return client.LoginV1alpha1().TokenCredentialRequests().Create(ctx, &loginv1alpha1.TokenCredentialRequest{
@@ -96,7 +96,7 @@ func TestImpersonationProxy(t *testing.T) {
}
// At the end of the test, clean up the ConfigMap.
t.Cleanup(func() {
ctx, cancel = context.WithTimeout(context.Background(), 10*time.Second)
ctx, cancel = context.WithTimeout(context.Background(), time.Minute)
defer cancel()
// Delete any version that was created by this test.
@@ -30,7 +30,7 @@ func TestKubeCertAgent(t *testing.T) {
library.AssertNoRestartsDuringTest(t, env.ConciergeNamespace, "")
ctx, cancel := context.WithTimeout(context.Background(), 1*time.Minute)
ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
defer cancel()
kubeClient := library.NewKubernetesClientset(t)
+2 -2
View File
@@ -240,7 +240,7 @@ func verifyTokenResponse(
nonceParam nonce.Nonce,
expectedIDTokenClaims []string,
) {
ctx, cancel := context.WithTimeout(context.Background(), 1*time.Minute)
ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
defer cancel()
// Verify the ID Token.
@@ -310,7 +310,7 @@ func (s *localCallbackServer) waitForCallback(timeout time.Duration) *http.Reque
}
func doTokenExchange(t *testing.T, config *oauth2.Config, tokenResponse *oauth2.Token, httpClient *http.Client, provider *coreosoidc.Provider) {
ctx, cancel := context.WithTimeout(context.Background(), 1*time.Minute)
ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
defer cancel()
// Form the HTTP POST request with the parameters specified by RFC8693.
@@ -99,7 +99,7 @@ func createSecret(ctx context.Context, t *testing.T, secrets corev1client.Secret
// Make sure the Secret is deleted when the test ends.
t.Cleanup(func() {
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
defer cancel()
err := secrets.Delete(ctx, secret.Name, metav1.DeleteOptions{})
notFound := k8serrors.IsNotFound(err)
+2 -2
View File
@@ -42,13 +42,13 @@ func TestAuthorizeCodeStorage(t *testing.T) {
secrets := client.CoreV1().Secrets(env.SupervisorNamespace)
t.Cleanup(func() {
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
defer cancel()
err := secrets.Delete(ctx, name, metav1.DeleteOptions{})
require.NoError(t, err)
})
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
defer cancel()
// get a session with most of the data filled out
+7 -7
View File
@@ -34,7 +34,7 @@ func TestWhoAmI_Kubeadm(t *testing.T) {
// we should add more robust logic around skipping clusters based on vendor
_ = library.IntegrationEnv(t).WithCapability(library.ClusterSigningKeyIsAvailable)
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
defer cancel()
whoAmI, err := library.NewConciergeClientset(t).IdentityV1alpha1().WhoAmIRequests().
@@ -63,7 +63,7 @@ func TestWhoAmI_Kubeadm(t *testing.T) {
func TestWhoAmI_ServiceAccount_Legacy(t *testing.T) {
_ = library.IntegrationEnv(t)
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
defer cancel()
kubeClient := library.NewKubernetesClientset(t).CoreV1()
@@ -107,7 +107,7 @@ func TestWhoAmI_ServiceAccount_Legacy(t *testing.T) {
return false, err
}
return len(secret.Data[corev1.ServiceAccountTokenKey]) > 0, nil
}, 30*time.Second, time.Second)
}, time.Minute, time.Second)
saConfig := library.NewAnonymousClientRestConfig(t)
saConfig.BearerToken = string(secret.Data[corev1.ServiceAccountTokenKey])
@@ -140,7 +140,7 @@ func TestWhoAmI_ServiceAccount_Legacy(t *testing.T) {
func TestWhoAmI_ServiceAccount_TokenRequest(t *testing.T) {
_ = library.IntegrationEnv(t)
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
defer cancel()
kubeClient := library.NewKubernetesClientset(t).CoreV1()
@@ -258,7 +258,7 @@ func TestWhoAmI_CSR(t *testing.T) {
// we should add more robust logic around skipping clusters based on vendor
_ = library.IntegrationEnv(t).WithCapability(library.ClusterSigningKeyIsAvailable)
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
defer cancel()
kubeClient := library.NewKubernetesClientset(t)
@@ -346,7 +346,7 @@ func TestWhoAmI_CSR(t *testing.T) {
func TestWhoAmI_Anonymous(t *testing.T) {
_ = library.IntegrationEnv(t)
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
defer cancel()
anonymousConfig := library.NewAnonymousClientRestConfig(t)
@@ -377,7 +377,7 @@ func TestWhoAmI_Anonymous(t *testing.T) {
func TestWhoAmI_ImpersonateDirectly(t *testing.T) {
_ = library.IntegrationEnv(t)
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
defer cancel()
impersonationConfig := library.NewClientConfig(t)
+1 -1
View File
@@ -88,7 +88,7 @@ func getRestartCounts(t *testing.T, namespace, labelSelector string) map[string]
t.Helper()
kubeClient := NewKubernetesClientset(t)
ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second)
ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
defer cancel()
pods, err := kubeClient.CoreV1().Pods(namespace).List(ctx, metav1.ListOptions{LabelSelector: labelSelector})
+10 -10
View File
@@ -156,7 +156,7 @@ func CreateTestWebhookAuthenticator(ctx context.Context, t *testing.T) corev1.Ty
client := NewConciergeClientset(t)
webhooks := client.AuthenticationV1alpha1().WebhookAuthenticators()
createContext, cancel := context.WithTimeout(ctx, 5*time.Second)
createContext, cancel := context.WithTimeout(ctx, time.Minute)
defer cancel()
webhook, err := webhooks.Create(createContext, &auth1alpha1.WebhookAuthenticator{
@@ -169,7 +169,7 @@ func CreateTestWebhookAuthenticator(ctx context.Context, t *testing.T) corev1.Ty
t.Cleanup(func() {
t.Helper()
t.Logf("cleaning up test WebhookAuthenticator %s/%s", webhook.Namespace, webhook.Name)
deleteCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
deleteCtx, cancel := context.WithTimeout(context.Background(), time.Minute)
defer cancel()
err := webhooks.Delete(deleteCtx, webhook.Name, metav1.DeleteOptions{})
require.NoErrorf(t, err, "could not cleanup test WebhookAuthenticator %s/%s", webhook.Namespace, webhook.Name)
@@ -219,7 +219,7 @@ func CreateTestJWTAuthenticator(ctx context.Context, t *testing.T, spec auth1alp
client := NewConciergeClientset(t)
jwtAuthenticators := client.AuthenticationV1alpha1().JWTAuthenticators()
createContext, cancel := context.WithTimeout(ctx, 5*time.Second)
createContext, cancel := context.WithTimeout(ctx, time.Minute)
defer cancel()
jwtAuthenticator, err := jwtAuthenticators.Create(createContext, &auth1alpha1.JWTAuthenticator{
@@ -232,7 +232,7 @@ func CreateTestJWTAuthenticator(ctx context.Context, t *testing.T, spec auth1alp
t.Cleanup(func() {
t.Helper()
t.Logf("cleaning up test JWTAuthenticator %s/%s", jwtAuthenticator.Namespace, jwtAuthenticator.Name)
deleteCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
deleteCtx, cancel := context.WithTimeout(context.Background(), time.Minute)
defer cancel()
err := jwtAuthenticators.Delete(deleteCtx, jwtAuthenticator.Name, metav1.DeleteOptions{})
require.NoErrorf(t, err, "could not cleanup test JWTAuthenticator %s/%s", jwtAuthenticator.Namespace, jwtAuthenticator.Name)
@@ -255,7 +255,7 @@ func CreateTestFederationDomain(ctx context.Context, t *testing.T, issuer string
t.Helper()
testEnv := IntegrationEnv(t)
createContext, cancel := context.WithTimeout(ctx, 5*time.Second)
createContext, cancel := context.WithTimeout(ctx, time.Minute)
defer cancel()
if issuer == "" {
@@ -276,7 +276,7 @@ func CreateTestFederationDomain(ctx context.Context, t *testing.T, issuer string
t.Cleanup(func() {
t.Helper()
t.Logf("cleaning up test FederationDomain %s/%s", federationDomain.Namespace, federationDomain.Name)
deleteCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
deleteCtx, cancel := context.WithTimeout(context.Background(), time.Minute)
defer cancel()
err := federationDomains.Delete(deleteCtx, federationDomain.Name, metav1.DeleteOptions{})
notFound := k8serrors.IsNotFound(err)
@@ -330,7 +330,7 @@ func RandHex(t *testing.T, numBytes int) string {
func CreateTestSecret(t *testing.T, namespace string, baseName string, secretType corev1.SecretType, stringData map[string]string) *corev1.Secret {
t.Helper()
client := NewKubernetesClientset(t)
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
defer cancel()
created, err := client.CoreV1().Secrets(namespace).Create(ctx, &corev1.Secret{
@@ -401,7 +401,7 @@ func CreateTestOIDCIdentityProvider(t *testing.T, spec idpv1alpha1.OIDCIdentityP
func CreateTestClusterRoleBinding(t *testing.T, subject rbacv1.Subject, roleRef rbacv1.RoleRef) *rbacv1.ClusterRoleBinding {
t.Helper()
client := NewKubernetesClientset(t)
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
defer cancel()
// Create the ClusterRoleBinding using GenerateName to get a random name.
@@ -426,7 +426,7 @@ func CreateTestClusterRoleBinding(t *testing.T, subject rbacv1.Subject, roleRef
func WaitForUserToHaveAccess(t *testing.T, user string, groups []string, shouldHaveAccessTo *authorizationv1.ResourceAttributes) {
t.Helper()
client := NewKubernetesClientset(t)
ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second)
ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
defer cancel()
RequireEventuallyWithoutError(t, func() (bool, error) {
@@ -441,7 +441,7 @@ func WaitForUserToHaveAccess(t *testing.T, user string, groups []string, shouldH
return false, err
}
return subjectAccessReview.Status.Allowed, nil
}, 10*time.Second, 500*time.Millisecond)
}, time.Minute, 500*time.Millisecond)
}
func testObjectMeta(t *testing.T, baseName string) metav1.ObjectMeta {
+1 -1
View File
@@ -22,7 +22,7 @@ func DumpLogs(t *testing.T, namespace string, labelSelector string) {
}
kubeClient := NewKubernetesClientset(t)
ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second)
ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
defer cancel()
logTailLines := int64(40)