[libvirt] [jenkins-ci PATCH v2] lcitool: check for virt-install / ansible-playbook in $PATH

Daniel P. Berrangé posted 1 patch 4 years, 11 months ago
Failed in applying to current master (apply log)
guests/lcitool | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
[libvirt] [jenkins-ci PATCH v2] lcitool: check for virt-install / ansible-playbook in $PATH
Posted by Daniel P. Berrangé 4 years, 11 months ago
This improves error reporting:

  $ ./lcitool install libvirt-fedora-29
  ./lcitool: Failed to install 'libvirt-fedora-29': [Errno 2] No such file or directory

To

  $ ./lcitool install libvirt-fedora-29
  ./lcitool: Cannot find virt-install in $PATH

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 guests/lcitool | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/guests/lcitool b/guests/lcitool
index 0f60704..d3937be 100755
--- a/guests/lcitool
+++ b/guests/lcitool
@@ -17,6 +17,7 @@
 # with this program. If not, see <https://www.gnu.org/licenses/>.
 
 import argparse
+import distutils.spawn
 import fnmatch
 import json
 import os
@@ -461,8 +462,12 @@ class Application:
             "git_branch": git_branch,
         })
 
+        ap_path = distutils.spawn.find_executable("ansible-playbook")
+        if ap_path is None:
+            raise Exception("Cannot find ansible-playbook in $PATH")
+
         cmd = [
-            "ansible-playbook",
+            ap_path,
             "--limit", ansible_hosts,
             "--extra-vars", extra_vars,
         ]
@@ -534,8 +539,12 @@ class Application:
             # a kernel argument
             extra_arg = "console=ttyS0 ks=file:/{}".format(install_config)
 
+            vi_path = distutils.spawn.find_executable("virt-install")
+            if vi_path is None:
+                raise Exception("Cannot find virt-install in $PATH")
+
             cmd = [
-                "virt-install",
+                vi_path,
                 "--name", host,
                 "--location", facts["install_url"],
                 "--virt-type", facts["install_virt_type"],
-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH v2] lcitool: check for virt-install / ansible-playbook in $PATH
Posted by Andrea Bolognani 4 years, 11 months ago
On Fri, 2019-05-03 at 11:02 +0100, Daniel P. Berrangé wrote:
> This improves error reporting:
> 
>   $ ./lcitool install libvirt-fedora-29
>   ./lcitool: Failed to install 'libvirt-fedora-29': [Errno 2] No such file or directory
> 
> To
> 
>   $ ./lcitool install libvirt-fedora-29
>   ./lcitool: Cannot find virt-install in $PATH

Much nicer indeed!

> @@ -461,8 +462,12 @@ class Application:
>              "git_branch": git_branch,
>          })
>  
> +        ap_path = distutils.spawn.find_executable("ansible-playbook")

Please call this 'ansible_playbook' instead of 'ap_path'...

> @@ -534,8 +539,12 @@ class Application:
>              # a kernel argument
>              extra_arg = "console=ttyS0 ks=file:/{}".format(install_config)
>  
> +            vi_path = distutils.spawn.find_executable("virt-install")

... and same here.

Other than that, this works and AFAICT addresses all issues Pino
raised for v1, so

  Reviewed-by: Andrea Bolognani <abologna@redhat.com>

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH v2] lcitool: check for virt-install / ansible-playbook in $PATH
Posted by Daniel P. Berrangé 4 years, 11 months ago
On Fri, May 03, 2019 at 05:37:58PM +0200, Andrea Bolognani wrote:
> On Fri, 2019-05-03 at 11:02 +0100, Daniel P. Berrangé wrote:
> > This improves error reporting:
> > 
> >   $ ./lcitool install libvirt-fedora-29
> >   ./lcitool: Failed to install 'libvirt-fedora-29': [Errno 2] No such file or directory
> > 
> > To
> > 
> >   $ ./lcitool install libvirt-fedora-29
> >   ./lcitool: Cannot find virt-install in $PATH
> 
> Much nicer indeed!
> 
> > @@ -461,8 +462,12 @@ class Application:
> >              "git_branch": git_branch,
> >          })
> >  
> > +        ap_path = distutils.spawn.find_executable("ansible-playbook")
> 
> Please call this 'ansible_playbook' instead of 'ap_path'...
> 
> > @@ -534,8 +539,12 @@ class Application:
> >              # a kernel argument
> >              extra_arg = "console=ttyS0 ks=file:/{}".format(install_config)
> >  
> > +            vi_path = distutils.spawn.find_executable("virt-install")
> 
> ... and same here.

I'll call this one  "virt_install" rather than "ansible_playbook" ;-P


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH v2] lcitool: check for virt-install / ansible-playbook in $PATH
Posted by Andrea Bolognani 4 years, 11 months ago
On Fri, 2019-05-03 at 16:41 +0100, Daniel P. Berrangé wrote:
> On Fri, May 03, 2019 at 05:37:58PM +0200, Andrea Bolognani wrote:
> > > @@ -461,8 +462,12 @@ class Application:
> > >              "git_branch": git_branch,
> > >          })
> > >  
> > > +        ap_path = distutils.spawn.find_executable("ansible-playbook")
> > 
> > Please call this 'ansible_playbook' instead of 'ap_path'...
> > 
> > > @@ -534,8 +539,12 @@ class Application:
> > >              # a kernel argument
> > >              extra_arg = "console=ttyS0 ks=file:/{}".format(install_config)
> > >  
> > > +            vi_path = distutils.spawn.find_executable("virt-install")
> > 
> > ... and same here.
> 
> I'll call this one  "virt_install" rather than "ansible_playbook" ;-P

Since you insist, I'll begrudgingly accept this compromise :P

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list