From 38b78f763982501599d04112dadd1f9d6397c3e1 Mon Sep 17 00:00:00 2001 From: Jerry Blakley Date: Wed, 10 May 2017 20:55:58 +0000 Subject: [PATCH 1/8] New and improved server tests --- tests/server-tests/api_test.go | 80 ++++++++++++++++++++++++++----- tests/server-tests/server_test.go | 28 +++++++++-- 2 files changed, 90 insertions(+), 18 deletions(-) diff --git a/tests/server-tests/api_test.go b/tests/server-tests/api_test.go index 040e828b..394017c1 100644 --- a/tests/server-tests/api_test.go +++ b/tests/server-tests/api_test.go @@ -5,6 +5,7 @@ import ( "encoding/hex" "fmt" "io" + "io/ioutil" "net/http" "strings" "testing" @@ -56,12 +57,6 @@ func collectResults(testOutput TestOutput) { fmt.Printf("%v\n", testOutput.result) } -/* TODO GET check content returned -if len(content) > 0 && string(body) != content { - t.Errorf("testGET: Error, expected content %v, got %v", content, resp.Body) -} -*/ - func testMethod(t *testing.T, testInput TestInput) error { var ( err error @@ -82,7 +77,7 @@ func testMethod(t *testing.T, testInput TestInput) error { for _, pair := range testInput.headers { req.Header.Add(pair.key, pair.value) - fmt.Printf("testMethod: method: %s; path: %s; header: %v\n", testInput.method, testInput.path, req.Header) + fmt.Printf("testMethod: Headers: method: %s; path: %s; header: %v\n", testInput.method, testInput.path, req.Header) } defer collectResults(testOutput) @@ -101,12 +96,72 @@ func testMethod(t *testing.T, testInput TestInput) error { testInput.method, testInput.path, testInput.expectedStatusCode, resp.Status) } + if testInput.method == "GET" { // Are there other methods which get content. We could genericize + body, err := ioutil.ReadAll(resp.Body) + if err != nil { + // handle error + t.Errorf("testMethod: Error on ioutil.ReadAll: %v", err) + } + + if testInput.expectedStatusCode != 404 && + len(testInput.content) > 0 && + string(body) != testInput.content { + t.Errorf("testMethod: Error, expected content %v, got %v", testInput.content, resp.Body) + } + } + testOutput.status = resp.Status testOutput.statusCode = resp.StatusCode return err } +func TestPaths(t *testing.T) { + type PathResult struct { + path string + statusCode int + } + + var testInput TestInput + var testOutput TestOutput + var paths []PathResult + + fmt.Println("TestPaths") + + testInput.client = getClient() + + paths = make([]PathResult, 7) + paths[0].path = getServerPath() + paths[0].statusCode = 200 + paths[1].path = paths[0].path + "sites/" + paths[1].statusCode = 405 + paths[2].path = paths[1].path + getSiteId() + "/" + paths[2].statusCode = 404 + paths[3].path = paths[2].path + "/environments/" + paths[3].statusCode = 405 + paths[4].path = paths[3].path + "self/" + paths[4].statusCode = 404 + paths[5].path = paths[3].path + "dev/" + paths[5].statusCode = 404 + paths[6].path = paths[5].path + "files/" + paths[6].statusCode = 404 + + testInput.content = "" // content is ignored + testInput.method = "GET" + + // for _, method := range []string{"HEAL", "TEST", "LOCK", "PROPPATCH", "OPTIONS"} { + // Temporarily remove HEAL, it takes a long time + for _, entry := range paths { + testInput.path = entry.path + testInput.expectedStatusCode = entry.statusCode + err := testMethod(t, testInput) + if err != nil { + // handle error + } + testOutput.result = "" + } +} + func TestAuxiliaryOps(t *testing.T) { var testInput TestInput var testOutput TestOutput @@ -114,7 +169,7 @@ func TestAuxiliaryOps(t *testing.T) { fmt.Println("TestAuxiliaryFileOps") testInput.client = getClient() - filepath := getServerPath() + filepath := getFilesPath() dirname := filepath + randstring(8) + "/" testInput.content = randstring(32) @@ -130,7 +185,6 @@ func TestAuxiliaryOps(t *testing.T) { if err != nil { // handle error } - // First assignment to res testOutput.result = "" } } @@ -144,7 +198,7 @@ func TestBasicFileOps(t *testing.T) { fmt.Println("TestBasicFileOps") testInput.client = getClient() - filepath := getServerPath() + filepath := getFilesPath() dirname := filepath + randstring(8) + "/" testInput.content = randstring(32) @@ -203,6 +257,9 @@ func TestBasicFileOps(t *testing.T) { // handle error } + // Clear the headers + testInput.headers = nil + testInput.method = "GET" testInput.expectedStatusCode = 200 err = testMethod(t, testInput) @@ -210,9 +267,6 @@ func TestBasicFileOps(t *testing.T) { // handle error } - // Clear the headers - testInput.headers = nil - // COPY // We don't COPY, the OS makes effectively a get/put // Kind of a meaningless test diff --git a/tests/server-tests/server_test.go b/tests/server-tests/server_test.go index 7d8fc73b..a3cf3461 100644 --- a/tests/server-tests/server_test.go +++ b/tests/server-tests/server_test.go @@ -70,13 +70,31 @@ func getClient() *http.Client { return client } -func getServerPath() string { - valhallapath := testcfg.ServerPath - valhallaport := testcfg.ServerPort +func getFilesPath() string { + serverpath := getServerPath() siteid := testcfg.SiteId // bindingid := testcfg.BindingId env := testcfg.Env - filepath := "https://" + valhallapath + ":" + valhallaport + "/sites/" + siteid + "/environments/" + env + "/files/" - return filepath + filespath := serverpath + "sites/" + siteid + "/environments/" + env + "/files/" + return filespath +} + +func getServerPath() string { + valhallapath := testcfg.ServerPath + valhallaport := testcfg.ServerPort + serverpath := "https://" + valhallapath + ":" + valhallaport + "/" + return serverpath +} + +func getBindingId() string { + return testcfg.BindingId +} + +func getSiteId() string { + return testcfg.SiteId +} + +func getEnv() string { + return testcfg.Env } From 917312eab6b86b590c30d327a77a2b760c30aa96 Mon Sep 17 00:00:00 2001 From: Jerry Blakley Date: Sat, 15 Jul 2017 18:21:21 +0000 Subject: [PATCH 2/8] Capture errors curl reports on fusedav-valhalla requests --- src/filecache.c | 2 +- src/session.c | 11 +++++++++++ src/session.h | 1 + 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/filecache.c b/src/filecache.c index aba74385..f0aad2f7 100644 --- a/src/filecache.c +++ b/src/filecache.c @@ -754,7 +754,7 @@ static void get_fresh_fd(filecache_t *cache, } else { // Not sure what to do here; goto finish, or try the loop another time? - log_print(LOG_WARNING, SECTION_FILECACHE_OPEN, "%s: returns %d; expected 304 or 200", funcname, response_code); + log_print(LOG_WARNING, SECTION_FILECACHE_OPEN, "%s: returns %d; expected 304 or 200; err: %s", funcname, response_code, curl_errorbuffer()); } // No check for O_TRUNC here because we skip server access and just diff --git a/src/session.c b/src/session.c index 007bd03a..329f39c5 100644 --- a/src/session.c +++ b/src/session.c @@ -70,6 +70,9 @@ void (*const state_table [NUM_STATES][NUM_EVENTS]) (void) = { #define LOGSTRSZ 80 static __thread char nodeaddr[LOGSTRSZ]; +// Capture errors and make them available +static __thread char curl_errbuf[CURL_ERROR_SIZE]; + // status of a node in a cluster static const unsigned int HEALTHY = 0; static const unsigned int RECOVERING = 1; @@ -408,6 +411,11 @@ static void print_ipaddr_pair(char *msg) { log_print(LOG_INFO, SECTION_SESSION_DEFAULT, "Using filesystem_host=%s", nodeaddr); } +// Return the contents of the error buffer +const char * curl_errorbuffer() { + return curl_errbuf; +} + static int session_debug(__unused CURL *handle, curl_infotype type, char *data, size_t size, __unused void *userp) { if (type == CURLINFO_TEXT) { char *msg = malloc(size + 1); @@ -1212,6 +1220,9 @@ CURL *session_request_init(const char *path, const char *query_string, bool tmp_ funcname, node_status.resolve_slist); curl_easy_setopt(session, CURLOPT_RESOLVE, node_status.resolve_slist); curl_easy_setopt(session, CURLOPT_DEBUGFUNCTION, session_debug); + // Empty the error buffer + curl_errbuf[0] = '\0'; + curl_easy_setopt(session, CURLOPT_ERRORBUFFER, curl_errbuf); curl_easy_setopt(session, CURLOPT_VERBOSE, 1L); escaped_path = escape_except_slashes(session, path); diff --git a/src/session.h b/src/session.h index 402a5016..dc57abcf 100644 --- a/src/session.h +++ b/src/session.h @@ -61,5 +61,6 @@ bool use_saint_mode(void); void timed_curl_easy_perform(CURL *session, CURLcode *res, long *response_code, long *elapsed_time); const char *get_filesystem_cluster(void); const char *get_nodeaddr(void); +const char *curl_errorbuffer(void); #endif From 89b333a3c38cc256e2ca722b5dd235a160fa6947 Mon Sep 17 00:00:00 2001 From: Jerry Blakley Date: Sat, 15 Jul 2017 20:11:30 +0000 Subject: [PATCH 3/8] Update tests for curl capture error --- src/filecache.c | 4 ++-- src/session.c | 9 +++++++-- src/session.h | 2 +- src/util.c | 27 +++++++++++++++++++++++++++ 4 files changed, 37 insertions(+), 5 deletions(-) diff --git a/src/filecache.c b/src/filecache.c index f0aad2f7..76f2b99e 100644 --- a/src/filecache.c +++ b/src/filecache.c @@ -531,7 +531,7 @@ static void get_fresh_fd(filecache_t *cache, // but on the close (dav_flush/release), the PUT fails and the file never makes it to the server. // On opening again, the server will deliver this unexpected 404. Changes for forensic-haven // should prevent these errors in the future (2013-08-29) - if (inject_error(filecache_error_fresh404)) response_code = 404; + if (inject_error(filecache_error_fresh404)) response_code = 400; if (response_code == 304) { // This should never happen with a well-behaved server. if (pdata == NULL || inject_error(filecache_error_freshcurl2)) { @@ -754,7 +754,7 @@ static void get_fresh_fd(filecache_t *cache, } else { // Not sure what to do here; goto finish, or try the loop another time? - log_print(LOG_WARNING, SECTION_FILECACHE_OPEN, "%s: returns %d; expected 304 or 200; err: %s", funcname, response_code, curl_errorbuffer()); + log_print(LOG_WARNING, SECTION_FILECACHE_OPEN, "%s: returns %d; expected 304 or 200; err: %s", funcname, response_code, curl_errorbuffer(response_code)); } // No check for O_TRUNC here because we skip server access and just diff --git a/src/session.c b/src/session.c index 329f39c5..f81af501 100644 --- a/src/session.c +++ b/src/session.c @@ -412,8 +412,13 @@ static void print_ipaddr_pair(char *msg) { } // Return the contents of the error buffer -const char * curl_errorbuffer() { - return curl_errbuf; +const char * curl_errorbuffer(CURLcode res) { + size_t len = strlen(curl_errbuf); + if(len) { + return curl_errbuf; + } else { + return curl_easy_strerror(res); + } } static int session_debug(__unused CURL *handle, curl_infotype type, char *data, size_t size, __unused void *userp) { diff --git a/src/session.h b/src/session.h index dc57abcf..0062775e 100644 --- a/src/session.h +++ b/src/session.h @@ -61,6 +61,6 @@ bool use_saint_mode(void); void timed_curl_easy_perform(CURL *session, CURLcode *res, long *response_code, long *elapsed_time); const char *get_filesystem_cluster(void); const char *get_nodeaddr(void); -const char *curl_errorbuffer(void); +const char *curl_errorbuffer(CURLcode res); #endif diff --git a/src/util.c b/src/util.c index ff99117e..085e16fa 100644 --- a/src/util.c +++ b/src/util.c @@ -206,6 +206,30 @@ static void propfind_test(void) { } } +/* test what happens on a GET 400 error */ +static void curl_error_capture_test(void) { + static int fdx = no_error; + static int tdx = no_error; + const int iters = 4096; // @TODO I just made this number up; figure out a better one! + + for (int iter = 0; iter < iters; iter++) { + // Sleep 11 seconds between injections + sleep(11); + + // flop between filecache_error_fresh404 and no_error + + if (tdx == no_error) tdx = filecache_error_fresh404; + else tdx = no_error; + + log_print(LOG_NOTICE, SECTION_UTIL_DEFAULT, "fce: %d Uninjecting %d; injecting %d", inject_error_count, fdx, tdx); + + // Make the new location true but turn off the locations for the old location. + inject_error_list[tdx] = true; + inject_error_list[fdx] = false; + fdx = tdx; + } +} + /* test conditions which might or might not land a file in the forensic haven * This is a pretty extensive test of the filecache errors, but not a complete one. */ @@ -316,6 +340,9 @@ void *inject_error_mechanism(__unused void *ptr) { log_print(LOG_NOTICE, SECTION_UTIL_DEFAULT, "inject_error_mechanism: Starting raise SIGINT on leveldb error"); leveldb_error_test(); + + log_print(LOG_NOTICE, SECTION_UTIL_DEFAULT, "inject_error_mechanism: Starting curl error capture test"); + curl_error_capture_test(); } free(inject_error_list); From 46f9bb3bc7835a8a5a74538259a335e32f1cde48 Mon Sep 17 00:00:00 2001 From: Jerry Blakley Date: Sat, 15 Jul 2017 20:55:50 +0000 Subject: [PATCH 4/8] Report curl errorbuffer errors via print_errors mechanism --- src/session.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/session.c b/src/session.c index f81af501..cb5dd0ad 100644 --- a/src/session.c +++ b/src/session.c @@ -240,7 +240,7 @@ static void print_errors(const int iter, const char *type_str, const char *fcn_n bool slow_request = false; if (res != CURLE_OK) { - asprintf(&error_str, "%s :: %s", curl_easy_strerror(res), "no rc"); + asprintf(&error_str, "%s :: %s", curl_errorbuffer(res), "no rc"); } else if (response_code >= 500) { asprintf(&error_str, "%s :: %lu", "no curl error", response_code); } else if (elapsed_time >= 0) { From e103c3f4ad885683a3bc00ea39cded552f878dfa Mon Sep 17 00:00:00 2001 From: Jerry Blakley Date: Sat, 9 Sep 2017 00:25:26 +0000 Subject: [PATCH 5/8] Dev curl errorbuffer 3 (#365) (#366) (#367) * Accommodate comments on previous review; update server tests --- src/filecache.c | 2 +- src/util.c | 6 +- src/util.h | 2 +- tests/server-tests/README.md | 11 +++ tests/server-tests/api_test.go | 90 +++++++++---------- .../server-tests/base-test-server-config.json | 8 ++ tests/server-tests/server_test.go | 24 +++-- tests/server-tests/test-server-config.json | 8 -- 8 files changed, 78 insertions(+), 73 deletions(-) create mode 100644 tests/server-tests/README.md create mode 100644 tests/server-tests/base-test-server-config.json delete mode 100644 tests/server-tests/test-server-config.json diff --git a/src/filecache.c b/src/filecache.c index 76f2b99e..a5a0e253 100644 --- a/src/filecache.c +++ b/src/filecache.c @@ -531,7 +531,7 @@ static void get_fresh_fd(filecache_t *cache, // but on the close (dav_flush/release), the PUT fails and the file never makes it to the server. // On opening again, the server will deliver this unexpected 404. Changes for forensic-haven // should prevent these errors in the future (2013-08-29) - if (inject_error(filecache_error_fresh404)) response_code = 400; + if (inject_error(filecache_error_fresh400)) response_code = 400; if (response_code == 304) { // This should never happen with a well-behaved server. if (pdata == NULL || inject_error(filecache_error_freshcurl2)) { diff --git a/src/util.c b/src/util.c index 085e16fa..f5307500 100644 --- a/src/util.c +++ b/src/util.c @@ -216,9 +216,9 @@ static void curl_error_capture_test(void) { // Sleep 11 seconds between injections sleep(11); - // flop between filecache_error_fresh404 and no_error + // flop between filecache_error_fresh400 and no_error - if (tdx == no_error) tdx = filecache_error_fresh404; + if (tdx == no_error) tdx = filecache_error_fresh400; else tdx = no_error; log_print(LOG_NOTICE, SECTION_UTIL_DEFAULT, "fce: %d Uninjecting %d; injecting %d", inject_error_count, fdx, tdx); @@ -253,7 +253,7 @@ static void filecache_forensic_haven_test(void) { {filecache_error_freshflock2, "filecache_error_freshflock2"}, {filecache_error_freshsession, "filecache_error_freshsession"}, {filecache_error_freshcurl1, "filecache_error_freshcurl1"}, - {filecache_error_fresh404, "filecache_error_fresh404"}, + {filecache_error_fresh400, "filecache_error_fresh400"}, {filecache_error_freshcurl2, "filecache_error_freshcurl2"}, {filecache_error_freshopen2, "filecache_error_freshopen2"}, {filecache_error_freshpdata, "filecache_error_freshpdata"}, diff --git a/src/util.h b/src/util.h index 0f5424a8..979f67b8 100644 --- a/src/util.h +++ b/src/util.h @@ -78,7 +78,7 @@ void *inject_error_mechanism(void *ptr); #define filecache_error_freshcurl2 35 #define filecache_error_freshopen2 36 #define filecache_error_freshpdata 37 -#define filecache_error_fresh404 38 +#define filecache_error_fresh400 38 #define filecache_error_opencalloc 39 #define filecache_error_readsdata 40 #define filecache_error_readread 41 diff --git a/tests/server-tests/README.md b/tests/server-tests/README.md new file mode 100644 index 00000000..d3daed66 --- /dev/null +++ b/tests/server-tests/README.md @@ -0,0 +1,11 @@ +These tests are designed to provide functionality similar to unit tests. +However, they are designed to run directly against a working fileserver. +As such, depending on the requirements of your file server, they require explicit parameters. +Edit the file: +test-server-config.json +to include a binding id, site id, server path, server port, and env. +Then run: +go test +If you run: +go test -v +it will also display certain log messages in addition to errors. diff --git a/tests/server-tests/api_test.go b/tests/server-tests/api_test.go index 394017c1..2cce28d4 100644 --- a/tests/server-tests/api_test.go +++ b/tests/server-tests/api_test.go @@ -3,7 +3,6 @@ package testserver import ( "crypto/rand" "encoding/hex" - "fmt" "io" "io/ioutil" "net/http" @@ -33,11 +32,10 @@ type TestOutput struct { content string } -func newrequest(file string, method string, body io.Reader) (*http.Request, error) { +func newrequest(file string, method string, body io.Reader, t *testing.T) (*http.Request, error) { req, err := http.NewRequest(method, file, body) if err != nil { - // handle err - fmt.Errorf("Error on NewRequest; exiting...") + t.Errorf("Error on NewRequest; exiting...") return nil, err } req.Header.Add("Log-To-Journal", "true") @@ -52,9 +50,9 @@ func randstring(len int) string { return hex.EncodeToString(randBytes) } -func collectResults(testOutput TestOutput) { +func collectResults(testOutput TestOutput, t *testing.T) { // TODO figure out how to collect results - fmt.Printf("%v\n", testOutput.result) + t.Logf("%v\n", testOutput.result) } func testMethod(t *testing.T, testInput TestInput) error { @@ -64,49 +62,48 @@ func testMethod(t *testing.T, testInput TestInput) error { testOutput TestOutput ) - fmt.Printf("testMethod: %s %d %s\n", testInput.method, testInput.expectedStatusCode, testInput.path) + t.Logf("testMethod: %s %d %s", testInput.method, testInput.expectedStatusCode, testInput.path) if testInput.method == "PUT" { // only PUT sends content? - req, err = newrequest(testInput.path, testInput.method, strings.NewReader(testInput.content)) + req, err = newrequest(testInput.path, testInput.method, strings.NewReader(testInput.content), t) } else { - req, err = newrequest(testInput.path, testInput.method, nil) + req, err = newrequest(testInput.path, testInput.method, nil, t) } if err != nil { - // handle error t.Errorf("testMethod: %s, Error: %v", testInput.method, err) + return err } for _, pair := range testInput.headers { req.Header.Add(pair.key, pair.value) - fmt.Printf("testMethod: Headers: method: %s; path: %s; header: %v\n", testInput.method, testInput.path, req.Header) + t.Logf("testMethod: Headers: method: %s; path: %s; header: %v", testInput.method, testInput.path, req.Header) } - defer collectResults(testOutput) + defer collectResults(testOutput, t) resp, err := testInput.client.Do(req) if err != nil { - // handle err - t.Errorf("Error on client.Do; method: %s; path: %s; exiting...\n", testInput.method, testInput.path) + t.Errorf("testMethod: Error on client.Do; method: %s; path: %s; exiting...\n", testInput.method, testInput.path) testOutput.err = err testOutput.result = "" return err } defer resp.Body.Close() + body, err := ioutil.ReadAll(resp.Body) if resp.StatusCode != testInput.expectedStatusCode { - t.Errorf("testMethod: %s; path: %s; Error, expected Status %d, got %v", - testInput.method, testInput.path, testInput.expectedStatusCode, resp.Status) + t.Errorf("testMethod: %s; path: %s; Error, expected Status %d, got %v; body: %v", + testInput.method, testInput.path, testInput.expectedStatusCode, resp.Status, string(body)) } if testInput.method == "GET" { // Are there other methods which get content. We could genericize - body, err := ioutil.ReadAll(resp.Body) if err != nil { - // handle error t.Errorf("testMethod: Error on ioutil.ReadAll: %v", err) + return err } if testInput.expectedStatusCode != 404 && len(testInput.content) > 0 && string(body) != testInput.content { - t.Errorf("testMethod: Error, expected content %v, got %v", testInput.content, resp.Body) + t.Errorf("testMethod: Error, expected content %v, got %v", testInput.content, string(body)) } } @@ -126,17 +123,18 @@ func TestPaths(t *testing.T) { var testOutput TestOutput var paths []PathResult - fmt.Println("TestPaths") + t.Log("TestPaths") - testInput.client = getClient() + testInput.client = getClient(t) paths = make([]PathResult, 7) paths[0].path = getServerPath() paths[0].statusCode = 200 paths[1].path = paths[0].path + "sites/" paths[1].statusCode = 405 - paths[2].path = paths[1].path + getSiteId() + "/" - paths[2].statusCode = 404 + // Response might be different if there is a trailing slash after site id + paths[2].path = paths[1].path + getSiteId() + paths[2].statusCode = 405 paths[3].path = paths[2].path + "/environments/" paths[3].statusCode = 405 paths[4].path = paths[3].path + "self/" @@ -149,14 +147,12 @@ func TestPaths(t *testing.T) { testInput.content = "" // content is ignored testInput.method = "GET" - // for _, method := range []string{"HEAL", "TEST", "LOCK", "PROPPATCH", "OPTIONS"} { - // Temporarily remove HEAL, it takes a long time for _, entry := range paths { testInput.path = entry.path testInput.expectedStatusCode = entry.statusCode err := testMethod(t, testInput) if err != nil { - // handle error + t.Errorf("TestPaths: Error on path %v: %v", testInput.path, err) } testOutput.result = "" } @@ -166,9 +162,9 @@ func TestAuxiliaryOps(t *testing.T) { var testInput TestInput var testOutput TestOutput - fmt.Println("TestAuxiliaryFileOps") + t.Log("TestAuxiliaryFileOps") - testInput.client = getClient() + testInput.client = getClient(t) filepath := getFilesPath() dirname := filepath + randstring(8) + "/" @@ -183,7 +179,7 @@ func TestAuxiliaryOps(t *testing.T) { testInput.method = method err := testMethod(t, testInput) if err != nil { - // handle error + t.Errorf("TestAuxiliaryOps: Error on method %s on path %v: %v", method, testInput.path, err) } testOutput.result = "" } @@ -195,9 +191,9 @@ func TestBasicFileOps(t *testing.T) { pair HeaderPair ) - fmt.Println("TestBasicFileOps") + t.Log("TestBasicFileOps") - testInput.client = getClient() + testInput.client = getClient(t) filepath := getFilesPath() dirname := filepath + randstring(8) + "/" @@ -211,7 +207,7 @@ func TestBasicFileOps(t *testing.T) { err := testMethod(t, testInput) testInput.expectedStatusCode = 200 // Reset to default if err != nil { - // handle error + t.Errorf("TestBasicFileOps: Error on method %s on path %v: %v", testInput.method, testInput.path, err) } // Add filename to dirname which was used in MKCOL @@ -221,7 +217,7 @@ func TestBasicFileOps(t *testing.T) { testInput.expectedStatusCode = 201 err = testMethod(t, testInput) if err != nil { - // handle error + t.Errorf("TestBasicFileOps: Error on method %s on path %v: %v", testInput.method, testInput.path, err) } // Sort of emulating the way propfind works in fusedav-valhalla world @@ -237,7 +233,7 @@ func TestBasicFileOps(t *testing.T) { err = testMethod(t, testInput) if err != nil { - // handle error + t.Errorf("TestBasicFileOps: Error on method %s on path %v: %v", testInput.method, testInput.path, err) } // PROPFIND on directory testInput.path = dirname @@ -245,7 +241,7 @@ func TestBasicFileOps(t *testing.T) { testInput.expectedStatusCode = 207 err = testMethod(t, testInput) if err != nil { - // handle error + t.Errorf("TestBasicFileOps: Error on method %s on path %v: %v", testInput.method, testInput.path, err) } // PROPFIND on file @@ -254,7 +250,7 @@ func TestBasicFileOps(t *testing.T) { testInput.expectedStatusCode = 207 err = testMethod(t, testInput) if err != nil { - // handle error + t.Errorf("TestBasicFileOps: Error on method %s on path %v: %v", testInput.method, testInput.path, err) } // Clear the headers @@ -264,7 +260,7 @@ func TestBasicFileOps(t *testing.T) { testInput.expectedStatusCode = 200 err = testMethod(t, testInput) if err != nil { - // handle error + t.Errorf("TestBasicFileOps: Error on method %s on path %v: %v", testInput.method, testInput.path, err) } // COPY @@ -274,7 +270,7 @@ func TestBasicFileOps(t *testing.T) { testInput.expectedStatusCode = 200 err = testMethod(t, testInput) if err != nil { - // handle error + t.Errorf("TestBasicFileOps: Error on method %s on path %v: %v", testInput.method, testInput.path, err) } // Sort of cheating. We should use the content we get from GET @@ -286,7 +282,7 @@ func TestBasicFileOps(t *testing.T) { testInput.expectedStatusCode = 201 err = testMethod(t, testInput) if err != nil { - // handle error + t.Errorf("TestBasicFileOps: Error on method %s on path %v: %v", testInput.method, testInput.path, err) } // Get the tofile as part of copy to verify @@ -294,7 +290,7 @@ func TestBasicFileOps(t *testing.T) { testInput.expectedStatusCode = 200 err = testMethod(t, testInput) if err != nil { - // handle error + t.Errorf("TestBasicFileOps: Error on method %s on path %v: %v", testInput.method, testInput.path, err) } // e COPY @@ -309,7 +305,7 @@ func TestBasicFileOps(t *testing.T) { testInput.expectedStatusCode = 204 err = testMethod(t, testInput) if err != nil { - // handle error + t.Errorf("TestBasicFileOps: Error on method %s on path %v: %v", testInput.method, testInput.path, err) } // Clear the headers @@ -321,7 +317,7 @@ func TestBasicFileOps(t *testing.T) { testInput.expectedStatusCode = 200 err = testMethod(t, testInput) if err != nil { - // handle error + t.Errorf("TestBasicFileOps: Error on method %s on path %v: %v", testInput.method, testInput.path, err) } // Get the original file. It should get a 404 @@ -330,7 +326,7 @@ func TestBasicFileOps(t *testing.T) { testInput.expectedStatusCode = 404 err = testMethod(t, testInput) if err != nil { - // handle error + t.Errorf("TestBasicFileOps: Error on method %s on path %v: %v", testInput.method, testInput.path, err) } // e MOVE @@ -341,7 +337,7 @@ func TestBasicFileOps(t *testing.T) { testInput.expectedStatusCode = 204 err = testMethod(t, testInput) if err != nil { - // handle error + t.Errorf("TestBasicFileOps: Error on method %s on path %v: %v", testInput.method, testInput.path, err) } testInput.path = newfile @@ -349,7 +345,7 @@ func TestBasicFileOps(t *testing.T) { testInput.expectedStatusCode = 204 err = testMethod(t, testInput) if err != nil { - // handle error + t.Errorf("TestBasicFileOps: Error on method %s on path %v: %v", testInput.method, testInput.path, err) } testInput.path = filename @@ -358,7 +354,7 @@ func TestBasicFileOps(t *testing.T) { testInput.expectedStatusCode = 204 err = testMethod(t, testInput) if err != nil { - // handle error + t.Errorf("TestBasicFileOps: Error on method %s on path %v: %v", testInput.method, testInput.path, err) } testInput.path = dirname @@ -366,6 +362,6 @@ func TestBasicFileOps(t *testing.T) { testInput.expectedStatusCode = 204 err = testMethod(t, testInput) if err != nil { - // handle error + t.Errorf("TestBasicFileOps: Error on method %s on path %v: %v", testInput.method, testInput.path, err) } } diff --git a/tests/server-tests/base-test-server-config.json b/tests/server-tests/base-test-server-config.json new file mode 100644 index 00000000..7a60366c --- /dev/null +++ b/tests/server-tests/base-test-server-config.json @@ -0,0 +1,8 @@ +{ + "BindingId": "", + "SiteId": "", + "ServerPath": "", + "ServerPort": "", + "Env": "" +} + diff --git a/tests/server-tests/server_test.go b/tests/server-tests/server_test.go index a3cf3461..f67a117f 100644 --- a/tests/server-tests/server_test.go +++ b/tests/server-tests/server_test.go @@ -3,9 +3,9 @@ package testserver import ( "crypto/tls" "encoding/json" - "fmt" "net/http" "os" + "testing" ) type TestConfiguration struct { @@ -30,40 +30,38 @@ var tlsClientConfig = &tls.Config{ InsecureSkipVerify: true, } -func getBindingCert(testcfg TestConfiguration) []tls.Certificate { +func getBindingCert(testcfg TestConfiguration, t *testing.T) []tls.Certificate { crtpath := "/srv/bindings/" + testcfg.BindingId + "/certs/binding.crt" keypath := "/srv/bindings/" + testcfg.BindingId + "/certs/binding.key" - fmt.Printf("getBindingCert: %s : %s\n", crtpath, keypath) + t.Logf("getBindingCert: %s : %s", crtpath, keypath) cert, err := tls.LoadX509KeyPair(crtpath, keypath) if err != nil { - // handle error + t.Errorf("getBindingCert: Failed: %v", err) } certs := []tls.Certificate{cert} return certs } -func readConfig() { +func readConfig(t *testing.T) { // Read config file and set variables file, err := os.Open("test-server-config.json") if err != nil { - // handle error - fmt.Errorf("readConfig: Error in Open\n") + t.Error("readConfig: Error in Open :: Make sure base-test-server-config.json is copied to test-server-config.json and populated") panic(err) } decoder := json.NewDecoder(file) testcfg = TestConfiguration{} err = decoder.Decode(&testcfg) if err != nil { - // handle error - fmt.Errorf("readConfig: Error in decoder") + t.Errorf("readConfig: Error in decoder: %v", err) panic(err) } - fmt.Printf("readConfig: cfg: %v\n", testcfg) + t.Logf("readConfig: cfg: %v\n", testcfg) } -func getClient() *http.Client { - readConfig() - tlsClientConfig.Certificates = getBindingCert(testcfg) +func getClient(t *testing.T) *http.Client { + readConfig(t) + tlsClientConfig.Certificates = getBindingCert(testcfg, t) transport := &http.Transport{TLSClientConfig: tlsClientConfig} client := &http.Client{Transport: transport} diff --git a/tests/server-tests/test-server-config.json b/tests/server-tests/test-server-config.json deleted file mode 100644 index b6aee24c..00000000 --- a/tests/server-tests/test-server-config.json +++ /dev/null @@ -1,8 +0,0 @@ -{ - "BindingId": "4a1221c4cac44a2bbb793270a78ab9e5", - "SiteId": "cf207f2f-e868-49ca-980b-137810fa6911", - "ServerPath": "valhalla3.onebox.panth.io", - "ServerPort": "448", - "Env": "dev" -} - From 1dd2055d8749a320cbefc158ced9f78e3cf92ca2 Mon Sep 17 00:00:00 2001 From: Jerry Blakley Date: Wed, 13 Sep 2017 20:07:51 +0000 Subject: [PATCH 6/8] Yolo to stage (#370) Accommodate review suggestions; improve server tests Stop building for fedora 20 --- scripts/docker-outer.sh | 2 +- scripts/push_packagecloud.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/docker-outer.sh b/scripts/docker-outer.sh index c598c076..0c0a5365 100755 --- a/scripts/docker-outer.sh +++ b/scripts/docker-outer.sh @@ -4,7 +4,7 @@ bin="$(cd -P -- "$(dirname -- "$0")" && pwd -P)" docker=$(which docker) # which fedora distros to build this rpm for -BUILD_VERSIONS=${BUILD_VERSIONS:-20 22} +BUILD_VERSIONS=${BUILD_VERSIONS:-22} echo "==> Running RPM builds for these Fedora version(s): $BUILD_VERSIONS" diff --git a/scripts/push_packagecloud.sh b/scripts/push_packagecloud.sh index 2a7bb6e9..e1c9c218 100755 --- a/scripts/push_packagecloud.sh +++ b/scripts/push_packagecloud.sh @@ -13,7 +13,7 @@ if [ -z "$1" ] ; then exit 1 fi -BUILD_VERSIONS=${BUILD_VERSIONS:-20 22} +BUILD_VERSIONS=${BUILD_VERSIONS:-22} for i in $BUILD_VERSIONS ; do package_cloud push "pantheon/$1/fedora/$i" $bin/../pkg/$i/fusedav/*.rpm done From 529d843d60694ebdaefcd05a4efd1a1a5948bf28 Mon Sep 17 00:00:00 2001 From: Jerry Blakley Date: Fri, 15 Sep 2017 20:41:01 +0000 Subject: [PATCH 7/8] Yolo to stage (#374) Capture Curl partial file error --- scripts/version.sh | 8 +++++--- src/filecache.c | 34 +++++++++++++++++++++++++++------- tests/server-tests/api_test.go | 26 ++++++++++++++++++++++++++ 3 files changed, 58 insertions(+), 10 deletions(-) diff --git a/scripts/version.sh b/scripts/version.sh index 26506470..cab4028a 100755 --- a/scripts/version.sh +++ b/scripts/version.sh @@ -1,5 +1,6 @@ #!/bin/bash -set -e +set -eou pipefail +AUTOTAG_URL=${AUTOTAG_URL:-} # ensure we have autotag if [ ! -d "$HOME/bin" ]; then @@ -7,12 +8,13 @@ if [ ! -d "$HOME/bin" ]; then fi if [ ! -f "$HOME/bin/autotag" ]; then - AUTOTAG_URL=$(curl -silent -o - -L https://api.github.com/repos/pantheon-systems/autotag/releases/latest | grep 'browser_' | cut -d\" -f4) + AUTOTAG_URL=$(curl -silent -o - -L https://api.github.com/repos/pantheon-systems/autotag/releases/latest | grep 'browser_' | cut -d\" -f4 | awk '{print $0}') # handle the off chance that this wont work with some pre-set version if [ -z "$AUTOTAG_URL" ] ; then AUTOTAG_URL="https://github.com/pantheon-systems/autotag/releases/download/v0.0.3/autotag.linux.x86_64" fi - curl -L $AUTOTAG_URL -o ~/bin/autotag + echo "Pulling $AUTOTAG_URL" + curl -sf -L $AUTOTAG_URL -o ~/bin/autotag > /dev/null chmod 755 ~/bin/autotag fi diff --git a/src/filecache.c b/src/filecache.c index a5a0e253..b5edfb8e 100644 --- a/src/filecache.c +++ b/src/filecache.c @@ -511,11 +511,18 @@ static void get_fresh_fd(filecache_t *cache, } if ((res != CURLE_OK || response_code >= 500) || inject_error(filecache_error_freshcurl1)) { - trigger_saint_event(CLUSTER_FAILURE); - set_dynamic_logging(); - g_set_error(gerr, curl_quark(), E_FC_CURLERR, "%s: curl_easy_perform is not CURLE_OK or 500: %s", - funcname, curl_easy_strerror(res)); - goto finish; + if (res == CURLE_PARTIAL_FILE) { + // We see errors on partial file transfers. This seems to mean that the + // file didn't actually make it to S3. Capture this particular error, + // and don't send into saint mode + response_code = 999; // fake code to avoid accidental match + } else { + trigger_saint_event(CLUSTER_FAILURE); + set_dynamic_logging(); + g_set_error(gerr, curl_quark(), E_FC_CURLERR, "%s: curl_easy_perform is not CURLE_OK or 500: %s", + funcname, curl_easy_strerror(res)); + goto finish; + } } else { trigger_saint_event(CLUSTER_SUCCESS); } @@ -531,7 +538,11 @@ static void get_fresh_fd(filecache_t *cache, // but on the close (dav_flush/release), the PUT fails and the file never makes it to the server. // On opening again, the server will deliver this unexpected 404. Changes for forensic-haven // should prevent these errors in the future (2013-08-29) + // Update: if the offload of the file to S3 fails for some reason, either a partial file + // transfer error, which is a curl error so res != CURLE_OK, or a 404 is returned. + if (inject_error(filecache_error_fresh400)) response_code = 400; + if (response_code == 304) { // This should never happen with a well-behaved server. if (pdata == NULL || inject_error(filecache_error_freshcurl2)) { @@ -713,16 +724,25 @@ static void get_fresh_fd(filecache_t *cache, stats_timer("exceeded-time-small-GET-latency", elapsed_time); } } - else if (response_code == 404) { + else if (response_code == 404 || res == CURLE_PARTIAL_FILE) { /* If two bindings are in a race condition on the same file, this * can occur. Binding A does a propfind; Binding B does a propfind * and the file is shown to exist. Binding A deletes the file * then Binding B tries to access it and the server returns 404. * Not sure how to remediate without doing way more work and * introducing way more problems than the fix will fix. + * If a file doesn't make it to S3, fusedav often sees 'partial file transfer' + * In this case, treat more or less like a 404. The problem is that + * Valhalla Cassandra will think the file exists, and only fail when + * it tries to get content from S3. This will happen on each request + * for the file. */ struct stat_cache_value *value; - g_set_error(gerr, filecache_quark(), ENOENT, "%s: File expected to exist returns 404.", funcname); + if (res == CURLE_PARTIAL_FILE) { + g_set_error(gerr, filecache_quark(), ENOENT, "%s: File expected to exist returns 404-partial file transfer.", funcname); + } else { + g_set_error(gerr, filecache_quark(), ENOENT, "%s: File expected to exist returns 404.", funcname); + } /* we get a 404 because the stat_cache returned that the file existed, but it * was not on the server. Deleting it from the stat_cache makes the stat_cache * consistent, so the next access to the file will be handled correctly. diff --git a/tests/server-tests/api_test.go b/tests/server-tests/api_test.go index 2cce28d4..e891c29c 100644 --- a/tests/server-tests/api_test.go +++ b/tests/server-tests/api_test.go @@ -107,6 +107,32 @@ func testMethod(t *testing.T, testInput TestInput) error { } } + if testInput.method == "GET" { // Are there other methods which get content. We could genericize + if err != nil { + t.Errorf("testMethod: Error on ioutil.ReadAll: %v", err) + return err + } + + if testInput.expectedStatusCode != 404 && + len(testInput.content) > 0 && + string(body) != testInput.content { + t.Errorf("testMethod: Error, expected content %v, got %v", testInput.content, string(body)) + } + } + + if testInput.method == "GET" { // Are there other methods which get content. We could genericize + if err != nil { + t.Errorf("testMethod: Error on ioutil.ReadAll: %v", err) + return err + } + + if testInput.expectedStatusCode != 404 && + len(testInput.content) > 0 && + string(body) != testInput.content { + t.Errorf("testMethod: Error, expected content %v, got %v", testInput.content, string(body)) + } + } + testOutput.status = resp.Status testOutput.statusCode = resp.StatusCode From 1bf9a332a966bc8efd6f04aaadcb5428887a4f09 Mon Sep 17 00:00:00 2001 From: Jerry Blakley Date: Tue, 19 Sep 2017 18:28:33 +0000 Subject: [PATCH 8/8] Yolo to stage (#377) * Make partial file transfer failure non-retriable --- src/filecache.c | 10 ++++++++-- src/fusedav.c | 25 ++++++++++++++++++++----- src/log.c | 2 +- src/props.c | 5 ++++- src/session.c | 33 ++++++++++++++++++++++++++------- src/session.h | 2 +- tests/server-tests/api_test.go | 13 ------------- 7 files changed, 60 insertions(+), 30 deletions(-) diff --git a/src/filecache.c b/src/filecache.c index b5edfb8e..370db5e1 100644 --- a/src/filecache.c +++ b/src/filecache.c @@ -507,7 +507,10 @@ static void get_fresh_fd(filecache_t *cache, if (slist) curl_slist_free_all(slist); - process_status(funcname, session, res, response_code, elapsed_time, idx, path, false); + bool non_retriable_error = process_status(funcname, session, res, response_code, elapsed_time, idx, path, false); + // Some errors should not be retried. (Non-errors will fail the + // for loop test and fall through naturally) + if (non_retriable_error) break; } if ((res != CURLE_OK || response_code >= 500) || inject_error(filecache_error_freshcurl1)) { @@ -1101,7 +1104,10 @@ static void put_return_etag(const char *path, int fd, char *etag, GError **gerr) if (slist) curl_slist_free_all(slist); - process_status(funcname, session, res, response_code, elapsed_time, idx, path, false); + bool non_retriable_error = process_status(funcname, session, res, response_code, elapsed_time, idx, path, false); + // Some errors should not be retried. (Non-errors will fail the + // for loop test and fall through naturally) + if (non_retriable_error) break; } if ((res != CURLE_OK || response_code >= 500) || inject_error(filecache_error_etagcurl1)) { diff --git a/src/fusedav.c b/src/fusedav.c index 70a3aef2..2b0abd6e 100644 --- a/src/fusedav.c +++ b/src/fusedav.c @@ -275,7 +275,10 @@ static void getdir_propfind_callback(__unused void *userdata, const char *path, "%s: saw 410; calling HEAD on %s", funcname, path); timed_curl_easy_perform(session, &res, &response_code, &elapsed_time); - process_status(funcname, session, res, response_code, elapsed_time, idx, path, tmp_session); + bool non_retriable_error = process_status(funcname, session, res, response_code, elapsed_time, idx, path, tmp_session); + // Some errors should not be retried. (Non-errors will fail the + // for loop test and fall through naturally) + if (non_retriable_error) break; } delete_tmp_session(session); @@ -930,7 +933,10 @@ static void common_unlink(const char *path, bool do_unlink, GError **gerr) { if (slist) curl_slist_free_all(slist); - process_status(funcname, session, res, response_code, elapsed_time, idx, path, false); + bool non_retriable_error = process_status(funcname, session, res, response_code, elapsed_time, idx, path, false); + // Some errors should not be retried. (Non-errors will fail the + // for loop test and fall through naturally) + if (non_retriable_error) break; } if(res != CURLE_OK || response_code >= 500 || inject_error(fusedav_error_cunlinkcurl)) { @@ -1050,7 +1056,10 @@ static int dav_rmdir(const char *path) { if (slist) curl_slist_free_all(slist); - process_status(funcname, session, res, response_code, elapsed_time, idx, path, false); + bool non_retriable_error = process_status(funcname, session, res, response_code, elapsed_time, idx, path, false); + // Some errors should not be retried. (Non-errors will fail the + // for loop test and fall through naturally) + if (non_retriable_error) break; } if (res != CURLE_OK || response_code >= 500) { @@ -1120,7 +1129,10 @@ static int dav_mkdir(const char *path, mode_t mode) { if (slist) curl_slist_free_all(slist); - process_status(funcname, session, res, response_code, elapsed_time, idx, path, false); + bool non_retriable_error = process_status(funcname, session, res, response_code, elapsed_time, idx, path, false); + // Some errors should not be retried. (Non-errors will fail the + // for loop test and fall through naturally) + if (non_retriable_error) break; } if (res != CURLE_OK || response_code >= 500) { @@ -1222,7 +1234,10 @@ static int dav_rename(const char *from, const char *to) { if (slist) curl_slist_free_all(slist); - process_status(funcname, session, res, response_code, elapsed_time, idx, to, false); + bool non_retriable_error = process_status(funcname, session, res, response_code, elapsed_time, idx, to, false); + // Some errors should not be retried. (Non-errors will fail the + // for loop test and fall through naturally) + if (non_retriable_error) break; } /* move: diff --git a/src/log.c b/src/log.c index ac359f00..6dd6d3b4 100644 --- a/src/log.c +++ b/src/log.c @@ -169,7 +169,7 @@ int logging(unsigned int log_level, unsigned int section) { static int print_it(const char const *formatwithtid, const char const *msg, int log_level) { int ret; - // fusedav-valhalla standardizing on names BINDING, SITE, and ENVIRONMENT + // fusedav-server standardizing on names BINDING, SITE, and ENVIRONMENT ret = sd_journal_send("MESSAGE=%s%s", formatwithtid, msg, "PRIORITY=%d", log_level, "USER_AGENT=%s", get_user_agent(), diff --git a/src/props.c b/src/props.c index 178ff404..f9f116db 100644 --- a/src/props.c +++ b/src/props.c @@ -451,7 +451,10 @@ int simple_propfind(const char *path, size_t depth, time_t last_updated, props_r free(query_string); query_string = NULL; - process_status(funcname, session, res, response_code, elapsed_time, idx, path, false); + bool non_retriable_error = process_status(funcname, session, res, response_code, elapsed_time, idx, path, false); + // Some errors should not be retried. (Non-errors will fail the + // for loop test and fall through naturally) + if (non_retriable_error) break; } if (res != CURLE_OK || response_code >= 500 || inject_error(props_error_spropfindcurl)) { diff --git a/src/session.c b/src/session.c index cb5dd0ad..220f9ff9 100644 --- a/src/session.c +++ b/src/session.c @@ -1006,27 +1006,44 @@ static void increment_node_failure(char *addr, const CURLcode res, const long re healthstatus->timestamp = time(NULL); // Most recent failure. We don't currently use this value, but it might be interesting } -void process_status(const char *fcn_name, CURL *session, const CURLcode res, +bool process_status(const char *fcn_name, CURL *session, const CURLcode res, const long response_code, const long elapsed_time, const int iter, const char *path, bool tmp_session) { + bool non_retriable_error = false; // default to retry + stats_counter("attempts", 1); if (res != CURLE_OK) { - print_errors(iter, "curl_failures", fcn_name, res, response_code, elapsed_time, path); - increment_node_failure(nodeaddr, res, response_code, elapsed_time); - delete_session(session, tmp_session); - return; + // Treat some curl failures differently than others + if (res == CURLE_PARTIAL_FILE) { + non_retriable_error = true; + print_errors(iter, "partial-file-transfer", fcn_name, res, response_code, elapsed_time, path); + // This error is not likely a reflection of the status of the node, + // but rather specific to the file being accessed. (We think it likely + // signals that the file was transferred to server but not to permanent storage.) + // So, don't retry, it's pretty certain it will have the same result. + } else { + print_errors(iter, "curl_failures", fcn_name, res, response_code, elapsed_time, path); + increment_node_failure(nodeaddr, res, response_code, elapsed_time); + delete_session(session, tmp_session); + } + return non_retriable_error; } if (response_code >= 500) { + // We could treat 50x errors differently here print_errors(iter, "status500_failures", fcn_name, res, response_code, elapsed_time, path); increment_node_failure(nodeaddr, res, response_code, elapsed_time); delete_session(session, tmp_session); - return; + return non_retriable_error; } + // We mark a node unhealthy if it takes longer than time_limit to respond, but since + // it is neither a curl error, nor a response_code >= 500 error, it won't retry + // anyway. For clarity, mark it non-retriable explicitly if (elapsed_time > time_limit) { + non_retriable_error = true; print_errors(iter, "slow_requests", fcn_name, res, response_code, elapsed_time, path); increment_node_failure(nodeaddr, res, response_code, elapsed_time); if (health_status_all_nodes() == UNHEALTHY) { @@ -1034,7 +1051,7 @@ void process_status(const char *fcn_name, CURL *session, const CURLcode res, set_dynamic_logging(); } delete_session(session, tmp_session); - return; + return non_retriable_error; } // If it wasn't an error, and it isn't the 0'th iter, then we must have failed previously and now recovered @@ -1044,6 +1061,8 @@ void process_status(const char *fcn_name, CURL *session, const CURLcode res, "%s: curl iter %d on path %s -- fusedav.%s.server-%s.recoveries", fcn_name, iter, path, filesystem_cluster, nodeaddr); increment_node_success(nodeaddr); } + + return non_retriable_error; } static bool valid_slist(void) { diff --git a/src/session.h b/src/session.h index 0062775e..0e9745ab 100644 --- a/src/session.h +++ b/src/session.h @@ -27,7 +27,7 @@ extern int num_filesystem_server_nodes; int session_config_init(char *base, char *ca_cert, char *client_cert, bool grace); CURL *session_request_init(const char *path, const char *query_string, bool temporary_handle); void session_config_free(void); -void process_status(const char *fcn_name, CURL *session, const CURLcode res, +bool process_status(const char *fcn_name, CURL *session, const CURLcode res, const long response_code, const long elapsed_time, const int iter, const char *path, bool tmp_session); const char *get_base_url(void); diff --git a/tests/server-tests/api_test.go b/tests/server-tests/api_test.go index e891c29c..c42fe558 100644 --- a/tests/server-tests/api_test.go +++ b/tests/server-tests/api_test.go @@ -120,19 +120,6 @@ func testMethod(t *testing.T, testInput TestInput) error { } } - if testInput.method == "GET" { // Are there other methods which get content. We could genericize - if err != nil { - t.Errorf("testMethod: Error on ioutil.ReadAll: %v", err) - return err - } - - if testInput.expectedStatusCode != 404 && - len(testInput.content) > 0 && - string(body) != testInput.content { - t.Errorf("testMethod: Error, expected content %v, got %v", testInput.content, string(body)) - } - } - testOutput.status = resp.Status testOutput.statusCode = resp.StatusCode