<div dir="ltr">Hi Michael,<div>Thanks a lot for your good comments. I will create a new patch to address your comments which only includes the items #1 and #2, and leave the static ip patch for now. </div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<span style="font-size:12.800000190734863px;font-family:arial,sans-serif">I have no heartburn over making that optional but the other password<br></span><span style="font-size:12.800000190734863px;font-family:arial,sans-serif">parameters are being done with tuning knobs. I think I would prefer to<br>
</span><span style="font-size:12.800000190734863px;font-family:arial,sans-serif">add a tuning knob for that.</span></blockquote><div><span style="font-size:12.800000190734863px;font-family:arial,sans-serif"><br></span></div>
<div>Would you please elaborate on this comments? Are you suggesting allow user to pass the password as a command line parameter? </div><div><br></div><div>I found that the centos template could get the password from an environment variable named "root_password", so I use it in this way:</div>
<div>root_password="my root password" lxc-create -t centos -n my_container -E</div><div>where -E is the newly added switch to not expire the root password.</div><div><br></div><div>Thanks again!</div></div><div class="gmail_extra">
<br><br><div class="gmail_quote">On Fri, Mar 14, 2014 at 8:11 AM, Michael H. Warfield <span dir="ltr"><<a href="mailto:mhw@wittsend.com" target="_blank">mhw@wittsend.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Ok... Back from the -users list, now I'll get into some details...<br>
<div class=""><br>
On Thu, 2014-03-13 at 23:49 +0800, Mingjiang Shi wrote:<br>
> Hi All,<br>
> This patch introduces the following enhancements to the centos<br>
> templates.<br>
> 1. Added option to not expire the root password<br>
<br>
</div>I have no heartburn over making that optional but the other password<br>
parameters are being done with tuning knobs. I think I would prefer to<br>
add a tuning knob for that.<br>
<br>
I should mentioned that I, personally, actually never use those<br>
generated passwords anyways and generally immediately follow the<br>
lxc-create command with a command like this:<br>
<br>
chroot /var/lib/lxc/${Container}/rootfs passwd<br>
<br>
That resets the password to one I'm providing without even using the<br>
generated one and it resets the expired bit, so you're done anyways. We<br>
really can't "prompt for" the password in the template because of issues<br>
regarding interactive prompts in API functions, from what I understand.<br>
<div class=""><br>
> 2. Added option to copy the host ssh public key to the container so<br>
> that one can ssh to the my containers without using password<br>
<br>
</div>Ubuntu and Gentoo seem to already do this in a different way using a<br>
different parameter than what you're suggesting...<br>
<br>
They:<br>
-S auth_key<br>
Copies specified auth_key to root users authorized_keys file.<br>
<br>
You:<br>
-s<br>
Copies the user's authorized_keys file to root in the container.<br>
<br>
I'm not sure I would implement that feature your way at all. It has<br>
some problems. That template can only be run under "uid 0" (root).<br>
Older versions of sudo would leave HOME set to the calling user, causing<br>
it to act one way, newer version set it to /root resulting in another<br>
behavior. That sort of potential ambiguity is not a good practice.<br>
<br>
I'm also not sure copying the two public id files is the right another<br>
either. It may be for you, it certainly wouldn't be for me. My hosts<br>
don't have the individual id files on them (and certainly no private<br>
keys) and only have the authoritized_keys file (which a number of public<br>
id keys). I have no need for the private keys on those hosts, since<br>
they're only connected to from a secure remote workstation and I use key<br>
agent forwarding for subsequent connections between remote systems.<br>
<br>
My choice there would be the method in the Ubuntu template (the Gentoo<br>
template has a typo in it that likely renders that feature non-operable<br>
in a non-obvious way {hangs on line 620}).<br>
<div class=""><br>
> 3. Added option to set static IP to the container. Used when run<br>
> services which needs static ip address, such as hadoop.<br>
<br>
</div>Ok... Here I have some heartburn. Not from adding the IP, but the way<br>
it's done. Two other templates, lxc-altlinux and lxc-openmadrivia, are<br>
already doing this but using different parameters.<br>
<br>
Your option:<br>
<br>
--ip) use_static_ip=yes; ip=$2;<br>
<br>
Their options:<br>
<br>
-4|--ipv4) ipv4=$2;<br>
-6|--ipv6) ipv6=$2;<br>
<br>
I realize that you may not care about IPv6, but I certainly do.<br>
<br>
Even in the IPv4 case, you've assumed a /24. That may be often true but<br>
it certainly is not universally true and you don't provide a way to<br>
specify that either by included CIDR or by netmask.<br>
<br>
You specify the /24 CIDR into the config but then don't specify the<br>
netmask in the ifcfg files. That would not end well on my network,<br>
which is an old "class B" subneted in may variable ways (to say nothing<br>
of the 10./8 or 172.16/12 or others below 128./8). If someone happen to<br>
be using the 10. net on their private bridge and you give one machine a<br>
10.1.1.1, the system is going to try and configure itself to <a href="http://10.1.1.1/8" target="_blank">10.1.1.1/8</a>,<br>
with a broadcast of 10.255.255.255. 172.17.1.1 would end up being<br>
<a href="http://172.17.1.1/16" target="_blank">172.17.1.1/16</a> with a broadcast of 172.17.255.255. Not the intended<br>
result at all.<br>
<br>
You're not sanity checking those IP addresses to insure they are valid<br>
addresses or usable addresses. Yeah, it'll cause the container's<br>
networking to blow up down the road but it would be nice to catch the<br>
easy typos before getting that far. Sanity checking addresses like that<br>
is a non-trivial task, though, I will admit.<br>
<br>
I also have some heartburn here as to multiple interfaces. You may have<br>
only one in each container, I happen to have at least two in each. It's<br>
not clear how that structures out when you're merely cat'ing the<br>
"lxc.network.ipv4 =" line to the end of the config file. It would be<br>
after the definition of veth1 (the second interface). I think it will<br>
only apply to veth0, since these are not tightly associated, but I'll<br>
have to tinker with it a bit. I generally find that just setting up the<br>
ifcfg-eth* files are sufficient.<br>
<br>
I think we could have a whole long discussion over assignment of static<br>
IPv4 and IPv6 addresses much along the lines that took place a couple of<br>
months ago with regards to assigning persistent mac addresses to<br>
containers. I also need to have a closer look at how those other two<br>
templates are handling netmasks and netblocks and multiple interfaces.<br>
<div class=""><br>
> Let me know if you have any questions. Thanks!<br>
<br>
</div>Other problems I have with your attached patch...<br>
<br>
It makes other changes to the template that are not documented,<br>
reasoned, or commented on.<br>
<br>
You've added packages to the package list, and I wouldn't normally<br>
disagree with them (I add most of them myself) but we've been trying to<br>
keep the minimal config as minimal as possible. Yeah, I would place<br>
"sudo" on the list of "should be there" but "wget"? Yeah, I like wget<br>
but I have no heartburn with curl and if we all added our fav's to the<br>
default for everybody, I'm not sure we'd be happy with the result.<br>
There's been some discussion over "additional packages" (like Oracle<br>
does) but not much.<br>
<br>
You've "flipped case" on some variables for no apparent reason. I know<br>
that's a gimish gamash of upper and lower case variable names and (even<br>
though I'm not the original author - 4th down on the list in fact) but,<br>
if you start making arbitrary changes in style, outside of what's been<br>
established, it creates unnecessary patching in git and the potential<br>
for "patch wars". Such changes are also potential for trouble if you<br>
"miss something" and "something unintended occurs". I inherited the<br>
gimish gamash and have made no effort to "clean it up", I admit.<br>
<br>
If you change something, even if you think it's trivial or just cleaning<br>
something up, you should comment on it either in the code or in the<br>
patch header comments or both.<br>
<br>
You've got some good ideas but this needs more discussion in the realm<br>
of all the templates and some of the details may work fine on your<br>
network but you break on mine and others. Options should "work in<br>
general" as much as possible and not be for specific setups.<br>
<div class=""><br>
> Tests have been run:<br>
> 1. Created a container without using the newly added options, the<br>
> current behaviors are preserved, i.e. root password need to be changed<br>
> at first log, dynamic ip address and no public key is copied.<br>
> 2. Tested the new behaviors are working by creating a container using<br>
> the newly added options.<br>
<br>
> Signed-off-by: Mingjiang Shi <mrjewes at gmail dot com><br>
<br>
</div>At minimum for the networking issues and parameter usage that is<br>
inconsistent with the other existing templates, this isn't ready yet.<br>
<br>
Nacked-by: Michael H. Warfield <mhw@WittsEnd.com><br>
<div class="HOEnZb"><div class="h5"><br>
> ---<br>
> diff --git a/templates/<a href="http://lxc-centos.in" target="_blank">lxc-centos.in</a> b/templates/<a href="http://lxc-centos.in" target="_blank">lxc-centos.in</a><br>
> index 55e0531..3c0e9e6 100644<br>
> --- a/templates/<a href="http://lxc-centos.in" target="_blank">lxc-centos.in</a><br>
> +++ b/templates/<a href="http://lxc-centos.in" target="_blank">lxc-centos.in</a><br>
> @@ -229,31 +229,57 @@ configure_centos()<br>
> cd ${rootfs_path}/etc/rc.d/rc6.d<br>
> ln -s ../init.d/lxc-halt S00lxc-reboot<br>
> )<br>
> fi<br>
><br>
> + if [ $use_static_ip == "yes" ];then<br>
> + # configure the network using static ip<br>
> + cat <<EOF ><br>
> ${rootfs_path}/etc/sysconfig/network-scripts/ifcfg-eth0<br>
> +DEVICE=eth0<br>
> +BOOTPROTO=none<br>
> +ONBOOT=yes<br>
> +HOSTNAME=${utsname}<br>
> +NM_CONTROLLED=no<br>
> +TYPE=Ethernet<br>
> +MTU=${MTU}<br>
> +EOF<br>
> + # set static route, add the default gateway<br>
> + cat <<EOF > ${rootfs_path}/etc/sysconfig/static-routes<br>
> +any net default gw ${gw}<br>
> +EOF<br>
> +<br>
> + # set minimal hosts, don't resolve the hostname to 127.0.0.1<br>
> + # resolve it to the static ip<br>
> + cat <<EOF > $rootfs_path/etc/hosts<br>
> +127.0.0.1 localhost<br>
> +$ip ${utsname} $name<br>
> +EOF<br>
> +<br>
> + else<br>
> # configure the network using the dhcp<br>
> cat <<EOF ><br>
> ${rootfs_path}/etc/sysconfig/network-scripts/ifcfg-eth0<br>
> DEVICE=eth0<br>
> BOOTPROTO=dhcp<br>
> ONBOOT=yes<br>
> -HOSTNAME=${UTSNAME}<br>
> +HOSTNAME=${utsname}<br>
> NM_CONTROLLED=no<br>
> TYPE=Ethernet<br>
> MTU=${MTU}<br>
> EOF<br>
><br>
> + # set minimal hosts<br>
> + cat <<EOF > $rootfs_path/etc/hosts<br>
> +127.0.0.1 localhost $name<br>
> +EOF<br>
> + fi<br>
> +<br>
> # set the hostname<br>
> cat <<EOF > ${rootfs_path}/etc/sysconfig/network<br>
> NETWORKING=yes<br>
> -HOSTNAME=${UTSNAME}<br>
> +HOSTNAME=${utsname}<br>
> EOF<br>
><br>
> - # set minimal hosts<br>
> - cat <<EOF > $rootfs_path/etc/hosts<br>
> -127.0.0.1 localhost $name<br>
> -EOF<br>
><br>
> # set minimal fstab<br>
> cat <<EOF > $rootfs_path/etc/fstab<br>
> /dev/root / rootfs defaults<br>
> 0 0<br>
> none /dev/shm tmpfs<br>
> nosuid,nodev 0 0<br>
> @@ -337,12 +363,15 @@ EOF<br>
> echo ${root_password} > ${config_path}/tmp_root_pass<br>
> echo "Storing root password in<br>
> '${config_path}/tmp_root_pass'"<br>
> fi<br>
><br>
> echo "root:$root_password" | chroot $rootfs_path chpasswd<br>
> - # Also set this password as expired to force the user to change<br>
> it!<br>
> - chroot $rootfs_path passwd -e root<br>
> +<br>
> + if [ $expire_root_passwd == "yes" ];then<br>
> + # Set this password as expired to force the user to change<br>
> it!<br>
> + chroot $rootfs_path passwd -e root<br>
> + fi<br>
><br>
> # This will need to be enhanced for CentOS 7 when systemd<br>
> # comes into play... /\/\|=mhw=|\/\/<br>
><br>
> return 0<br>
> @@ -370,11 +399,11 @@ download_centos()<br>
> fi<br>
><br>
> # download a mini centos into a cache<br>
> echo "Downloading centos minimal ..."<br>
> YUM="yum --installroot $INSTALL_ROOT -y --nogpgcheck"<br>
> - PKG_LIST="yum initscripts passwd rsyslog vim-minimal<br>
> openssh-server openssh-clients dhclient chkconfig rootfiles<br>
> policycoreutils"<br>
> + PKG_LIST="yum initscripts passwd rsyslog vim openssh-server<br>
> openssh-clients dhclient chkconfig rootfiles policycoreutils wget tar<br>
> sudo zip unzip which"<br>
><br>
> # use temporary repository definition<br>
> REPO_FILE=$INSTALL_ROOT/etc/yum.repos.d/lxc-centos-temp.repo<br>
> mkdir -p $(dirname $REPO_FILE)<br>
> if [ -n "$repo" ]; then<br>
> @@ -559,10 +588,15 @@ lxc.rootfs = $rootfs_path<br>
> fi<br>
> fi<br>
> done < $config_path/config.def<br>
><br>
> rm -f $config_path/config.def<br>
> +<br>
> + # append the container ip address<br>
> + if [ $use_static_ip == "yes" ];then<br>
> + echo "lxc.network.ipv4 = ${ip}/24" >> $config_path/config<br>
> + fi<br>
><br>
> if [ -e "@LXCTEMPLATECONFIG@/centos.common.conf" ]; then<br>
> echo "<br>
> # Include common configuration<br>
> lxc.include = @LXCTEMPLATECONFIG@/centos.common.conf<br>
> @@ -635,46 +669,87 @@ Optional args:<br>
> -p,--path path to where the container rootfs will be<br>
> created, defaults to /var/lib/lxc/name.<br>
> -c,--clean clean the cache<br>
> -R,--release Centos release for the new container. if the host<br>
> is Centos, then it will defaultto the host's release.<br>
> --fqdn fully qualified domain name (FQDN) for DNS and<br>
> system naming<br>
> --repo repository to use (url)<br>
> + --ip specify a static ip, must use with --gw option<br>
> + --gw specify the default gateway, required if --ip<br>
> option is used.<br>
> + -E, don't set the root password expired<br>
> + -s, Copy the current ssh public key to the authorized<br>
> host list of the container<br>
> -a,--arch Define what arch the container will be<br>
> [i686,x86_64]<br>
> -h,--help print this help<br>
> EOF<br>
> return 0<br>
> }<br>
><br>
> -options=$(getopt -o a:hp:n:cR: -l<br>
> help,path:,rootfs:,name:,clean,release:,repo:,arch:,fqdn: -- "$@")<br>
> +copy_ssh_key_to_container()<br>
> +{<br>
> + # create the .ssh folder and set permission<br>
> + container_ssh_dir=${rootfs_path}/root/.ssh<br>
> + if [ ! -d $container_ssh_dir ];then<br>
> + mkdir -p $container_ssh_dir<br>
> + chmod 700 $container_ssh_dir<br>
> + fi<br>
> +<br>
> + # copy the id_rsa.pub to authorized_keys if exists<br>
> + my_ssh_id=$HOME/.ssh/id_rsa.pub<br>
> + if [ -f $my_ssh_id ];then<br>
> + cat $my_ssh_id >> $container_ssh_dir/authorized_keys<br>
> + fi<br>
> +<br>
> + # copy the id_dsa.pub to authorized_keys if exists<br>
> + my_ssh_id=$HOME/.ssh/id_dsa.pub<br>
> + if [ -f $my_ssh_id ];then<br>
> + cat $my_ssh_id >> $container_ssh_dir/authorized_keys<br>
> + fi<br>
> +}<br>
> +<br>
> +options=$(getopt -o a:hp:n:cR:Es -l<br>
> help,path:,rootfs:,name:,clean,release:,repo:,arch:,fqdn:,ip:,gw: --<br>
> "$@")<br>
> if [ $? -ne 0 ]; then<br>
> usage $(basename $0)<br>
> exit 1<br>
> fi<br>
><br>
> arch=$(arch)<br>
> +use_static_ip=no<br>
> +ip=<br>
> +gw=<br>
> +expire_root_passwd=yes<br>
> +copy_ssh_id=no<br>
> eval set -- "$options"<br>
> while true<br>
> do<br>
> case "$1" in<br>
> -h|--help) usage $0 && exit 0;;<br>
> -p|--path) path=$2; shift 2;;<br>
> --rootfs) rootfs=$2; shift 2;;<br>
> -n|--name) name=$2; shift 2;;<br>
> -c|--clean) clean=$2; shift 2;;<br>
> -R|--release) release=$2; shift 2;;<br>
> - --repo) repo="$2"; shift 2;;<br>
> + --repo) repo="$2"; shift 2;;<br>
> -a|--arch) newarch=$2; shift 2;;<br>
> --fqdn) utsname=$2; shift 2;;<br>
> + --ip) use_static_ip=yes; ip=$2; shift 2;;<br>
> + --gw) gw=$2; shift 2;;<br>
> + -E) expire_root_passwd=no; shift 1;;<br>
> + -s) copy_ssh_id=yes; shift 1;;<br>
> --) shift 1; break ;;<br>
> *) break ;;<br>
> esac<br>
> done<br>
><br>
> if [ ! -z "$clean" -a -z "$path" ]; then<br>
> clean || exit 1<br>
> exit 0<br>
> fi<br>
><br>
> +if [ ! -z "$ip" -a -z "$gw" ];then<br>
> + echo "Missing the default gateway, use --gw option to specify the<br>
> default gateway"<br>
> + usage $0<br>
> + exit 1<br>
> +fi<br>
> +<br>
> basearch=${arch}<br>
> # Map a few architectures to their generic CentOS repository archs.<br>
> # The two ARM archs are a bit of a guesstimate for the v5 and v6<br>
> # archs. V6 should have hardware floating point (Rasberry Pi).<br>
> # The "arm" arch is safer (no hardware floating point). So<br>
> @@ -846,10 +921,15 @@ if [ $? -ne 0 ]; then<br>
> exit 1<br>
> fi<br>
><br>
> configure_centos_init<br>
><br>
> +# copy the ssh public key to authorized keys in the container<br>
> +if [ $copy_ssh_id == "yes" ];then<br>
> + copy_ssh_key_to_container<br>
> +fi<br>
> +<br>
> if [ ! -z $clean ]; then<br>
> clean || exit 1<br>
> exit 0<br>
> fi<br>
> echo "<br>
> @@ -879,15 +959,17 @@ then<br>
><br>
> chroot ${rootfs_path} passwd<br>
> "<br>
> chroot ${rootfs_path} passwd<br>
> else<br>
> - echo "<br>
> -The root password is set up as "expired" and will require it to be<br>
> changed<br>
> -at first login, which you should do as soon as possible. If you lose<br>
> the<br>
> -root password or wish to change it without starting the container,<br>
> you<br>
> -can change it from the host by running the following command (which<br>
> will<br>
> -also reset the expired flag):<br>
> -<br>
> - chroot ${rootfs_path} passwd<br>
> -"<br>
> + if [ $expire_root_passwd == "yes" ];then<br>
> + echo "<br>
> + The root password is set up as "expired" and will require it to be<br>
> changed<br>
> + at first login, which you should do as soon as possible. If you<br>
> lose the<br>
> + root password or wish to change it without starting the container,<br>
> you<br>
> + can change it from the host by running the following command (which<br>
> will<br>
> + also reset the expired flag):<br>
> +<br>
> + chroot ${rootfs_path} passwd<br>
> + "<br>
> + fi<br>
> fi<br>
> ---<br>
><br>
><br>
> --<br>
> Thanks<br>
> -Mingjiang<br>
><br>
</div></div><span class="HOEnZb"><font color="#888888">> --<br>
> This message has been scanned for viruses and<br>
> dangerous content by MailScanner, and is<br>
> believed to be clean.<br>
> _______________________________________________<br>
> lxc-devel mailing list<br>
> <a href="mailto:lxc-devel@lists.linuxcontainers.org">lxc-devel@lists.linuxcontainers.org</a><br>
> <a href="http://lists.linuxcontainers.org/listinfo/lxc-devel" target="_blank">http://lists.linuxcontainers.org/listinfo/lxc-devel</a><br>
<br>
--<br>
Michael H. Warfield (AI4NB) | <a href="tel:%28770%29%20978-7061" value="+867709787061">(770) 978-7061</a> | mhw@WittsEnd.com<br>
/\/\|=mhw=|\/\/ | (678) 463-0932 | <a href="http://www.wittsend.com/mhw/" target="_blank">http://www.wittsend.com/mhw/</a><br>
NIC whois: MHW9 | An optimist believes we live in the best of all<br>
PGP Key: 0x674627FF | possible worlds. A pessimist is sure of it!<br>
<br>
</font></span><br>_______________________________________________<br>
lxc-devel mailing list<br>
<a href="mailto:lxc-devel@lists.linuxcontainers.org">lxc-devel@lists.linuxcontainers.org</a><br>
<a href="http://lists.linuxcontainers.org/listinfo/lxc-devel" target="_blank">http://lists.linuxcontainers.org/listinfo/lxc-devel</a><br>
<br></blockquote></div><br><br clear="all"><div><br></div>-- <br>Thanks<br>-Mingjiang
</div>