[GitHub] brooklyn-server pull request #1003: Update IPTables save method

[ahgittin]

Github user ahgittin commented on a diff in the pull request:

    https://github.com/apache/brooklyn-server/pull/1003#discussion_r235932933
  
    --- Diff: 
utils/common/src/main/java/org/apache/brooklyn/util/ssh/IptablesCommands.java 
---
    @@ -130,8 +131,9 @@ public static String firewalldServiceIsActive() {
          *
          */
         public static String saveIptablesRules() {
    -        return alternatives(sudo("service iptables save"),
    -                            chain(installPackage("iptables-persistent"), 
sudo("/etc/init.d/iptables-persistent save")));
    +        return alternatives(
    +                ifExecutableElse1("iptables–save", "if [ ${UID} -eq 0 ] 
; then iptables–save > /etc/sysconfig/iptables ; else sudo iptables-save | 
sudo tee /etc/sysconfig/iptables ; fi"),
    --- End diff --
    
    this removes `service iptables save` altogether, is that intended?  a 
comment to that effect would be useful,
    or if it is still useful sometimes (older OS's?) then keep it in the 
alternatives list
    
    also worth checking whether `sudoNew("iptables-save > 
/etc/sysconfig/iptables")` works; the way it does `echo COMMAND | sudo bash` 
_should_ support redirect for non-root users; if not maybe refactor to add a 
`sudoRedirect` method in `BashCommands` capturing the conditional `tee` so that 
you could write `ifExecutableElse1("iptables-save", 
sudoRedirect("iptable-save", "/etc/sysconfig/iptables"))`



---