aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorKaren Arutyunov <karen@codesynthesis.com>2023-04-26 21:34:12 +0300
committerKaren Arutyunov <karen@codesynthesis.com>2023-05-17 19:00:24 +0300
commit2fca6d23f87304ceed78e93d2a52d137c5ffd0c7 (patch)
treec67fb039261b9417ca78b318104e8ada9f87f530
parenta8f7447cf43184160ade0de01199462c11f3c109 (diff)
Add support for build artifacts upload in agent
-rw-r--r--bbot/agent/agent.cxx438
-rw-r--r--bbot/agent/http-service.cxx465
-rw-r--r--bbot/agent/http-service.hxx71
-rw-r--r--bbot/utility.hxx11
-rw-r--r--bbot/worker/worker.cxx1
-rwxr-xr-xdoc/cli.sh1
-rw-r--r--doc/manual.cli37
-rw-r--r--tests/integration/testscript2
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>
diff --git a/doc/cli.sh b/doc/cli.sh
index 22fe0c2..404e4d6 100755
--- a/doc/cli.sh
+++ b/doc/cli.sh
@@ -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
: