riptano / ComboAMI

The AMI takes a set of input parameters via the EC2 user-data to install, RAID, ring, and launch a DataStax Enterprise/Community cluster.
69 stars 59 forks source link

base64postscript fails when there are multiple commands #103

Closed alexrudd closed 9 years ago

alexrudd commented 9 years ago

Currently the commands provided through the --base64postscript option are executed by the following lines:

command = base64.b64decode(options.base64postscript)
process = subprocess.Popen(shlex.split(command), stderr=subprocess.PIPE, stdout=subprocess.PIPE, shell=True)
read = process.communicate()

As far as I can tell, the shlex.split function makes it impossible to pass more than one command in the --base64postscript option, making it very hard to do any sort of custom configuration. Removing the shlex.split completely so that it matched the running of postscript_url (here) seems to have the desired effect.

The reason I want to use base64postscript opposed to postscript_url is so that I can pass CloudFormation parameters to the post script commands like so:

"UserData" : { "Fn::Base64":
  { "Fn::Join": [ "", [
    "--clustername ", {"Ref": "ClusterName"}, " ",
    "--totalnodes ", {"Ref": "ClusterSize"}, " ",
    "--version ", {"Ref": "CassandraVersion"}, " ",
    "--customreservation ", {"Ref": "AWS::StackId"}, " ",
    "--repository ", {"Ref": "ConfigRepository"}, " ",
    "--opscenter ", {"Ref": "UseOpsCenter"}, " ",
    "--release ", {"Ref": "ReleaseVersion"}, " ",
    "--base64postscript ", { "Fn::Base64": { "Fn::Join": [ "", [
        "sudo /bin/sed -i -e 's/^server/# server/' /etc/ntp.conf;",
        "sudo /bin/sed -i -e '$ a\\server ", {"Ref": "NtpServer"} ,"' /etc/ntp.conf;",
        "sudo service ntp restart;"
      ] ] } }
    ] ]
  }
}
mlococo commented 9 years ago

Yeah, I'm pretty sure shlex.split is at least unnecessary (and maybe wrong) when used in conjunction with subprocess.popen(shell=True). I'm not likely to have time to test this soon, but if you submit a PR removing shlex.split, and submit a comment confirming that you've executed a successful test using a simple --base64postscript example (without the cloudformation stuff) and documenting the userdata flags that you used for the test, then I'll merge it.

You can use the --repository flag to test a fork/branch.

mlococo commented 9 years ago

Merged. Thanks for your work on this. It's a tiny code change, but the extra effort you put in on testing and documentation made this an instant and painless merge rather than something I had to throw on the back burner. Much thanks.