You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Damodar Reddy Talakanti <da...@citrix.com> on 2014/04/17 11:41:22 UTC

Review Request 20445: Add new Command line options to setup/bindir/cloud-setup-databases.in and remove OS specific commands

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20445/
-----------------------------------------------------------

Review request for cloudstack, Alex Huang, Frank Zhang, and Hugo Trippaers.


Bugs: https://issues.apache.org/jira/browse/CLOUDSTACK-6435
    https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/CLOUDSTACK-6435


Repository: cloudstack-git


Description
-------

Currently the python script "setup/bindir/cloud-setup-databases.in" for setup databases uses template based parameters which will get replaced during rpm build. 
Along with this also enable new command line options to over ride those template based parameters if the same get passed as command line options.
Accept the following options:
1. db-conf-path
2. db-files-path
3. encryption-jar-path
4. encryption-key-file
Also replace code that calls OS specific commands with python specific libraries.


Diffs
-----

  setup/bindir/cloud-setup-databases.in 2ba7d51 

Diff: https://reviews.apache.org/r/20445/diff/


Testing
-------

Tested the script on both windows and linux environment

Tested for the following scenarios

1. With out passing new options on linux environment
2. With passing new options on linux environment
3. With passing new options on windows environment
4. With out passing new options on windows environment (Script fails here as there are no replacements happened for template parameters during build)


Thanks,

Damodar Reddy Talakanti


Re: Review Request 20445: Add new Command line options to setup/bindir/cloud-setup-databases.in and remove OS specific commands

Posted by Damodar Reddy Talakanti <da...@citrix.com>.

> On April 18, 2014, 7:33 p.m., Frank Zhang wrote:
> >  This script is very old it really needs some improvement, thanks for the patch. Here are some my thoughts:
> >  
> >  1. instead of calling socket.gethostbyname, can we handle the case where the machine has multiple nics? the ip will be used as node ip in multiple management server nodes environment, it would be nice to have an option that admin can specify the ip, if they don't, by default the script should automatically find ip of nic for default route. Yes the old code doesn't handle this, as you are improving it we should take this chance to make it further.
> >  2. I see there are some new options, but it seems they are not used in code?

For (1) I can add a command line option right now but to handle multiple nics case I can file a defect and will track it further as I can not finish that currently.
For (2) I in case of new options I have over ridden existing variables which has default values, if those options are passed. These are not completely new options.


- Damodar Reddy


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20445/#review40804
-----------------------------------------------------------


On April 17, 2014, 9:41 a.m., Damodar Reddy Talakanti wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20445/
> -----------------------------------------------------------
> 
> (Updated April 17, 2014, 9:41 a.m.)
> 
> 
> Review request for cloudstack, Alex Huang, Frank Zhang, and Hugo Trippaers.
> 
> 
> Bugs: https://issues.apache.org/jira/browse/CLOUDSTACK-6435
>     https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/CLOUDSTACK-6435
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> Currently the python script "setup/bindir/cloud-setup-databases.in" for setup databases uses template based parameters which will get replaced during rpm build. 
> Along with this also enable new command line options to over ride those template based parameters if the same get passed as command line options.
> Accept the following options:
> 1. db-conf-path
> 2. db-files-path
> 3. encryption-jar-path
> 4. encryption-key-file
> Also replace code that calls OS specific commands with python specific libraries.
> 
> 
> Diffs
> -----
> 
>   setup/bindir/cloud-setup-databases.in 2ba7d51 
> 
> Diff: https://reviews.apache.org/r/20445/diff/
> 
> 
> Testing
> -------
> 
> Tested the script on both windows and linux environment
> 
> Tested for the following scenarios
> 
> 1. With out passing new options on linux environment
> 2. With passing new options on linux environment
> 3. With passing new options on windows environment
> 4. With out passing new options on windows environment (Script fails here as there are no replacements happened for template parameters during build)
> 
> 
> Thanks,
> 
> Damodar Reddy Talakanti
> 
>


