thr3ads.net - Libguestfs - [Libguestfs] [PATCH 3/5] mltools: add simple tests for external

If this information is useful, please help other people find it:
Share via:

Pino Toscano

2019-Jan-16 14:17 UTC

[Libguestfs] [PATCH 0/5] [RFC] builder: handle unavailable repos

In case a repository of virt-builder references files (e.g. the index)
that cannot be downloaded (network issues, 404, etc) then virt-builder
errors out on this situation. This is not a nice situation, from an user
POV.

This series does some refactoring to allow to better handle downloading
failures, and handle the failures gracefully in virt-builder.

RFC because I'm not yet too convinced the approach I used (especially
for the changes in the Curl module) is optimal, so looking for feedback
on this.

Pino Toscano (5):
  mltools: split helper do_check_exitcode
  mltools: create a new external_command_code
  mltools: add simple tests for external_command
  mltools: curl: turn Curl.run to raise exceptions
  builder: ignore repositories with download failures

 builder/builder.ml                  | 39 ++++++++++++++++++++---------
 common/mltools/curl.ml              | 15 ++++++++++-
 common/mltools/curl.mli             |  3 +++
 common/mltools/tools_utils.ml       | 26 +++++++++----------
 common/mltools/tools_utils.mli      |  8 ++++++
 common/mltools/tools_utils_tests.ml | 22 ++++++++++++++++
 v2v/copy_to_local.ml                |  9 ++++++-
 v2v/v2v.ml                          |  9 ++++++-
 8 files changed, 103 insertions(+), 28 deletions(-)

-- 
2.20.1

Pino Toscano

2019-Jan-16 14:17 UTC

head link

[Libguestfs] [PATCH 1/5] mltools: split helper do_check_exitcode

It will be used later in more parts of Tools_utils.

Simple refactoring with no behaviour changes.
---
 common/mltools/tools_utils.ml | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/common/mltools/tools_utils.ml b/common/mltools/tools_utils.ml
index 24641369e..3b1554c5a 100644
--- a/common/mltools/tools_utils.ml
+++ b/common/mltools/tools_utils.ml
@@ -427,7 +427,9 @@ and do_run ?(echo_cmd = true) ?stdout_fd ?stderr_fd args 
and do_teardown app outfd errfd exitstat    Option.may Unix.close outfd;
   Option.may Unix.close errfd;
-  match exitstat with
+  do_check_exitcode app exitstat
+
+and do_check_exitcode app = function
   | Unix.WEXITED i ->
     i
   | Unix.WSIGNALED i ->
-- 
2.20.1

Pino Toscano

2019-Jan-16 14:17 UTC

head link

[Libguestfs] [PATCH 2/5] mltools: create a new external_command_code

Split most of the code from external_command to a new
external_command_code, so it is possible to get the exit code of the
process without considering it fatal.
---
 common/mltools/tools_utils.ml  | 22 ++++++++++------------
 common/mltools/tools_utils.mli |  8 ++++++++
 2 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/common/mltools/tools_utils.ml b/common/mltools/tools_utils.ml
index 3b1554c5a..e6b2b5713 100644
--- a/common/mltools/tools_utils.ml
+++ b/common/mltools/tools_utils.ml
@@ -338,7 +338,13 @@ let create_standard_options argspec ?anon_fun ?(key_opts =
false) ?(machine_read
   }
 
 (* Run an external command, slurp up the output as a list of lines. *)
-let external_command ?(echo_cmd = true) cmd +let rec external_command
?(echo_cmd = true) cmd +  let lines, exitcode = external_command_code ~echo_cmd
cmd in
+  if exitcode <> 0 then
+    error (f_"external command ‘%s’ exited with error %d") cmd
exitcode;
+  lines
+
+and external_command_code ?(echo_cmd = true) cmd    if echo_cmd then
     debug "%s" cmd;
   let chan = Unix.open_process_in cmd in
@@ -347,18 +353,10 @@ let external_command ?(echo_cmd = true) cmd     with
End_of_file -> ());
   let lines = List.rev !lines in
   let stat = Unix.close_process_in chan in
