Skip to content

Commit

Permalink
Merge pull request #1040 from haoming29/better-waituntilworking
Browse files Browse the repository at this point in the history
More error tolerant web ui check
  • Loading branch information
jhiemstrawisc authored Apr 5, 2024
2 parents 8beb82c + 7236f1e commit 939eda9
Show file tree
Hide file tree
Showing 9 changed files with 64 additions and 28 deletions.
4 changes: 2 additions & 2 deletions broker/broker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ func TestBroker(t *testing.T) {
// Run the web engine, wait for it to be online.
err = web_ui.RunEngineRoutine(ctx, engine, egrp, false)
require.NoError(t, err)
err = server_utils.WaitUntilWorking(ctx, "GET", param.Server_ExternalWebUrl.GetString()+"/", "Web UI", http.StatusNotFound)
err = server_utils.WaitUntilWorking(ctx, "GET", param.Server_ExternalWebUrl.GetString()+"/", "Web UI", http.StatusNotFound, false)
require.NoError(t, err)

// Create a HTTP server we'll use to serve up the reversed connection
Expand Down Expand Up @@ -291,7 +291,7 @@ func TestRetrieveTimeout(t *testing.T) {
// Run the web engine, wait for it to be online.
err = web_ui.RunEngineRoutine(ctx, engine, egrp, false)
require.NoError(t, err)
err = server_utils.WaitUntilWorking(ctx, "GET", param.Server_ExternalWebUrl.GetString()+"/", "Web UI", http.StatusNotFound)
err = server_utils.WaitUntilWorking(ctx, "GET", param.Server_ExternalWebUrl.GetString()+"/", "Web UI", http.StatusNotFound, false)
require.NoError(t, err)

viper.Set("Federation.BrokerUrl", param.Server_ExternalWebUrl.GetString())
Expand Down
2 changes: 1 addition & 1 deletion cmd/fed_serve_cache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ func TestFedServeCache(t *testing.T) {
}

// In this case 403 means the cache is running
err = server_utils.WaitUntilWorking(ctx, "GET", param.Cache_Url.GetString(), "xrootd", 403)
err = server_utils.WaitUntilWorking(ctx, "GET", param.Cache_Url.GetString(), "xrootd", 403, false)
require.NoError(t, err)

fileTests := server_utils.TestFileTransferImpl{}
Expand Down
2 changes: 1 addition & 1 deletion cmd/fed_serve_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ func TestFedServePosixOrigin(t *testing.T) {
}

desiredURL := param.Server_ExternalWebUrl.GetString() + "/.well-known/openid-configuration"
err = server_utils.WaitUntilWorking(ctx, "GET", desiredURL, "director", 200)
err = server_utils.WaitUntilWorking(ctx, "GET", desiredURL, "director", 200, false)
require.NoError(t, err)

httpc := http.Client{
Expand Down
2 changes: 1 addition & 1 deletion cmd/plugin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ func (f *FedTest) Spinup() {
}

desiredURL := param.Server_ExternalWebUrl.GetString() + "/.well-known/openid-configuration"
err = server_utils.WaitUntilWorking(ctx, "GET", desiredURL, "director", 200)
err = server_utils.WaitUntilWorking(ctx, "GET", desiredURL, "director", 200, false)
require.NoError(f.T, err)

httpc := http.Client{
Expand Down
2 changes: 1 addition & 1 deletion fed_test_utils/fed.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ func NewFedTest(t *testing.T, originConfig string) (ft *FedTest) {
}

desiredURL := param.Server_ExternalWebUrl.GetString() + "/api/v1.0/health"
err = server_utils.WaitUntilWorking(ctx, "GET", desiredURL, "director", 200)
err = server_utils.WaitUntilWorking(ctx, "GET", desiredURL, "director", 200, false)
require.NoError(t, err)

httpc := http.Client{
Expand Down
9 changes: 5 additions & 4 deletions launchers/launcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -236,8 +236,9 @@ func LaunchModules(ctx context.Context, modules config.ServerType) (servers []se
return nil
})

if err = server_utils.WaitUntilWorking(ctx, "GET", param.Server_ExternalWebUrl.GetString()+"/api/v1.0/health", "Web UI", http.StatusOK); err != nil {
log.Errorln("Web engine startup appears to have failed:", err)
healthCheckUrl := param.Server_ExternalWebUrl.GetString() + "/api/v1.0/health"
if err = server_utils.WaitUntilWorking(ctx, "GET", healthCheckUrl, "Web UI", http.StatusOK, true); err != nil {
log.Errorln("Web engine check failed: ", err)
return
}

Expand Down Expand Up @@ -289,7 +290,7 @@ func LaunchModules(ctx context.Context, modules config.ServerType) (servers []se
errCh <- errors.Wrapf(err, "Failed to join path %s for origin advertisement check", prefix)
return
}
if err = server_utils.WaitUntilWorking(ctx, "GET", urlToCheck.String(), "director", 307); err != nil {
if err = server_utils.WaitUntilWorking(ctx, "GET", urlToCheck.String(), "director", 307, false); err != nil {
errCh <- errors.Wrapf(err, "The prefix %s does not seem to have advertised correctly", prefix)
}

Expand All @@ -316,7 +317,7 @@ func LaunchModules(ctx context.Context, modules config.ServerType) (servers []se
if modules.IsEnabled(config.CacheType) {
// Give five seconds for the origin to finish advertising to the director
desiredURL := param.Federation_DirectorUrl.GetString() + "/.well-known/openid-configuration"
if err = server_utils.WaitUntilWorking(ctx, "GET", desiredURL, "director", 200); err != nil {
if err = server_utils.WaitUntilWorking(ctx, "GET", desiredURL, "director", 200, false); err != nil {
log.Errorln("Director does not seem to be working:", err)
return
}
Expand Down
49 changes: 42 additions & 7 deletions server_utils/server_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,17 @@ import (

// Wait until given `reqUrl` returns a HTTP 200.
// Logging messages emitted will refer to `server` (e.g., origin, cache, director)
func WaitUntilWorking(ctx context.Context, method, reqUrl, server string, expectedStatus int) error {
// Pass true to statusMismatch to allow a mismatch of expected status code and what's returned not fail immediately
func WaitUntilWorking(ctx context.Context, method, reqUrl, server string, expectedStatus int, statusMismatch bool) error {
expiry := time.Now().Add(10 * time.Second)
ctx, cancel := context.WithDeadline(ctx, expiry)
defer cancel()
ticker := time.NewTicker(50 * time.Millisecond)
success := false
logged := false
var statusError error
statusErrLogged := false

for !(success || time.Now().After(expiry)) {
select {
case <-ticker.C:
Expand Down Expand Up @@ -75,24 +79,55 @@ func WaitUntilWorking(ctx context.Context, method, reqUrl, server string, expect
}
bytes, err := io.ReadAll(resp.Body)
if err != nil {
// We didn't get the expected status
return errors.Errorf("Received bad status code in reply to server ping at %s: %d. Expected %d. Can't read response body with error %v.", reqUrl, resp.StatusCode, expectedStatus, err)
statusError = errors.Errorf("Received bad status code in reply to server ping at %s: %d. Expected %d. Can't read response body with error %v.", reqUrl, resp.StatusCode, expectedStatus, err)
if statusMismatch {
if !statusErrLogged {
log.Info(statusError, "Will retry until timeout")
statusErrLogged = true
}
} else {
// We didn't get the expected status
return statusError
}
} else {
if len(bytes) != 0 {
// We didn't get the expected status
return errors.Errorf("Received bad status code in reply to server ping at %s: %d. Expected %d. Response body: %s", reqUrl, resp.StatusCode, expectedStatus, string(bytes))
statusError = errors.Errorf("Received bad status code in reply to server ping at %s: %d. Expected %d. Response body: %s", reqUrl, resp.StatusCode, expectedStatus, string(bytes))
if statusMismatch {
if !statusErrLogged {
log.Info(statusError, "Will retry until timeout")
statusErrLogged = true
}
} else {
// We didn't get the expected status
return statusError
}
} else {
return errors.Errorf("Received bad status code in reply to server ping at %s: %d. Expected %d. Response body is empty.", reqUrl, resp.StatusCode, expectedStatus)
statusError = errors.Errorf("Received bad status code in reply to server ping at %s: %d. Expected %d. Response body is empty.", reqUrl, resp.StatusCode, expectedStatus)
if statusMismatch {
if !statusErrLogged {
log.Info(statusError, "Will retry until timeout")
statusErrLogged = true
}
} else {
return statusError
}
}
}

}
case <-ctx.Done():
if statusError != nil {
return errors.Wrapf(statusError, "url %s didn't respond with the expected status code %d within 10s", reqUrl, expectedStatus)
}
return ctx.Err()
}
}

return errors.Errorf("Server %s at %s did not startup after 10s of waiting", server, reqUrl)
if statusError != nil {
return errors.Wrapf(statusError, "url %s didn't respond with the expected status code %d within 10s", reqUrl, expectedStatus)
} else {
return errors.Errorf("Server %s at %s did not startup after 10s of waiting", server, reqUrl)
}
}

// For calling from within the server. Returns the server's issuer URL/port
Expand Down
12 changes: 6 additions & 6 deletions server_utils/server_utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func TestWaitUntilWorking(t *testing.T) {
}))
defer server.Close()

err := WaitUntilWorking(ctx, "GET", server.URL, "testServer", http.StatusOK)
err := WaitUntilWorking(ctx, "GET", server.URL, "testServer", http.StatusOK, false)
require.NoError(t, err)

assert.Equal(t, logrus.DebugLevel, hook.LastEntry().Level)
Expand All @@ -62,7 +62,7 @@ func TestWaitUntilWorking(t *testing.T) {
}))
defer server.Close()

err := WaitUntilWorking(ctx, "GET", server.URL, "testServer", http.StatusOK)
err := WaitUntilWorking(ctx, "GET", server.URL, "testServer", http.StatusOK, false)
require.Error(t, err)
expectedErrorMsg := fmt.Sprintf("Received bad status code in reply to server ping at %s: %d. Expected %d. Response body is empty.", server.URL, 500, 200)
assert.Contains(t, err.Error(), expectedErrorMsg)
Expand All @@ -76,7 +76,7 @@ func TestWaitUntilWorking(t *testing.T) {
}))
defer server.Close()

err := WaitUntilWorking(ctx, "GET", server.URL, "testServer", http.StatusOK)
err := WaitUntilWorking(ctx, "GET", server.URL, "testServer", http.StatusOK, false)
require.Error(t, err)
expectedErrorMsg := fmt.Sprintf("Received bad status code in reply to server ping at %s: %d. Expected %d. Response body: %s", server.URL, 404, 200, "404 page not found")
assert.Equal(t, expectedErrorMsg, err.Error())
Expand All @@ -94,7 +94,7 @@ func TestWaitUntilWorking(t *testing.T) {
}))
defer server.Close()

err = WaitUntilWorking(ctx, "GET", server.URL, "testServer", http.StatusOK)
err = WaitUntilWorking(ctx, "GET", server.URL, "testServer", http.StatusOK, false)
require.Error(t, err)
expectedErrorMsg := fmt.Sprintf("Received bad status code in reply to server ping at %s: %d. Expected %d. Response body: %s", server.URL, 400, 200, string(jsonBytes))
assert.Equal(t, expectedErrorMsg, err.Error())
Expand All @@ -107,7 +107,7 @@ func TestWaitUntilWorking(t *testing.T) {
<-time.After(200 * time.Millisecond)
earlyCancel()
}()
err := WaitUntilWorking(earlyCancelCtx, "GET", "https://noserverexists.com", "testServer", http.StatusOK)
err := WaitUntilWorking(earlyCancelCtx, "GET", "https://noserverexists.com", "testServer", http.StatusOK, false)
require.Error(t, err)
expectedErrorMsg := fmt.Sprintf("Failed to send request to testServer at %s; likely server is not up (will retry in 50ms):", "https://noserverexists.com")
assert.Equal(t, logrus.InfoLevel, hook.LastEntry().Level)
Expand All @@ -128,7 +128,7 @@ func TestWaitUntilWorking(t *testing.T) {
}))
defer server.Close()

