[GitHub] cloudstack pull request: Feature cenik123 vpcvrr 1.1.1

2014-12-11 Thread lsimons 
Github user lsimons commented on the pull request:

https://github.com/apache/cloudstack/pull/33#issuecomment-66652799
  
Hey folks, should be public now, sorry, I didn't realize :)


---
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.
---

[GitHub] cloudstack pull request: Feature cenik123 vpcvrr 1.1.1

2014-12-02 Thread lsimons 
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.

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.

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...

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 :)

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.
---

[GitHub] cloudstack pull request: CLOUDSTACK-7143: Refactoring of the syste...

2014-09-23 Thread lsimons 
Github user lsimons commented on the pull request:

https://github.com/apache/cloudstack/pull/16#issuecomment-56589622
  
Thanks for all the help Rohit! I'm sure to most people some bash script 
rearrangement looks boring, but nevermind that, to me it just feels good to 
sumbit more than just simple one-line patches to an apache project after such a 
long time :-)


---
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.
---

[GitHub] cloudstack pull request: CLOUDSTACK-7143: Refactoring of the syste...

2014-09-22 Thread lsimons 
Github user lsimons commented on the pull request:

https://github.com/apache/cloudstack/pull/16#issuecomment-56385199
  
thanks for testing rohit. We've never seen this (with xen).

If you look at the error,

```
/dev/vda5: Superblock last mount time (Fri Sep 19 14:54...)
   now = Mon Sep 15 20:34... is in the future
```

the fsck is unhappy with the last recorded mount being sep 19 (last friday, 
which is correct, i.e. it matches with when we ran the build on jenkins) 
because the date of the system as its booting is in the past, sep 15. So 
somehow the system clock of the VM is wrong...?

Either during the build process the time was moved backward some time after 
mounting the filesystem, or during the boot process time moved backward, or ... 
?

I've checked and the date of the build slave where the image was baked is 
correct and managed with ntp so very likely was always correct...so I really 
don't see how the build could result in that timestamp existing anywhere.

Can you check the date of your KVM host? Or perhaps if you have an NTP 
server with a wrong date?


---
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.
---

[GitHub] cloudstack pull request: CLOUDSTACK-7143: Refactoring of the syste...

2014-09-22 Thread lsimons 
Github user lsimons commented on the pull request:

https://github.com/apache/cloudstack/pull/16#issuecomment-56387394
  
sorry, nevermind the last comment.

From the jenkins build log:
```
[2014-09-19 14:57:59] INFO: stoppping all virtualbox vms for jenkins
+ bundle exec ./vbox_vm_clean.rb
VBoxManage controlvm systemvmtemplate poweroff
VBoxManage: error: Invalid machine state: PoweredOff (must be Running, 
Paused or Stuck)
VBoxManage: error: Details: code VBOX_E_INVALID_VM_STATE (0x80bb0002), 
component Console, interface IConsole, callee nsISupports
VBoxManage: error: Context: PowerDown(progress.asOutParam()) at line 222 
of file VBoxManageControlVM.cpp
VBoxManage controlvm systemvm64template poweroff
VBoxManage: error: Invalid machine state: PoweredOff (must be Running, 
Paused or Stuck)
VBoxManage: error: Details: code VBOX_E_INVALID_VM_STATE (0x80bb0002), 
component Console, interface IConsole, callee nsISupports
VBoxManage: error: Context: PowerDown(progress.asOutParam()) at line 222 
of file VBoxManageControlVM.cpp
kill -SIGKILL 19324
kill -SIGTERM 19324
./vbox_vm_clean.rb:49:in `kill'
./vbox_vm_clean.rb:49:in `block (2 levels) in main'

/usr/local/rvm/gems/ruby-1.9.3-p547/gems/sys-proctable-0.9.4-universal-linux/lib/linux/sys/proctable.rb:237:in
 `block in ps'

/usr/local/rvm/gems/ruby-1.9.3-p547/gems/sys-proctable-0.9.4-universal-linux/lib/linux/sys/proctable.rb:110:in
 `foreach'

/usr/local/rvm/gems/ruby-1.9.3-p547/gems/sys-proctable-0.9.4-universal-linux/lib/linux/sys/proctable.rb:110:in
 `ps'
./vbox_vm_clean.rb:31:in `block in main'
./vbox_vm_clean.rb:14:in `each'
./vbox_vm_clean.rb:14:in `main'
VBoxManage controlvm systemvm64template-systemvm-refactor-for-upstream-9 
poweroff
VBoxManage: error: Invalid machine state: Aborted (must be Running, Paused 
or Stuck)
VBoxManage: error: Details: code VBOX_E_INVALID_VM_STATE (0x80bb0002), 
component Console, interface IConsole, callee nsISupports
VBoxManage: error: Context: PowerDown(progress.asOutParam()) at line 222 
of file VBoxManageControlVM.cpp
```

The lines
```bash
kill -SIGKILL 19324
kill -SIGTERM 19324
```

