> On June 13, 2016, 1:03 p.m., Andrew Onischuk wrote: > > ambari-common/src/main/python/ambari_commons/firewall.py, line 123 > > <https://reviews.apache.org/r/48309/diff/1/?file=1408121#file1408121line123> > > > > Can you please explain why did we remove the check for firewalld? > > Masahiro Tanaka wrote: > Thank you for reviewing. > Acutually I didn't remove the check for firewalld. `systemctl is-active` > can take multiple arguments. > This is a sample output on CentOS7.2 > > ``` > # systemctl is-active iptables firewalld > inactive > active > # systemctl is-active iptables > inactive > # systemctl is-active firewalld > active > ``` > > Andrew Onischuk wrote: > Oh, I see. > > So: > if first is active and second is inactive what is the return code of > command? > if second is active and first is inactive what is the return code of > command? > > Masahiro Tanaka wrote: > Accoding to the `man systemctl`: > ``` > is-active PATTERN... > Check whether any of the specified units are active (i.e. running). > Returns an exit > code 0 if at least one is active, or non-zero otherwise. Unless > --quiet is specified, > this will also print the current unit state to standard output. > ``` > > We can expect that if at least one unit is active, then return code (exit > code) equals 0. > > BUT there is a bug in *systemd*. See [this PR on > GitHub](https://github.com/systemd/systemd/pull/2430) > > So the actual behavior on CentOS 7.2 is below: > ``` > # systemctl is-active iptables firewalld > inactive > active > # systemctl is-active iptables > inactive > # systemctl is-active firewalld > active > # systemctl is-active iptables firewalld > inactive > active > # echo $? > 3 > # systemctl is-active firewalld iptables > active > inactive > # echo $? > 3 > ``` > > That's why I checked if stdout includes the string `active` in my code. > > Andrew Onischuk wrote: > What will happen is firewalld or iptables is absent as a services, could > that be the case for centos7?
Basically try doing something like this: # systemctl is-active firewalld somethingunknown # echo $? - Andrew ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48309/#review137283 ----------------------------------------------------------- On June 7, 2016, 11:11 a.m., Masahiro Tanaka wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/48309/ > ----------------------------------------------------------- > > (Updated June 7, 2016, 11:11 a.m.) > > > Review request for Ambari, Andrew Onischuk, Dmytro Sen, Florian Barca, and > Yusaku Sako. > > > Bugs: AMBARI-17047 > https://issues.apache.org/jira/browse/AMBARI-17047 > > > Repository: ambari > > > Description > ------- > > In firewall.py, `systemctl is-active iptables || systemctl is-active > firewalld` is passed to `run_in_shell` function, which splits cmd string by > using `shlex.split`. > > run_in_shell function finally calls `subprocess.Popen` with `shell=True`, so > the cmd string is evaluated like `Popen(['/bin/sh', '-c', 'systemctl', > 'is-active', 'iptables', '||', 'systemctl', 'is-active', 'firewalld'])`. This > doesn't returns values as expected, because after args[1] (in this case, > after the first `is-active`) are evaluated as sh arguements. > > `systemctl is-active` can take multiple arugments, so we can use it. > > > Diffs > ----- > > ambari-common/src/main/python/ambari_commons/firewall.py 72e6d26 > > Diff: https://reviews.apache.org/r/48309/diff/ > > > Testing > ------- > > mvn clean test & manual test > > > Thanks, > > Masahiro Tanaka > >
