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

added 'appsv1.DeploymentReplicaFailure' condition #878

Merged
merged 3 commits into from
Jan 13, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions pkg/kapp/resourcesmisc/apps_v1_deployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,10 @@ func (s AppsV1Deployment) IsDoneApplying() DoneApplyState {
"Deployment is not progressing: %s (message: %s)", cond.Reason, cond.Message)}
}

case "FailedDelete":
case appsv1.DeploymentReplicaFailure:
if cond.Status == corev1.ConditionTrue {
return DoneApplyState{Done: true, Successful: false, Message: fmt.Sprintf(
"Deployment failed to delete pods: %s (message: %s)", cond.Reason, cond.Message)}
return DoneApplyState{Done: false, Successful: false, Message: fmt.Sprintf(
"Deployment has encountered replica failure: %s (message: %s)", cond.Reason, cond.Message)}
}
}
}
Expand Down
61 changes: 61 additions & 0 deletions pkg/kapp/resourcesmisc/apps_v1_deployment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,3 +65,64 @@ func buildDep(resourcesBs string, t *testing.T) *ctlresm.AppsV1Deployment {

return ctlresm.NewAppsV1Deployment(newResources[0], newResources[1:])
}
func TestIsDoneApplying(t *testing.T) {
Copy link
Member

Choose a reason for hiding this comment

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

Let's move the test above the buildDep helper function. Also, let's rename it to TestAppsV1DeploymentReplicaFailure.

// Define a Deployment with a Progressing condition set to False
Copy link
Member

Choose a reason for hiding this comment

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

The code is self explanatory, so we can remove the comments.

depYAML := `
apiVersion: apps/v1
kind: Deployment
metadata:
praveenrewar marked this conversation as resolved.
Show resolved Hide resolved
name: test-deployment
status:
observedGeneration: 1
conditions:
- type: Progressing
status: "False"
reason: "ProgressDeadlineExceeded"
message: "Progress deadline exceeded"
`

// Parse the Deployment YAML into kapp resources
resources, err := ctlres.NewFileResource(ctlres.NewBytesSource([]byte(depYAML))).Resources()
require.NoError(t, err, "Expected resources to parse")

// Create an AppsV1Deployment instance with the parsed resources
deployment := ctlresm.NewAppsV1Deployment(resources[0], nil)
Copy link
Member

Choose a reason for hiding this comment

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

We can use the existing buildDep helper function for this.


// Invoke the IsDoneApplying method to check for Progressing condition
state := deployment.IsDoneApplying()

// Define the expected state based on the test case
expectedState := ctlresm.DoneApplyState{
Done: true,
Successful: false,
Message: "Deployment is not progressing: ProgressDeadlineExceeded (message: Progress deadline exceeded)",
}

require.Equal(t, expectedState, state, "Found incorrect state")

// Case: ReplicaFailure condition is True
depYAML = `
apiVersion: apps/v1
kind: Deployment
metadata:
name: test-deployment
status:
observedGeneration: 1
conditions:
- type: ReplicaFailure
status: "True"
reason: "FailedCreate"
message: "Failed to create pods"
`
resources, err = ctlres.NewFileResource(ctlres.NewBytesSource([]byte(depYAML))).Resources()
require.NoError(t, err, "Expected resources to parse")
deployment = ctlresm.NewAppsV1Deployment(resources[0], nil)
Copy link
Member

Choose a reason for hiding this comment

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

Same as above, we can use the existing helper function.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have made the requested changes. Please have a look.

state = deployment.IsDoneApplying()
expectedState = ctlresm.DoneApplyState{
Done: false,
Successful: false,
Message: "Deployment has encountered replica failure: FailedCreate (message: Failed to create pods)",
}
require.Equal(t, expectedState, state, "Found incorrect state")

}