----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8406/#review14472 -----------------------------------------------------------
Ship it! Thanks Bill, thank you for your patch. Applied on master; (pl. test and let me know if this works, I cleaned up some code, see the commit after yours) commit 1ae2d720a3ca3bde2baacb0d06610e7466f95bfe Author: Bill Rich <bill.r...@gmail.com> Date: Fri Dec 7 08:39:13 2012 -0800 CLOUDSTACK-591: Changed bridge name parsing in security_group.py to support bridges named with dashes I removed the list pop-ing which makes no sense and instead of getting and splitting on newline and take the first element from the list, I fixed the execute statement to take the first from the list, using head -1: commit 6f29317a8492008d37c7eb0770f0cee650c14c40 Author: Rohit Yadav <bhais...@apache.org> Date: Thu Dec 13 15:26:05 2012 -0800 CLOUDSTACK-591: Fix execute and string processing logic for reboot_vm in security_group - Since we're always getting the first from the list, use head -1 to get the first of the results instead of processing again - Remove unecessay pop (why was it even there) Signed-off-by: Rohit Yadav <bhais...@apache.org> - Rohit Yadav On Dec. 7, 2012, 5:21 p.m., Bill Rich wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/8406/ > ----------------------------------------------------------- > > (Updated Dec. 7, 2012, 5:21 p.m.) > > > Review request for cloudstack. > > > Description > ------- > > When checking for rebooted VMs, security_group.py assumed bridge names would > not include dashed. The original code split the name found in iptables and > used the 2nd member of the returned array. In a case where the bridge had a > hyphen in the name, only a partial name was returned. For example, with a > bridge named br-public, the line 'iptables-save |grep physdev-is-bridged > |grep FORWARD |grep BF |grep '\-o' |awk '{print $9}'' returns BF-br-public. > The name is parsed by splitting the name by hyphens and taking the 2nd member > of the array. This returns br resulting in the script attempting to modify > chains 'BF-br-IN' and 'BF-br-OUT' which don't exist. > > I changed the code to use regular expressions to remove "^BF-" from the chain > name. This will consistently remove just the unwanted part of the chain name > to get the standard brname used throughout the rest of the script. > > > This addresses bug CLOUDSTACK-591. > > > Diffs > ----- > > scripts/vm/network/security_group.py b079890 > > Diff: https://reviews.apache.org/r/8406/diff/ > > > Testing > ------- > > -Modified /var/run/cloud/<VM>.log on hv to include a different ID. > -Waited for security_group.py get_rule_logs_for_vms to be run. > -Checked /var/log/cloud/security_group.log for errors and that the correct > information was being parsed > -Confirmed network connectivity for VM > > > Thanks, > > Bill Rich > >