Re: Review Request 20445: Add new Command line options to setup/bindir/cloud-setup-databases.in and remove OS specific commands

Posted by Frank Zhang <fr...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20445/#review40804
-----------------------------------------------------------


 This script is very old it really needs some improvement, thanks for the patch. Here are some my thoughts:
 
 1. instead of calling socket.gethostbyname, can we handle the case where the machine has multiple nics? the ip will be used as node ip in multiple management server nodes environment, it would be nice to have an option that admin can specify the ip, if they don't, by default the script should automatically find ip of nic for default route. Yes the old code doesn't handle this, as you are improving it we should take this chance to make it further.
 2. I see there are some new options, but it seems they are not used in code?

- Frank Zhang


On April 17, 2014, 9:41 a.m., Damodar Reddy Talakanti wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20445/
> -----------------------------------------------------------
> 
> (Updated April 17, 2014, 9:41 a.m.)
> 
> 
> Review request for cloudstack, Alex Huang, Frank Zhang, and Hugo Trippaers.
> 
> 
> Bugs: https://issues.apache.org/jira/browse/CLOUDSTACK-6435
>     https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/CLOUDSTACK-6435
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> Currently the python script "setup/bindir/cloud-setup-databases.in" for setup databases uses template based parameters which will get replaced during rpm build. 
> Along with this also enable new command line options to over ride those template based parameters if the same get passed as command line options.
> Accept the following options:
> 1. db-conf-path
> 2. db-files-path
> 3. encryption-jar-path
> 4. encryption-key-file
> Also replace code that calls OS specific commands with python specific libraries.
> 
> 
> Diffs
> -----
> 
>   setup/bindir/cloud-setup-databases.in 2ba7d51 
> 
> Diff: https://reviews.apache.org/r/20445/diff/
> 
> 
> Testing
> -------
> 
> Tested the script on both windows and linux environment
> 
> Tested for the following scenarios
> 
> 1. With out passing new options on linux environment
> 2. With passing new options on linux environment
> 3. With passing new options on windows environment
> 4. With out passing new options on windows environment (Script fails here as there are no replacements happened for template parameters during build)
> 
> 
> Thanks,
> 
> Damodar Reddy Talakanti
> 
>


Re: Review Request 20445: Add new Command line options to setup/bindir/cloud-setup-databases.in and remove OS specific commands

Posted by Frank Zhang <fr...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20445/#review41164
-----------------------------------------------------------

Ship it!


Ship It!

- Frank Zhang


On April 17, 2014, 9:41 a.m., Damodar Reddy Talakanti wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20445/
> -----------------------------------------------------------
> 
> (Updated April 17, 2014, 9:41 a.m.)
> 
> 
> Review request for cloudstack, Alex Huang, Frank Zhang, and Hugo Trippaers.
> 
> 
> Bugs: https://issues.apache.org/jira/browse/CLOUDSTACK-6435
>     https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/CLOUDSTACK-6435
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> Currently the python script "setup/bindir/cloud-setup-databases.in" for setup databases uses template based parameters which will get replaced during rpm build. 
> Along with this also enable new command line options to over ride those template based parameters if the same get passed as command line options.
> Accept the following options:
> 1. db-conf-path
> 2. db-files-path
> 3. encryption-jar-path
> 4. encryption-key-file
> Also replace code that calls OS specific commands with python specific libraries.
> 
> 
> Diffs
> -----
> 
>   setup/bindir/cloud-setup-databases.in 2ba7d51 
> 
> Diff: https://reviews.apache.org/r/20445/diff/
> 
> 
> Testing
> -------
> 
> Tested the script on both windows and linux environment
> 
> Tested for the following scenarios
> 
> 1. With out passing new options on linux environment
> 2. With passing new options on linux environment
> 3. With passing new options on windows environment
> 4. With out passing new options on windows environment (Script fails here as there are no replacements happened for template parameters during build)
> 
> 
> Thanks,
> 
> Damodar Reddy Talakanti
> 
>