Re: [Xen-devel] [OSSTEST PATCH v6 1/3] ts-openstack-deploy: Deploy OpenStack on a host with devstack

[Ian Jackson]

Anthony PERARD writes ("[OSSTEST PATCH v6 1/3] ts-openstack-deploy: Deploy 
OpenStack on a host with devstack"):
> This script installs any necessary packages and clones all of the OpenStack
> trees which are used by devstack to deploy OpenStack.

Thanks for this contribution.  I have no knowledge of OpenStack so I
can't really know whether what you are doing here is right, but it
looks basically OK to me.


I have some minor comments.  The most annoying one will be that I
would like you to file bugs in appropriate upstream bug trackers
(OpenStack or Debian), for each of the the workarounds.  Sorry...



> +sub packages () {
> +  # Install open-iscsi ahead of devstack ...
> +  target_install_packages($ho, qw(git sudo open-iscsi));

The indentation is a bit odd.  I won't insist you fix it, but usual
indent in osstest is 4 spaces.

> +  # ... and start open-iscsi to have /etc/iscsi/initiatorname.iscsi
> +  # generated. This is done on install on Ubuntu.
> +  target_cmd_root($ho, 'service open-iscsi start');

This looks like it is a workaround.  Can you please
 1. file a bug against Debian
 2. quote the bug number in your code
 3. make the workaround conditional on the suite

> +sub checkout () {
> +  prepbuilddirs();
> +  build_clone($ho, 'cinder', $builddir, 'cinder');
> +  build_clone($ho, 'devstack', $builddir, 'devstack');
> +  build_clone($ho, 'glance', $builddir, 'glance');
> +  build_clone($ho, 'keystone', $builddir, 'keystone');
> +  build_clone($ho, 'nova', $builddir, 'nova');
> +  build_clone($ho, 'requirements', $builddir, 'requirements');
> +  build_clone($ho, 'tempest', $builddir, 'tempest');

This could profitably be reformatted with extra whitespace so that the
columns line up.  (Again I won't insist on this.)

> +LOGFILE=\$DEST/logs/stack.sh.log
> +# stackrc set this but don't take \$DEST into account

ITYM "sets this but doesn't take".

> +  # stackrc does not take $DEST from local.conf into account, so fix it here
> +  target_editfile($ho, "$builddir/devstack/stackrc", sub {

This is a workround for an upstream bug ?  Is there a corresponding
upstream bug report ?  Should there be ?

In osstest I like to reduce, where possible, unconditional workarounds
for bugs in the software under test.  In general there should be an
upstream bug report, referred to in the osstest code.  Ideally there
would be a way to automatically disable the workaround eventually,
although that's probably too difficult here.

> +  while () {
> +if (m/^DEST=\/opt\/stack$/) {
> +  s/DEST=\/opt\/stack/DEST=$builddir/;

Using a regexp delimiter other than // would make this much clearer.

  +if (m{^DEST=/opt/stack$}) {
  +if (m#^DEST=/opt/stack$}) {
  +  s{DEST=/opt/stack}{DEST=$builddir};
  +  s#DEST=/opt/stack#DEST=$builddir#;

For example.  Take your pick.  I see later you used % % which is also
OK, although it is not the most idiomatic Perl (since the human reader
will tend to see % as a reference to a hash).

> +  # libvirt is already installed, but not as a package, so avoid 
> installation of
> +  # the libvirt package with devstack
> +  target_editfile($ho, "$builddir/devstack/files/debs/nova", sub {
> +  while () {
> +next if m/.*libvirt.*/;
> +print EO or die $!;
> +  }
> +  });
> +  target_editfile($ho, 
> "$builddir/devstack/lib/nova_plugins/functions-libvirt", sub {
> +  while () {
> +next if m/install_package.*libvirt.*/;
> +print EO or die $!;
> +  }
> +  });

These look like they ought to come with an upstream wishlist bug
asking for a front-door way to achieve this.

> +  # devstack blindly assume that systemd is used if systemctl is present
> +  target_editfile($ho, "$builddir/devstack/functions-common", sub {
> +  while () {
> +if (m%\[ -x /bin/systemctl%) {
> +  s%\[ -x /bin/systemctl \]%false%
> +}
> +print EO or die $!;
> +  }
> +  });

Again, an upstream bug.

> +  # OpenStack needs access to libvirt from a user.
> +  target_cmd_root($ho, < +if ! getent group libvirt >/dev/null; then
> +  groupadd libvirt

You should probably use "addgroup --system".  That is idempotent, so
you do not need the test.

> +# For unknown reason, restart fail when done by devstack
> +# so stop the service here, and devstack will start it.

Again, upstream bug.

> +  # devstack is going to setup the host, install some dependency.
> +  target_putfilecontents_root_stash($ho, 100, 
> < +  target_putfilecontents_root_stash($ho, 60, tgt_init(), "/etc/init.d/tgt");
> +  target_cmd_root($ho, < +chmod +x /etc/init.d/tgt
> +END
> +}

You may find it better to combine some of these target_cmd_root
stanzas.  That would reduce the overall amount of clutter in the perl
code.

> +# This is missing from Debian but 

[Xen-devel] [OSSTEST PATCH v6 1/3] ts-openstack-deploy: Deploy OpenStack on a host with devstack

[Anthony PERARD]

This script installs any necessary packages and clones all of the OpenStack
trees which are used by devstack to deploy OpenStack.

Signed-off-by: Anthony PERARD 

---
Changes in V6:
- rebased
- fix issues due to new debian and newer devstack:
  - add missing libvirt group
  - switch back to old nova-network instead of neutron
  - have devstack use 'service' instead of 'systemctl' to restart
services

Only change in V5:
- edit stackrc from devstack file to change the hardcoded path DEST

No change in V4:
- acked

Change in V3:
- Use host as argument to run the job.
- Use selectjob() and get rid of the unused $gho.
- Use target_jobdir() instead of builddirsprops().
- Remove GIT_BASE from devstack config.
- Rename the script to ts-openstack-deploy (from ts-openstack-devstack).
---
 sg-run-job  |   5 +
 ts-openstack-deploy | 338 
 2 files changed, 343 insertions(+)
 create mode 100755 ts-openstack-deploy

diff --git a/sg-run-job b/sg-run-job
index 9f8d003..5146dd1 100755
--- a/sg-run-job
+++ b/sg-run-job
@@ -474,6 +474,11 @@ proc run-job/test-rumprun {} {
  ts-guest-destroy-hardhost   $g   +
 }
 
+proc need-hosts/test-devstack {} { return host }
+proc run-job/test-devstack {} {
+  run-ts . = ts-openstack-deploy host
+}
+
 if {[file exists sg-run-job-adhoc]} {
 source sg-run-job-adhoc
 }
diff --git a/ts-openstack-deploy b/ts-openstack-deploy
new file mode 100755
index 000..5758f82
--- /dev/null
+++ b/ts-openstack-deploy
@@ -0,0 +1,338 @@
+#!/usr/bin/perl
+# This is part of "osstest", an automated testing framework for Xen.
+# Copyright (C) 2015 Citrix Inc.
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU Affero General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU Affero General Public License for more details.
+#
+# You should have received a copy of the GNU Affero General Public License
+# along with this program.  If not, see .
+
+use strict qw(vars);
+use Osstest;
+use Osstest::TestSupport;
+use Osstest::BuildSupport;
+
+tsreadconfig();
+our ($whhost) = @ARGV;
+$whhost ||= 'host';
+our $ho = selecthost($whhost);
+our $builddir = target_jobdir($ho);
+
+sub tgt_init ();
+
+sub packages () {
+  # Install open-iscsi ahead of devstack ...
+  target_install_packages($ho, qw(git sudo open-iscsi));
+  # ... and start open-iscsi to have /etc/iscsi/initiatorname.iscsi
+  # generated. This is done on install on Ubuntu.
+  target_cmd_root($ho, 'service open-iscsi start');
+}
+
+sub checkout () {
+  prepbuilddirs();
+  build_clone($ho, 'cinder', $builddir, 'cinder');
+  build_clone($ho, 'devstack', $builddir, 'devstack');
+  build_clone($ho, 'glance', $builddir, 'glance');
+  build_clone($ho, 'keystone', $builddir, 'keystone');
+  build_clone($ho, 'nova', $builddir, 'nova');
+  build_clone($ho, 'requirements', $builddir, 'requirements');
+  build_clone($ho, 'tempest', $builddir, 'tempest');
+
+  my $vg = target_choose_vg($ho, 10*1024); # 10GB
+  target_putfilecontents_stash($ho, 60, <