err := WaitUntilWorking(earlyCancelCtx, "GET", server.URL, "testServer", http.StatusOK)
err := WaitUntilWorking(earlyCancelCtx, "GET", server.URL, "testServer", http.StatusOK, false)
require.Error(t, err)
expectedErrorMsg := fmt.Sprintf("Failed to send request to testServer at %s; likely server is not up (will retry in 50ms):", server.URL)
assert.Equal(t, logrus.InfoLevel, hook.LastEntry().Level)
Expand Down
10 changes: 5 additions & 5 deletions xrootd/origin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ func TestOrigin(t *testing.T) {
defer mockupCancel()

// In this case a 403 means its running
err := server_utils.WaitUntilWorking(ctx, "GET", param.Origin_Url.GetString(), "xrootd", 403)
err := server_utils.WaitUntilWorking(ctx, "GET", param.Origin_Url.GetString(), "xrootd", 403, false)
if err != nil {
t.Fatalf("Unsuccessful test: Server encountered an error: %v", err)
}
Expand Down Expand Up @@ -210,7 +210,7 @@ func TestMultiExportOrigin(t *testing.T) {
defer mockupCancel()

// In this case a 403 means its running
err = server_utils.WaitUntilWorking(ctx, "GET", param.Origin_Url.GetString(), "xrootd", 403)
err = server_utils.WaitUntilWorking(ctx, "GET", param.Origin_Url.GetString(), "xrootd", 403, false)
if err != nil {
t.Fatalf("Unsuccessful test: Server encountered an error: %v", err)
}
Expand Down Expand Up @@ -255,7 +255,7 @@ func TestS3OriginConfig(t *testing.T) {
// Check if MinIO is running (by default at localhost:9000)
endpoint := "localhost:9000"
// Expect a 403 from this endpoint -- that means it's running
err = server_utils.WaitUntilWorking(ctx, "GET", fmt.Sprintf("http://%s", endpoint), "xrootd", 403)
err = server_utils.WaitUntilWorking(ctx, "GET", fmt.Sprintf("http://%s", endpoint), "xrootd", 403, false)
if err != nil {
t.Fatalf("Unsuccessful test: Server encountered an error: %v", err)
}
Expand Down Expand Up @@ -332,7 +332,7 @@ func TestS3OriginConfig(t *testing.T) {
originEndpoint := fmt.Sprintf("%s/%s/%s/%s/%s", param.Origin_Url.GetString(), serviceName, regionName, bucketName, objectName)
// Until we sort out the things we mentioned above, we should expect a 403 because we don't try to pass tokens
// and we don't actually export any keys for token validation.
err = server_utils.WaitUntilWorking(ctx, "GET", originEndpoint, "xrootd", 403)
err = server_utils.WaitUntilWorking(ctx, "GET", originEndpoint, "xrootd", 403, false)
if err != nil {
t.Fatalf("Unsucessful test: Server encountered an error: %v", err)
}
Expand All @@ -349,7 +349,7 @@ func TestS3OriginConfig(t *testing.T) {
defer cancel()
defer mockupCancel()

err = server_utils.WaitUntilWorking(ctx, "GET", originEndpoint, "xrootd", 403)
err = server_utils.WaitUntilWorking(ctx, "GET", originEndpoint, "xrootd", 403, false)
if err != nil {
t.Fatalf("Unsucessful test: Server encountered an error: %v", err)
}
Expand Down

0 comments on commit 939eda9

Please sign in to comment.