From 23103a6e42d2901b0f5e52d56b232368a0035f0d Mon Sep 17 00:00:00 2001 From: Karen Arutyunov Date: Thu, 20 Apr 2023 21:40:13 +0300 Subject: Add support for bbot.bindist.upload and bbot.upload steps in worker --- bbot/agent/agent.cxx | 112 +++++++++++- bbot/agent/tftp.cxx | 2 +- bbot/worker/worker.cxx | 467 ++++++++++++++++++++++++++++++++++++++++++------- 3 files changed, 509 insertions(+), 72 deletions(-) (limited to 'bbot') diff --git a/bbot/agent/agent.cxx b/bbot/agent/agent.cxx index 7dc6794..ed90da0 100644 --- a/bbot/agent/agent.cxx +++ b/bbot/agent/agent.cxx @@ -840,6 +840,7 @@ try path tf (gd / "task.manifest"); // Task manifest file. path rf (pd / "result.manifest.lz4"); // Result manifest file. + path af (pd / "upload.tar"); // Archive of build artifacts to upload. serialize_manifest (tm, tf, "task"); @@ -862,10 +863,43 @@ try } r = parse_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")); // -- // @@ -985,11 +1019,12 @@ try l3 ([&]{trace << "completed startup in " << startup_to - to << "s";}); - // Next the worker builds things and then uploads the result manifest. - // So on our side we serve TFTP requests while checking for the - // manifest file. To workaround some obscure filesystem races (the - // file's mtime/size is updated several seconds later; maybe tmpfs - // issue?), we periodically re-check. + // Next the worker builds things and then uploads optional archive of + // build artifacts and the result manifest afterwards. So on our side + // we serve TFTP requests while checking for the manifest file. To + // workaround some obscure filesystem races (the file's mtime/size is + // updated several seconds later; maybe tmpfs issue?), we periodically + // re-check. // for (to = build_to; to != 0; ) { @@ -1015,14 +1050,77 @@ try // Parse the result manifest. // + optional rm; + try { - r = parse_manifest (rf, "result", false); + rm = parse_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. + // + 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 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 { @@ -1089,7 +1187,7 @@ handle_signal (int sig) } } -static const string agent_checksum ("1"); // Logic version. +static const string agent_checksum ("2"); // Logic version. int main (int argc, char* argv[]) diff --git a/bbot/agent/tftp.cxx b/bbot/agent/tftp.cxx index b671060..58aaabc 100644 --- a/bbot/agent/tftp.cxx +++ b/bbot/agent/tftp.cxx @@ -126,7 +126,7 @@ namespace bbot ops.tftp ()); // This is not really accurate since tftpd will, for example, serve - // an upload request until it is complete. But it's close anough for + // an upload request until it is complete. But it's close enough for // our needs. // sec -= (inc - static_cast (timeout.tv_sec)); diff --git a/bbot/worker/worker.cxx b/bbot/worker/worker.cxx index aee7d3c..9d3962c 100644 --- a/bbot/worker/worker.cxx +++ b/bbot/worker/worker.cxx @@ -73,6 +73,17 @@ catch (const system_error& e) fail << "unable to stat path " << d << ": " << e << endf; } +static bool +exists (const path& f) +try +{ + return file_exists (f, true /* follow_symlinks */); +} +catch (const system_error& e) +{ + fail << "unable to stat path " << f << ": " << e << endf; +} + static dir_path current_directory () try @@ -84,35 +95,66 @@ catch (const system_error& e) fail << "unable to obtain current directory: " << e << endf; } -static dir_path -change_wd (tracer& t, string* log, const dir_path& d, bool create = false) +static void +mk_p (tracer& t, string* log, const dir_path& d) try { - if (create) - { - if (verb >= 3) - t << "mkdir -p " << d; + if (verb >= 3) + t << "mkdir -p " << d; - if (log != nullptr) - *log += "mkdir -p " + d.representation () + '\n'; + if (log != nullptr) + *log += "mkdir -p " + d.representation () + '\n'; - try_mkdir_p (d); - } + try_mkdir_p (d); +} +catch (const system_error& e) +{ + fail << "unable to create directory " << d << ": " << e << endf; +} - dir_path r (current_directory ()); +static void +mk (tracer& t, string* log, const dir_path& d) +try +{ + if (verb >= 3) + t << "mkdir " << d; + + if (log != nullptr) + *log += "mkdir " + d.representation () + '\n'; + + try_mkdir (d); +} +catch (const system_error& e) +{ + fail << "unable to create directory " << d << ": " << e << endf; +} +static bool +empty (const dir_path& d) +try +{ + return dir_empty (d); +} +catch (const system_error& e) +{ + fail << "unable to scan directory " << d << ": " << e << endf; +} + +static void +cp_into (tracer& t, string* log, const path& p, const dir_path& d) +try +{ if (verb >= 3) - t << "cd " << d; + t << "cp " << p << ' ' << d; if (log != nullptr) - *log += "cd " + d.representation () + '\n'; + *log += "cp " + p.string () + ' ' + d.representation () + '\n'; - dir_path::current_directory (d); - return r; + cpfile_into (p, d); } catch (const system_error& e) { - fail << "unable to change current directory to " << d << ": " << e << endf; + fail << "unable to copy file " << p << " into " << d << ": " << e << endf; } static void @@ -135,6 +177,25 @@ catch (const system_error& e) } static void +mv_into (tracer& t, string* log, const path& from, const dir_path& into) +try +{ + if (verb >= 3) + t << "mv " << from << ' ' << into; + + if (log != nullptr) + *log += "mv " + from.representation () + ' ' + into.representation () + + "\n"; + + mventry_into (from, into); +} +catch (const system_error& e) +{ + fail << "unable to move entry '" << from << "' into '" << into << "': " << e + << endf; +} + +static void rm_r (tracer& t, string* log, const dir_path& d) try { @@ -151,6 +212,29 @@ catch (const system_error& e) fail << "unable to remove directory " << d << ": " << e << endf; } +static dir_path +change_wd (tracer& t, string* log, const dir_path& d, bool create = false) +try +{ + if (create) + mk_p (t, log, d); + + dir_path r (current_directory ()); + + if (verb >= 3) + t << "cd " << d; + + if (log != nullptr) + *log += "cd " + d.representation () + '\n'; + + dir_path::current_directory (d); + return r; +} +catch (const system_error& e) +{ + fail << "unable to change current directory to " << d << ": " << e << endf; +} + // Step IDs. // // NOTE: keep ids ordered according to the sequence of steps and remember to @@ -272,11 +356,21 @@ enum class step_id bpkg_test_separate_installed_update, //: bpkg_update bpkg_test_separate_installed_test, //: bpkg_test - bpkg_uninstall, - bbot_sys_uninstall_apt_get_remove, bbot_sys_uninstall_dnf_remove, + bpkg_uninstall, + + bbot_bindist_upload, // Note: disabled by default, not a breakpoint. + + // Note that this step is considered disabled unless the upload/ directory + // is not empty. Note: not a breakpoint. + // + bbot_upload, + + bbot_upload_tar_create, + bbot_upload_tar_list, + end }; @@ -342,11 +436,17 @@ static const strings step_id_str { "bpkg.test-separate-installed.update", "bpkg.test-separate-installed.test", - "bpkg.uninstall", - "bbot.sys-uninstall.apt-get.remove", "bbot.sys-uninstall.dnf.remove", + "bpkg.uninstall", + + "bbot.bindist.upload", + + "bbot.upload", + "bbot.upload.tar.create", + "bbot.upload.tar.list", + "end"}; static inline const string& @@ -385,6 +485,26 @@ const char* comment_begin ("#"); const char* comment_begin ("rem"); #endif +static void +log_step_id (tracer& t, string& log, step_id id) +{ + string ts (to_string (system_clock::now (), + "%Y-%m-%d %H:%M:%S %Z", + true /* special */, + true /* local */)); + + const string& sid (to_string (id)); + + l3 ([&]{t << "step id: " << sid << ' ' << ts;}); + + log += comment_begin; + log += " step id: "; + log += sid; + log += ' '; + log += ts; + log += '\n'; +} + // Run the worker script command. Name is used for logging and diagnostics // only. Match lines read from the command's stderr against the regular // expressions and return the warning result status (instead of success) in @@ -462,28 +582,12 @@ run_cmd (step_id step, auto prompt_step = [step, &t, &log, &bkp_step, &prompt, &pre_run] () { - const string& sid (to_string (step)); - // Prompt the user if the breakpoint is reached. // if (bkp_step && *bkp_step == step) - prompt (sid + " step reached"); - - string ts (to_string (system_clock::now (), - "%Y-%m-%d %H:%M:%S %Z", - true /* special */, - true /* local */)); - - // Log the step id and the command to be executed. - // - l3 ([&]{t << "step id: " << sid << ' ' << ts;}); + prompt (to_string (step) + " step reached"); - log += comment_begin; - log += " step id: "; - log += sid; - log += ' '; - log += ts; - log += '\n'; + log_step_id (t, log, step); if (pre_run) pre_run (); @@ -502,6 +606,8 @@ run_cmd (step_id step, prompt_step (); + // Log the command to be executed. + // t (c, n); log += next_cmd; @@ -939,6 +1045,7 @@ run_dnf (step_id step, "dnf", cmd, forward (a)...); } +#ifndef _WIN32 template static result_status run_tar (step_id step, @@ -947,19 +1054,31 @@ run_tar (step_id step, const optional& bkp_step, const optional& bkp_status, string& last_cmd, + bool sudo, A&&... a) { -#ifndef _WIN32 return run_cmd (step, t, log, nullptr /* out_str */, path () /* out_file*/, warn_detect, - "sudo tar", + sudo ? "sudo tar" : "tar", bkp_step, bkp_status, last_cmd, - process_env ("sudo"), - "tar", forward (a)...); + process_env (sudo ? "sudo" : "tar"), + sudo ? "tar" : nullptr, forward (a)...); +} #else +template +static result_status +run_tar (step_id step, + tracer& t, + string& log, const regexes& warn_detect, + const optional& bkp_step, + const optional& bkp_status, + string& last_cmd, + bool /* sudo */, + A&&... a) +{ // Note: using bsdtar which can unpack .zip archives (and also not an MSYS // executable). // @@ -972,8 +1091,8 @@ run_tar (step_id step, bkp_step, bkp_status, last_cmd, process_env ("bsdtar"), forward (a)...); -#endif } +#endif // Upload compressed manifest to the specified TFTP URL with curl. Issue // diagnostics and throw failed on invalid manifest or process management @@ -1084,11 +1203,12 @@ build (size_t argc, const char* argv[]) // 1. Parse the task manifest (it should be in CWD). // // 2. Run bpkg to create the package/tests configurations, add the - // repository to them, and configure, build, test, optionally install, - // test installed and uninstall the package all while saving the logs in - // the result manifest. + // repository to them, and configure, build, test, optionally install or + // alternatively bindist and sys-install, test installed, and + // (sys-)uninstall the package all while saving the logs in the result + // manifest. // - // 3. Upload the result manifest. + // 3. Upload the result manifest and, optionally, the build artifacts. // // NOTE: consider updating worker_checksum if making any logic changes. // @@ -1125,6 +1245,10 @@ build (size_t argc, const char* argv[]) dir_path rwd; // Root working directory. + // Archive of the build artifacts for upload. + // + path upload_archive ("upload.tar"); + // Resolve the breakpoint specified by the interactive manifest value into // the step id or the result status breakpoint. If the breakpoint is // invalid, then log the error and abort the build. Note that we reuse the @@ -1274,7 +1398,9 @@ build (size_t argc, const char* argv[]) prefix != "bbot.sys-install.ldconfig" && prefix != "b.test-installed.test" && prefix != "bpkg.test-separate-installed.update" && - prefix != "bpkg.test-separate-installed.test") + prefix != "bpkg.test-separate-installed.test" && + prefix != "bbot.bindist.upload" && + prefix != "bbot.upload") { return nullopt; // Prefix is invalid. } @@ -1805,6 +1931,12 @@ build (size_t argc, const char* argv[]) rwd = current_directory (); + // Create directory for the build artifacts to archive and upload. + // + dir_path upload_dir ("upload"); + + mk (trace, nullptr /* log */, upload_dir); + // If the package comes from a version control-based repository, then we // will also test its dist meta-operation. Specifically, we will checkout // the package outside the configuration directory passing --checkout-root @@ -1943,6 +2075,7 @@ build (size_t argc, const char* argv[]) } bool sys_install (bindist && !step_disabled (step_id::bbot_sys_install)); + bool bindist_upload (bindist && step_enabled (step_id::bbot_bindist_upload)); // Unless a bpkg.bindist.* step is enabled or bpkg.install step is // disabled, search for config.install.root variable. If it is present and @@ -3683,6 +3816,12 @@ build (size_t argc, const char* argv[]) // Generate the binary distribution package. // + // Note that if bbot.bindist.upload step is enabled, it makes sense to + // only copy all the generated binary distribution files to the + // upload/bindist// directory after the binary + // distribution packages are testes, i.e. after the potential + // bbot.sys-uninstall.* steps. + // // The following bindist_* structures contain a subset of members of the // corresponding structures described in the STRUCTURED RESULT section // of the bpkg-pkg-bindist(1) man page. Note: needed later for @@ -3701,12 +3840,20 @@ build (size_t argc, const char* argv[]) struct bindist_result_type { + string distribution; bindist_package package; vector dependencies; }; bindist_result_type bindist_result; + const dir_path& bindist_conf ( + create_install ? install_conf : main_pkg_conf); + + // Make it absolute for the sake of diagnostics. + // + path bindist_result_file (rwd / bindist_conf / "bindist-result.json"); + if (bindist) { operation_result& r (add_result ("bindist")); @@ -3733,13 +3880,6 @@ build (size_t argc, const char* argv[]) if (!target_pkg && create_target) rm_r (trace, &r.log, rwd / target_conf); - const dir_path& bindist_conf ( - create_install ? install_conf : main_pkg_conf); - - // Make it absolute for the sake of diagnostics. - // - path out_file (rwd / bindist_conf / "bindist-result.json"); - string distribution; dir_path output_root; @@ -3780,7 +3920,7 @@ build (size_t argc, const char* argv[]) r.status |= run_bpkg ( b, - trace, r.log, out_file, wre, + trace, r.log, bindist_result_file, wre, bkp_step, bkp_status, last_cmd, "-v", "bindist", @@ -3803,8 +3943,8 @@ build (size_t argc, const char* argv[]) // try { - ifdstream is (out_file); - json::parser p (is, out_file.string ()); + ifdstream is (bindist_result_file); + json::parser p (is, bindist_result_file.string ()); using event = json::event; @@ -3894,11 +4034,11 @@ build (size_t argc, const char* argv[]) if (n == "distribution") { - string dist (p.next_expect_string ()); + bindist_result.distribution = p.next_expect_string (); - if (dist != distribution) + if (bindist_result.distribution != distribution) bad_json ("expected distribution '" + distribution + - "' instead of '" + dist + "'"); + "' instead of '" + bindist_result.distribution + "'"); } else if (n == "package") { @@ -3931,7 +4071,7 @@ build (size_t argc, const char* argv[]) } catch (const io_error& e) { - fail << "unable to read " << out_file << ": " << e; + fail << "unable to read " << bindist_result_file << ": " << e; } if (!r.status) @@ -4133,6 +4273,7 @@ build (size_t argc, const char* argv[]) b, trace, r.log, wre, bkp_step, bkp_status, last_cmd, + true /* sudo */, "-xf", f, step_args (env_args, s), @@ -5043,7 +5184,7 @@ build (size_t argc, const char* argv[]) *bkp_step >= step_id::b_test_installed_create && *bkp_step <= step_id::bpkg_test_separate_installed_test) { - fail_unreached_breakpoint (add_result ("test-install")); + fail_unreached_breakpoint (add_result ("test-installed")); break; } @@ -5209,6 +5350,171 @@ build (size_t argc, const char* argv[]) fail_unreached_breakpoint (add_result ("uninstall")); break; } + + // Now, after the package is fully tested, let's prepare the build + // artifacts for the upload, using the upload operation log. + // + + // Prepare the bindist artifacts. + // + // Move all the generated binary distribution files and + // bindist-result.json to the upload/bindist// directory. + // + // Fail if the breakpoint refers to the bbot.bindist.upload step since + // it has no specific command associated. + // + if (bkp_step && *bkp_step == step_id::bbot_bindist_upload) + { + fail_unreached_breakpoint (add_result ("upload")); + break; + } + + if (bindist_upload) + { + operation_result& r (add_result ("upload")); + + dir_path d (upload_dir / + dir_path ("bindist") / + dir_path (bindist_result.distribution)); + + log_step_id (trace, r.log, step_id::bbot_bindist_upload); + + mk_p (trace, &r.log, d); + + // Move a file to the upload/bindist// directory. + // + // On Windows copy the file instead of moving not to fail if it is + // being scanned by the Windows Defender or some such. + // + auto mv = [&d, &r, &rwd, &trace] (const path& p) + { +#ifndef _WIN32 + bool mv (true); +#else + bool mv (false); +#endif + // Use relative paths to keep the operation log tidy. + // + if (mv) + mv_into (trace, &r.log, p.leaf (rwd), d); + else + cp_into (trace, &r.log, p.leaf (rwd), d); + }; + + auto move_files = [&mv] (const vector& bfs) + { + for (const bindist_file& f: bfs) + mv (f.path); + }; + + move_files (bindist_result.package.files); + + for (const bindist_package& d: bindist_result.dependencies) + move_files (d.files); + + mv (bindist_result_file); + } + + // Create the archive of the build artifacts for subsequent upload, if + // the upload/ directory is not empty. + // + // Note that the archive will be uploaded later, right before uploading + // the result manifest, unless the task has been aborted by the user or + // the result status is error or worse. + // + if (!empty (rwd / upload_dir) && !step_disabled (step_id::bbot_upload)) + { + // The upload operation log must have been added as part of the + // build artifacts preparation for upload. + // + operation_result& r (rm.results.back ()); + + // Fail if the breakpoint refers to the bbot.upload step since it has + // no specific command associated. + // + if (bkp_step && *bkp_step == step_id::bbot_upload) + { + fail_unreached_breakpoint (r); + break; + } + + change_wd (trace, &r.log, rwd); + + // Archive the build artifacts. + // + { + // tar -cf upload.tar + // + // + // upload/ + // + step_id b (step_id::bbot_upload_tar_create); + step_id s (step_id::bbot_upload_tar_create); + + // Make sure the archive is portable. + // + // Note that OpenBSD tar does not support --format but it appear + // ustar is the default (see bpkg/system-package-manager-archive.cxx + // for details). + // + r.status |= run_tar ( + b, + trace, r.log, wre, + bkp_step, bkp_status, last_cmd, + false /* sudo */, +#ifndef __OpenBSD__ + "--format", "ustar", +#endif + "-cf", + upload_archive, + step_args (env_args, s), + step_args (tgt_args, s), + step_args (pkg_args, s), + upload_dir); + + if (!r.status) + break; + } + + // It feels useful to also print the archive content to the log. + // + { + // tar -tf upload.tar + // + // + // + step_id b (step_id::bbot_upload_tar_list); + step_id s (step_id::bbot_upload_tar_list); + + r.status |= run_tar ( + b, + trace, r.log, wre, + bkp_step, bkp_status, last_cmd, + false /* sudo */, + "-tf", + upload_archive, + step_args (env_args, s), + step_args (tgt_args, s), + step_args (pkg_args, s)); + + if (!r.status) + break; + } + + rm.status |= r.status; + } + // + // Fail if the breakpoint refers to some of the bbot.upload.* steps but + // there is either nothing to upload or the bbot.upload step is + // disabled. + // + else if (bkp_step && + *bkp_step >= step_id::bbot_upload && + *bkp_step <= step_id::bbot_upload_tar_list) + { + fail_unreached_breakpoint (add_result ("upload")); + break; + } } // // Fail if the breakpoint refers to bpkg.update or any dependent step but @@ -5216,7 +5522,7 @@ build (size_t argc, const char* argv[]) // else if (bkp_step && *bkp_step >= step_id::bpkg_update && - *bkp_step <= step_id::bpkg_uninstall) + *bkp_step <= step_id::bbot_upload) { fail_unreached_breakpoint (add_result ("update")); break; @@ -5295,8 +5601,41 @@ build (size_t argc, const char* argv[]) rm.status == result_status::skip); if (!rwd.empty ()) + { change_wd (trace, nullptr /* log */, rwd); + // Upload the build artifacts archive, if exists. + // + bool error (!rm.status); + if (exists (upload_archive) && !error) + { + const string url ( + "tftp://" + ops.tftp_host () + '/' + upload_archive.string ()); + + try + { + tftp_curl c (trace, + upload_archive, + nullfd, + curl::put, + url, + "--tftp-blksize", tftp_blksize, + "--max-time", tftp_put_timeout); + + if (!c.wait ()) + fail << "curl " << *c.exit; + } + catch (const process_error& e) + { + fail << "unable to execute curl: " << e; + } + catch (const system_error& e) + { + fail << "unable to upload build artifacts to " << url << ": " << e; + } + } + } + // Upload the result. // const string url ("tftp://" + ops.tftp_host () + "/result.manifest.lz4"); -- cgit v1.1