diff options
| -rw-r--r-- | bbot/agent/agent.cxx | 438 | ||||
| -rw-r--r-- | bbot/agent/http-service.cxx | 465 | ||||
| -rw-r--r-- | bbot/agent/http-service.hxx | 71 | ||||
| -rw-r--r-- | bbot/utility.hxx | 11 | ||||
| -rw-r--r-- | bbot/worker/worker.cxx | 1 | ||||
| -rwxr-xr-x | doc/cli.sh | 1 | ||||
| -rw-r--r-- | doc/manual.cli | 37 | ||||
| -rw-r--r-- | tests/integration/testscript | 2 |
8 files changed, 883 insertions, 143 deletions
diff --git a/bbot/agent/agent.cxx b/bbot/agent/agent.cxx index 6163c14..e56f742 100644 --- a/bbot/agent/agent.cxx +++ b/bbot/agent/agent.cxx @@ -31,6 +31,7 @@ #include <system_error> // generic_category() #include <libbutl/pager.hxx> +#include <libbutl/base64.hxx> #include <libbutl/sha256.hxx> #include <libbutl/openssl.hxx> #include <libbutl/filesystem.hxx> // dir_iterator, try_rmfile(), readsymlink() @@ -47,6 +48,7 @@ #include <bbot/agent/tftp.hxx> #include <bbot/agent/machine.hxx> +#include <bbot/agent/http-service.hxx> using namespace butl; using namespace bbot; @@ -1249,7 +1251,35 @@ catch (const system_error& e) // struct interrupt {}; -static result_manifest +struct perform_task_result +{ + auto_rmdir work_dir; // <tftp>/build/<toolchain>-<instance>/ + result_manifest manifest; + + // Uploaded archive, if any (somewhere inside work_dir). + // + optional<path> upload_archive; + + // Create the special empty result. + // + perform_task_result () = default; + + // Create task result without build artifacts. + // + explicit + perform_task_result (auto_rmdir&& d, result_manifest&& m) + : work_dir (move (d)), + manifest (move (m)) {} + + // Create task result with build artifacts. + // + perform_task_result (auto_rmdir&& d, result_manifest&& m, path&& a) + : work_dir (move (d)), + manifest (move (m)), + upload_archive (move (a)) {} +}; + +static perform_task_result perform_task (toolchain_lock tl, // Note: assumes ownership. machine_lock& ml, const dir_path& md, @@ -1267,6 +1297,11 @@ try sigurs1.store (0, std::memory_order_release); tl.unlock (); + const string in_name (tc_name + '-' + to_string (inst)); + auto_rmdir arm ((dir_path (ops.tftp ()) /= "build") /= in_name); + + try_mkdir_p (arm.path); + result_manifest r { tm.name, tm.version, @@ -1276,7 +1311,7 @@ try nullopt /* dependency_checksum */}; if (ops.fake_build ()) - return r; + return perform_task_result (move (arm), move (r)); // The overall plan is as follows: // @@ -1295,12 +1330,9 @@ try // TFTP server mapping (server chroot is --tftp): // - // GET requests to .../build/<name>-<instance>/get/* - // PUT requests to .../build/<name>-<instance>/put/* + // GET requests to .../build/<toolchain>-<instance>/get/* + // PUT requests to .../build/<toolchain>-<instance>/put/* // - const string in_name (tc_name + '-' + to_string (inst)); - auto_rmdir arm ((dir_path (ops.tftp ()) /= "build") /= in_name); - dir_path gd (dir_path (arm.path) /= "get"); dir_path pd (dir_path (arm.path) /= "put"); @@ -1334,43 +1366,11 @@ try } r = parse_manifest<result_manifest> (rf, "result"); - - // If archive of build artifacts is present, then just list its content as - // a sanity check. - // - bool err (!r.status); - if (!err && file_exists (af)) - { - try - { - auto_fd null (fdopen_null ()); - - // Redirect stdout to stderr if the command is traced and to /dev/null - // otherwise. - // - process_exit pe ( - process_run_callback ( - trace, - null.get (), // Don't expect to read from stdin. - verb >= 3 ? 2 : null.get (), - 2, - "tar", - "-tf", af)); - - if (!pe) - fail << "tar " << pe; - } - catch (const process_error& e) - { - fail << "unable execute tar: " << e; - } - } } else { try_rmfile (rf); try_rmfile (af); - try_rmdir_r (pd / dir_path ("upload")); // <name>-<toolchain>-<xxx> // @@ -1418,7 +1418,7 @@ try } })); - auto soft_fail = [&trace, &ml, &xp, &m, &r] (const char* msg) + auto soft_fail = [&trace, &ml, &xp, &m, &arm, &r] (const char* msg) { { diag_record dr (error); @@ -1441,7 +1441,7 @@ try } catch (const failed&) {} - return r; + return perform_task_result (move (arm), move (r)); }; auto check_machine = [&xp, &m] () @@ -1501,7 +1501,7 @@ try if (!check_machine ()) { run_btrfs (trace, "subvolume", "delete", xp); - return r; + return perform_task_result (move (arm), move (r)); } } @@ -1541,7 +1541,7 @@ try if (!file_not_empty (rf)) { run_btrfs (trace, "subvolume", "delete", xp); - return r; + return perform_task_result (move (arm), move (r)); } } @@ -1558,81 +1558,14 @@ try // Parse the result manifest. // - optional<result_manifest> rm; - try { - rm = parse_manifest<result_manifest> (rf, "result", false); + r = parse_manifest<result_manifest> (rf, "result", false); } catch (const failed&) { r.status = result_status::abnormal; // Soft-fail below. } - - // Upload the build artifacts if the result manifest is parsed - // successfully, the result status is not an error, and upload.tar - // exists. - // - // Note that while the worker doesn't upload the build artifacts - // archives on errors, there can be the case when the error occurred - // while uploading the archive and so the partially uploaded file - // may exist. Thus, we check if the result status is not an error. - // - // Note also that we will not bother with interrupting this process - // assuming it will be quick (relative to the amount of work that - // would be wasted). - // - bool err (!rm || !rm->status); - if (!err && file_exists (af)) - { - // Extract the build artifacts from the archive and upload them to - // the controller. On error keep the result status as abort for - // transient errors (network failure, etc) and set it to abnormal - // otherwise (for subsequent machine suspension and - // investigation). - // - optional<bool> err; // True if the error is transient. - - try - { - process_exit pe ( - process_run_callback ( - trace, - fdopen_null (), // Don't expect to read from stdin. - 2, // Redirect stdout to stderr. - 2, - "tar", - "-xf", af, - "-C", pd)); - - if (!pe) - { - err = false; - error << "tar " << pe; - } - } - catch (const process_error& e) - { - err = false; - error << "unable execute tar: " << e; - } - - if (!err) - { - // @@ Upload the extracted artifacts. - } - - if (err) - { - if (!*err) // Non-transient? - r.status = result_status::abnormal; // Soft-fail below. - - rm = nullopt; // Drop the parsed manifest. - } - } - - if (rm) - r = move (*rm); } else { @@ -1681,7 +1614,9 @@ try r.version = tm.version; } - return r; + return (!r.status || !file_exists (af) + ? perform_task_result (move (arm), move (r)) + : perform_task_result (move (arm), move (r), move (af))); } catch (const system_error& e) { @@ -2206,9 +2141,10 @@ try auto t (parse_manifest<task_manifest> (ops.fake_request (), "task")); tr = task_response_manifest { - "fake-session", // Dummy session. - nullopt, // No challenge. - string (), // Empty result URL. + "fake-session", // Dummy session. + nullopt, // No challenge. + string (), // Empty result URL. + vector<upload_url> (), agent_checksum, move (t)}; @@ -2381,7 +2317,7 @@ try // that we have already received a task, responding with an interrupt // feels like the most sensible option). // - result_manifest r; + perform_task_result r; bootstrapped_machine* pm (nullptr); try { @@ -2592,21 +2528,27 @@ try } catch (const interrupt&) { - r = result_manifest { - t.name, - t.version, - result_status::interrupt, - operation_results {}, - nullopt /* worker_checksum */, - nullopt /* dependency_checksum */}; + // Note: no work_dir. + // + r = perform_task_result ( + auto_rmdir (), + result_manifest { + t.name, + t.version, + result_status::interrupt, + operation_results {}, + nullopt /* worker_checksum */, + nullopt /* dependency_checksum */}); } if (pm != nullptr && pm->lock.locked ()) pm->lock.unlock (); // No need to hold the lock any longer. + result_manifest& rm (r.manifest); + if (ops.dump_result ()) { - serialize_manifest (r, cout, "stdout", "result"); + serialize_manifest (rm, cout, "stdout", "result"); return 0; } @@ -2642,14 +2584,258 @@ try fail << "unable to sign task response challenge: " << e; } - result_status rs (r.status); + // Re-package the build artifacts, if present, into the type/instance- + // specific archives and upload them to the type-specific URLs, if + // provided. + // + // Note that the initial upload archive content is organized as a bunch of + // upload/<type>/<instance>/*, where the second level directories are the + // upload types and the third level sub-directories are their instances. + // The resulting <instance>.tar archives content (which is what we submit + // to the type-specific handler) are organized as <instance>/*. + // + if (r.upload_archive && !tr.upload_urls.empty ()) + { + const path& ua (*r.upload_archive); + + // Extract the archive content into the parent directory of the archive + // file. But first, make sure the resulting directory doesn't exist. + // + // Note that while we don't assume where inside the working directory + // the archive is, we do assume that there is nothing clashing/precious + // in the upload/ directory which we are going to cleanup. + // + dir_path d (ua.directory ()); + + const dir_path ud (d / dir_path ("upload")); + try_rmdir_r (ud); + + try + { + process_exit pe ( + process_run_callback ( + trace, + fdopen_null (), // Don't expect to read from stdin. + 2, // Redirect stdout to stderr. + 2, + "tar", + "-xf", ua, + "-C", d)); + + if (!pe) + fail << "tar " << pe; + } + catch (const system_error& e) + { + // There must be something wrong with the setup or there is no space + // left on the disk, thus the failure is fatal. + // + fail << "unable to extract build artifacts from archive: " << e; + } + + try_rmfile (ua); // Let's free up the disk space. + + // To decrease nesting a bit, let's collect the type-specific upload + // directories and the corresponding URLs first. This way we can also + // create the archive files as the upload/ directory sub-entries without + // interfering with iterating over this directory. + // + vector<pair<dir_path, string>> urls; + + try + { + for (const dir_entry& te: dir_iterator (ud, dir_iterator::no_follow)) + { + const string& t (te.path ().string ()); + + // Can only be a result of the worker malfunction, thus the failure + // is fatal. + // + if (te.type () != entry_type::directory) + fail << "unexpected filesystem entry '" << t << "' in " << ud; + + auto i (find_if (tr.upload_urls.begin (), tr.upload_urls.end (), + [&t] (const upload_url& u) {return u.type == t;})); + + if (i == tr.upload_urls.end ()) + continue; + + urls.emplace_back (ud / path_cast<dir_path> (te.path ()), i->url); + } + } + catch (const system_error& e) + { + fail << "unable to iterate over " << ud << ": " << e; + } + + // Now create archives and upload. + // + for (const pair<dir_path, string>& p: urls) + { + const dir_path& td (p.first); // <type>/ + const string& url (p.second); + + try + { + for (const dir_entry& ie: dir_iterator (td, dir_iterator::no_follow)) + { + const string& i (ie.path ().string ()); // <instance> + + // Can only be a result of the worker malfunction, thus the + // failure is fatal. + // + if (ie.type () != entry_type::directory) + fail << "unexpected filesystem entry '" << i << "' in " << td; + + // Archive the upload instance files and, while at it, calculate + // the resulting archive checksum. + // + sha256 sha; + auto_rmfile ari (ud / (i + ".tar")); + + try + { + // Instruct tar to print the archive to stdout. + // + fdpipe in_pipe (fdopen_pipe (fdopen_mode::binary)); + + process pr ( + process_start_callback ( + trace, + fdopen_null (), // Don't expect to read from stdin. + in_pipe, + 2 /* stderr */, + "tar", + "--format", "ustar", + "-c", + "-C", td, + i)); + + // Shouldn't throw, unless something is severely damaged. + // + in_pipe.out.close (); + + ifdstream is ( + move (in_pipe.in), fdstream_mode::skip, ifdstream::badbit); + + ofdstream os (ari.path, fdopen_mode::binary); + + char buf[8192]; + while (!eof (is)) + { + is.read (buf, sizeof (buf)); + + if (size_t n = static_cast<size_t> (is.gcount ())) + { + sha.append (buf, n); + os.write (buf, n); + } + } + + os.close (); + + if (!pr.wait ()) + fail << "tar " << *pr.exit; + } + catch (const system_error& e) + { + // There must be something wrong with the setup or there is no + // space left on the disk, thus the failure is fatal. + // + fail << "unable to archive " << td << i << "/: " << e; + } + + // Post the upload instance archive. + // + using namespace http_service; + + parameters params ({ + {parameter::text, "session", tr.session}, + {parameter::text, "instance", i}, + {parameter::file, "archive", ari.path.string ()}, + {parameter::text, "sha256sum", sha.string ()}}); + + if (challenge) + params.push_back ({ + parameter::text, "challenge", base64_encode (*challenge)}); + + result pr (post (ops, url, params)); + + // Turn the potential upload failure into the "upload" operation + // error, amending the task result manifest. + // + if (pr.error) + { + // The "upload" operation result must be present (otherwise + // there would be nothing to upload). We can assume it is last. + // + assert (!rm.results.empty ()); + + operation_result& r (rm.results.back ()); + + // The "upload" operation result must be the last, if present. + // + assert (r.operation == "upload"); + + auto log = [&r, indent = false] (const string& t, + const string& l) mutable + { + if (indent) + r.log += " "; + else + indent = true; + + r.log += t; + r.log += ": "; + r.log += l; + r.log += '\n'; + }; + + log ("error", + "unable to upload " + td.leaf ().string () + '/' + i + + " build artifacts"); + + log ("error", *pr.error); + + if (!pr.message.empty ()) + log ("reason", pr.message); + + if (pr.reference) + log ("reference", *pr.reference); + + for (const manifest_name_value& nv: pr.body) + { + if (!nv.name.empty ()) + log (nv.name, nv.value); + } + + r.status |= result_status::error; + rm.status |= r.status; + + break; + } + } + + // Bail out on the instance archive upload failure. + // + if (!rm.status) + break; + } + catch (const system_error& e) + { + fail << "unable to iterate over " << td << ": " << e; + } + } + } + + result_status rs (rm.status); // Upload the result. // result_request_manifest rq {tr.session, move (challenge), agent_checksum, - move (r)}; + move (rm)}; { const string& u (*tr.result_url); diff --git a/bbot/agent/http-service.cxx b/bbot/agent/http-service.cxx new file mode 100644 index 0000000..28b4d94 --- /dev/null +++ b/bbot/agent/http-service.cxx @@ -0,0 +1,465 @@ +// file : bbot/agent/http-service.cxx -*- C++ -*- +// license : MIT; see accompanying LICENSE file + +#include <bbot/agent/http-service.hxx> + +#include <cstdlib> // strtoul() + +#include <bbot/diagnostics.hxx> + +using namespace std; +using namespace butl; + +namespace bbot +{ + namespace http_service + { + result + post (const agent_options& o, const string& u, const parameters& params) + { + tracer trace ("http_service::post"); + + using parser = manifest_parser; + using parsing = manifest_parsing; + using name_value = manifest_name_value; + + // The overall plan is to post the data using the curl program, read + // the HTTP response status and content type, read and parse the body + // according to the content type, and obtain the result message and + // optional reference in case of both the request success and failure. + // + // The successful request response (HTTP status code 200) is expected to + // contain the result manifest (text/manifest content type). The faulty + // response (HTTP status code other than 200) can either contain the + // result manifest or a plain text error description (text/plain content + // type) or some other content (for example text/html). We will return + // the manifest message value, if available or the first line of the + // plain text error description or, as a last resort, construct the + // message from the HTTP status code and reason phrase. We will also + // return the error description if anything goes wrong with the HTTP + // request or the response manifest status value is not 200. + // + string message; + optional<uint16_t> status; // Request result manifest status value. + optional<string> reference; + vector<name_value> body; + optional<string> error; + + // None of the 3XX redirect code semantics assume automatic re-posting. + // We will treat all such codes as failures, adding the location header + // value to the message for troubleshooting. + // + optional<string> location; + + // Convert the submit arguments to curl's --form* options and cache the + // pointer to the file_text parameter value, if present, for writing + // into curl's stdin. + // + strings fos; + const string* file_text (nullptr); + + for (const parameter& p: params) + { + if (p.type == parameter::file_text) + { + assert (file_text == nullptr); + file_text = &p.value; + } + + fos.emplace_back (p.type == parameter::file || + p.type == parameter::file_text + ? "--form" + : "--form-string"); + + fos.emplace_back ( + p.type == parameter::file ? p.name + "=@" + p.value : + p.type == parameter::file_text ? p.name + "=@-" : + p.name + '=' + p.value); + } + + // Note that we prefer the low-level process API for running curl over + // using butl::curl because in this context it is restrictive and + // inconvenient. + // + // Start curl program. + // + // Text mode seems appropriate. + // + fdpipe in_pipe; + fdpipe out_pipe; + process pr; + + try + { + in_pipe = fdopen_pipe (); + + out_pipe = (file_text != nullptr + ? fdopen_pipe () + : fdpipe {fdopen_null (), nullfd}); + + pr = process_start_callback (trace, + out_pipe.in.get () /* stdin */, + in_pipe /* stdout */, + 2 /* stderr */, + "curl", + + // Include the response headers in the + // output so we can get the status + // code/reason, content type, and the + // redirect location. + // + "--include", + + "--max-time", o.request_timeout (), + "--connect-timeout", o.connect_timeout (), + fos, + u); + + // Shouldn't throw, unless something is severely damaged. + // + in_pipe.out.close (); + out_pipe.in.close (); + } + catch (const process_error& e) + { + fail << "unable to execute curl: " << e; + } + catch (const io_error& e) + { + fail << "unable to open pipe: " << e; + } + + auto finish = [&pr, &error] (bool io_read = false, bool io_write = false) + { + if (!pr.wait ()) + error = "curl " + to_string (*pr.exit); + else if (io_read) + error = "error reading curl output"; + else if (io_write) + error = "error writing curl input"; + }; + + bool io_write (false); + bool io_read (false); + + try + { + // First we read the HTTP response status line and headers. At this + // stage we will read until the empty line (containing just CRLF). Not + // being able to reach such a line is an error, which is the reason + // for the exception mask choice. + // + ifdstream is ( + move (in_pipe.in), + fdstream_mode::skip, + ifdstream::badbit | ifdstream::failbit | ifdstream::eofbit); + + if (file_text != nullptr) + { + ofdstream os (move (out_pipe.out)); + os << *file_text; + os.close (); + + // Indicate to the potential IO error handling that we are done with + // writing. + // + file_text = nullptr; + } + + // Parse and return the HTTP status code. Return 0 if the argument is + // invalid. + // + auto status_code = [] (const string& s) + { + char* e (nullptr); + unsigned long c (strtoul (s.c_str (), &e, 10)); // Can't throw. + assert (e != nullptr); + + return *e == '\0' && c >= 100 && c < 600 + ? static_cast<uint16_t> (c) + : 0; + }; + + // Read the CRLF-terminated line from the stream stripping the + // trailing CRLF. + // + auto read_line = [&is] () + { + string l; + getline (is, l); // Strips the trailing LF (0xA). + + // Note that on POSIX CRLF is not automatically translated into + // LF, so we need to strip CR (0xD) manually. + // + if (!l.empty () && l.back () == '\r') + l.pop_back (); + + return l; + }; + + auto bad_response = [] (const string& d) {throw runtime_error (d);}; + + // Read and parse the HTTP response status line, return the status + // code and the reason phrase. + // + struct http_status + { + uint16_t code; + string reason; + }; + + auto read_status = [&read_line, &status_code, &bad_response] () + { + string l (read_line ()); + + for (;;) // Breakout loop. + { + if (l.compare (0, 5, "HTTP/") != 0) + break; + + size_t p (l.find (' ', 5)); // The protocol end. + if (p == string::npos) + break; + + p = l.find_first_not_of (' ', p + 1); // The code start. + if (p == string::npos) + break; + + size_t e (l.find (' ', p + 1)); // The code end. + if (e == string::npos) + break; + + uint16_t c (status_code (string (l, p, e - p))); + if (c == 0) + break; + + string r; + p = l.find_first_not_of (' ', e + 1); // The reason start. + if (p != string::npos) + { + e = l.find_last_not_of (' '); // The reason end. + assert (e != string::npos && e >= p); + + r = string (l, p, e - p + 1); + } + + return http_status {c, move (r)}; + } + + bad_response ("invalid HTTP response status line '" + l + '\''); + + assert (false); // Can't be here. + return http_status {}; + }; + + // The curl output for a successfull request looks like this: + // + // HTTP/1.1 100 Continue + // + // HTTP/1.1 200 OK + // Content-Length: 83 + // Content-Type: text/manifest;charset=utf-8 + // + // : 1 + // status: 200 + // message: submission is queued + // reference: 256910ca46d5 + // + // curl normally sends the 'Expect: 100-continue' header for uploads, + // so we need to handle the interim HTTP server response with the + // continue (100) status code. + // + // Interestingly, Apache can respond with the continue (100) code and + // with the not found (404) code afterwords. Can it be configured to + // just respond with 404? + // + http_status rs (read_status ()); + + if (rs.code == 100) + { + while (!read_line ().empty ()) ; // Skips the interim response. + rs = read_status (); // Reads the final status code. + } + + // Read through the response headers until the empty line is + // encountered and obtain the content type and/or the redirect + // location, if present. + // + optional<string> ctype; + + // Check if the line contains the specified header and return its + // value if that's the case. Return nullopt otherwise. + // + // Note that we don't expect the header values that we are interested + // in to span over multiple lines. + // + string l; + auto header = [&l] (const char* name) -> optional<string> + { + size_t n (string::traits_type::length (name)); + if (!(icasecmp (name, l, n) == 0 && l[n] == ':')) + return nullopt; + + string r; + size_t p (l.find_first_not_of (' ', n + 1)); // The value begin. + if (p != string::npos) + { + size_t e (l.find_last_not_of (' ')); // The value end. + assert (e != string::npos && e >= p); + + r = string (l, p, e - p + 1); + } + + return optional<string> (move (r)); + }; + + while (!(l = read_line ()).empty ()) + { + if (optional<string> v = header ("Content-Type")) + ctype = move (v); + else if (optional<string> v = header ("Location")) + { + if ((rs.code >= 301 && rs.code <= 303) || rs.code == 307) + location = move (v); + } + } + + assert (!eof (is)); // Would have already failed otherwise. + + // Now parse the response payload if the content type is specified and + // is recognized (text/manifest or text/plain), skip it (with the + // ifdstream's close() function) otherwise. + // + // Note that eof and getline() fail conditions are not errors anymore, + // so we adjust the exception mask accordingly. + // + is.exceptions (ifdstream::badbit); + + if (ctype) + { + if (icasecmp ("text/manifest", *ctype, 13) == 0) + { + parser p (is, "manifest"); + name_value nv (p.next ()); + + if (nv.empty ()) + bad_response ("empty manifest"); + + const string& n (nv.name); + string& v (nv.value); + + // The format version pair is verified by the parser. + // + assert (n.empty () && v == "1"); + + body.push_back (move (nv)); // Save the format version pair. + + auto bad_value = [&p, &nv] (const string& d) { + throw parsing (p.name (), nv.value_line, nv.value_column, d);}; + + // Get and verify the HTTP status. + // + nv = p.next (); + if (n != "status") + bad_value ("no status specified"); + + uint16_t c (status_code (v)); + if (c == 0) + bad_value ("invalid HTTP status '" + v + '\''); + + if (c != rs.code) + bad_value ("status " + v + " doesn't match HTTP response " + "code " + to_string (rs.code)); + + // Get the message. + // + nv = p.next (); + if (n != "message" || v.empty ()) + bad_value ("no message specified"); + + message = move (v); + + // Try to get an optional reference. + // + nv = p.next (); + + if (n == "reference") + { + if (v.empty ()) + bad_value ("empty reference specified"); + + reference = move (v); + + nv = p.next (); + } + + // Save the remaining name/value pairs. + // + for (; !nv.empty (); nv = p.next ()) + body.push_back (move (nv)); + + status = c; + } + else if (icasecmp ("text/plain", *ctype, 10) == 0) + getline (is, message); // Can result in the empty message. + } + + is.close (); // Detect errors. + + // The only meaningful result we expect is the manifest (status code + // is not necessarily 200). We unable to interpret any other cases and + // so report them as a bad response. + // + if (!status) + { + if (rs.code == 200) + bad_response ("manifest expected"); + + if (message.empty ()) + { + message = "HTTP status code " + to_string (rs.code); + + if (!rs.reason.empty ()) + message += " (" + lcase (rs.reason) + ')'; + } + + if (location) + message += ", new location: " + *location; + + bad_response ("bad server response"); + } + } + catch (const io_error&) + { + // Presumably the child process failed and issued diagnostics so let + // finish() try to deal with that first. + // + (file_text != nullptr ? io_write : io_read) = true; + } + // Handle all parsing errors, including the manifest_parsing exception + // that inherits from the runtime_error exception. + // + // Note that the io_error class inherits from the runtime_error class, + // so this catch-clause must go last. + // + catch (const runtime_error& e) + { + finish (); // Sets the error variable on process failure. + + if (!error) + error = e.what (); + } + + if (!error) + finish (io_read, io_write); + + assert (error || (status && !message.empty ())); + + if (!error && *status != 200) + error = "status code " + to_string (*status); + + return result { + move (error), move (message), move (reference), move (body)}; + } + } +} diff --git a/bbot/agent/http-service.hxx b/bbot/agent/http-service.hxx new file mode 100644 index 0000000..b50c6b7 --- /dev/null +++ b/bbot/agent/http-service.hxx @@ -0,0 +1,71 @@ +// file : bbot/agent/http-service.hxx -*- C++ -*- +// license : MIT; see accompanying LICENSE file + +#ifndef BBOT_AGENT_HTTP_SERVICE_HXX +#define BBOT_AGENT_HTTP_SERVICE_HXX + +#include <libbutl/manifest-types.hxx> + +#include <bbot/types.hxx> +#include <bbot/utility.hxx> + +#include <bbot/agent/agent-options.hxx> + +// NOTE: this implementation is inspired by the bdep's http_service::post() +// function. The key difference is the result::error member which is used +// to return rather than fail on upload errors. + +namespace bbot +{ + namespace http_service + { + // If type is file, then the value is a path to be uploaded. If type is + // file_text, then the value is a file content to be uploaded. + // + struct parameter + { + enum {text, file, file_text} type; + string name; + string value; + }; + using parameters = vector<parameter>; + + struct result + { + // If error is present, then it contains the description of why the + // upload failed. In this case message contains additional information. + // + optional<string> error; + string message; + optional<string> reference; + + // Does not include status, message, or reference. + // + vector<butl::manifest_name_value> body; + }; + + // Submit text parameters and/or upload files to an HTTP service via the + // POST method. Use the multipart/form-data content type if any files are + // uploaded and application/x-www-form-urlencoded otherwise. + // + // Note: currently only one file_text parameter can be specified. + // + // Return the response manifest message and reference (if present, see + // below) and the rest of the manifest values, if any. If unable to + // retrieve the response manifest, the message can also be set to the + // first line of the plain text error description or, as a last resort, + // constructed from the HTTP status code and reason phrase. Issue + // diagnostics and throw failed if something is wrong with the setup + // (unable to execute curl, etc). + // + // Note that the HTTP service is expected to respond with the result + // manifest that starts with the 'status' (HTTP status code) and 'message' + // (diagnostics message) values optionally followed by 'reference' and + // then other manifest values. + // + result + post (const agent_options&, const string& url, const parameters&); + } +} + +#endif // BBOT_AGENT_HTTP_SERVICE_HXX diff --git a/bbot/utility.hxx b/bbot/utility.hxx index 35136bd..9bc517c 100644 --- a/bbot/utility.hxx +++ b/bbot/utility.hxx @@ -4,11 +4,12 @@ #ifndef BBOT_UTILITY_HXX #define BBOT_UTILITY_HXX -#include <memory> // make_shared() -#include <string> // to_string(), stoull() -#include <utility> // move(), forward(), declval(), make_pair() -#include <cassert> // assert() -#include <iterator> // make_move_iterator() +#include <memory> // make_shared() +#include <string> // to_string(), stoull() +#include <utility> // move(), forward(), declval(), make_pair() +#include <cassert> // assert() +#include <iterator> // make_move_iterator() +#include <algorithm> // * #include <libbutl/ft/lang.hxx> diff --git a/bbot/worker/worker.cxx b/bbot/worker/worker.cxx index 4895c76..c2ee4ba 100644 --- a/bbot/worker/worker.cxx +++ b/bbot/worker/worker.cxx @@ -12,7 +12,6 @@ #include <cstring> // strchr(), strncmp() #include <sstream> #include <iostream> -#include <algorithm> // find(), find_if(), remove_if() #include <libbutl/b.hxx> #include <libbutl/pager.hxx> @@ -96,6 +96,7 @@ function compile_doc () # <file> <prefix> <suffix> --html-epilogue-file doc-epilogue.xhtml \ --link-regex '%bpkg([-.].+)%../../bpkg/doc/bpkg$1%' \ --link-regex '%bpkg(#.+)?%../../bpkg/doc/build2-package-manager-manual.xhtml$1%' \ +--link-regex '%brep(#.+)?%../../brep/doc/build2-repository-interface-manual.xhtml$1%' \ --output-prefix "$2" \ --output-suffix "$3" \ "$1" diff --git a/doc/manual.cli b/doc/manual.cli index eb48a31..e61e6ef 100644 --- a/doc/manual.cli +++ b/doc/manual.cli @@ -45,8 +45,8 @@ Let's now examine the workflow in the other direction, that is, from a worker to a controller. Once a build machine is booted (by the agent), the worker inside connects to the TFTP server running on the build host and downloads the \i{build task manifest}. It then proceeds to perform the build task and -uploads the \i{build result manifest} (which includes build logs) to the TFTP -server. +uploads the \i{build artifacts archive}, if any, followed by the \i{build +result manifest} (which includes build logs) to the TFTP server. Once an agent receives a build task for a specific build machine, it goes through the following steps. First, it creates a directory on its TFTP server @@ -79,14 +79,18 @@ agents. In this case we say that the \i{controller act as an agent}. The controller may also be configured to monitor build sources, such as SCM repositories, directly in which case it generates build tasks itself. -In this architecture the build results are propagated up the chain: from a -worker, to its agent, to its controller, and so on. A controller that is the -final destination of a build result uses email to notify interested parties of -the outcome. For example, \c{brep} would send a notification to the package -owner if the build failed. Similarly, a \c{bbot} controller that monitors a -\cb{git} repository would send an email to a committer if their commit caused a -build failure. The email would include a link (normally HTTP/HTTPS) to the -build logs hosted by the controller. +In this architecture the build results and optional build artifacts are +propagated up the chain: from a worker, to its agent, to its controller, and +so on. A controller that is the final destination of a build result uses email +to notify interested parties of the outcome. For example, \c{brep} would send +a notification to the package owner if the build failed. Similarly, a \c{bbot} +controller that monitors a \cb{git} repository would send an email to a +committer if their commit caused a build failure. The email would include a +link (normally HTTP/HTTPS) to the build logs hosted by the controller. The +build artifacts, such as generated binary distribution packages, are normally +made available for the interested parties to download. See \l{brep#upload +Build Artifacts Upload} for details on the \c{brep} controller's +implementation of the build artifacts upload handling. \h#arch-machine-config|Configurations| @@ -851,6 +855,7 @@ subsequent sections. session: <id> [challenge]: <text> [result-url]: <url> +[*-upload-url]: <url> [agent-checksum]: <checksum> \ @@ -889,6 +894,18 @@ private key and then \c{base64}-encoding the result. The URL to POST (upload) the result request to. +\h2#arch-task-res-upload-url|\c{*-upload-url}| + +\ +[*-upload-url]: <url> +\ + +The URLs to upload the build artifacts to, if any, via the HTTP \c{POST} +method using the \c{multipart/form-data} content type (see \l{brep#upload +Build Artifacts Upload} for details on the upload protocol). The substring +matched by \c{*} in \c{*-upload-url} denotes the upload type. + + \h2#arch-task-res-agent-checksum|\c{agent-checksum}| \ diff --git a/tests/integration/testscript b/tests/integration/testscript index 75b61cb..a608bef 100644 --- a/tests/integration/testscript +++ b/tests/integration/testscript @@ -417,7 +417,7 @@ end tftp = 127.0.0.1:55123 a = $0 -+ sed -e 's/-agent$/-worker/' <"$0" | set w ++sed -e 's/-agent$/-worker/' <"$0" | set w : agent : |