On Tue, Dec 2, 2014 at 3:50 AM, lsimons <g...@git.apache.org> wrote:
> Github user lsimons commented on the pull request:
>
> https://github.com/apache/cloudstack/pull/33#issuecomment-65199170
>
> Hey folks,
>
> Karl thanks for contributing. Looks like quite a bit of work!
>
> Rohit thanks for CC-ing me in :)
>
> It's a bit hard to read the pull request right now and provide review
> comments:
> * Github says there's nearly 7000 changed files so at a guess
> something went wrong there? I had a look, and it looks like
> KarlHarrisSungardAS/cloudstack is quite a ways behind master, which may
> explain that...? It's probably a good idea to make a copy of the feature
> branch, rebase it against current master, squash some of the commits
> together, and then re-send the pull request. Or, start another branch, and
> cherry-pick over just the changes that _should_ be in the change set.
> * git hasn't picked up on the relation between veewee and packer, it's
> probably good idea to give it some more hints. For example it would be good
> to see
>
> https://github.com/KarlHarrisSungardAS/cloudstack/commit/892b11ec0230ce87d14370fd1055ac223096cc77#diff-67bdab8b08f1e110ceabe1a23c3d4011R1
> compared to the veewee equivalent.
> * there's a lot of 'shuffling' and/or adding comments intermixed with
> functional changes. For example
> https://github.com/KarlHarrisSungardAS/cloudstack/commit/7f55a1ef032c29cb955e02c90da43c91739108d7#diff-95748c84d8b6be2da502091e69b748d9L30
> you might think the 'set -x' is getting removed here, but it just moved
> down a bit amidst a few new comments.
> * there's a lot of commits that are seemingly unrelated to the actual
> change (probably cherry-picked from master onto the feature branch?), those
> will need to be filtered out I think.
> * there's a lot of 'archive' commits which have unfinished
> 'checkpoint' code. I imagine all that work gets finished 'later on' (like
> renaming _gitignore to .gitignore is one that I tracked) but it's not so
> easy to see. It'd be great if those things could be squashed together.
>
> I will issue a revised pull request shortly....Thanks for the input and
suggestions.
As far as content goes...
>
> 1) Like sebastien says, veewee is in maintenance mode and moving to
> packer (replacing veewee) is definitely a good idea. This looks like a
> straightforward port, which if it works ok seems like the way to go. We
> _should_ make sure to do it in a way that's somewhat compatible with the
> jenkins.buildacloud.org setup. I don't think we could sustain maintenance
> of both packer and veewee builds for long, it'll be a pain to keep changes
> in sync. I'd want to drop veewee use completely.
>
My intent with moving to Packer was to make/continue the systemvm builds
compatible with jenkins. This is part of building a self supporting unit
test framework using Packer/Vagrant. See below.
>
> 2) Testing the systemvm using vagrant is also obviously a good idea
> (we're doing much the same, over at
> https://github.com/schubergphilis/cloudstack/tree/feature/systemvm-persistent-config/tools/vagrant/systemvm
> see
> https://github.com/schubergphilis/cloudstack/commits/feature/systemvm-test
> for an abandoned branch having just test-related commits), though I don't
> understand why we'd need to use vagrant templates? The .box file that we
> get out of the veewee-based systemvm build imports (
> https://github.com/apache/cloudstack/blob/master/tools/appliance/build.sh#L466)
> just fine...
>
> Yes, the use of vagrant templates is a bit redundant given the self
patching nature of the systemvms. I will look at the references above as
our work progresses.
> 3) For the stuff that seems to be planned _beyond_ these changes, it
> might be a good idea to pop by on the development mailing list and discuss
> plans. A few people at Schuberg Philis (my employer, where there's a few
> people working on cloudstack) have been working on redundancy for the
> virtual router for a few months now. We've found that making it work also
> involves a lot of changes outside of the systemvm, too, i.e. changing the
> orchestration logic in the java code to match changed behavior of the vpc
> router (see
> https://cwiki.apache.org/confluence/display/CLOUDSTACK/Refactor+for+Redundant+Virtual+Router+Implementation
> /
> https://www.slideshare.net/LeoSimons1/toolkit-posterccceu14-v2 ).
> @spark404 please do have a look at Karl's approach :)
>
After looking a how Cloudstack implements both public and VPC routers; I
have chosen to concentrate on the systemvm and what is required to create a
VPC redundant router with the minimum changes to the interface between the
system virtual machine (scripts) and the Cloudstack management server. The
idea is to add the ability for the existing VPC routers to route
dynamically configured network topologies (tiers) by using the existing
Create, Read, Update and Delete (CRUD) network scripts. The goal is to
only add a boolean "is redundant" to the interface along with supporting
script code to configure Keepalived and Conntrackd for VPC redundant
routers. Note: this boolead variable is already used for the "router type"
and the type "VPC router" ignores this variable. Currently the public
routers are redundant but only allow for a "static" network topology.
Building the test framework using vagrant help with understanding how the
public routers work (keepalived/conntracd) and what is required to allow
for "dynamically configured network topologies".
We are currently looking a the refactoring of the Java code and still
believe our work will complement this refactoring work quite well.;
As always, if you have an questions or comments do not hesitate to post
them on this thread.
> cheers,
>
> Leo
>
>
> ---
> If your project is set up for it, you can reply to this email and have your
> reply appear on GitHub as well. If your project does not have this feature
> enabled and wishes so, or if the feature is enabled but not working, please
> contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
> with INFRA.
> ---
>
--
Karl O. Harris
Cloud Software Engineer
Sungard Availability Services
Office: 215-446-1772
Cell: 215-264-1855
karl.har...@sungardas.com
