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.

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

[k8s] Fix panic in WaitUntilDeploymentAvailable #1330

Conversation

antoninbas
Copy link
Contributor

Description

Fixes #1329

The func (err DeploymentNotAvailable) Error() method should not assume that Status.Conditions[0] is always valid for the Deployment.

TODOs

Read the Gruntwork contribution guidelines.

  • Update the docs. N/A
  • Run the relevant tests successfully, including pre-commit checks.
  • Ensure any 3rd party code adheres with our license policy or delete this line if its not applicable. N/A
  • Include release notes. If this PR is backward incompatible, include a migration guide.

Release Notes (draft)

Fixed panic in WaitUntilDeploymentAvailable in the k8s module.

Migration Guide

N/A

@antoninbas antoninbas force-pushed the fix-panic-in-WaitUntilDeploymentAvailable branch from fc83b09 to 2e00fea Compare August 9, 2023 21:23
@antoninbas
Copy link
Contributor Author

antoninbas commented Aug 9, 2023

Relevant tests:

$ go test -v -run=.*DeploymentAvailable.* --tags=kubeall ./modules/k8s/...
=== RUN   TestWaitUntilDeploymentAvailable
=== PAUSE TestWaitUntilDeploymentAvailable
=== RUN   TestTestIsDeploymentAvailable
=== RUN   TestTestIsDeploymentAvailable/TestIsDeploymentAvailableWithProgressingNewReplicaSetAvailable
=== PAUSE TestTestIsDeploymentAvailable/TestIsDeploymentAvailableWithProgressingNewReplicaSetAvailable
=== RUN   TestTestIsDeploymentAvailable/TestIsDeploymentAvailableWithoutProgressingNewReplicaSetAvailable
=== PAUSE TestTestIsDeploymentAvailable/TestIsDeploymentAvailableWithoutProgressingNewReplicaSetAvailable
=== CONT  TestTestIsDeploymentAvailable/TestIsDeploymentAvailableWithProgressingNewReplicaSetAvailable
=== CONT  TestTestIsDeploymentAvailable/TestIsDeploymentAvailableWithoutProgressingNewReplicaSetAvailable
--- PASS: TestTestIsDeploymentAvailable (0.00s)
    --- PASS: TestTestIsDeploymentAvailable/TestIsDeploymentAvailableWithoutProgressingNewReplicaSetAvailable (0.00s)
    --- PASS: TestTestIsDeploymentAvailable/TestIsDeploymentAvailableWithProgressingNewReplicaSetAvailable (0.00s)
=== CONT  TestWaitUntilDeploymentAvailable
TestWaitUntilDeploymentAvailable 2023-08-09T14:27:00-07:00 logger.go:66: Running command kubectl with args [--namespace ph2leb apply -f /var/folders/9q/0_7t6cs557d_v8mfkhcw5r780000gp/T/TestWaitUntilDeploymentAvailable1824699040]
TestWaitUntilDeploymentAvailable 2023-08-09T14:27:01-07:00 logger.go:66: namespace/ph2leb created
TestWaitUntilDeploymentAvailable 2023-08-09T14:27:01-07:00 logger.go:66: deployment.apps/nginx-deployment created
TestWaitUntilDeploymentAvailable 2023-08-09T14:27:01-07:00 retry.go:91: Wait for deployment nginx-deployment to be provisioned.
TestWaitUntilDeploymentAvailable 2023-08-09T14:27:01-07:00 client.go:42: Configuring Kubernetes client using config file /Users/abas/vmware/antrea/test/e2e/infra/vagrant/playbook/kube/config with context
TestWaitUntilDeploymentAvailable 2023-08-09T14:27:01-07:00 retry.go:103: Wait for deployment nginx-deployment to be provisioned. returned an error: Deployment nginx-deployment is not available as 'Progressing' condition indicates that the Deployment is not complete, status: True, reason: NewReplicaSetCreated, message: Created new replica set "nginx-deployment-dc56b9db4". Sleeping for 1s and will try again.
TestWaitUntilDeploymentAvailable 2023-08-09T14:27:02-07:00 retry.go:91: Wait for deployment nginx-deployment to be provisioned.
TestWaitUntilDeploymentAvailable 2023-08-09T14:27:02-07:00 client.go:42: Configuring Kubernetes client using config file /Users/abas/vmware/antrea/test/e2e/infra/vagrant/playbook/kube/config with context
TestWaitUntilDeploymentAvailable 2023-08-09T14:27:02-07:00 retry.go:103: Wait for deployment nginx-deployment to be provisioned. returned an error: Deployment nginx-deployment is not available as 'Progressing' condition indicates that the Deployment is not complete, status: True, reason: ReplicaSetUpdated, message: ReplicaSet "nginx-deployment-dc56b9db4" is progressing.. Sleeping for 1s and will try again.
TestWaitUntilDeploymentAvailable 2023-08-09T14:27:03-07:00 retry.go:91: Wait for deployment nginx-deployment to be provisioned.
TestWaitUntilDeploymentAvailable 2023-08-09T14:27:03-07:00 client.go:42: Configuring Kubernetes client using config file /Users/abas/vmware/antrea/test/e2e/infra/vagrant/playbook/kube/config with context
TestWaitUntilDeploymentAvailable 2023-08-09T14:27:03-07:00 deployment.go:95: Deployment is now available
TestWaitUntilDeploymentAvailable 2023-08-09T14:27:03-07:00 logger.go:66: Running command kubectl with args [--namespace ph2leb delete -f /var/folders/9q/0_7t6cs557d_v8mfkhcw5r780000gp/T/TestWaitUntilDeploymentAvailable57124874]
TestWaitUntilDeploymentAvailable 2023-08-09T14:27:03-07:00 logger.go:66: Warning: deleting cluster-scoped resources, not scoped to the provided namespace
TestWaitUntilDeploymentAvailable 2023-08-09T14:27:03-07:00 logger.go:66: namespace "ph2leb" deleted
TestWaitUntilDeploymentAvailable 2023-08-09T14:27:03-07:00 logger.go:66: deployment.apps "nginx-deployment" deleted
--- PASS: TestWaitUntilDeploymentAvailable (7.74s)
PASS
ok  	github.com/gruntwork-io/terratest/modules/k8s	8.170s

Pre-commit hooks:

goimports................................................................Passed
Terraform fmt........................................(no files to check)Skipped
test-interfaces-used.....................................................Passed

dc.Reason == "NewReplicaSetAvailable" {
return true
dc := getDeploymentCondition(deploy, appsv1.DeploymentProgressing)
return dc != nil && dc.Status == v1.ConditionTrue && dc.Reason == "NewReplicaSetAvailable"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was wondering if it's possible to implement a test case that would generate such a situation.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which situation are you referring to? The panic?
Won't be possible to reproduce with a real K8s apiserver. However, I just updated the commit to add unit tests.

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 force-pushed the fix-panic-in-WaitUntilDeploymentAvailable branch from 2e00fea to f689e8c Compare August 10, 2023 18:25
@denis256 denis256 merged commit 9e475c5 into gruntwork-io:master Aug 11, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[k8s] Calling WaitUntilDeploymentAvailable can cause a panic
2 participants