Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[k8s] Calling WaitUntilDeploymentAvailable can cause a panic #1329

Closed
antoninbas opened this issue Aug 9, 2023 · 1 comment · Fixed by #1330
Closed

[k8s] Calling WaitUntilDeploymentAvailable can cause a panic #1329

antoninbas opened this issue Aug 9, 2023 · 1 comment · Fixed by #1330
Labels
bug Something isn't working

Comments

@antoninbas
Copy link
Contributor

Describe the bug
Calling k8s.WaitUntilDeploymentAvailable too early can cause a panic.
If the function is called too quickly after the Deployment was created, a panic can occur because Status.Conditions have not been populated yet.
This code assumes that Status.Conditions[0] is always valid:

terratest/modules/k8s/errors.go

Lines 70 to 78 in 2df0170

// Error is a simple function to return a formatted error message as a string
func (err DeploymentNotAvailable) Error() string {
return fmt.Sprintf(
"Deployment %s is not available, reason: %s, message: %s",
err.deploy.Name,
err.deploy.Status.Conditions[0].Reason,
err.deploy.Status.Conditions[0].Message,
)
}

To Reproduce
Something like this can reproduce the issue (with a Helm chart that includes a Deployment), but it won't happen very often as the failure is timing-dependent.

helm.Install(t, helmOptions, helmChartPath, releaseName)
defer helm.Delete(t, helmOptions, releaseName, true)
k8s.WaitUntilDeploymentAvailable(t, kubectlOptions, deploymentName, 60, 1*time.Second)

Expected behavior
Calling k8s.WaitUntilDeploymentAvailable should never cause the test to panic.
The implementation of DeploymentNotAvailable should be updated to remove the assumption that Status.Conditions[0] always exists.

Nice to have
Here are some logs of a panic happening during a test:

2023-08-09T04:55:06.5454822Z TestInstall/https_-_auto 2023-08-09T04:55:06Z retry.go:91: Wait for deployment antrea-ui to be provisioned.
2023-08-09T04:55:06.5456228Z TestInstall/https_-_auto 2023-08-09T04:55:06Z client.go:42: Configuring Kubernetes client using config file /home/runner/.kube/config with context 
2023-08-09T04:55:06.6051023Z TestInstall/https_-_auto 2023-08-09T04:55:06Z logger.go:66: Running command helm with args [delete --namespace kube-system antrea-ui-swu3ts]
2023-08-09T04:55:07.1117086Z TestInstall/https_-_auto 2023-08-09T04:55:07Z logger.go:66: release "antrea-ui-swu3ts" uninstalled
2023-08-09T04:55:07.1140862Z --- FAIL: TestInstall (8.69s)
2023-08-09T04:55:07.1149031Z     --- PASS: TestInstall/default (4.60s)
2023-08-09T04:55:07.1149506Z     --- FAIL: TestInstall/https_-_auto (4.08s)
2023-08-09T04:55:07.1165535Z panic: runtime error: index out of range [0] with length 0 [recovered]
2023-08-09T04:55:07.1166167Z 	panic: runtime error: index out of range [0] with length 0
2023-08-09T04:55:07.1264251Z 
2023-08-09T04:55:07.1264603Z goroutine 67 [running]:
2023-08-09T04:55:07.1266283Z testing.tRunner.func1.2({0x182e180, 0xc00060c180})
2023-08-09T04:55:07.1268024Z 	/opt/hostedtoolcache/go/1.20.6/x64/src/testing/testing.go:1526 +0x24e
2023-08-09T04:55:07.1269440Z testing.tRunner.func1()
2023-08-09T04:55:07.1269779Z 	/opt/hostedtoolcache/go/1.20.6/x64/src/testing/testing.go:1529 +0x39f
2023-08-09T04:55:07.1270087Z panic({0x182e180, 0xc00060c180})
2023-08-09T04:55:07.1271348Z 	/opt/hostedtoolcache/go/1.20.6/x64/src/runtime/panic.go:884 +0x213
2023-08-09T04:55:07.1273105Z github.com/gruntwork-io/terratest/modules/k8s.DeploymentNotAvailable.Error({0x1d0c8b0?})
2023-08-09T04:55:07.1274753Z 	/home/runner/go/pkg/mod/github.com/gruntwork-io/terratest@v0.43.11/modules/k8s/errors.go:75 +0xf8
2023-08-09T04:55:07.1276550Z github.com/gruntwork-io/terratest/modules/retry.DoWithRetryInterfaceE({0x1d0c8b0, 0xc0000ad380}, {0xc00004d410, 0x30}, 0x3c, 0x1?, 0xc000757c88)
2023-08-09T04:55:07.1278227Z 	/home/runner/go/pkg/mod/github.com/gruntwork-io/terratest@v0.43.11/modules/retry/retry.go:103 +0x71
2023-08-09T04:55:07.1280806Z github.com/gruntwork-io/terratest/modules/retry.DoWithRetryE({0x1d0c8b0?, 0xc0000ad380?}, {0xc00004d410?, 0x1?}, 0x1?, 0x1cf58c0?, 0xc00011a008?)
2023-08-09T04:55:07.1281487Z 	/home/runner/go/pkg/mod/github.com/gruntwork-io/terratest@v0.43.11/modules/retry/retry.go:68 +0x45
2023-08-09T04:55:07.1283202Z github.com/gruntwork-io/terratest/modules/k8s.WaitUntilDeploymentAvailableE({0x1d0c8b0, 0xc0000ad380}, 0xc0000406e0, {0x191e384, 0x9}, 0x1d0c8b0?, 0xc0000ad380?)
2023-08-09T04:55:07.1285317Z 	/home/runner/go/pkg/mod/github.com/gruntwork-io/terratest@v0.43.11/modules/k8s/deployment.go:75 +0x126
2023-08-09T04:55:07.1287076Z github.com/gruntwork-io/terratest/modules/k8s.WaitUntilDeploymentAvailable({0x1d0c8b0?, 0xc0000ad380}, 0xc0000406e0?, {0x191e384?, 0x9?}, 0xc00072a6e0?, 0x599?)
2023-08-09T04:55:07.1290355Z 	/home/runner/go/pkg/mod/github.com/gruntwork-io/terratest@v0.43.11/modules/k8s/deployment.go:61 +0x2d
2023-08-09T04:55:07.1291915Z antrea.io/antrea-ui/test/e2e_helm.TestInstall.func1(0xc0000ad380)
2023-08-09T04:55:07.1293370Z 	/home/runner/work/antrea-ui/antrea-ui/test/e2e_helm/install_test.go:109 +0x205
2023-08-09T04:55:07.1293732Z testing.tRunner(0xc0000ad380, 0xc00041c3c0)
2023-08-09T04:55:07.1294063Z 	/opt/hostedtoolcache/go/1.20.6/x64/src/testing/testing.go:1576 +0x10b
2023-08-09T04:55:07.1295485Z created by testing.(*T).Run
2023-08-09T04:55:07.1297201Z 	/opt/hostedtoolcache/go/1.20.6/x64/src/testing/testing.go:1629 +0x3ea
2023-08-09T04:55:07.1298816Z FAIL	antrea.io/antrea-ui/test/e2e_helm	8.709s

