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 > >
