-
Notifications
You must be signed in to change notification settings - Fork 38
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
[KO-245] Deleting PVC of local volumes during node maintenance. #256
Conversation
@@ -32,7 +32,7 @@ func CheckPodFailed(pod *corev1.Pod) error { | |||
return fmt.Errorf("pod %s has failed status", pod.Name) | |||
} | |||
|
|||
if pod.Status.Phase == corev1.PodPending && isPodReasonUnschedulable(pod) { | |||
if pod.Status.Phase == corev1.PodPending && IsPodReasonUnschedulable(pod) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about moving this pod.Status.Phase == corev1.PodPending
inside IsPodReasonUnschedulable
and IsPodNeedsToMigrate
func ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
anything is fine. Both functions are just on the conditions, even we can just pass conditions also in these functions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, anything is fine. My thinking was to keep every un-schedulable condition under a func so that we don't have to call multiple conditions ANDed together at multiple places if required.
It( | ||
"Should delete local PVC if pod is unschedulable", func() { | ||
aeroCluster.Spec.Storage.LocalStorageClasses = []string{storageClass} | ||
aeroCluster.Spec.Storage.CleanLocalPVC = true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should also add a test-case where CleanLocalPVC = false
and PVC should not be deleted (i.e. match UUID)
No description provided.