Here's a patch [1] addressing the location commands failures from the email below.
Svet. [1] https://github.com/apache/brooklyn-server/pull/169 <https://github.com/apache/brooklyn-server/pull/169> > On 31.05.2016 г., at 12:21, Svetoslav Neykov > <[email protected]> wrote: > > Hi Geoff, > > Agree we should change it. As you say breaking an existing blueprint will > actually reveal existing problems. > > Coincidentally I am working on similar changes but on locations so I'd like > to include them here for discussion. Here are the commands which don't have > exist code checks: > * setup.scripts > * installDevUrandom > * generate.hostname > * openIptables > * stopIptables > * extraSshPublicKeyUrls > > Of those openIpTables and stopIptables are already deprecated so perhaps > logging a warning will be enough for them. > > Svet. > > >> On 31.05.2016 г., at 12:12, Geoff Macartney >> <[email protected]> wrote: >> >> hi Brooklyn devs, >> >> Shouldn't an “install” step fail if a pre-install script exits with return >> status non-zero? >> >> At the moment in AbstractSoftwareProcessSshDriver we have >> >> @Override >> public void runPreInstallCommand() { >> >> if(Strings.isNonBlank(getEntity().getConfig(BrooklynConfigKeys.PRE_INSTALL_COMMAND))) >> { >> >> execute(ImmutableList.of(getEntity().getConfig(BrooklynConfigKeys.PRE_INSTALL_COMMAND)), >> "running pre-install commands"); >> } >> } >> >> Not a mention of “failIfNonZero”. Shouldn’t that be the case, however? >> >> There are a handful of similar methods - pre/post install, pre/post >> configure etc. In general I'd imagine you'd want to have the default >> behaviour be to fail if any returned non-zero. >> >> Not too surprisingly, I came across this because a step in a pre-install >> script of mine was failing, but it took time to find because the install >> phase appeared to complete successfully. >> >> I moot we change this. While this could break some existing blueprints, I >> think in such cases it's more likely that it is highlighting a problem that >> has been missed, rather than causing a problem because someone is explicitly >> relying on that behaviour. >> >> What do you think? >> >> all the best >> Geoff >> >> >> ———————————————————— >> Gnu PGP key - http://is.gd/TTTTuI >> >> >
