fix: for readiness check both the phase and ready status #277
Conversation
Observed that the ready status = true when the pod is terminating.
deepthidevaki
force-pushed
the
fix-readiness-check
branch
from
e3eba5f to
c82a71e
Compare
There was a problem hiding this comment.
I would feel better if we have a test for that. Could you take a look at this?
Like adding a test where you setup a pod, with pod status Running, Failed etc. and verify the result of that check function?
Here you find the functions which creates the pod https://github.com/zeebe-io/zeebe-chaos/blob/main/go-chaos/internal/helper_test.go#L73-L80 in the fake k8 environment
Here https://github.com/zeebe-io/zeebe-chaos/blob/main/go-chaos/internal/pods_test.go I would see the test like
- Test_ShouldReturnTrueWhenBrokerPodsAreReady
- Test_ShouldReturnFalseWhenOneBrokerPodIsNotReady
|
@Zelldon Do you know how I can mock a respose for "get pods" to return ready or not ready pods? When creating a pod using the fakeclient we are only providing the spec of the pod. We cannot set the status. |
|
@deepthidevaki you can define the status here in the struct :) https://github.com/zeebe-io/zeebe-chaos/blob/main/go-chaos/internal/helper_test.go#L73-L80 Maybe create another function which creates the pod with a status. |
Aah, Right. I tried to add it in the Spec 🙈 |
deepthidevaki
force-pushed
the
fix-readiness-check
branch
from
128697e to
b6590aa
Compare
There was a problem hiding this comment.
Thank you very much for adding additional tests @deepthidevaki 👍 🚀
Please have a look at my comments, maybe you can do that as follow up. I will merge it anyway, since I'm currently testing the QA and this helps here.
| case <-timedOut: | ||
| return errors.New(fmt.Sprintf("Awaited readiness of pods in namespace %v, but timed out after %v", c.GetCurrentNamespace(), timeout)) | ||
| case <-ticker: |
There was a problem hiding this comment.
👍 Yeah I thought also last week that we can write this better with channels thanks for improving this 🚀
| k8Client.CreateDeploymentWithLabelsAndName(t, gatewaySelector, "gateway") | ||
|
|
||
| // when | ||
| err = k8Client.AwaitReadinessWithTimeout(2 * time.Second) |
There was a problem hiding this comment.
🔧 Could we reduce the timeout otherwise the tests take a bit long
|
|
||
| func (c K8Client) CreateReadyPodWithLabelsAndName(t *testing.T, selector *metav1.LabelSelector, podName string) { | ||
| _, err := c.Clientset.CoreV1().Pods(c.GetCurrentNamespace()).Create(context.TODO(), &v1.Pod{ | ||
| ObjectMeta: metav1.ObjectMeta{Labels: selector.MatchLabels, Name: podName}, | ||
| Spec: v1.PodSpec{}, | ||
| }, metav1.CreateOptions{}) | ||
|
|
||
| require.NoError(t, err) | ||
| } | ||
|
|
There was a problem hiding this comment.
❌ I think this is not used? BTW you could also call the function above.
Observed that the ready status = true when the pod is terminating.