Skip to content

Commit 9e475c5

Browse files
authored
Merge pull request #1330 from antoninbas/fix-panic-in-WaitUntilDeploymentAvailable
[k8s] Fix panic in WaitUntilDeploymentAvailable
2 parents a6467d4 + f689e8c commit 9e475c5

4 files changed

Lines changed: 92 additions & 10 deletions

File tree

modules/k8s/deployment.go

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -98,13 +98,16 @@ func WaitUntilDeploymentAvailableE(
9898

9999
// IsDeploymentAvailable returns true if all pods within the deployment are ready and started
100100
func IsDeploymentAvailable(deploy *appsv1.Deployment) bool {
101-
for _, dc := range deploy.Status.Conditions {
102-
if dc.Type == appsv1.DeploymentProgressing &&
103-
dc.Status == v1.ConditionTrue &&
104-
dc.Reason == "NewReplicaSetAvailable" {
105-
return true
101+
dc := getDeploymentCondition(deploy, appsv1.DeploymentProgressing)
102+
return dc != nil && dc.Status == v1.ConditionTrue && dc.Reason == "NewReplicaSetAvailable"
103+
}
104+
105+
func getDeploymentCondition(deploy *appsv1.Deployment, cType appsv1.DeploymentConditionType) *appsv1.DeploymentCondition {
106+
for idx := range deploy.Status.Conditions {
107+
dc := &deploy.Status.Conditions[idx]
108+
if dc.Type == cType {
109+
return dc
106110
}
107111
}
108-
109-
return false
112+
return nil
110113
}

modules/k8s/deployment_test.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,15 @@ func TestTestIsDeploymentAvailable(t *testing.T) {
110110
},
111111
expectedResult: false,
112112
},
113+
{
114+
title: "TestIsDeploymentAvailableWithoutProgressingCondition",
115+
deploy: &appsv1.Deployment{
116+
Status: appsv1.DeploymentStatus{
117+
Conditions: []appsv1.DeploymentCondition{},
118+
},
119+
},
120+
expectedResult: false,
121+
},
113122
}
114123

115124
for _, tc := range testCases {

modules/k8s/errors.go

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -69,11 +69,21 @@ type DeploymentNotAvailable struct {
6969

7070
// Error is a simple function to return a formatted error message as a string
7171
func (err DeploymentNotAvailable) Error() string {
72+
dc := getDeploymentCondition(err.deploy, appsv1.DeploymentProgressing)
73+
if dc == nil {
74+
return fmt.Sprintf(
75+
"Deployment %s is not available, missing '%s' condition",
76+
err.deploy.Name,
77+
appsv1.DeploymentProgressing,
78+
)
79+
}
7280
return fmt.Sprintf(
73-
"Deployment %s is not available, reason: %s, message: %s",
81+
"Deployment %s is not available as '%s' condition indicates that the Deployment is not complete, status: %v, reason: %s, message: %s",
7482
err.deploy.Name,
75-
err.deploy.Status.Conditions[0].Reason,
76-
err.deploy.Status.Conditions[0].Message,
83+
appsv1.DeploymentProgressing,
84+
dc.Status,
85+
dc.Reason,
86+
dc.Message,
7787
)
7888
}
7989

modules/k8s/errors_test.go

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
package k8s
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/assert"
7+
8+
appsv1 "k8s.io/api/apps/v1"
9+
v1 "k8s.io/api/core/v1"
10+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
11+
)
12+
13+
func TestErrorDeploymentNotAvailable(t *testing.T) {
14+
testCases := []struct {
15+
title string
16+
deploy *appsv1.Deployment
17+
expectedErr string
18+
}{
19+
{
20+
title: "NoProgressingCondition",
21+
deploy: &appsv1.Deployment{
22+
ObjectMeta: metav1.ObjectMeta{
23+
Name: "foo",
24+
},
25+
Status: appsv1.DeploymentStatus{
26+
Conditions: []appsv1.DeploymentCondition{},
27+
},
28+
},
29+
expectedErr: "Deployment foo is not available, missing 'Progressing' condition",
30+
},
31+
{
32+
title: "DeploymentNotComplete",
33+
deploy: &appsv1.Deployment{
34+
ObjectMeta: metav1.ObjectMeta{
35+
Name: "foo",
36+
},
37+
Status: appsv1.DeploymentStatus{
38+
Conditions: []appsv1.DeploymentCondition{
39+
{
40+
Type: appsv1.DeploymentProgressing,
41+
Status: v1.ConditionTrue,
42+
Reason: "ReplicaSetUpdated",
43+
Message: "bar",
44+
},
45+
},
46+
},
47+
},
48+
expectedErr: "Deployment foo is not available as 'Progressing' condition indicates that the Deployment is not complete, status: True, reason: ReplicaSetUpdated, message: bar",
49+
},
50+
}
51+
52+
for _, tc := range testCases {
53+
tc := tc
54+
t.Run(tc.title, func(t *testing.T) {
55+
t.Parallel()
56+
err := NewDeploymentNotAvailableError(tc.deploy)
57+
assert.EqualError(t, err, tc.expectedErr)
58+
})
59+
}
60+
}

0 commit comments

Comments
 (0)