Skip to content

Commit 96b94e4

Browse files
authored
CI: in-cluster scaler tests poll their dependency instead of sleeping between checks (#8153)
Five scaler tests that run without cloud credentials each waited for a dependency with a hand-rolled loop: a fixed sleep, a check, and another sleep if the check failed. This replaces those loops with KedaEventually, so the wait ends when the dependency answers rather than at the next tick, and a failure names what the last attempt saw rather than just that the attempts ran out. - activemq: the broker health query dropped its 10s head start; the loop kept its 10-minute budget and 10s interval. An exec that fails is now returned as the reason instead of being discarded. - solr: same shape, same 1-minute budget and 5s interval, and it is called twice, before and after the restart that enables BasicAuth. - metrics_api: the recursive retry (5 x 5s, with a log line claiming 1s) is one wait for every metrics-server pod to be running with an IP. It is the only wait the test has on that Deployment, so the budget is 2 minutes to absorb scheduling ten replicas on a cold node. - elastic_forecast: two deadline loops keep their budgets (3 minutes) and intervals (3s and 5s). An exec or JSON error is returned as the reason rather than skipped. - couchdb: the 10s "wait until client is ready" becomes WaitForPodReady on the client pod, which is what the exec that follows actually needs. Sleeps under tests/scalers: 38 -> 31. None of these tests need external credentials, so they are the ones that reach OpenShift CI. Relates to #7991 Signed-off-by: Mikhail Fedosin <mfedosin@redhat.com>
1 parent 59815e1 commit 96b94e4

5 files changed

Lines changed: 133 additions & 80 deletions

File tree

‎tests/scalers/activemq/activemq_test.go‎

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@
44
package activemq_test
55

66
import (
7+
"context"
78
"encoding/base64"
8-
"errors"
99
"fmt"
1010
"strings"
1111
"testing"
@@ -475,19 +475,35 @@ func setupActiveMQ(t *testing.T, kc *kubernetes.Clientset) {
475475
require.NoErrorf(t, err, "%s", err)
476476
}
477477

478+
// The StatefulSet reports the broker ready as soon as its container is up, which is before the
479+
// broker has finished starting, so readiness is established by asking the broker's health MBean.
480+
// Ten minutes is the budget the previous 60 x 10s loop had; the broker usually answers Good well
481+
// inside the first minute, and the wait ends as soon as it does.
482+
const (
483+
activeMQReadyTimeout = 10 * time.Minute
484+
activeMQReadyInterval = 10 * time.Second
485+
)
486+
478487
func checkIfActiveMQStatusIsReady(t *testing.T, name string) error {
479488
t.Log("--- checking activemq status ---")
480-
time.Sleep(time.Second * 10)
481-
for i := 0; i < 60; i++ {
482-
out, errOut, _ := ExecCommandOnSpecificPod(t, name, testNamespace, fmt.Sprintf("%s query –objname type=Broker,brokerName=localhost,Service=Health", activemqPath))
489+
490+
ctx, cancel := context.WithTimeout(context.Background(), activeMQReadyTimeout)
491+
defer cancel()
492+
493+
// A failed exec is handed back rather than swallowed, so that a broker which never answers
494+
// is reported with the reason instead of as a bare timeout.
495+
err := KedaEventually(ctx, func(_ context.Context) (bool, error) {
496+
out, errOut, err := ExecCommandOnSpecificPod(t, name, testNamespace, fmt.Sprintf("%s query –objname type=Broker,brokerName=localhost,Service=Health", activemqPath))
483497
t.Logf("Output: %s, Error: %s", out, errOut)
484-
if !strings.Contains(out, "CurrentStatus = Good") {
485-
time.Sleep(time.Second * 10)
486-
continue
498+
if err != nil {
499+
return false, fmt.Errorf("cannot query activemq health: %w", err)
487500
}
488-
return nil
501+
return strings.Contains(out, "CurrentStatus = Good"), nil
502+
}, activeMQReadyInterval)
503+
if err != nil {
504+
return fmt.Errorf("activemq is not ready: %w", err)
489505
}
490-
return errors.New("activemq is not ready")
506+
return nil
491507
}
492508

493509
func testActivation(t *testing.T, kc *kubernetes.Clientset) {

‎tests/scalers/couchdb/couchdb_test.go‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import (
99
"fmt"
1010
"strings"
1111
"testing"
12-
"time"
1312

1413
"github.com/google/uuid"
1514
"github.com/joho/godotenv"
@@ -165,8 +164,10 @@ func TestCouchDBScaler(t *testing.T) {
165164
data, templates := getTemplateData(t, kc)
166165
KubectlApplyMultipleWithTemplate(t, data, templates)
167166

168-
// wait until client is ready
169-
time.Sleep(10 * time.Second)
167+
// The database is created through the client pod, which has no readiness probe, so Ready means its
168+
// container is running and exec will reach it. A minute covers pulling the curl image on a cold node.
169+
require.True(t, WaitForPodReady(t, kc, clientName, testNamespace, 60, 1),
170+
"client pod %s should be ready", clientName)
170171
// create database
171172
_, _, err := ExecCommandOnSpecificPod(t, clientName, testNamespace, fmt.Sprintf("curl -X PUT http://admin:%s@test-release-svc-couchdb.%s.svc.cluster.local:5984/animals", getPassword(t, kc), testNamespace))
172173
require.NoErrorf(t, err, "cannot execute command - %s", err)

‎tests/scalers/elastic_forecast/elastic_forecast_test.go‎

Lines changed: 43 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
package elastic_forecast_test
55

66
import (
7+
"context"
78
"encoding/base64"
89
"encoding/json"
910
"fmt"
@@ -402,23 +403,29 @@ func activateTrialAndVerify(t *testing.T) {
402403
// requireDatafeedStopped polls the datafeed status until it reports "stopped", which means the bounded historical run has completed.
403404
func requireDatafeedStopped(t *testing.T, timeout time.Duration) {
404405
t.Helper()
405-
deadline := time.Now().Add(timeout)
406-
for time.Now().Before(deadline) {
406+
407+
ctx, cancel := context.WithTimeout(context.Background(), timeout)
408+
defer cancel()
409+
410+
// A failed exec is returned rather than skipped, so a datafeed that never reports is failed
411+
// with the reason instead of a bare timeout.
412+
err := KedaEventually(ctx, func(_ context.Context) (bool, error) {
407413
out, err := ExecuteCommand(fmt.Sprintf(
408414
"%s -XGET http://127.0.0.1:9200/_ml/datafeeds/datafeed-%s/_stats",
409415
forecastKubectlExecCmd, forecastJobID,
410416
))
411-
if err == nil {
412-
body := string(out)
413-
if strings.Contains(body, `"state":"stopped"`) {
414-
t.Log("[datafeed] state is stopped — historical processing complete")
415-
return
416-
}
417-
t.Logf("[datafeed] not yet stopped, waiting… (body: %s)", body)
417+
if err != nil {
418+
return false, fmt.Errorf("cannot read datafeed stats: %w", err)
418419
}
419-
time.Sleep(3 * time.Second)
420-
}
421-
require.Fail(t, fmt.Sprintf("datafeed-%s did not reach state 'stopped' within %s", forecastJobID, timeout))
420+
body := string(out)
421+
if strings.Contains(body, `"state":"stopped"`) {
422+
t.Log("[datafeed] state is stopped — historical processing complete")
423+
return true, nil
424+
}
425+
t.Logf("[datafeed] not yet stopped, waiting… (body: %s)", body)
426+
return false, nil
427+
}, 3*time.Second)
428+
require.NoErrorf(t, err, "datafeed-%s did not reach state 'stopped' within %s", forecastJobID, timeout)
422429
}
423430

424431
// requireJobHasBuckets asserts that the ML job has processed enough result buckets for the model to produce a reliable forecast.
@@ -467,26 +474,35 @@ func ingestForecastTrainingData(t *testing.T, count int, window time.Duration) {
467474
func waitForForecastDocs(t *testing.T) {
468475
t.Helper()
469476

470-
deadline := time.Now().Add(3 * time.Minute)
471-
for time.Now().Before(deadline) {
477+
const timeout = 3 * time.Minute
478+
ctx, cancel := context.WithTimeout(context.Background(), timeout)
479+
defer cancel()
480+
481+
// A failed exec or an unparseable body is returned rather than skipped, so a count that never
482+
// comes back is failed with the reason instead of a bare timeout.
483+
err := KedaEventually(ctx, func(_ context.Context) (bool, error) {
472484
out, err := ExecuteCommand(fmt.Sprintf(
473485
`%s -XGET 'http://127.0.0.1:9200/.ml-anomalies-shared/_count' -d '{"query":{"bool":{"filter":[{"term":{"job_id":"%s"}},{"term":{"result_type":"model_forecast"}}]}}}'`,
474486
forecastKubectlExecCmd, forecastJobID,
475487
))
476-
if err == nil {
477-
body := string(out)
478-
var countResp struct {
479-
Count int64 `json:"count"`
480-
}
481-
if jsonErr := json.Unmarshal([]byte(body), &countResp); jsonErr == nil && countResp.Count > 0 {
482-
t.Logf("[forecast] %d forecast document(s) indexed — proceeding", countResp.Count)
483-
return
484-
}
485-
t.Logf("[forecast] 0 documents yet, waiting... (response: %s)", body)
488+
if err != nil {
489+
return false, fmt.Errorf("cannot count forecast documents: %w", err)
486490
}
487-
time.Sleep(5 * time.Second)
488-
}
489-
require.Fail(t, "forecast documents did not appear in .ml-anomalies-shared within 3 minutes")
491+
body := string(out)
492+
var countResp struct {
493+
Count int64 `json:"count"`
494+
}
495+
if jsonErr := json.Unmarshal([]byte(body), &countResp); jsonErr != nil {
496+
return false, fmt.Errorf("cannot parse count response: %w (body: %s)", jsonErr, body)
497+
}
498+
if countResp.Count > 0 {
499+
t.Logf("[forecast] %d forecast document(s) indexed — proceeding", countResp.Count)
500+
return true, nil
501+
}
502+
t.Logf("[forecast] 0 documents yet, waiting... (response: %s)", body)
503+
return false, nil
504+
}, 5*time.Second)
505+
require.NoErrorf(t, err, "forecast documents did not appear in .ml-anomalies-shared within %s", timeout)
490506
}
491507

492508
func testElasticForecastScaler(t *testing.T, kc *kubernetes.Clientset) {

‎tests/scalers/metrics_api/metrics_api_test.go‎

Lines changed: 38 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -266,50 +266,55 @@ func getSplitArrayFromAverage(t *testing.T, average, splitInNValues int) []int {
266266
return result
267267
}
268268

269-
func getUpdateUrlsForAllMetricAllMetricsServerReplicas(t *testing.T, kc *kubernetes.Clientset, expectedAverageMetric int, nbReplicasForMetricsServer int, nbRetry int) map[string]string {
270-
nbRetriesMax := 5
269+
// Nothing else waits for the metrics server's Deployment, so this is where the test first depends on
270+
// its pods being scheduled and running: the budget has to absorb image pull and start-up for up to ten
271+
// replicas on a cold node. The previous retry allowed about 25 seconds.
272+
const metricsServerPodsTimeout = 2 * time.Minute
273+
274+
func getUpdateUrlsForAllMetricAllMetricsServerReplicas(t *testing.T, kc *kubernetes.Clientset, expectedAverageMetric int, nbReplicasForMetricsServer int) map[string]string {
271275
// get an array for which all elements' average would give expectedAverageMetric without any of its elements being exactly expectedAverageMetric
272276
individualMetrics := getSplitArrayFromAverage(t, expectedAverageMetric, nbReplicasForMetricsServer)
273-
// use kc to curl all metrics-server replicas
274-
// Get pods with the specified label selector
275-
pods, err := kc.CoreV1().Pods(testNamespace).List(context.Background(), metav1.ListOptions{
276-
LabelSelector: "app=" + metricsServerDeploymentName,
277-
})
278-
if err != nil {
279-
t.Fatalf("Error listing pods: %v", err)
280-
return nil
281-
}
282277

283-
retryFunc := func(message string) map[string]string {
284-
if nbRetry >= nbRetriesMax {
285-
t.Fatal(message)
286-
return nil
278+
ctx, cancel := context.WithTimeout(context.Background(), metricsServerPodsTimeout)
279+
defer cancel()
280+
281+
// Every metrics-server replica has to be running with an IP before any of them is posted to,
282+
// because the values are split so that only their average equals the expected metric: posting
283+
// to a subset would give the scaler a different average. The reason a check fell short is
284+
// returned as the error, so a timeout says which pod was not ready rather than just that one was not.
285+
var postUrls map[string]string
286+
err := KedaEventually(ctx, func(ctx context.Context) (bool, error) {
287+
pods, err := kc.CoreV1().Pods(testNamespace).List(ctx, metav1.ListOptions{
288+
LabelSelector: "app=" + metricsServerDeploymentName,
289+
})
290+
if err != nil {
291+
return false, fmt.Errorf("cannot list metrics server pods: %w", err)
287292
}
288-
t.Logf("%s. Retry calling getUpdateUrlsForAllMetricAllMetricsServerReplicas() after 1 second", message)
289-
time.Sleep(5 * time.Second)
290-
return getUpdateUrlsForAllMetricAllMetricsServerReplicas(t, kc, expectedAverageMetric, nbReplicasForMetricsServer, nbRetry+1)
291-
}
292-
if len(pods.Items) == 0 {
293-
return retryFunc("No pods found with the given selector.")
294-
}
295293

296-
if len(pods.Items) != nbReplicasForMetricsServer {
297-
return retryFunc(fmt.Sprintf("Number of replicas of metrics server (%d) does not match expected value (%d).", len(pods.Items), nbReplicasForMetricsServer))
298-
}
299-
postUrls := make(map[string]string, nbReplicasForMetricsServer)
300-
// Iterate through the pods and send HTTP requests
301-
for nbPod, pod := range pods.Items {
302-
if pod.Status.Phase != v1.PodRunning || pod.Status.PodIP == "" {
303-
return retryFunc(fmt.Sprintf("Pod %s was expected to be running and to have an IP.", pod.Name))
294+
if len(pods.Items) != nbReplicasForMetricsServer {
295+
return false, fmt.Errorf("metrics server has %d pods, expected %d", len(pods.Items), nbReplicasForMetricsServer)
296+
}
297+
298+
urls := make(map[string]string, nbReplicasForMetricsServer)
299+
for nbPod, pod := range pods.Items {
300+
if pod.Status.Phase != v1.PodRunning || pod.Status.PodIP == "" {
301+
return false, fmt.Errorf("pod %s was expected to be running and to have an IP (phase %s, ip %q)", pod.Name, pod.Status.Phase, pod.Status.PodIP)
302+
}
303+
urls[pod.Name] = fmt.Sprintf("http://%s:8080/api/value/%d", pod.Status.PodIP, individualMetrics[nbPod])
304304
}
305-
url := fmt.Sprintf("http://%s:8080/api/value/%d", pod.Status.PodIP, individualMetrics[nbPod])
306-
postUrls[pod.Name] = url
305+
postUrls = urls
306+
return true, nil
307+
}, IntervalShort)
308+
if err != nil {
309+
t.Fatalf("metrics server replicas were not all running within %s: %v", metricsServerPodsTimeout, err)
310+
return nil
307311
}
312+
308313
return postUrls
309314
}
310315

311316
func updateAllMetricsServerReplicas(t *testing.T, kc *kubernetes.Clientset, data templateData, metricValue int, nbReplicasForMetricsServer int) {
312-
for targetPodName, urlToPost := range getUpdateUrlsForAllMetricAllMetricsServerReplicas(t, kc, metricValue, nbReplicasForMetricsServer, 0) {
317+
for targetPodName, urlToPost := range getUpdateUrlsForAllMetricAllMetricsServerReplicas(t, kc, metricValue, nbReplicasForMetricsServer) {
313318
if urlToPost == "" {
314319
t.Fatalf("target pod %s should have non emoty url but got one", targetPodName)
315320
return

‎tests/scalers/solr/solr_test.go‎

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@
44
package solr_test
55

66
import (
7+
"context"
78
"encoding/base64"
8-
"errors"
99
"fmt"
1010
"strings"
1111
"testing"
@@ -226,19 +226,34 @@ func setupSolr(t *testing.T, kc *kubernetes.Clientset) {
226226
t.Log("--- solr is ready ---")
227227
}
228228

229+
// Solr is asked directly whether it is serving, because the StatefulSet reports ready before the
230+
// server does, and again after the in-place restart that enables BasicAuth, which the StatefulSet
231+
// does not see at all. One minute is the budget the previous 12 x 5s loop had.
232+
const (
233+
solrReadyTimeout = time.Minute
234+
solrReadyInterval = 5 * time.Second
235+
)
236+
229237
func checkIfSolrStatusIsReady(t *testing.T, name string) error {
230238
t.Log("--- checking solr status ---")
231239

232-
for i := 0; i < 12; i++ {
233-
out, errOut, _ := ExecCommandOnSpecificPod(t, name, testNamespace, fmt.Sprintf("%s status", solrPath))
240+
ctx, cancel := context.WithTimeout(context.Background(), solrReadyTimeout)
241+
defer cancel()
242+
243+
// A failed exec is handed back rather than swallowed, so that a server which never answers
244+
// is reported with the reason instead of as a bare timeout.
245+
err := KedaEventually(ctx, func(_ context.Context) (bool, error) {
246+
out, errOut, err := ExecCommandOnSpecificPod(t, name, testNamespace, fmt.Sprintf("%s status", solrPath))
234247
t.Logf("Output: %s, Error: %s", out, errOut)
235-
if !strings.Contains(out, "running on port") {
236-
time.Sleep(time.Second * 5)
237-
continue
248+
if err != nil {
249+
return false, fmt.Errorf("cannot query solr status: %w", err)
238250
}
239-
return nil
251+
return strings.Contains(out, "running on port"), nil
252+
}, solrReadyInterval)
253+
if err != nil {
254+
return fmt.Errorf("solr is not ready: %w", err)
240255
}
241-
return errors.New("solr is not ready")
256+
return nil
242257
}
243258

244259
func getTemplateData() (templateData, []Template) {

0 commit comments

Comments
 (0)