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

    https://github.com/apache/incubator-brooklyn/pull/980#discussion_r42612710
  
    --- Diff: 
software/base/src/main/java/org/apache/brooklyn/entity/machine/MachineInitTasks.java
 ---
    @@ -105,28 +103,85 @@ protected void openIptablesImpl(Iterable<Integer> 
inboundPorts, SshMachineLocati
                      }
                 } else {
                     iptablesRules = 
createIptablesRulesForNetworkInterface(inboundPorts);
    -                iptablesRules.add(IptablesCommands.saveIptablesRules());
    +                iptablesInstallCommands = 
IptablesCommands.saveIptablesRules();
                 }
    -            List<String> batch = Lists.newArrayList();
    -
    -            ByteArrayOutputStream outStream = new ByteArrayOutputStream();
    -            ByteArrayOutputStream errStream = new ByteArrayOutputStream();
    -            
Tasks.addTagDynamically(BrooklynTaskTags.tagForStreamSoft(BrooklynTaskTags.STREAM_STDOUT,
 outStream));
    -            
Tasks.addTagDynamically(BrooklynTaskTags.tagForStreamSoft(BrooklynTaskTags.STREAM_STDERR,
 errStream));
    -            // Some entities, such as Riak (erlang based) have a huge 
range of ports, which leads to a script that
    -            // is too large to run (fails with a broken pipe). Batch the 
rules into batches of 50
    -            for (String rule : iptablesRules) {
    -                batch.add(rule);
    -                if (batch.size() == 50) {
    -                    machine.execCommands(ImmutableMap.of("out", outStream, 
"err", errStream), "Inserting iptables rules, 50 command batch", batch);
    -                    batch.clear();
    -                }
    -            }
    -            if (batch.size() > 0) {
    -                machine.execCommands(ImmutableMap.of("out", outStream, 
"err", errStream), "Inserting iptables rules", batch);
    +            insertIptablesRules(iptablesRules, iptablesInstallCommands, 
machine);
    +            listIptablesRules(machine);
    +        }
    +    }
    +
    +    /**
    +     * Returns a queued {@link Task} which inserts iptables rules.
    +     */
    +    public Task<Void> insertIptablesRules(final List<String> 
iptablesRules, final String installCommands, final SshMachineLocation machine) {
    +        return DynamicTasks.queue("insert rules", new Callable<Void>() {
    --- End diff --
    
    There's already API which will create a task and attach the streams to it, 
making it available in the log and in the UI. Use 
`SshTasks.newSshExecTaskFactory` when you have a machine available (as in this 
case), or `SshEffectorTasks.ssh` when you wish to retrieve the machine from the 
current context.
    
    The helpers give you some additional goodies like:
    
    ```
    SshTasks.newSshExecTaskFactory(machine, installCommands)
      .requiringExitCodeZero()  // checks the exit code, throwing if not 0, if 
a task fails subsequent task are not executed
      .summary(...) // short description used in logging
    ```
    
    this will give you a `TaskAdaptable` which you can then 
`DynamicTasks.queue`.
    
    ---
    
    When using `new Callable<Void>()` consider using `new Runnable()` if the 
API supports it.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---

Reply via email to