search

andsens / build-debian-cloud

This project has been superseeded by andsens/bootstrap-vz and is no longer maintained - Script to create Debian Squeeze & Wheezy Amazon Machine Images (AMIs) and Google Compute Engine images
Other
117 stars 43 forks source link

plugin image_commands #95

Closed osallou closed 11 years ago

osallou commented 11 years ago

Hi, here is a new plugin proposal. It helps the user to execute some commands or some local scripts in the image. This is helpful to customize the image with custom commands/scripts.

Olivier

andsens commented 11 years ago

I like the general idea of this. Though I would prefer making plugins so easy to code that people use them instead of scripts I can see how this is easier when doing a "quick test/debug" or one has a rather large script that may not be made specifically for the bootstrapper.

I added comments directly in the code. Maybe one way of running external commands would be enough. After all, it is quite easy to create a command which runs a script, so the second option is kind of superfluous.

Instead of running the command directly in chroot, how about letting the user decide where to run it? This would give him the option to maybe copy data from the host machine. Also: What about variables? Check out my AMIName task. It replaces any variables in the AMI name with actual values. Date, time and arch may not make much sense, but mount_dir, device_path etc. do, they can be used as parameters in the command.

osallou commented 11 years ago

Le 9 sept. 2013 00:17, "Anders Ingemann" notifications@github.com a écrit :

I like the general idea of this. Though I would prefer making plugins so easy to code that people use them instead of scripts I can see how this is easier when doing a "quick test/debug" or one has a rather large script that may not be made specifically for the bootstrapper.

I added comments directly in the code.

Thanks I gonna look those

Maybe one way of running external commands would be enough. After all, it is quite easy to create a command which runs a script, so the second option is kind of superfluous.

Depending on script complexity, it may be complex to go only with commands, especially with conditons or loops.

Instead of running the command directly in chroot, how about letting the user decide where to run it? This would give him the option to maybe copy data from the host machine.

Why not indeed, I will think about this

Also: What about variables? Check out my AMIName task. It replaces any variables in the AMI name with actual values. Date, time and arch may not make much sense, but mount_dir, device_path etc. do, they can be used as parameters in the command.

I will have a look

— Reply to this email directly or view it on GitHub.

andsens commented 11 years ago

Depending on script complexity, it may be complex to go only with commands, especially with conditons or loops.

Agreed, but what you do in the script task is to simply invoke the script, that can just as easily be done with the command task (and on top of that you get the option to specify your own parameters to the script).

osallou commented 11 years ago

2013/9/9 Anders Ingemann notifications@github.com

Depending on script complexity, it may be complex to go only with commands, especially with conditons or loops.

Agreed, but what you do in the script task is to simply invoke the script, that can just as easily be done with the command task (and on top of that you get the option to specify your own parameters to the script).

oh, ok, I understand what you mean

Olivier

— Reply to this email directly or view it on GitHubhttps://github.com/andsens/build-debian-cloud/pull/95#issuecomment-24049875 .

gpg key id: 4096R/326D8438 (keyring.debian.org)

Key fingerprint = 5FB4 6F83 D3B9 5204 6335 D26D 78DC 68DB 326D 8438

osallou commented 11 years ago

I have remove scripts and kept commands only (and used your suggestions). commands can include some variables to be used in command (only a few for the moment, we could add others later on). command is executed in user context to allow copies to image etc...

osallou commented 11 years ago

I made updates according o your commands

andsens commented 11 years ago

This is beginning to look really good! Maybe you could rebase it all into one commit?

osallou commented 11 years ago

I made modifications for manifest and add a different commit to fix a missing package in kvm and virtualbox providers.

I tried to make a rebase, that worked fine locally (to group plugin commits), but it seems to fail when I pushed to github repo. I have my commit with everything, but I still see the individual commits before... :-( I don't know what I should do here with git.

andsens commented 11 years ago

Well, you rebased, sooo.. git push -f?

Anders

On 9 September 2013 20:49, Olivier Sallou notifications@github.com wrote:

I made modifications for manifest and add a different commit to fix a missing package in kvm and virtualbox providers.

I tried to make a rebase, that worked fine locally (to group plugin commits), but it seems to fail when I pushed to github repo. I have my commit with everything, but I still see the individual commits before... :-( I don't know what I should do here with git.

— Reply to this email directly or view it on GitHubhttps://github.com/andsens/build-debian-cloud/pull/95#issuecomment-24104552 .

osallou commented 11 years ago

thanks, I am not used to use rebase....

I mixed the commits related to plugin and kept separate commit for kpartx

andsens commented 11 years ago

This is looking great! Normally you'd separate the two fixes in different pull requests but this is fine. If you could just fix the indentation issue I'll merge :-) (it's thoroughly tested, right? My setup is broken right now.)

osallou commented 11 years ago

where do you have indent issue ? I had to work on 2 different systems (one that manage indent for python, not the other...).

I tested the plugin on my server.

2013/9/9 Anders Ingemann notifications@github.com

This is looking great! Normally you'd separate the two fixes in different pull requests but this is fine. If you could just fix the indentation issue I'll merge :-) (it's thoroughly tested, right? My setup is broken right now.)

— Reply to this email directly or view it on GitHubhttps://github.com/andsens/build-debian-cloud/pull/95#issuecomment-24110626 .

gpg key id: 4096R/326D8438 (keyring.debian.org)

Key fingerprint = 5FB4 6F83 D3B9 5204 6335 D26D 78DC 68DB 326D 8438

osallou commented 11 years ago

I had not seen the indent message in the code. I fixed it with a new rebase.

andsens commented 11 years ago

Great stuff! Merging...