You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by frogfather <gi...@git.apache.org> on 2018/10/01 13:28:38 UTC
[GitHub] brooklyn-server pull request #1003: Update IPTables save method
GitHub user frogfather opened a pull request:
https://github.com/apache/brooklyn-server/pull/1003
Update IPTables save method
Service iptables save doesn't work with Centos7
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/frogfather/brooklyn-server amend_iptables_save
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/brooklyn-server/pull/1003.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1003
----
commit 77b50be911148c7414dbb1b30364b495b4c8f363
Author: frogfather <j....@...>
Date: 2018-10-01T13:23:15Z
Update IPTables save method
----
---
[GitHub] brooklyn-server pull request #1003: Update IPTables save method
Posted by grkvlt <gi...@git.apache.org>.
Github user grkvlt commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/1003#discussion_r221687382
--- Diff: utils/common/src/main/java/org/apache/brooklyn/util/ssh/IptablesCommands.java ---
@@ -130,8 +130,8 @@ 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("if [ ${UID} -eq 0 ] ; then iptables–save > /etc/sysconfig/iptables ; else sudo iptables-save | sudo tee /etc/sysconfig/iptables ; fi",
--- End diff --
Maybe surround this with `BashCommands.ifExecutableElse1("iptables-save", ...)` to ensure the command exists
---
[GitHub] brooklyn-server issue #1003: Update IPTables save method
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the issue:
https://github.com/apache/brooklyn-server/pull/1003
as @aledsage this is largely included in #1006 . @frogfather your call whether there is value in the recent comments here and if so update this PR, make sure to `git merge master` in to this branch, or if not just close this.
---
[GitHub] brooklyn-server issue #1003: Update IPTables save method
Posted by grkvlt <gi...@git.apache.org>.
Github user grkvlt commented on the issue:
https://github.com/apache/brooklyn-server/pull/1003
> Service iptables save doesn't work with Centos7
That is, doesn't work with the CentOS 7 AMI we use on EC2, which has a cut-down `service` command only supporting the standard _start_, _stop_, _restart_ and _status_ command verbs.
---
[GitHub] brooklyn-server issue #1003: Update IPTables save method
Posted by grkvlt <gi...@git.apache.org>.
Github user grkvlt commented on the issue:
https://github.com/apache/brooklyn-server/pull/1003
I think Jenkins is failing due to permissions, maybe on the ASF side?
---
[GitHub] brooklyn-server issue #1003: Update IPTables save method
Posted by grkvlt <gi...@git.apache.org>.
Github user grkvlt commented on the issue:
https://github.com/apache/brooklyn-server/pull/1003
Thanks @frogfather LGTM, will merge assuming tests pass
---
[GitHub] brooklyn-server pull request #1003: Update IPTables save method
Posted by ahgittin <gi...@git.apache.org>.
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"))`
---
[GitHub] brooklyn-server issue #1003: Update IPTables save method
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the issue:
https://github.com/apache/brooklyn-server/pull/1003
LGTM; happy for this to be merged (once confusion with the identical-looking https://github.com/apache/brooklyn-server/pull/1005 is cleared up).
---