Include elasticsearch statefulset nodes in availability check #371
Conversation
Signed-off-by: Pavol Loffay <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #371 +/- ##
==========================================
- Coverage 90.01% 89.71% -0.31%
==========================================
Files 64 64
Lines 3076 3093 +17
==========================================
+ Hits 2769 2775 +6
- Misses 207 216 +9
- Partials 100 102 +2
Continue to review full report at Codecov.
|
|
The coverage dropped, I think we do not test wait in other controller either. Not sure how to test it either in unit tests. |
| @@ -83,6 +83,21 @@ func waitForAvailableElastic(c client.Client, es esv1.Elasticsearch) error { | |||
| available++ | |||
| } | |||
| } | |||
| ssList := corev1.StatefulSetList{} | |||
There was a problem hiding this comment.
What's the relationship between the deployment above and this stateful set? Is it sufficient to just check the stateful set?
There was a problem hiding this comment.
SS is used exclusively for only master nodes. The deployments are used for data or other combinations.
| return false, err | ||
| } | ||
| for _, d := range ssList.Items { | ||
| available += d.Status.ReadyReplicas |
There was a problem hiding this comment.
That may seem a bit confusing when debugging an issue: when there's a mismatch, which ones aren't ready?
Wouldn't it be better to have two counters, one for the deployments and one for the stateful sets?
There was a problem hiding this comment.
+1 I have split them into SS and deployments.
Signed-off-by: Pavol Loffay <[email protected]>
Resolves #343
Signed-off-by: Pavol Loffay [email protected]