mean that we forcibly shut down a virtualbox process, probably the one that 
was running the systemvm that was built. This means that the line further up
```bash
bundle exec veewee vbox halt 
systemvm64template-systemvm-refactor-for-upstream-9
```

did not succeed in fully shutting down the new machine, and then the next 
attempt to cleanly power off
```bash
VBoxManage controlvm systemvm64template-systemvm-refactor-for-upstream-9 
poweroff
```

also failed, and so we don't umount cleanly, and so stuff is broken.

FWIW, a successful build has something more like

```bash
VBoxManage controlvm systemvmtemplate-systemvm-persistent-config-4.5.0.78 
poweroff
0%...10%...20%...30%...40%...50%...60%...70%...80%...90%...100%```
```

I'll investigate what went wrong.

We should probably also just fail the build if the poweroff fails rather 
than attempting a 'kill'.



---
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.
---

[GitHub] cloudstack pull request: CLOUDSTACK-7143: Refactoring of the syste...

2014-09-22 Thread lsimons 
Github user lsimons commented on the pull request:

https://github.com/apache/cloudstack/pull/16#issuecomment-56402358
  
Hey Rohit, looks like that did it. Can you try the outputs from 
http://jenkins.buildacloud.org/job/systemvm-refactor-CLOUDSTACK-7143/10/console 
when it finishes archiving? Cheers!


---
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.
---

[GitHub] cloudstack pull request: CLOUDSTACK-7143: Refactoring of the syste...

2014-09-17 Thread lsimons 
Github user lsimons commented on the pull request:

https://github.com/apache/cloudstack/pull/16#issuecomment-55866128
  
Hey Rohit, like I thought, a ruby issue.I'm surprised RVM isn't enabled 
already since the build was already using it (I think...). The fix should be 
along the lines of http://rvm.io/integration/jenkins .

The other part of the fix is making sure that the inline shell scripts act 
as login scripts so that rvm is loaded, i.e. they need to start with
```
#!/bin/bash -l
```


---
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.
---

[GitHub] cloudstack pull request: bugfix/CLOUDSTACK-7476 for 4.4 branch.

2014-09-08 Thread lsimons 
Github user lsimons commented on the pull request:

https://github.com/apache/cloudstack/pull/17#issuecomment-54785217
  
Hey Daan, _this_ change is finedone as-is. Like Rajani mentioned, the 
other changes to that file could be ported, too. I'm +0 on that -- on the one 
hand, it's a good change and I can't see any way that it would ever break 
anything, on the other hand it's a (however minor) behavioral change so I 
personally wouldn't put that in a point release.


---
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.
---

[GitHub] cloudstack pull request: Fix CLOUDSTACK-7476: always pass along $J...

2014-09-05 Thread lsimons 
Github user lsimons commented on the pull request:

https://github.com/apache/cloudstack/pull/15#issuecomment-54600044
  
I'm afraid not. This fix actually counts on setJavaHome() succeeding -- but 
_after_ that, that value does not make it to JSVC. The call to daemon calls 
runuser which, depending on configuration, *does not pass along environment 
variables like JAVA_HOME*.


---
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.
---

[GitHub] cloudstack pull request: Fix CLOUDSTACK-7476: always pass along $J...

2014-09-05 Thread lsimons 
Github user lsimons commented on the pull request:

https://github.com/apache/cloudstack/pull/15#issuecomment-54626548
  
Sweet, thanks!

Personally I think just having it on master is ok -- anyone that is likely 
to run into this issue is also likely to have configuration management (like 
chef) in place to edit their cloudstack-usage script if they need to do so.

Of course it's a low-risk merge so there isn't anything _wrong_ with 
merging to 4.4, but, 4.4 still has the old style logic for choosing $JAVA_HOME, 
so those changes would have to be cherry-picked too, and that implies changes 
to preferential status of 1.7 vs 1.6 for 4.4.so I would guess, just 
leave it for 4.5.


---
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.
---

[GitHub] cloudstack pull request: Fix CLOUDSTACK-7476: always pass along $J...

2014-09-05 Thread lsimons 
Github user lsimons closed the pull request at:

https://github.com/apache/cloudstack/pull/15


---
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.
---

[GitHub] cloudstack pull request: bugfix/CLOUDSTACK-7476 for 4.4 branch.

2014-09-05 Thread lsimons 
GitHub user lsimons opened a pull request:

https://github.com/apache/cloudstack/pull/17

bugfix/CLOUDSTACK-7476 for 4.4 branch.

See https://github.com/apache/cloudstack/pull/15 for the pull request
that was merged to master  associated discussion.


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/schubergphilis/cloudstack hotfix/4.4-7476

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/cloudstack/pull/17.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #17


commit c6136255b95ac81d6da9cebc1b80639242d6a029
Author: Leo Simons lsim...@schubergphilis.com
Date:   2014-09-05T17:54:36Z

bugfix/CLOUDSTACK-7476 for 4.4 branch.

See https://github.com/apache/cloudstack/pull/15 for the pull request
that was merged to master  associated discussion.




---
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.
---

[GitHub] cloudstack pull request: Fix CLOUDSTACK-7476: always pass along $J...

2014-09-04 Thread lsimons 
GitHub user lsimons opened a pull request:

https://github.com/apache/cloudstack/pull/15

Fix CLOUDSTACK-7476: always pass along $JAVA_HOME

On a secured environment (selinux w/ env_reset enabled in sudoers), the
runuser command that is invoked by the daemon() function does not pass
along environment variables, so $JAVA_HOME is empty, and JSVC falls
back to its default behavior, which may not find java or may not find
the intended java.

This fix simply passes $JAVA_HOME explicitly using the -home argument to
JSVC.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/schubergphilis/cloudstack 
bugfix/CLOUDSTACK-7476

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/cloudstack/pull/15.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #15


commit da9647205aafccee3ae8b60b6ccdee0dcad8c712
Author: Leo Simons lsim...@schubergphilis.com
Date:   2014-09-03T07:32:32Z

Fix CLOUDSTACK-7476: always pass along $JAVA_HOME

On a secured environment (selinux w/ env_reset enabled in sudoers), the
runuser command that is invoked by the daemon() function does not pass
along environment variables, so $JAVA_HOME is empty, and JSVC falls
back to its default behavior, which may not find java or may not find
the intended java.

This fix simply passes $JAVA_HOME explicitly using the -home argument to
JSVC.




---
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.
---

[GitHub] cloudstack pull request: CLOUDSTACK-7143: Refactoring of the syste...

2014-09-04 Thread lsimons 
GitHub user lsimons opened a pull request:

https://github.com/apache/cloudstack/pull/16

CLOUDSTACK-7143: Refactoring of the systemvm build process

E-mail thread:
  
http://mail-archives.apache.org/mod_mbox/cloudstack-dev/201407.mbox/%3C7A6CF878-7A28-4D4A-BCD2-0C264F8C90B7%40schubergphilis.com%3E

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.

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.

Most of these commits are now a while old but I wanted to wait with
sending this upstream until we had sufficiently tested the systemvms
built with this changed approach locally.


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/schubergphilis/cloudstack 
feature/systemvm-refactor-for-upstream

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/cloudstack/pull/16.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #16


commit 49cbb4e20b70a19286c626ac8d9111be8752bc3f
Author: Leo Simons lsim...@schubergphilis.com
Date:   2014-07-21T07:54:13Z

CLOUDSTACK-7143: upgrade systemvm to latest debian stable, 7.6.0.

commit 35b1875226201d5923dea57db64bbd789d9ad908
Author: Leo Simons lsim...@schubergphilis.com
Date:   2014-07-21T07:55:37Z

CLOUDSTACK-7143: split base.sh into its two functions.

commit fb258b506964ca339ef85aa92a6217dc12259811
Author: Leo Simons lsim...@schubergphilis.com
Date:   2014-07-21T07:57:49Z

CLOUDSTACK-7143: move network tuning from cleanup.sh to its own script.

commit 50e2c0177b0df04aa80d19264d52c8d55dc02eb3
Author: Leo Simons lsim...@schubergphilis.com
Date:   2014-07-21T08:36:11Z

CLOUDSTACK-7143: merge systemvm templates, step 1: remove differences

commit 03dba3d901997619d9095e33afe4b39a423ac3f6
Author: Leo Simons lsim...@schubergphilis.com
Date:   2014-07-21T08:44:38Z

[GitHub] cloudstack pull request: CLOUDSTACK-7143: Refactoring of the syste...

2014-09-04 Thread lsimons 
Github user lsimons commented on the pull request:

https://github.com/apache/cloudstack/pull/16#issuecomment-54490782
  
Hey Rohit, thanks for reviewing!

We actually have a new vagrant-based component test setup for the systemvm 
to contribute, too; I'm working on extracting that now from the schubergphilis 
feature/systemvm-persistent-config branch. Our internal jenkins setup is 
already running those tests, as well as wider-scale marvin-driven integration 
tests that use these new-style systemvms. That's why I think this is ready for 
upstream: I'm pretty confident this stuff all still works properly.tough of 
course that _really_ _really_ needs careful verification outside of our 
assumptions/environment.

I'll try and work with Hugo to port those jenkins builds to 
buildacloud.org...hope he has time for it :)



---
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.
---

[GitHub] cloudstack pull request: Pull request of changes in the cloud-ser...

2014-09-02 Thread lsimons 
Github user lsimons commented on the pull request:

https://github.com/apache/cloudstack/pull/14#issuecomment-54167891
  
Thanks Travis! So cool that this is working...the failure is due to the 
change here:


https://github.com/schubergphilis/cloudstack/commit/680dd7ece887829645afee99c45d32af6fe57134#diff-11

Wilder will update the pull request to filter it out.



---
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.
---