Enhance ES index cleaner e2e test to verify indices have been removed #378
Conversation
Signed-off-by: Gary Brown <[email protected]>
test/e2e/es_index_cleaner_test.go
Outdated
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| err = WaitForCronJob(t, f.KubeClient, namespace, fmt.Sprintf("%s-es-index-cleaner", name), retryInterval, timeout) |
There was a problem hiding this comment.
Currently removed the intermediate checks of whether the cronjob completed - as was focusing on the result. But wondering whether this should be added back in - just to check that the job also reports success.
There was a problem hiding this comment.
I would leave it there to see that the job run successfully.
Signed-off-by: Gary Brown <[email protected]>
test/e2e/es_index_cleaner_test.go
Outdated
| bodyBytes, err := ioutil.ReadAll(resp.Body) | ||
| bodyString := string(bodyBytes) | ||
|
|
||
| return strings.Contains(bodyString, "jaeger-span-"), nil |
There was a problem hiding this comment.
a minor thing but could you also check the service names index or just do jaeger- in a case we add new indices..
test/e2e/es_index_cleaner_test.go
Outdated
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| err = WaitForCronJob(t, f.KubeClient, namespace, fmt.Sprintf("%s-es-index-cleaner", name), retryInterval, timeout) |
There was a problem hiding this comment.
I would leave it there to see that the job run successfully.
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| err = WaitForCronJob(t, f.KubeClient, namespace, fmt.Sprintf("%s-es-index-cleaner", name), retryInterval, timeout) | ||
| portForwES, closeChanES, err := CreatePortForward(esPod.Namespace, esPod.Name, []string{"9200"}, f.KubeConfig) |
There was a problem hiding this comment.
nit move es port forward closer to has-index.
test/e2e/es_index_cleaner_test.go
Outdated
| }) | ||
| } | ||
|
|
||
| func hasIndex(t *testing.T) (bool, error) { |
There was a problem hiding this comment.
why passing t? maybe it's better to change the signature to bool containsIndex(string) bool, error
There was a problem hiding this comment.
Yes, t no longer necessary so will remove. Will also update the signature and check for jaeger- for now.
Signed-off-by: Gary Brown <[email protected]>
Signed-off-by: Gary Brown <[email protected]>
Signed-off-by: Gary Brown <[email protected]>
Signed-off-by: Gary Brown <[email protected]>
| return nil | ||
|
|
||
| return wait.Poll(retryInterval, timeout, func() (done bool, err error) { | ||
| flag, err := hasIndexWithPrefix("jaeger-") |
There was a problem hiding this comment.
Is this needed hasIndexWithPrefix is also called on line 97.
There was a problem hiding this comment.
Line 97 is checking the indices exist, this part of checking they no longer exist.
|
Still investigating the build failure in travis - the problem is not lint, but the |
Signed-off-by: Gary Brown <[email protected]>
Signed-off-by: Gary Brown <[email protected]>
Signed-off-by: Gary Brown <[email protected]>
Signed-off-by: Gary Brown <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #378 +/- ##
==========================================
+ Coverage 89.71% 89.72% +<.01%
==========================================
Files 64 64
Lines 3093 3094 +1
==========================================
+ Hits 2775 2776 +1
Misses 216 216
Partials 102 102
Continue to review full report at Codecov.
|
|
I have created a separate issue #379 for the |
Existing test only verifies that the cronjob completes successfully.
This PR extends the test to verify that the job has correctly triggered the es-index-cleaner image and that the task has been performed correctly (i.e. the indices have been removed).
Signed-off-by: Gary Brown [email protected]