-  (match stat with
-  | Unix.WEXITED 0 -> ()
-  | Unix.WEXITED i ->
-    error (f_"external command ‘%s’ exited with error %d") cmd i
-  | Unix.WSIGNALED i ->
-    error (f_"external command ‘%s’ killed by signal %d") cmd i
-  | Unix.WSTOPPED i ->
-    error (f_"external command ‘%s’ stopped by signal %d") cmd i
-  );
-  lines
+  let exitcode = do_check_exitcode cmd stat in
+  lines, exitcode
 
-let rec run_commands ?(echo_cmd = true) cmds +and run_commands ?(echo_cmd =
true) cmds    let res = Array.make (List.length cmds) 0 in
   let pids      List.mapi (
diff --git a/common/mltools/tools_utils.mli b/common/mltools/tools_utils.mli
index ab70f583e..fb998697c 100644
--- a/common/mltools/tools_utils.mli
+++ b/common/mltools/tools_utils.mli
@@ -100,6 +100,14 @@ val create_standard_options : Getopt.speclist ->
?anon_fun:Getopt.anon_fun -> ?k
 
 val external_command : ?echo_cmd:bool -> string -> string list
 (** Run an external command, slurp up the output as a list of lines.
+    A non-zero exit code of the command is considered a fatal error.
+
+    [echo_cmd] specifies whether to output the full command on verbose
+    mode, and it's on by default. *)
+
+val external_command_code : ?echo_cmd:bool -> string -> string list * int
+(** Run an external command, slurp up the output as a list of lines,
+    and return it together with the exit code of the command.
 
     [echo_cmd] specifies whether to output the full command on verbose
     mode, and it's on by default. *)
-- 
2.20.1

Pino Toscano

2019-Jan-16 14:17 UTC

head link

[Libguestfs] [PATCH 3/5] mltools: add simple tests for external_command

---
 common/mltools/tools_utils_tests.ml | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/common/mltools/tools_utils_tests.ml
b/common/mltools/tools_utils_tests.ml
index 1489fe699..f3c39514d 100644
--- a/common/mltools/tools_utils_tests.ml
+++ b/common/mltools/tools_utils_tests.ml
@@ -28,6 +28,7 @@ let assert_equal_string = assert_equal ~printer:(fun x ->
x)
 let assert_equal_int = assert_equal ~printer:(fun x -> string_of_int x)
 let assert_equal_int64 = assert_equal ~printer:(fun x -> Int64.to_string x)
 let assert_equal_intlist = assert_equal ~printer:(fun x -> "(" ^
(String.concat ";" (List.map string_of_int x)) ^ ")")
+let assert_equal_stringlist = assert_equal ~printer:(fun x -> "("
^ (String.escaped (String.concat "," x)) ^ ")")
 
 (* Test Tools_utils.parse_size and Tools_utils.parse_resize. *)
 let test_parse_resize ctx @@ -156,6 +157,26 @@ let test_run_commands ctx   
end;
   ()
 
+let test_external_command ctx +  assert_equal_stringlist [] (external_command
"true");
+  assert_equal_stringlist ["out"] (external_command "echo
out");
+  begin
+    let lines, code = external_command_code "true" in
+    assert_equal_int 0 code;
+    assert_equal_stringlist [] lines
+  end;
+  begin
+    let lines, code = external_command_code "false" in
+    assert_equal_int 1 code;
+    assert_equal_stringlist [] lines
+  end;
+  begin
+    let lines, code = external_command_code "echo out" in
+    assert_equal_int 0 code;
+    assert_equal_stringlist ["out"] lines
+  end;
+  ()
+
 (* Suites declaration. *)
 let suite    "mltools Tools_utils" >:::
@@ -164,6 +185,7 @@ let suite        "sizes.human_size" >::
test_human_size;
       "run_command" >:: test_run_command;
       "run_commands" >:: test_run_commands;
+      "external_command" >:: test_external_command;
     ]
 
 let () -- 
2.20.1

Pino Toscano

2019-Jan-16 14:17 UTC

head link

[Libguestfs] [PATCH 4/5] mltools: curl: turn Curl.run to raise exceptions

Add a new Curl_failed exception, and raise it when Curl.run fails,
instead of exiting directly with error.  This allows users of Curl to
handle failed downloads gracefully.

Add wrappers to the "main" functions of virt-builder, virt-v2v, and
virt-v2v-copy-to-local to catch Curl_failed, and show a better message
for them.
---
 builder/builder.ml      |  9 ++++++++-
 common/mltools/curl.ml  | 15 ++++++++++++++-
 common/mltools/curl.mli |  3 +++
 v2v/copy_to_local.ml    |  9 ++++++++-
 v2v/v2v.ml              |  9 ++++++++-
 5 files changed, 41 insertions(+), 4 deletions(-)

diff --git a/builder/builder.ml b/builder/builder.ml
index c7cbcaedb..651db83f0 100644
--- a/builder/builder.ml
+++ b/builder/builder.ml
@@ -791,4 +791,11 @@ let main ()  
   Option.may print_string stats
 
-let () = run_main_and_handle_errors main
+let () +  let main_wrap () +    try main ()
+    with
+    | Curl.Curl_failed (code, url) ->
+      error (f_"curl error: failed to download ‘%s’, error code %d")
url code
+  in
+  run_main_and_handle_errors main_wrap
diff --git a/common/mltools/curl.ml b/common/mltools/curl.ml
index 6dba97534..eb32a4748 100644
--- a/common/mltools/curl.ml
+++ b/common/mltools/curl.ml
@@ -40,6 +40,8 @@ let args_of_proxy = function
   | SystemProxy ->     []
   | ForcedProxy url -> [ "proxy", Some url; "noproxy",
Some "" ]
 
+exception Curl_failed of (int * string) (* exit code, URL *)
+
 let create ?(curl = "curl") ?(proxy = SystemProxy) ?tmpdir args   
let args = safe_args @ args_of_proxy proxy @ args in
   { curl = curl; args = args; tmpdir = tmpdir }
@@ -71,8 +73,19 @@ let run { curl; args; tmpdir }    close_out chan;
 
   let cmd = sprintf "%s -q --config %s" (quote curl) (quote
config_file) in
-  let lines = external_command ~echo_cmd:false cmd in
+  let lines, exitcode = external_command_code ~echo_cmd:false cmd in
   Unix.unlink config_file;
+  if exitcode <> 0 then (
+    let url +      try
+        List.find_map (
+          function
+          | "url", Some url -> Some url
+          | _, _ -> None
+        ) args
+      with Not_found -> s_"(unknown)" in
+    raise (Curl_failed (exitcode, url))
+  );
   lines
 
 let to_string { curl; args } diff --git a/common/mltools/curl.mli
b/common/mltools/curl.mli
index a3e98dc68..34507b4a2 100644
--- a/common/mltools/curl.mli
+++ b/common/mltools/curl.mli
@@ -27,6 +27,9 @@ type proxy    | SystemProxy           (** Use the system
settings. *)
   | ForcedProxy of string (** The proxy is forced to the specified URL. *)
 
+exception Curl_failed of (int * string) (* exit code, URL *)
+(** Exception thrown by [run] when the download fails. *)
+
 val create : ?curl:string -> ?proxy:proxy -> ?tmpdir:string -> args
-> t
 (** Create a curl command handle.
 
diff --git a/v2v/copy_to_local.ml b/v2v/copy_to_local.ml
index 408cbdebc..e2749fa0c 100644
--- a/v2v/copy_to_local.ml
+++ b/v2v/copy_to_local.ml
@@ -311,4 +311,11 @@ and parse_libvirt_xml guest_name xml    let xml =
Xml.to_string doc ~format:true in
   get_disks (), dcpath, xml
 
-let () = run_main_and_handle_errors main
+let () +  let main_wrap () +    try main ()
+    with
+    | Curl.Curl_failed (code, url) ->
+      error (f_"curl error: failed to download ‘%s’, error code %d")
url code
+  in
+  run_main_and_handle_errors main_wrap
diff --git a/v2v/v2v.ml b/v2v/v2v.ml
index 277d8f2c7..1b3ec0b6c 100644
--- a/v2v/v2v.ml
+++ b/v2v/v2v.ml
@@ -870,4 +870,11 @@ and rcaps_from_source source      rcaps_video = video;
   }
 
-let () = run_main_and_handle_errors main
+let () +  let main_wrap () +    try main ()
+    with
+    | Curl.Curl_failed (code, url) ->
+      error (f_"curl error: failed to download ‘%s’, error code %d")
url code
+  in
+  run_main_and_handle_errors main_wrap
-- 
2.20.1

Pino Toscano

2019-Jan-16 14:17 UTC

head link

[Libguestfs] [PATCH 5/5] builder: ignore repositories with download failures

If any URL of a repository (the index itself, or any of the other
resources associated except the templates themselves) cannot be
downloaded, ignore the repository with a warning message.

This way, if a repository is temporary unavailable, the rest of the
repositories can still be used.
---
 builder/builder.ml | 30 +++++++++++++++++++-----------
 1 file changed, 19 insertions(+), 11 deletions(-)

diff --git a/builder/builder.ml b/builder/builder.ml
index 651db83f0..b8d36f6fa 100644
--- a/builder/builder.ml
+++ b/builder/builder.ml
@@ -199,18 +199,26 @@ let main ()    let sources = List.append sources repos in
   let index : Index.index      List.concat (
-      List.map (
+      List.filter_map (
         fun source ->
-          let sigchecker -            Sigchecker.create ~gpg:cmdline.gpg
-                              ~check_signature:cmdline.check_signature
-                              ~gpgkey:source.Sources.gpgkey
-                              ~tmpdir in
-          match source.Sources.format with
-          | Sources.FormatNative ->
-            Index_parser.get_index ~downloader ~sigchecker source
-          | Sources.FormatSimpleStreams ->
-            Simplestreams_parser.get_index ~downloader ~sigchecker source
+          try
+            let sigchecker +              Sigchecker.create ~gpg:cmdline.gpg
+                                ~check_signature:cmdline.check_signature
+                                ~gpgkey:source.Sources.gpgkey
+                                ~tmpdir in
+            let parsed_index +              match source.Sources.format with
+              | Sources.FormatNative ->
+                Index_parser.get_index ~downloader ~sigchecker source
+              | Sources.FormatSimpleStreams ->
+                Simplestreams_parser.get_index ~downloader ~sigchecker source
in
+            Some parsed_index
+          with
+          | Curl.Curl_failed (code, url) ->
+            warning (f_"failed to download ‘%s’ (code %d), ignoring
repository ‘%s’")
+              url code source.Sources.name;
+            None
       ) sources
     ) in
   let index = remove_duplicates index in
-- 
2.20.1

Richard W.M. Jones

2019-Jan-16 14:28 UTC

head link

Re: [Libguestfs] [PATCH 1/5] mltools: split helper do_check_exitcode

On Wed, Jan 16, 2019 at 03:17:31PM +0100, Pino Toscano
wrote:> It will be used later in more parts of Tools_utils.
> 
> Simple refactoring with no behaviour changes.
> ---
>  common/mltools/tools_utils.ml | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/common/mltools/tools_utils.ml b/common/mltools/tools_utils.ml
> index 24641369e..3b1554c5a 100644
> --- a/common/mltools/tools_utils.ml
> +++ b/common/mltools/tools_utils.ml
> @@ -427,7 +427,9 @@ and do_run ?(echo_cmd = true) ?stdout_fd ?stderr_fd
args >  and do_teardown app outfd errfd exitstat >    Option.may
Unix.close outfd;
>    Option.may Unix.close errfd;
> -  match exitstat with
> +  do_check_exitcode app exitstat
> +
> +and do_check_exitcode app = function
The change is fine, but I think it's probably better to call the
function ‘error_<something>’ which is consistent with the functions in
virt-v2v which call error.  Perhaps ‘error_if_signaled_or_stopped’?

But anyway, ACK.

Rich.
>    | Unix.WEXITED i ->
>      i
>    | Unix.WSIGNALED i ->
> -- 
> 2.20.1
> 
> _______________________________________________
> Libguestfs mailing list
> Libguestfs@redhat.com
> https://www.redhat.com/mailman/listinfo/libguestfs
-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org

Richard W.M. Jones

2019-Jan-16 14:31 UTC

head link

Re: [Libguestfs] [PATCH 2/5] mltools: create a new external_command_code

On Wed, Jan 16, 2019 at 03:17:32PM +0100, Pino Toscano
wrote:> Split most of the code from external_command to a new
> external_command_code, so it is possible to get the exit code of the
> process without considering it fatal.
> ---
>  common/mltools/tools_utils.ml  | 22 ++++++++++------------
>  common/mltools/tools_utils.mli |  8 ++++++++
>  2 files changed, 18 insertions(+), 12 deletions(-)
> 
> diff --git a/common/mltools/tools_utils.ml b/common/mltools/tools_utils.ml
> index 3b1554c5a..e6b2b5713 100644
> --- a/common/mltools/tools_utils.ml
> +++ b/common/mltools/tools_utils.ml
> @@ -338,7 +338,13 @@ let create_standard_options argspec ?anon_fun
?(key_opts = false) ?(machine_read
>    }
>  
>  (* Run an external command, slurp up the output as a list of lines. *)
> -let external_command ?(echo_cmd = true) cmd > +let rec external_command
?(echo_cmd = true) cmd > +  let lines, exitcode = external_command_code
~echo_cmd cmd in
> +  if exitcode <> 0 then
> +    error (f_"external command ‘%s’ exited with error %d") cmd
exitcode;
> +  lines
> +
> +and external_command_code ?(echo_cmd = true) cmd >    if echo_cmd then
>      debug "%s" cmd;
>    let chan = Unix.open_process_in cmd in
> @@ -347,18 +353,10 @@ let external_command ?(echo_cmd = true) cmd >    
with End_of_file -> ());
>    let lines = List.rev !lines in
>    let stat = Unix.close_process_in chan in
> -  (match stat with
> -  | Unix.WEXITED 0 -> ()
> -  | Unix.WEXITED i ->
> -    error (f_"external command ‘%s’ exited with error %d") cmd i
> -  | Unix.WSIGNALED i ->
> -    error (f_"external command ‘%s’ killed by signal %d") cmd i
> -  | Unix.WSTOPPED i ->
> -    error (f_"external command ‘%s’ stopped by signal %d") cmd i
> -  );
> -  lines
> +  let exitcode = do_check_exitcode cmd stat in
> +  lines, exitcode
>  
> -let rec run_commands ?(echo_cmd = true) cmds > +and run_commands
?(echo_cmd = true) cmds >    let res = Array.make (List.length cmds) 0 in
>    let pids >      List.mapi (
> diff --git a/common/mltools/tools_utils.mli
b/common/mltools/tools_utils.mli
> index ab70f583e..fb998697c 100644
> --- a/common/mltools/tools_utils.mli
> +++ b/common/mltools/tools_utils.mli
> @@ -100,6 +100,14 @@ val create_standard_options : Getopt.speclist ->
?anon_fun:Getopt.anon_fun -> ?k
>  
>  val external_command : ?echo_cmd:bool -> string -> string list
>  (** Run an external command, slurp up the output as a list of lines.
> +    A non-zero exit code of the command is considered a fatal error.
> +
> +    [echo_cmd] specifies whether to output the full command on verbose
> +    mode, and it's on by default. *)
> +
> +val external_command_code : ?echo_cmd:bool -> string -> string list
* int
> +(** Run an external command, slurp up the output as a list of lines,
> +    and return it together with the exit code of the command.
Can you amend the documentation to say something on the lines of:

  Run an external command, slurp up the output as a list of lines.
  If the command exits normally with any exit code then the exit
  code is returned.  If the command is signalled or stopped then
  that is a fatal error.

ACK with this or similar change.

BTW we use debug + Sys.command all over the place and it might be
worth considering replacing those instances with this new function
where appropriate.

Rich.
>      [echo_cmd] specifies whether to output the full command on verbose
>      mode, and it's on by default. *)
> -- 
> 2.20.1
> 
> _______________________________________________
> Libguestfs mailing list
> Libguestfs@redhat.com
> https://www.redhat.com/mailman/listinfo/libguestfs
-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top

Richard W.M. Jones

2019-Jan-16 14:32 UTC

head link

Re: [Libguestfs] [PATCH 3/5] mltools: add simple tests for external_command

On Wed, Jan 16, 2019 at 03:17:33PM +0100, Pino Toscano
wrote:> ---
>  common/mltools/tools_utils_tests.ml | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/common/mltools/tools_utils_tests.ml
b/common/mltools/tools_utils_tests.ml
> index 1489fe699..f3c39514d 100644
> --- a/common/mltools/tools_utils_tests.ml
> +++ b/common/mltools/tools_utils_tests.ml
> @@ -28,6 +28,7 @@ let assert_equal_string = assert_equal ~printer:(fun x
-> x)
>  let assert_equal_int = assert_equal ~printer:(fun x -> string_of_int x)
>  let assert_equal_int64 = assert_equal ~printer:(fun x ->
Int64.to_string x)
>  let assert_equal_intlist = assert_equal ~printer:(fun x ->
"(" ^ (String.concat ";" (List.map string_of_int x)) ^
")")
> +let assert_equal_stringlist = assert_equal ~printer:(fun x ->
"(" ^ (String.escaped (String.concat "," x)) ^
")")
>  
>  (* Test Tools_utils.parse_size and Tools_utils.parse_resize. *)
>  let test_parse_resize ctx > @@ -156,6 +157,26 @@ let test_run_commands
ctx >    end;
>    ()
>  
> +let test_external_command ctx > +  assert_equal_stringlist []
(external_command "true");
> +  assert_equal_stringlist ["out"] (external_command "echo
out");
> +  begin
> +    let lines, code = external_command_code "true" in
> +    assert_equal_int 0 code;
> +    assert_equal_stringlist [] lines
> +  end;
> +  begin
> +    let lines, code = external_command_code "false" in
> +    assert_equal_int 1 code;
> +    assert_equal_stringlist [] lines
> +  end;
> +  begin
> +    let lines, code = external_command_code "echo out" in
> +    assert_equal_int 0 code;
> +    assert_equal_stringlist ["out"] lines
> +  end;
begin..end isn't really necessary here, but it doesn't matter.
> +  ()
> +
>  (* Suites declaration. *)
>  let suite >    "mltools Tools_utils" >:::
> @@ -164,6 +185,7 @@ let suite >        "sizes.human_size"
>:: test_human_size;
>        "run_command" >:: test_run_command;
>        "run_commands" >:: test_run_commands;
> +      "external_command" >:: test_external_command;
>      ]
ACK.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW

Richard W.M. Jones

2019-Jan-16 14:38 UTC

head link

Re: [Libguestfs] [PATCH 4/5] mltools: curl: turn Curl.run to raise exceptions

I can see where you're going with this, but it does make the Curl
module more error-prone.  You need to use an exception since there is
a long distance between where the Curl command fails and where you
want to handle the failure.

I would say that you should add a special variation of [Curl.run]
which raises an exception and leave the existing [Curl.run] alone.

Existing users won't be accidentally affected while virt-builder can
use the exception-raising variant where appropriate.

At the same time I would also change common/mltools/tools_utils.ml:
run_main_and_handle_errors so it knows about the new exception and can
print a suitable (internal?) error.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/

Richard W.M. Jones

2019-Jan-16 14:42 UTC

head link

Re: [Libguestfs] [PATCH 5/5] builder: ignore repositories with download failures

On Wed, Jan 16, 2019 at 03:17:35PM +0100, Pino Toscano
wrote:> If any URL of a repository (the index itself, or any of the other
> resources associated except the templates themselves) cannot be
> downloaded, ignore the repository with a warning message.
> 
> This way, if a repository is temporary unavailable, the rest of the
> repositories can still be used.
It's worth noting that DNF has a special flag
‘skip_if_unavailableTrue|False’ to indicate repositories which are skipped with
a warning
versus an error.  Well it's not _quite_ the same thing because AIUI
DNF will disable repositories (permanently I think?) if they are
unavailable, which we don't want, but adding some sort of skippable
flag might be worth considering for virt-builder anyway.

   skip_if_unavailable
          boolean

          If enabled, DNF will continue running and disable the repository
          that couldn't be contacted for any reason when downloading meta‐
          data.  This  option doesn't affect skipping of unavailable pack‐
          ages after dependency resolution. To  check  inaccessibility  of
          repository  use  it  in  combination  with  refresh command line
          option. The default is True.

Rich.
>  builder/builder.ml | 30 +++++++++++++++++++-----------
>  1 file changed, 19 insertions(+), 11 deletions(-)
> 
> diff --git a/builder/builder.ml b/builder/builder.ml
> index 651db83f0..b8d36f6fa 100644
> --- a/builder/builder.ml
> +++ b/builder/builder.ml
> @@ -199,18 +199,26 @@ let main () >    let sources = List.append sources
repos in
>    let index : Index.index >      List.concat (
> -      List.map (
> +      List.filter_map (
>          fun source ->
> -          let sigchecker > -            Sigchecker.create
~gpg:cmdline.gpg
> -                              ~check_signature:cmdline.check_signature
> -                              ~gpgkey:source.Sources.gpgkey
> -                              ~tmpdir in
> -          match source.Sources.format with
> -          | Sources.FormatNative ->
> -            Index_parser.get_index ~downloader ~sigchecker source
> -          | Sources.FormatSimpleStreams ->
> -            Simplestreams_parser.get_index ~downloader ~sigchecker source
> +          try
> +            let sigchecker > +              Sigchecker.create
~gpg:cmdline.gpg
> +                                ~check_signature:cmdline.check_signature
> +                                ~gpgkey:source.Sources.gpgkey
> +                                ~tmpdir in
> +            let parsed_index > +              match
source.Sources.format with
> +              | Sources.FormatNative ->
> +                Index_parser.get_index ~downloader ~sigchecker source
> +              | Sources.FormatSimpleStreams ->
> +                Simplestreams_parser.get_index ~downloader ~sigchecker
source in
> +            Some parsed_index
> +          with
> +          | Curl.Curl_failed (code, url) ->
> +            warning (f_"failed to download ‘%s’ (code %d), ignoring
repository ‘%s’")
> +              url code source.Sources.name;
> +            None
>        ) sources
>      ) in
>    let index = remove_duplicates index in
> -- 
> 2.20.1
> 
> _______________________________________________
> Libguestfs mailing list
> Libguestfs@redhat.com
> https://www.redhat.com/mailman/listinfo/libguestfs
-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/

Reasonably Related Threads

Search for more maybe matching threads

Libguestfs - Jan 2019 - [PATCH 3/5] mltools: add simple tests for external_command

[Libguestfs] [PATCH 0/5] [RFC] builder: handle unavailable repos

[Libguestfs] [PATCH 1/5] mltools: split helper do_check_exitcode

[Libguestfs] [PATCH 2/5] mltools: create a new external_command_code

[Libguestfs] [PATCH 3/5] mltools: add simple tests for external_command

[Libguestfs] [PATCH 4/5] mltools: curl: turn Curl.run to raise exceptions

[Libguestfs] [PATCH 5/5] builder: ignore repositories with download failures

Re: [Libguestfs] [PATCH 1/5] mltools: split helper do_check_exitcode

Re: [Libguestfs] [PATCH 2/5] mltools: create a new external_command_code

Re: [Libguestfs] [PATCH 3/5] mltools: add simple tests for external_command

Re: [Libguestfs] [PATCH 4/5] mltools: curl: turn Curl.run to raise exceptions

Re: [Libguestfs] [PATCH 5/5] builder: ignore repositories with download failures

Reasonably Related Threads