From 0bd51586088588c352e04f5e4727f856b66e5aeb Mon Sep 17 00:00:00 2001 From: Chewing_Bever Date: Thu, 15 Dec 2022 09:46:48 +0100 Subject: [PATCH 01/10] feat(client): handle empty and non-successful responses --- src/client/client.v | 29 +++++++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/src/client/client.v b/src/client/client.v index aa6094a..3541555 100644 --- a/src/client/client.v +++ b/src/client/client.v @@ -1,8 +1,8 @@ module client -import net.http { Method } +import net.http { Method, Status } import net.urllib -import web.response { Response } +import web.response { Response, new_data_response } import json pub struct Client { @@ -56,8 +56,29 @@ fn (c &Client) send_request(method Method, url string, params map[string]stri // send_request_with_body calls send_request_raw_response & parses its // output as a Response object. fn (c &Client) send_request_with_body(method Method, url string, params map[string]string, body string) !Response { - res_text := c.send_request_raw_response(method, url, params, body)! - data := json.decode(Response, res_text)! + res := c.send_request_raw(method, url, params, body)! + + // Just return an empty successful response + if res.status_code == Status.no_content.int() { + return new_data_response(T{}) + } + + // Non-successful requests are expected to return either an empty body or + // Response + if res.status_code < 200 || res.status_code > 299 { + status_string := http.status_from_int(res.status_code).str() + + // A non-successful status call will have an empty body + if res.body == '' { + return error('Error $res.status_code ($status_string): (empty response)') + } + + data := json.decode(Response, res.body)! + + return error('Status $res.status_code ($status_string): $data.message') + } + + data := json.decode(Response, res.body)! return data } From 0727d0fd2517cb5dd67aa6f25d94b651c0f351f1 Mon Sep 17 00:00:00 2001 From: Chewing_Bever Date: Thu, 15 Dec 2022 10:01:45 +0100 Subject: [PATCH 02/10] refactor(client): streamline code & improve error propagation --- src/client/client.v | 13 ++++++------- src/client/jobs.v | 7 ++----- src/client/logs.v | 19 ++++--------------- src/client/targets.v | 13 ++++++------- src/console/logs/logs.v | 12 ++---------- src/console/targets/targets.v | 32 +++++++++----------------------- 6 files changed, 29 insertions(+), 67 deletions(-) diff --git a/src/client/client.v b/src/client/client.v index 3541555..5f24197 100644 --- a/src/client/client.v +++ b/src/client/client.v @@ -1,6 +1,6 @@ module client -import net.http { Method, Status } +import net.http { Method } import net.urllib import web.response { Response, new_data_response } import json @@ -57,25 +57,24 @@ fn (c &Client) send_request(method Method, url string, params map[string]stri // output as a Response object. fn (c &Client) send_request_with_body(method Method, url string, params map[string]string, body string) !Response { res := c.send_request_raw(method, url, params, body)! + status := http.status_from_int(res.status_code) // Just return an empty successful response - if res.status_code == Status.no_content.int() { + if status.is_success() && res.body == '' { return new_data_response(T{}) } // Non-successful requests are expected to return either an empty body or // Response - if res.status_code < 200 || res.status_code > 299 { - status_string := http.status_from_int(res.status_code).str() - + if status.is_error() { // A non-successful status call will have an empty body if res.body == '' { - return error('Error $res.status_code ($status_string): (empty response)') + return error('Error $res.status_code ($status.str()): (empty response)') } data := json.decode(Response, res.body)! - return error('Status $res.status_code ($status_string): $data.message') + return error('Status $res.status_code ($status.str()): $data.message') } data := json.decode(Response, res.body)! diff --git a/src/client/jobs.v b/src/client/jobs.v index 440affa..a545499 100644 --- a/src/client/jobs.v +++ b/src/client/jobs.v @@ -1,7 +1,6 @@ module client import build { BuildConfig } -import web.response { Response } // poll_jobs requests a list of new build jobs from the server. pub fn (c &Client) poll_jobs(arch string, max int) ![]BuildConfig { @@ -15,12 +14,10 @@ pub fn (c &Client) poll_jobs(arch string, max int) ![]BuildConfig { // queue_job adds a new one-time build job for the given target to the job // queue. -pub fn (c &Client) queue_job(target_id int, arch string, force bool) !Response { - data := c.send_request(.post, '/api/v1/jobs/queue', { +pub fn (c &Client) queue_job(target_id int, arch string, force bool) ! { + c.send_request(.post, '/api/v1/jobs/queue', { 'target': target_id.str() 'arch': arch 'force': force.str() })! - - return data } diff --git a/src/client/logs.v b/src/client/logs.v index eaddc8c..85063bc 100644 --- a/src/client/logs.v +++ b/src/client/logs.v @@ -6,29 +6,18 @@ import web.response { Response } import time // get_build_logs returns all build logs. -pub fn (c &Client) get_build_logs(filter BuildLogFilter) !Response<[]BuildLog> { +pub fn (c &Client) get_build_logs(filter BuildLogFilter) ![]BuildLog { params := models.params_from(filter) data := c.send_request<[]BuildLog>(Method.get, '/api/v1/logs', params)! - return data -} - -// get_build_logs_for_target returns all build logs for a given target. -pub fn (c &Client) get_build_logs_for_target(target_id int) !Response<[]BuildLog> { - params := { - 'repo': target_id.str() - } - - data := c.send_request<[]BuildLog>(Method.get, '/api/v1/logs', params)! - - return data + return data.data } // get_build_log returns a specific build log. -pub fn (c &Client) get_build_log(id int) !Response { +pub fn (c &Client) get_build_log(id int) !BuildLog { data := c.send_request(Method.get, '/api/v1/logs/$id', {})! - return data + return data.data } // get_build_log_content returns the contents of the build log file. diff --git a/src/client/targets.v b/src/client/targets.v index fd4254c..40bfdae 100644 --- a/src/client/targets.v +++ b/src/client/targets.v @@ -2,7 +2,6 @@ module client import models { Target, TargetFilter } import net.http { Method } -import web.response { Response } // get_targets returns a list of targets, given a filter object. pub fn (c &Client) get_targets(filter TargetFilter) ![]Target { @@ -49,24 +48,24 @@ pub struct NewTarget { } // add_target adds a new target to the server. -pub fn (c &Client) add_target(t NewTarget) !Response { +pub fn (c &Client) add_target(t NewTarget) !int { params := models.params_from(t) data := c.send_request(Method.post, '/api/v1/targets', params)! - return data + return data.data } // remove_target removes the target with the given id from the server. -pub fn (c &Client) remove_target(id int) !Response { +pub fn (c &Client) remove_target(id int) !string { data := c.send_request(Method.delete, '/api/v1/targets/$id', {})! - return data + return data.data } // patch_target sends a PATCH request to the given target with the params as // payload. -pub fn (c &Client) patch_target(id int, params map[string]string) !Response { +pub fn (c &Client) patch_target(id int, params map[string]string) !string { data := c.send_request(Method.patch, '/api/v1/targets/$id', params)! - return data + return data.data } diff --git a/src/console/logs/logs.v b/src/console/logs/logs.v index 1330dd0..3064a58 100644 --- a/src/console/logs/logs.v +++ b/src/console/logs/logs.v @@ -183,15 +183,7 @@ fn print_log_list(logs []BuildLog, raw bool) ! { // list prints a list of all build logs. fn list(conf Config, filter BuildLogFilter, raw bool) ! { c := client.new(conf.address, conf.api_key) - logs := c.get_build_logs(filter)!.data - - print_log_list(logs, raw)! -} - -// list prints a list of all build logs for a given target. -fn list_for_target(conf Config, target_id int, raw bool) ! { - c := client.new(conf.address, conf.api_key) - logs := c.get_build_logs_for_target(target_id)!.data + logs := c.get_build_logs(filter)! print_log_list(logs, raw)! } @@ -199,7 +191,7 @@ fn list_for_target(conf Config, target_id int, raw bool) ! { // info print the detailed info for a given build log. fn info(conf Config, id int) ! { c := client.new(conf.address, conf.api_key) - log := c.get_build_log(id)!.data + log := c.get_build_log(id)! print(log) } diff --git a/src/console/targets/targets.v b/src/console/targets/targets.v index b527896..b277410 100644 --- a/src/console/targets/targets.v +++ b/src/console/targets/targets.v @@ -215,8 +215,7 @@ pub fn cmd() cli.Command { } c := client.new(conf.address, conf.api_key) - res := c.queue_job(target_id, arch, force)! - println(res.message) + c.queue_job(target_id, arch, force)! } else { build(conf, target_id, force)! } @@ -245,23 +244,19 @@ fn list(conf Config, filter TargetFilter, raw bool) ! { // add adds a new repository to the server's list. fn add(conf Config, t &NewTarget, raw bool) ! { c := client.new(conf.address, conf.api_key) - res := c.add_target(t)! + target_id := c.add_target(t)! if raw { - println(res.data) + println(target_id) } else { - println('Target added with id $res.data') + println('Target added with id $target_id') } } // remove removes a repository from the server's list. fn remove(conf Config, id string) ! { - id_int := id.int() - - if id_int != 0 { - c := client.new(conf.address, conf.api_key) - c.remove_target(id_int)! - } + c := client.new(conf.address, conf.api_key) + c.remove_target(id.int())! } // patch patches a given repository with the provided params. @@ -274,22 +269,13 @@ fn patch(conf Config, id string, params map[string]string) ! { } } - id_int := id.int() - if id_int != 0 { - c := client.new(conf.address, conf.api_key) - c.patch_target(id_int, params)! - } + c := client.new(conf.address, conf.api_key) + c.patch_target(id.int(), params)! } // info shows detailed information for a given repo. fn info(conf Config, id string) ! { - id_int := id.int() - - if id_int == 0 { - return - } - c := client.new(conf.address, conf.api_key) - repo := c.get_target(id_int)! + repo := c.get_target(id.int())! println(repo) } From b634775ca387d9827f933af1c7c1a521e1c4926b Mon Sep 17 00:00:00 2001 From: Chewing_Bever Date: Thu, 15 Dec 2022 10:46:58 +0100 Subject: [PATCH 03/10] refactor(server): clean up server responses a bit --- src/server/api_logs.v | 23 +++++++++++------------ src/server/api_targets.v | 11 +++++------ src/web/web.v | 7 ------- 3 files changed, 16 insertions(+), 25 deletions(-) diff --git a/src/server/api_logs.v b/src/server/api_logs.v index fcbf024..c7521dd 100644 --- a/src/server/api_logs.v +++ b/src/server/api_logs.v @@ -1,7 +1,6 @@ module server import web -import net.http import net.urllib import web.response { new_data_response, new_response } import db @@ -15,7 +14,7 @@ import models { BuildLog, BuildLogFilter } ['/api/v1/logs'; auth; get] fn (mut app App) v1_get_logs() web.Result { filter := models.from_params(app.query) or { - return app.json(http.Status.bad_request, new_response('Invalid query parameters.')) + return app.json(.bad_request, new_response('Invalid query parameters.')) } logs := app.db.get_build_logs(filter) @@ -25,7 +24,7 @@ fn (mut app App) v1_get_logs() web.Result { // v1_get_single_log returns the build log with the given id. ['/api/v1/logs/:id'; auth; get] fn (mut app App) v1_get_single_log(id int) web.Result { - log := app.db.get_build_log(id) or { return app.not_found() } + log := app.db.get_build_log(id) or { return app.status(.not_found) } return app.json(.ok, new_data_response(log)) } @@ -33,7 +32,7 @@ fn (mut app App) v1_get_single_log(id int) web.Result { // v1_get_log_content returns the actual build log file for the given id. ['/api/v1/logs/:id/content'; auth; get] fn (mut app App) v1_get_log_content(id int) web.Result { - log := app.db.get_build_log(id) or { return app.not_found() } + log := app.db.get_build_log(id) or { return app.status(.not_found) } file_name := log.start_time.custom_format('YYYY-MM-DD_HH-mm-ss') full_path := os.join_path(app.conf.data_dir, logs_dir_name, log.target_id.str(), log.arch, file_name) @@ -57,25 +56,25 @@ fn (mut app App) v1_post_log() web.Result { start_time_int := app.query['startTime'].int() if start_time_int == 0 { - return app.json(http.Status.bad_request, new_response('Invalid or missing start time.')) + return app.json(.bad_request, new_response('Invalid or missing start time.')) } start_time := time.unix(start_time_int) end_time_int := app.query['endTime'].int() if end_time_int == 0 { - return app.json(http.Status.bad_request, new_response('Invalid or missing end time.')) + return app.json(.bad_request, new_response('Invalid or missing end time.')) } end_time := time.unix(end_time_int) if 'exitCode' !in app.query { - return app.json(http.Status.bad_request, new_response('Missing exit code.')) + return app.json(.bad_request, new_response('Missing exit code.')) } exit_code := app.query['exitCode'].int() if 'arch' !in app.query { - return app.json(http.Status.bad_request, new_response("Missing parameter 'arch'.")) + return app.json(.bad_request, new_response("Missing parameter 'arch'.")) } arch := app.query['arch'] @@ -83,7 +82,7 @@ fn (mut app App) v1_post_log() web.Result { target_id := app.query['target'].int() if !app.db.target_exists(target_id) { - return app.json(http.Status.bad_request, new_response('Unknown target.')) + return app.json(.bad_request, new_response('Unknown target.')) } // Store log in db @@ -105,7 +104,7 @@ fn (mut app App) v1_post_log() web.Result { os.mkdir_all(repo_logs_dir) or { app.lerror("Couldn't create dir '$repo_logs_dir'.") - return app.json(http.Status.internal_server_error, new_response('An error occured while processing the request.')) + return app.status(.internal_server_error) } } @@ -117,10 +116,10 @@ fn (mut app App) v1_post_log() web.Result { util.reader_to_file(mut app.reader, length.int(), full_path) or { app.lerror('An error occured while receiving logs: $err.msg()') - return app.json(http.Status.internal_server_error, new_response('Failed to upload logs.')) + return app.status(.internal_server_error) } } else { - return app.status(http.Status.length_required) + return app.status(.length_required) } return app.json(.ok, new_data_response(log_id)) diff --git a/src/server/api_targets.v b/src/server/api_targets.v index dc39d37..cd5cb0a 100644 --- a/src/server/api_targets.v +++ b/src/server/api_targets.v @@ -1,7 +1,6 @@ module server import web -import net.http import web.response { new_data_response, new_response } import db import models { Target, TargetArch, TargetFilter } @@ -10,7 +9,7 @@ import models { Target, TargetArch, TargetFilter } ['/api/v1/targets'; auth; get] fn (mut app App) v1_get_targets() web.Result { filter := models.from_params(app.query) or { - return app.json(http.Status.bad_request, new_response('Invalid query parameters.')) + return app.json(.bad_request, new_response('Invalid query parameters.')) } targets := app.db.get_targets(filter) @@ -20,7 +19,7 @@ fn (mut app App) v1_get_targets() web.Result { // v1_get_single_target returns the information for a single target. ['/api/v1/targets/:id'; auth; get] fn (mut app App) v1_get_single_target(id int) web.Result { - target := app.db.get_target(id) or { return app.not_found() } + target := app.db.get_target(id) or { return app.status(.not_found) } return app.json(.ok, new_data_response(target)) } @@ -37,12 +36,12 @@ fn (mut app App) v1_post_target() web.Result { } mut new_target := models.from_params(params) or { - return app.json(http.Status.bad_request, new_response(err.msg())) + return app.json(.bad_request, new_response(err.msg())) } // Ensure someone doesn't submit an invalid kind if new_target.kind !in models.valid_kinds { - return app.json(http.Status.bad_request, new_response('Invalid kind.')) + return app.json(.bad_request, new_response('Invalid kind.')) } id := app.db.add_target(new_target) @@ -61,7 +60,7 @@ fn (mut app App) v1_delete_target(id int) web.Result { app.db.delete_target(id) app.job_queue.invalidate(id) - return app.json(.ok, new_response('')) + return app.status(.ok) } // v1_patch_target updates a target's data with the given query params. diff --git a/src/web/web.v b/src/web/web.v index 1b40e7a..565baff 100644 --- a/src/web/web.v +++ b/src/web/web.v @@ -260,13 +260,6 @@ pub fn (mut ctx Context) redirect(url string) Result { return Result{} } -// not_found Send an not_found response -pub fn (mut ctx Context) not_found() Result { - ctx.send_custom_response(http_404) or {} - - return Result{} -} - interface DbInterface { db voidptr } From dbbe5c1e51cbd54483d2a4aee89a194960106ff5 Mon Sep 17 00:00:00 2001 From: Chewing_Bever Date: Thu, 15 Dec 2022 12:09:43 +0100 Subject: [PATCH 04/10] fix(agent): remove infinite loop and account for externally removed images --- src/agent/images.v | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/agent/images.v b/src/agent/images.v index 185192e..dd32656 100644 --- a/src/agent/images.v +++ b/src/agent/images.v @@ -73,7 +73,21 @@ fn (mut m ImageManager) clean_old_images() { // wasn't deleted. Therefore, we move the index over. If the function // returns true, the array's length has decreased by one so we don't // move the index. - dd.remove_image(m.images[image][i]) or { i += 1 } + dd.remove_image(m.images[image][i]) or { + // The image was removed by an external event + if err.code() == 404 { + m.images[image].delete(i) + } + // The image couldn't be removed, so we need to keep track of + // it + else { + i += 1 + } + + continue + } + + m.images[image].delete(i) } } } From a48358fd75101b82aa78c5b196324f08719b918f Mon Sep 17 00:00:00 2001 From: Jef Roosens Date: Thu, 15 Dec 2022 23:47:41 +0100 Subject: [PATCH 05/10] fix: don't run prepare step twice in builds --- src/build/build_script_git.sh | 4 ++-- src/build/build_script_git_branch.sh | 4 ++-- src/build/build_script_url.sh | 4 ++-- src/build/shell.v | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/build/build_script_git.sh b/src/build/build_script_git.sh index 73e0965..2644243 100644 --- a/src/build/build_script_git.sh +++ b/src/build/build_script_git.sh @@ -16,5 +16,5 @@ echo -e '+ curl -s --head --fail https://example.com/vieter/x86_64/$pkgname-$pkg curl -s --head --fail https://example.com/vieter/x86_64/$pkgname-$pkgver-$pkgrel && exit 0 echo -e '+ [ "$(id -u)" == 0 ] && exit 0' [ "$(id -u)" == 0 ] && exit 0 -echo -e '+ MAKEFLAGS="-j$(nproc)" makepkg -s --noconfirm --needed && for pkg in $(ls -1 *.pkg*); do curl -XPOST -T "$pkg" -H "X-API-KEY: $API_KEY" https://example.com/vieter/publish; done' -MAKEFLAGS="-j$(nproc)" makepkg -s --noconfirm --needed && for pkg in $(ls -1 *.pkg*); do curl -XPOST -T "$pkg" -H "X-API-KEY: $API_KEY" https://example.com/vieter/publish; done +echo -e '+ MAKEFLAGS="-j$(nproc)" makepkg -s --noconfirm --needed --noextract && for pkg in $(ls -1 *.pkg*); do curl -XPOST -T "$pkg" -H "X-API-KEY: $API_KEY" https://example.com/vieter/publish; done' +MAKEFLAGS="-j$(nproc)" makepkg -s --noconfirm --needed --noextract && for pkg in $(ls -1 *.pkg*); do curl -XPOST -T "$pkg" -H "X-API-KEY: $API_KEY" https://example.com/vieter/publish; done diff --git a/src/build/build_script_git_branch.sh b/src/build/build_script_git_branch.sh index be1ff4f..9f36bdc 100644 --- a/src/build/build_script_git_branch.sh +++ b/src/build/build_script_git_branch.sh @@ -16,5 +16,5 @@ echo -e '+ curl -s --head --fail https://example.com/vieter/x86_64/$pkgname-$pkg curl -s --head --fail https://example.com/vieter/x86_64/$pkgname-$pkgver-$pkgrel && exit 0 echo -e '+ [ "$(id -u)" == 0 ] && exit 0' [ "$(id -u)" == 0 ] && exit 0 -echo -e '+ MAKEFLAGS="-j$(nproc)" makepkg -s --noconfirm --needed && for pkg in $(ls -1 *.pkg*); do curl -XPOST -T "$pkg" -H "X-API-KEY: $API_KEY" https://example.com/vieter/publish; done' -MAKEFLAGS="-j$(nproc)" makepkg -s --noconfirm --needed && for pkg in $(ls -1 *.pkg*); do curl -XPOST -T "$pkg" -H "X-API-KEY: $API_KEY" https://example.com/vieter/publish; done +echo -e '+ MAKEFLAGS="-j$(nproc)" makepkg -s --noconfirm --needed --noextract && for pkg in $(ls -1 *.pkg*); do curl -XPOST -T "$pkg" -H "X-API-KEY: $API_KEY" https://example.com/vieter/publish; done' +MAKEFLAGS="-j$(nproc)" makepkg -s --noconfirm --needed --noextract && for pkg in $(ls -1 *.pkg*); do curl -XPOST -T "$pkg" -H "X-API-KEY: $API_KEY" https://example.com/vieter/publish; done diff --git a/src/build/build_script_url.sh b/src/build/build_script_url.sh index 3bc97e1..2d27de7 100644 --- a/src/build/build_script_url.sh +++ b/src/build/build_script_url.sh @@ -18,5 +18,5 @@ echo -e '+ curl -s --head --fail https://example.com/vieter/x86_64/$pkgname-$pkg curl -s --head --fail https://example.com/vieter/x86_64/$pkgname-$pkgver-$pkgrel && exit 0 echo -e '+ [ "$(id -u)" == 0 ] && exit 0' [ "$(id -u)" == 0 ] && exit 0 -echo -e '+ MAKEFLAGS="-j$(nproc)" makepkg -s --noconfirm --needed && for pkg in $(ls -1 *.pkg*); do curl -XPOST -T "$pkg" -H "X-API-KEY: $API_KEY" https://example.com/vieter/publish; done' -MAKEFLAGS="-j$(nproc)" makepkg -s --noconfirm --needed && for pkg in $(ls -1 *.pkg*); do curl -XPOST -T "$pkg" -H "X-API-KEY: $API_KEY" https://example.com/vieter/publish; done +echo -e '+ MAKEFLAGS="-j$(nproc)" makepkg -s --noconfirm --needed --noextract && for pkg in $(ls -1 *.pkg*); do curl -XPOST -T "$pkg" -H "X-API-KEY: $API_KEY" https://example.com/vieter/publish; done' +MAKEFLAGS="-j$(nproc)" makepkg -s --noconfirm --needed --noextract && for pkg in $(ls -1 *.pkg*); do curl -XPOST -T "$pkg" -H "X-API-KEY: $API_KEY" https://example.com/vieter/publish; done diff --git a/src/build/shell.v b/src/build/shell.v index ac61e07..6aa2413 100644 --- a/src/build/shell.v +++ b/src/build/shell.v @@ -79,7 +79,7 @@ fn create_build_script(address string, config BuildConfig, build_arch string) st } commands << [ - 'MAKEFLAGS="-j\$(nproc)" makepkg -s --noconfirm --needed && for pkg in \$(ls -1 *.pkg*); do curl -XPOST -T "\$pkg" -H "X-API-KEY: \$API_KEY" $repo_url/publish; done', + 'MAKEFLAGS="-j\$(nproc)" makepkg -s --noconfirm --needed --noextract && for pkg in \$(ls -1 *.pkg*); do curl -XPOST -T "\$pkg" -H "X-API-KEY: \$API_KEY" $repo_url/publish; done', ] return echo_commands(commands).join('\n') From 1ce7b9d5715d8d93deda284bd8b0dbd3d113dc26 Mon Sep 17 00:00:00 2001 From: Chewing_Bever Date: Fri, 16 Dec 2022 11:21:28 +0100 Subject: [PATCH 06/10] feat: add option to specify subdirectory in repo to use --- src/build/build.v | 4 +- .../{build_script_git.sh => scripts/git.sh} | 0 .../git_branch.sh} | 0 src/build/scripts/git_path.sh | 20 +++++++ src/build/scripts/git_path_spaces.sh | 20 +++++++ .../{build_script_url.sh => scripts/url.sh} | 0 src/build/shell.v | 7 ++- src/build/shell_test.v | 60 +++++++++++++------ src/client/targets.v | 1 + src/console/targets/targets.v | 11 ++++ src/db/db.v | 2 + src/db/migrations/005-repo-path/down.sql | 1 + src/db/migrations/005-repo-path/up.sql | 1 + src/models/targets.v | 21 ++++--- 14 files changed, 120 insertions(+), 28 deletions(-) rename src/build/{build_script_git.sh => scripts/git.sh} (100%) rename src/build/{build_script_git_branch.sh => scripts/git_branch.sh} (100%) create mode 100644 src/build/scripts/git_path.sh create mode 100644 src/build/scripts/git_path_spaces.sh rename src/build/{build_script_url.sh => scripts/url.sh} (100%) create mode 100644 src/db/migrations/005-repo-path/down.sql create mode 100644 src/db/migrations/005-repo-path/up.sql diff --git a/src/build/build.v b/src/build/build.v index 3d916bf..c6aa7f1 100644 --- a/src/build/build.v +++ b/src/build/build.v @@ -22,6 +22,7 @@ pub: kind string url string branch string + path string repo string base_image string force bool @@ -29,7 +30,7 @@ pub: // str return a single-line string representation of a build log pub fn (c BuildConfig) str() string { - return '{ target: $c.target_id, kind: $c.kind, url: $c.url, branch: $c.branch, repo: $c.repo, base_image: $c.base_image, force: $c.force }' + return '{ target: $c.target_id, kind: $c.kind, url: $c.url, branch: $c.branch, path: $c.path, repo: $c.repo, base_image: $c.base_image, force: $c.force }' } // create_build_image creates a builder image given some base image which can @@ -116,6 +117,7 @@ pub fn build_target(address string, api_key string, base_image_id string, target kind: target.kind url: target.url branch: target.branch + path: target.path repo: target.repo base_image: base_image_id force: force diff --git a/src/build/build_script_git.sh b/src/build/scripts/git.sh similarity index 100% rename from src/build/build_script_git.sh rename to src/build/scripts/git.sh diff --git a/src/build/build_script_git_branch.sh b/src/build/scripts/git_branch.sh similarity index 100% rename from src/build/build_script_git_branch.sh rename to src/build/scripts/git_branch.sh diff --git a/src/build/scripts/git_path.sh b/src/build/scripts/git_path.sh new file mode 100644 index 0000000..65b7fb9 --- /dev/null +++ b/src/build/scripts/git_path.sh @@ -0,0 +1,20 @@ +echo -e '+ echo -e '\''[vieter]\\nServer = https://example.com/$repo/$arch\\nSigLevel = Optional'\'' >> /etc/pacman.conf' +echo -e '[vieter]\nServer = https://example.com/$repo/$arch\nSigLevel = Optional' >> /etc/pacman.conf +echo -e '+ pacman -Syu --needed --noconfirm' +pacman -Syu --needed --noconfirm +echo -e '+ su builder' +su builder +echo -e '+ git clone --single-branch --depth 1 '\''https://examplerepo.com'\'' repo' +git clone --single-branch --depth 1 'https://examplerepo.com' repo +echo -e '+ cd '\''repo/example/path'\''' +cd 'repo/example/path' +echo -e '+ makepkg --nobuild --syncdeps --needed --noconfirm' +makepkg --nobuild --syncdeps --needed --noconfirm +echo -e '+ source PKGBUILD' +source PKGBUILD +echo -e '+ curl -s --head --fail https://example.com/vieter/x86_64/$pkgname-$pkgver-$pkgrel && exit 0' +curl -s --head --fail https://example.com/vieter/x86_64/$pkgname-$pkgver-$pkgrel && exit 0 +echo -e '+ [ "$(id -u)" == 0 ] && exit 0' +[ "$(id -u)" == 0 ] && exit 0 +echo -e '+ MAKEFLAGS="-j$(nproc)" makepkg -s --noconfirm --needed --noextract && for pkg in $(ls -1 *.pkg*); do curl -XPOST -T "$pkg" -H "X-API-KEY: $API_KEY" https://example.com/vieter/publish; done' +MAKEFLAGS="-j$(nproc)" makepkg -s --noconfirm --needed --noextract && for pkg in $(ls -1 *.pkg*); do curl -XPOST -T "$pkg" -H "X-API-KEY: $API_KEY" https://example.com/vieter/publish; done diff --git a/src/build/scripts/git_path_spaces.sh b/src/build/scripts/git_path_spaces.sh new file mode 100644 index 0000000..b632b91 --- /dev/null +++ b/src/build/scripts/git_path_spaces.sh @@ -0,0 +1,20 @@ +echo -e '+ echo -e '\''[vieter]\\nServer = https://example.com/$repo/$arch\\nSigLevel = Optional'\'' >> /etc/pacman.conf' +echo -e '[vieter]\nServer = https://example.com/$repo/$arch\nSigLevel = Optional' >> /etc/pacman.conf +echo -e '+ pacman -Syu --needed --noconfirm' +pacman -Syu --needed --noconfirm +echo -e '+ su builder' +su builder +echo -e '+ git clone --single-branch --depth 1 '\''https://examplerepo.com'\'' repo' +git clone --single-branch --depth 1 'https://examplerepo.com' repo +echo -e '+ cd '\''repo/example/path with spaces'\''' +cd 'repo/example/path with spaces' +echo -e '+ makepkg --nobuild --syncdeps --needed --noconfirm' +makepkg --nobuild --syncdeps --needed --noconfirm +echo -e '+ source PKGBUILD' +source PKGBUILD +echo -e '+ curl -s --head --fail https://example.com/vieter/x86_64/$pkgname-$pkgver-$pkgrel && exit 0' +curl -s --head --fail https://example.com/vieter/x86_64/$pkgname-$pkgver-$pkgrel && exit 0 +echo -e '+ [ "$(id -u)" == 0 ] && exit 0' +[ "$(id -u)" == 0 ] && exit 0 +echo -e '+ MAKEFLAGS="-j$(nproc)" makepkg -s --noconfirm --needed --noextract && for pkg in $(ls -1 *.pkg*); do curl -XPOST -T "$pkg" -H "X-API-KEY: $API_KEY" https://example.com/vieter/publish; done' +MAKEFLAGS="-j$(nproc)" makepkg -s --noconfirm --needed --noextract && for pkg in $(ls -1 *.pkg*); do curl -XPOST -T "$pkg" -H "X-API-KEY: $API_KEY" https://example.com/vieter/publish; done diff --git a/src/build/build_script_url.sh b/src/build/scripts/url.sh similarity index 100% rename from src/build/build_script_url.sh rename to src/build/scripts/url.sh diff --git a/src/build/shell.v b/src/build/shell.v index 6aa2413..c459a99 100644 --- a/src/build/shell.v +++ b/src/build/shell.v @@ -59,8 +59,13 @@ fn create_build_script(address string, config BuildConfig, build_arch string) st } } + commands << if config.path != '' { + "cd 'repo/$config.path'" + } else { + 'cd repo' + } + commands << [ - 'cd repo', 'makepkg --nobuild --syncdeps --needed --noconfirm', 'source PKGBUILD', ] diff --git a/src/build/shell_test.v b/src/build/shell_test.v index 8bb22d9..e44c5ff 100644 --- a/src/build/shell_test.v +++ b/src/build/shell_test.v @@ -1,5 +1,46 @@ module build +fn test_create_build_script_git() { + config := BuildConfig{ + target_id: 1 + kind: 'git' + url: 'https://examplerepo.com' + repo: 'vieter' + base_image: 'not-used:latest' + } + + build_script := create_build_script('https://example.com', config, 'x86_64') + expected := $embed_file('scripts/git.sh') + + assert build_script == expected.to_string().trim_space() +} + +fn test_create_build_script_git_path() { + mut config := BuildConfig{ + target_id: 1 + kind: 'git' + url: 'https://examplerepo.com' + repo: 'vieter' + path: 'example/path' + base_image: 'not-used:latest' + } + + mut build_script := create_build_script('https://example.com', config, 'x86_64') + mut expected := $embed_file('scripts/git_path.sh') + + assert build_script == expected.to_string().trim_space() + + config = BuildConfig{ + ...config + path: 'example/path with spaces' + } + + build_script = create_build_script('https://example.com', config, 'x86_64') + expected = $embed_file('scripts/git_path_spaces.sh') + + assert build_script == expected.to_string().trim_space() +} + fn test_create_build_script_git_branch() { config := BuildConfig{ target_id: 1 @@ -11,22 +52,7 @@ fn test_create_build_script_git_branch() { } build_script := create_build_script('https://example.com', config, 'x86_64') - expected := $embed_file('build_script_git_branch.sh') - - assert build_script == expected.to_string().trim_space() -} - -fn test_create_build_script_git() { - config := BuildConfig{ - target_id: 1 - kind: 'git' - url: 'https://examplerepo.com' - repo: 'vieter' - base_image: 'not-used:latest' - } - - build_script := create_build_script('https://example.com', config, 'x86_64') - expected := $embed_file('build_script_git.sh') + expected := $embed_file('scripts/git_branch.sh') assert build_script == expected.to_string().trim_space() } @@ -41,7 +67,7 @@ fn test_create_build_script_url() { } build_script := create_build_script('https://example.com', config, 'x86_64') - expected := $embed_file('build_script_url.sh') + expected := $embed_file('scripts/url.sh') assert build_script == expected.to_string().trim_space() } diff --git a/src/client/targets.v b/src/client/targets.v index 40bfdae..da6a9e4 100644 --- a/src/client/targets.v +++ b/src/client/targets.v @@ -44,6 +44,7 @@ pub struct NewTarget { url string branch string repo string + path string arch []string } diff --git a/src/console/targets/targets.v b/src/console/targets/targets.v index b277410..a134926 100644 --- a/src/console/targets/targets.v +++ b/src/console/targets/targets.v @@ -82,6 +82,11 @@ pub fn cmd() cli.Command { description: "Which branch to clone; only applies to kind 'git'." flag: cli.FlagType.string }, + cli.Flag{ + name: 'path' + description: 'Subdirectory inside Git repository to use.' + flag: cli.FlagType.string + }, ] execute: fn (cmd cli.Command) ! { config_file := cmd.flags.get_string('config-file')! @@ -92,6 +97,7 @@ pub fn cmd() cli.Command { url: cmd.args[0] repo: cmd.args[1] branch: cmd.flags.get_string('branch') or { '' } + path: cmd.flags.get_string('path') or { '' } } raw := cmd.flags.get_bool('raw')! @@ -159,6 +165,11 @@ pub fn cmd() cli.Command { description: 'Kind of target.' flag: cli.FlagType.string }, + cli.Flag{ + name: 'path' + description: 'Subdirectory inside Git repository to use.' + flag: cli.FlagType.string + }, ] execute: fn (cmd cli.Command) ! { config_file := cmd.flags.get_string('config-file')! diff --git a/src/db/db.v b/src/db/db.v index 1a0160e..98ee000 100644 --- a/src/db/db.v +++ b/src/db/db.v @@ -18,12 +18,14 @@ const ( $embed_file('migrations/002-rename-to-targets/up.sql'), $embed_file('migrations/003-target-url-type/up.sql'), $embed_file('migrations/004-nullable-branch/up.sql'), + $embed_file('migrations/005-repo-path/up.sql'), ] migrations_down = [ $embed_file('migrations/001-initial/down.sql'), $embed_file('migrations/002-rename-to-targets/down.sql'), $embed_file('migrations/003-target-url-type/down.sql'), $embed_file('migrations/004-nullable-branch/down.sql'), + $embed_file('migrations/005-repo-path/down.sql'), ] ) diff --git a/src/db/migrations/005-repo-path/down.sql b/src/db/migrations/005-repo-path/down.sql new file mode 100644 index 0000000..8a6f021 --- /dev/null +++ b/src/db/migrations/005-repo-path/down.sql @@ -0,0 +1 @@ +ALTER TABLE Target DROP COLUMN path; diff --git a/src/db/migrations/005-repo-path/up.sql b/src/db/migrations/005-repo-path/up.sql new file mode 100644 index 0000000..f7e5c29 --- /dev/null +++ b/src/db/migrations/005-repo-path/up.sql @@ -0,0 +1 @@ +ALTER TABLE Target ADD COLUMN path TEXT; diff --git a/src/models/targets.v b/src/models/targets.v index c8aa535..cb60650 100644 --- a/src/models/targets.v +++ b/src/models/targets.v @@ -28,21 +28,24 @@ pub mut: repo string [nonull] // Cron schedule describing how frequently to build the repo. schedule string + // Subdirectory in the Git repository to cd into + path string // On which architectures the package is allowed to be built. In reality, - // this controls which builders will periodically build the image. + // this controls which agents will build this package when scheduled. arch []TargetArch [fkey: 'target_id'] } // str returns a string representation. -pub fn (gr &Target) str() string { +pub fn (t &Target) str() string { mut parts := [ - 'id: $gr.id', - 'kind: $gr.kind', - 'url: $gr.url', - 'branch: $gr.branch', - 'repo: $gr.repo', - 'schedule: $gr.schedule', - 'arch: ${gr.arch.map(it.value).join(', ')}', + 'id: $t.id', + 'kind: $t.kind', + 'url: $t.url', + 'branch: $t.branch', + 'path: $t.path', + 'repo: $t.repo', + 'schedule: $t.schedule', + 'arch: ${t.arch.map(it.value).join(', ')}', ] str := parts.join('\n') From 489931eaa809bf4b0997c86f2da2dd38f86d3f0f Mon Sep 17 00:00:00 2001 From: Chewing_Bever Date: Fri, 16 Dec 2022 11:37:51 +0100 Subject: [PATCH 07/10] fix: don't buffer stdout even if not a terminal --- Dockerfile | 1 + src/main.v | 5 +++++ 2 files changed, 6 insertions(+) diff --git a/Dockerfile b/Dockerfile index 210ae66..a27ad44 100644 --- a/Dockerfile +++ b/Dockerfile @@ -23,6 +23,7 @@ RUN if [ -n "${CI_COMMIT_SHA}" ]; then \ "https://s3.rustybever.be/vieter/commits/${CI_COMMIT_SHA}/vieter-$(echo "${TARGETPLATFORM}" | sed 's:/:-:g')" && \ chmod +x vieter ; \ else \ + cd src && v install && cd .. && \ LDFLAGS='-lz -lbz2 -llzma -lexpat -lzstd -llz4 -lsqlite3 -static' make prod && \ mv pvieter vieter ; \ fi diff --git a/src/main.v b/src/main.v index 34387bf..fe0364f 100644 --- a/src/main.v +++ b/src/main.v @@ -12,6 +12,11 @@ import cron import agent fn main() { + // Stop buffering output so logs always show up immediately + unsafe { + C.setbuf(C.stdout, 0) + } + mut app := cli.Command{ name: 'vieter' description: 'Vieter is a lightweight implementation of an Arch repository server.' From 0604de26c48c4fd8e4c3285ce4d6008a6e1d64ca Mon Sep 17 00:00:00 2001 From: Chewing_Bever Date: Fri, 16 Dec 2022 14:33:16 +0100 Subject: [PATCH 08/10] feat(agent): ensure images exist when starting build --- src/agent/daemon.v | 19 +++++++++++++++---- src/agent/images.v | 42 ++++++++++++++++++++++++++++++++++-------- 2 files changed, 49 insertions(+), 12 deletions(-) diff --git a/src/agent/daemon.v b/src/agent/daemon.v index 0647733..8fa3816 100644 --- a/src/agent/daemon.v +++ b/src/agent/daemon.v @@ -80,13 +80,24 @@ pub fn (mut d AgentDaemon) run() { last_poll_time = time.now() for config in new_configs { - // TODO handle this better than to just skip the config // Make sure a recent build base image is available for // building the config - d.images.refresh_image(config.base_image) or { - d.lerror(err.msg()) - continue + if !d.images.up_to_date(config.base_image) { + d.linfo('Building builder image from base image $config.base_image') + + // TODO handle this better than to just skip the config + d.images.refresh_image(config.base_image) or { + d.lerror(err.msg()) + continue + } } + + // It's technically still possible that the build image is + // removed in the very short period between building the + // builder image and starting a build container with it. If + // this happens, faith really just didn't want you to do this + // build. + d.start_build(config) } diff --git a/src/agent/images.v b/src/agent/images.v index dd32656..23b741d 100644 --- a/src/agent/images.v +++ b/src/agent/images.v @@ -33,16 +33,42 @@ pub fn (m &ImageManager) get(base_image string) string { return m.images[base_image].last() } -// refresh_image builds a new builder image from the given base image if the -// previous builder image is too old or non-existent. This function will do -// nothing if these conditions aren't met, so it's safe to call it every time -// you want to ensure an image is up to date. -fn (mut m ImageManager) refresh_image(base_image string) ! { - if base_image in m.timestamps - && m.timestamps[base_image].add_seconds(m.max_image_age) > time.now() { - return +// up_to_date returns whether the last known builder image is exists and is up +// to date. If this function returns true, the last builder image may be used +// to perform a build. +pub fn (mut m ImageManager) up_to_date(base_image string) bool { + if base_image !in m.timestamps + || m.timestamps[base_image].add_seconds(m.max_image_age) <= time.now() { + return false } + // It's possible the image has been removed by some external event, so we + // check whether it actually exists as well. + mut dd := docker.new_conn() or { return false } + + defer { + dd.close() or {} + } + + dd.image_inspect(m.images[base_image].last()) or { + // Image doesn't exist, so we stop tracking it + if err.code() == 404 { + m.images[base_image].delete_last() + m.timestamps.delete(base_image) + } + + // If the inspect fails, it's either because the image doesn't exist or + // because of some other error. Either we can't know *for certain* that + // the image exists, so we return false. + return false + } + + return true +} + +// refresh_image builds a new builder image from the given base image. This +// function should only be called if `up_to_date` return false. +fn (mut m ImageManager) refresh_image(base_image string) ! { // TODO use better image tags for built images new_image := build.create_build_image(base_image) or { return error('Failed to build builder image from base image $base_image') From af4c9e1d004ac2d6277f1333d95963eda1cf3f02 Mon Sep 17 00:00:00 2001 From: Chewing_Bever Date: Fri, 16 Dec 2022 16:35:40 +0100 Subject: [PATCH 09/10] chore: updated changelog --- CHANGELOG.md | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c55e16b..54d833a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,24 +7,30 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased](https://git.rustybever.be/vieter-v/vieter/src/branch/dev) +### Added + +* Allow specifying subdirectory inside Git repository +* Added option to deploy using agent-server architecture instead of cron daemon +* Allow scheduling builds on the server from the CLI tool instead of building + them locally +* Allow force-building packages, meaning the build won't check if the + repository is already up to date + ### Changed * Migrated codebase to V 0.3.2 * Cron expression parser now uses bitfields instead of bool arrays -* Added option to deploy using agent-server architecture instead of cron daemon -* Allow force-building packages, meaning the build won't check if the - repository is already up to date -* Allow scheduling builds on the server from the CLI tool instead of building - them locally ### Fixed * Arch value for target is now properly set if not provided -* All API endpoints now return proper JSON on success - * CLI no longer exits with non-zero status code when removing/patching - target * Allow NULL values for branch in database * Endpoint for adding targets now returns the correct id +* CLI now correctly errors and doesn't error when sending requests +* Fixed possible infinite loop when removing old build images +* Check whether build image still exists before starting build +* Don't run makepkg `prepare()` function twice +* Don't buffer stdout in Docker containers ## [0.4.0](https://git.rustybever.be/vieter-v/vieter/src/tag/0.4.0) From fe3e6e2babce6b41973b10599d9c38c5ba09dcc1 Mon Sep 17 00:00:00 2001 From: Chewing_Bever Date: Fri, 16 Dec 2022 18:18:25 +0100 Subject: [PATCH 10/10] chore: some final revisions before pr merge --- src/agent/images.v | 12 ++++++------ src/client/client.v | 12 ++++++------ src/console/targets/targets.v | 21 +++++++++------------ 3 files changed, 21 insertions(+), 24 deletions(-) diff --git a/src/agent/images.v b/src/agent/images.v index 23b741d..1fec567 100644 --- a/src/agent/images.v +++ b/src/agent/images.v @@ -33,9 +33,9 @@ pub fn (m &ImageManager) get(base_image string) string { return m.images[base_image].last() } -// up_to_date returns whether the last known builder image is exists and is up -// to date. If this function returns true, the last builder image may be used -// to perform a build. +// up_to_date returns true if the last known builder image exists and is up to +// date. If this function returns true, the last builder image may be used to +// perform a build. pub fn (mut m ImageManager) up_to_date(base_image string) bool { if base_image !in m.timestamps || m.timestamps[base_image].add_seconds(m.max_image_age) <= time.now() { @@ -58,8 +58,8 @@ pub fn (mut m ImageManager) up_to_date(base_image string) bool { } // If the inspect fails, it's either because the image doesn't exist or - // because of some other error. Either we can't know *for certain* that - // the image exists, so we return false. + // because of some other error. Either way, we can't know *for certain* + // that the image exists, so we return false. return false } @@ -67,7 +67,7 @@ pub fn (mut m ImageManager) up_to_date(base_image string) bool { } // refresh_image builds a new builder image from the given base image. This -// function should only be called if `up_to_date` return false. +// function should only be called if `up_to_date` returned false. fn (mut m ImageManager) refresh_image(base_image string) ! { // TODO use better image tags for built images new_image := build.create_build_image(base_image) or { diff --git a/src/client/client.v b/src/client/client.v index 5f24197..cce4e70 100644 --- a/src/client/client.v +++ b/src/client/client.v @@ -57,12 +57,7 @@ fn (c &Client) send_request(method Method, url string, params map[string]stri // output as a Response object. fn (c &Client) send_request_with_body(method Method, url string, params map[string]string, body string) !Response { res := c.send_request_raw(method, url, params, body)! - status := http.status_from_int(res.status_code) - - // Just return an empty successful response - if status.is_success() && res.body == '' { - return new_data_response(T{}) - } + status := res.status() // Non-successful requests are expected to return either an empty body or // Response @@ -77,6 +72,11 @@ fn (c &Client) send_request_with_body(method Method, url string, params map[s return error('Status $res.status_code ($status.str()): $data.message') } + // Just return an empty successful response + if res.body == '' { + return new_data_response(T{}) + } + data := json.decode(Response, res.body)! return data diff --git a/src/console/targets/targets.v b/src/console/targets/targets.v index a134926..94deebd 100644 --- a/src/console/targets/targets.v +++ b/src/console/targets/targets.v @@ -13,7 +13,7 @@ struct Config { base_image string = 'archlinux:base-devel' } -// cmd returns the cli submodule that handles the repos API interaction +// cmd returns the cli submodule that handles the targets API interaction pub fn cmd() cli.Command { return cli.Command{ name: 'targets' @@ -236,14 +236,11 @@ pub fn cmd() cli.Command { } } -// get_repo_by_prefix tries to find the repo with the given prefix in its -// ID. If multiple or none are found, an error is raised. - // list prints out a list of all repositories. fn list(conf Config, filter TargetFilter, raw bool) ! { c := client.new(conf.address, conf.api_key) - repos := c.get_targets(filter)! - data := repos.map([it.id.str(), it.kind, it.url, it.repo]) + targets := c.get_targets(filter)! + data := targets.map([it.id.str(), it.kind, it.url, it.repo]) if raw { println(console.tabbed_table(data)) @@ -252,7 +249,7 @@ fn list(conf Config, filter TargetFilter, raw bool) ! { } } -// add adds a new repository to the server's list. +// add adds a new target to the server's list. fn add(conf Config, t &NewTarget, raw bool) ! { c := client.new(conf.address, conf.api_key) target_id := c.add_target(t)! @@ -264,13 +261,13 @@ fn add(conf Config, t &NewTarget, raw bool) ! { } } -// remove removes a repository from the server's list. +// remove removes a target from the server's list. fn remove(conf Config, id string) ! { c := client.new(conf.address, conf.api_key) c.remove_target(id.int())! } -// patch patches a given repository with the provided params. +// patch patches a given target with the provided params. fn patch(conf Config, id string, params map[string]string) ! { // We check the cron expression first because it's useless to send an // invalid one to the server. @@ -284,9 +281,9 @@ fn patch(conf Config, id string, params map[string]string) ! { c.patch_target(id.int(), params)! } -// info shows detailed information for a given repo. +// info shows detailed information for a given target. fn info(conf Config, id string) ! { c := client.new(conf.address, conf.api_key) - repo := c.get_target(id.int())! - println(repo) + target := c.get_target(id.int())! + println(target) }