Versions

  • Terratest version: v0.43.11
@antoninbas antoninbas added the bug Something isn't working label Aug 9, 2023
antoninbas added a commit to antoninbas/antrea-ui that referenced this issue Aug 9, 2023
@antoninbas
Avoid transient failures in e2e_helm tests
4bb009a 
WaitUntilDeploymentAvailable can sometimes panic if called too quickly
after creating the Deployment. We add a short sleep to avoid the issue.

See gruntwork-io/terratest#1329

Signed-off-by: Antonin Bas <abas@vmware.com>
@antoninbas antoninbas mentioned this issue Aug 9, 2023
Merged
antoninbas added a commit to antoninbas/terratest that referenced this issue Aug 9, 2023
@antoninbas
[k8s] Fix panic in WaitUntilDeploymentAvailable
fc83b09 
The `func (err DeploymentNotAvailable) Error()` method should not assume
that `Status.Conditions[0]` is always valid for the Deployment.

Fixes gruntwork-io#1329

Signed-off-by: Antonin Bas <abas@vmware.com>
@antoninbas antoninbas mentioned this issue Aug 9, 2023
Merged
4 tasks
antoninbas added a commit to antoninbas/terratest that referenced this issue Aug 9, 2023
@antoninbas
[k8s] Fix panic in WaitUntilDeploymentAvailable
2e00fea 
The `func (err DeploymentNotAvailable) Error()` method should not assume
that `Status.Conditions[0]` is always valid for the Deployment.

Fixes gruntwork-io#1329

Signed-off-by: Antonin Bas <abas@vmware.com>
antoninbas added a commit to antrea-io/antrea-ui that referenced this issue Aug 9, 2023
@antoninbas
Avoid transient failures in e2e_helm tests (#57)
c2962da 
WaitUntilDeploymentAvailable can sometimes panic if called too quickly
after creating the Deployment. We add a short sleep to avoid the issue.

See gruntwork-io/terratest#1329

Signed-off-by: Antonin Bas <abas@vmware.com>
antoninbas added a commit to antoninbas/terratest that referenced this issue Aug 10, 2023
@antoninbas
[k8s] Fix panic in WaitUntilDeploymentAvailable
f689e8c 
The `func (err DeploymentNotAvailable) Error()` method should not assume
that `Status.Conditions[0]` is always valid for the Deployment.

Fixes gruntwork-io#1329

Signed-off-by: Antonin Bas <abas@vmware.com>
@denis256
Copy link
Member

Released in https://github.com/gruntwork-io/terratest/releases/tag/v0.43.12

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[k8s] Fix panic in WaitUntilDeploymentAvailable antoninbas/terratest
2 participants
@antoninbas @denis256