Thanks Leo

From: Leo Simons <lsim...@schubergphilis.com<mailto:lsim...@schubergphilis.com>>
Date: Monday, July 21, 2014 at 7:24 AM
To: Chiradeep Vittal 
<chiradeep.vit...@citrix.com<mailto:chiradeep.vit...@citrix.com>>, 
"dev@cloudstack.apache.org<mailto:dev@cloudstack.apache.org>" 
<dev@cloudstack.apache.org<mailto:dev@cloudstack.apache.org>>, int-toolkit 
<int-tool...@schubergphilis.com<mailto:int-tool...@schubergphilis.com>>
Subject: Re: rfc: newsystemvm

Hey Chiradeep,

Thanks for taking a look. I’ve now re-done this work, but carefully and cleanly 
and on top of current master, in 37 small commits instead of 2 scary ones.

Please take a look at

  https://github.com/schubergphilis/cloudstack/compare/systemvm-refactor

Summarizing this kind of thing is always hard...it’s many little things...the 
interesting stuff is at the end/bottom, in particular the two main improvements

  
https://github.com/schubergphilis/cloudstack/commit/142d087f6a97f6ac70a858a35d2fe8b638c58cbb
When working on the systemvm in isolation, or using vagrant or similar tools,
it can be useful to inject a custom SSH key before merging a management server
systemvm.iso into it. This option allows that. It should _not_ have effect
on management-server-managed vms which always get their SSH keys injected.

  
https://github.com/schubergphilis/cloudstack/commit/e2240eaed18000d4d94dbf6a6e40612db1aeda34
The current build downloads its script from master by fetching a cloudstack
tarball. Besides being an unneeded load on the apache git server, this is a
problem when working on a branch and wanting to inject a different set of
scripts. It also makes it pretty likely that the injected copy of the script
will not match what a production release wants, so there is very little
chance of not needing to overwrite the scripts.

Ideally we would just rsync over some files. However, veewee does not provide
an option to do that. In order to keep a 'cleanly veewee-only' build possible,
and work with any recent veewee version, in this change we restor to using
shar (http://en.wikipedia.org/wiki/Shar) to produce an archive which can
execute as a script, which we feed to veewee to execute.

In order to avoid having to re-do this cleanup twice, I also ended up merging 
the systemvm and systemvm64 template definitions, factoring out their small 
differences by inspecting the os architecture.

  
https://github.com/schubergphilis/cloudstack/commit/f570b3921cd52672f841fc5f99cdd96f9737d629
  
https://github.com/schubergphilis/cloudstack/commit/50e91217f90fc952182dccac02a5af06ac33fb45

Everything else…well it pretty much falls into two categories:
  * general code cleanup without functional changes
  * general code defensiveness to survive various jenkins build scenarios

All in all it should help with ongoing maintenance, I think.

Note I still have some work to do (testing, merging this version back into our 
redundant vpc branch, moar testing, ...) before submitting a merge-able 
patchset. But since it’s such a big change and since the testing is a bit slow 
(what with creating and destroying VMs) any early comments would be quite 
useful so I don’t have to re-re-do lots of testing.


Thanks!


Leo

On Jul 18, 2014, at 7:35 PM, Chiradeep Vittal 
<chiradeep.vit...@citrix.com<mailto:chiradeep.vit...@citrix.com>> wrote:

Thanks Leo. Can you summarize the changes (it is a lot of changes)
From: Leo Simons <lsim...@schubergphilis.com<mailto:lsim...@schubergphilis.com>>
Reply-To: "dev@cloudstack.apache.org<mailto:dev@cloudstack.apache.org>" 
<dev@cloudstack.apache.org<mailto:dev@cloudstack.apache.org>>
Date: Friday, July 18, 2014 at 7:42 AM
To: int-toolkit 
<int-tool...@schubergphilis.com<mailto:int-tool...@schubergphilis.com>>, 
"dev@cloudstack.apache.org<mailto:dev@cloudstack.apache.org>" 
<dev@cloudstack.apache.org<mailto:dev@cloudstack.apache.org>>
Subject: rfc: newsystemvm
Hey folks,
https://github.com/schubergphilis/cloudstack/commit/f125f1564e8921def00dc0235ecca51470a2a22e
https://github.com/schubergphilis/cloudstack/tree/f125f1564e8921def00dc0235ecca51470a2a22e/tools/appliance
This started out as wanting the systemvm build to take 
systemvm/patches/debian/{debian,vpn} from the local machine/branch, rather than 
downloading from the apache git master [1]. In working out how on earth to get 
veewee to do that cleanly (hint: you can’t, hence resorting to shar usage) I 
got quite frustrated with the image rebuild times.
It so happens that veewee has a --skip-to-postinstall instruction which is 
_quite_ useful while debugging these scripts. To get that working requires the 
post install steps to be retryable/convergent. Of course, our existing scripts 
weren’t set up for that. So I had to add a bunch of tests whether changes had 
applied already. Which implied a pretty significant refactor.
I think I was careful enough and I expect this new template will work just as 
well as the old one. This is a change that we can (and probably should?) merge 
to master independently of the redundant VPC work (though the `apt-get install 
chef` would need to be taken out). But, given how big of a chunk of code has 
changed here, before upstreaming (a version of) this to apache we (I) need to 
do more testing. So for now I’ve put this change next to the existing 
definitions rather than replace ‘em, to not block anything else.
Comments/thoughts?
cheers,
Leo
[1] 
https://github.com/schubergphilis/cloudstack/blob/master/tools/appliance/definitions/systemvmtemplate/postinstall.sh#L228
Begin forwarded message:
...
     M tools/appliance/build.sh
...
     A tools/appliance/definitions/newsystemvm/apt_upgrade.sh
     A tools/appliance/definitions/newsystemvm/authorized_keys.sh
     A tools/appliance/definitions/newsystemvm/build_time.sh
     A tools/appliance/definitions/newsystemvm/cleanup.sh
     A tools/appliance/definitions/newsystemvm/configure_acpid.sh
     A tools/appliance/definitions/newsystemvm/configure_conntrack.sh
     A tools/appliance/definitions/newsystemvm/configure_grub.sh
     A tools/appliance/definitions/newsystemvm/configure_locale.sh
     A tools/appliance/definitions/newsystemvm/configure_login.sh
     A tools/appliance/definitions/newsystemvm/configure_networking.sh
     A tools/appliance/definitions/newsystemvm/configure_systemvm_services.sh
     A tools/appliance/definitions/newsystemvm/definition.rb
     A tools/appliance/definitions/newsystemvm/install_systemvm_packages.sh
     A tools/appliance/definitions/newsystemvm/preseed.cfg
     A tools/appliance/definitions/newsystemvm/zerodisk.sh
     A tools/appliance/shar_cloud_scripts.sh
     A tools/appliance/test.sh
...
Work in progress to rewrite systemvm box definition in clean code.
* Refactor build.sh to use functions and pretty logging.
* Add a new test.sh which tries several different build.sh invocations.
* Add a new 'debianbase' definition which is just the basic debian box (no 
systemvm functionality). This is used for testing.
* Add a new 'newsystemvm' definition which is a work-in-progress replacement of 
the 'systemvmtemplate' definition:
** reduce duplication between preseed.cfg and postinstall.sh
** remove duplication between postinstall.sh and cloud-scripts
** install cloud-scripts from current working copy instead of tying to a 
download of the apache master branch
** split up postinstall.sh into small utility scripts and clean up that code
** clean up preseed.cfg to match current veewee/debian best practices and 
remove cruft


Reply via email to