[#1215] Ensure module postrun command only runs for modified envs #1216
Conversation
Magisus
force-pushed
the
module-postrun-bug
branch
3 times, most recently
from
68e0605 to
aea1812
Compare
Magisus
changed the title
Magisus
force-pushed
the
module-postrun-bug
branch
from
aea1812 to
2c242fe
Compare
|
Tested this out, deploying a module to all environments, but only expecting it to be actually updated in one of them. Previously, this caused the following: Now, it only runs for the one env where an update happened. |
| expect(R10K::Util::Subprocess).to receive(:new). | ||
| with(["/generate/types/wrapper", "first"]). | ||
| and_return(mock_subprocess) | ||
|
|
||
| expect(subject).to receive(:visit_environment).and_wrap_original do |original, environment, &block| | ||
| if environment.name == 'first' | ||
| expect(environment).to receive(:deploy).and_return(true) | ||
| expect(environment).to receive(:modules).and_return([mock_mod]) |
|
@Magisus Thanks for all your work on this! I've tested this PR out today in my environment and everything seems to work as advertised. If I create a branch of a module with a random name not matching any environment name, the postrun isn't called. |
|
This will also need a changelog entry. |
| def self.concurrent_accept(modules, visitor, loader, pool_size, logger) | ||
| mods_queue = modules_visit_queue(modules, visitor, loader) | ||
| sync_queue(mods_queue, pool_size, logger) | ||
| end | ||
|
|
||
| # Returns a Queue of the names of modules actually updated |
There was a problem hiding this comment.
fwiw, I think this will return either a Queue of module names, or nil, if there was an error during a deprecated accept call, or raise if there was an error during the newer syncs.
There was a problem hiding this comment.
Do you want me to update this? I didn't dig into these methods, but I can if you want.
There was a problem hiding this comment.
That's fine, I was more trying to tie together why we were checking for both nil? and empty? in the caller
…d envs Previously, when a postrun command with interpolated environments was specified, the `deploy module` command with no environments specified would cause the command to be run for all environments that contained the module, whether or not the env was actually updated by the deploy. Similarly, `puppet generate types` would be run for any environment that contained the module, not just those for which it was updated. Together, these two things could result in a huge amount of unnecessary work. This commit makes modules return the result of their syncing, true for "updated" and false for "not updated". The environment's `deploy` function collects this information, recording the names of each module that was updated as part of the deploy. The `module deploy` action then uses this to determine which environments actually had modules modified, and uses that to populate the `$modifiedenvs` in the postrun command. So now, the command will only be run for environments that actually updated a module. This also updates the logic for running `puppet generate types` to follow the same pattern: it is only run for environments that actually had an updated module. Note this does not affect environment deploys.
mwaggett
force-pushed
the
module-postrun-bug
branch
from
1243626 to
a4f6b58
Compare
Previously, when a postrun command with interpolated environments was
specified, the
deploy modulecommand with no environments specifiedwould cause the command to be run for all environments that contained
the module, whether or not the env was actually updated by the
deploy.
Similarly,
puppet generate typeswould be run for any environment thatcontained the module, not just those for which it was updated. Together,
these two things could result in a huge amount of unnecessary work.
This commit makes modules return the result of their syncing, true for
"updated" and false for "not updated". The environment's
deployfunction collects this information, recording the names of each module
that was updated as part of the deploy. The
module deployaction thenuses this to determine which environments actually had modules modified,
and uses that to populate the
$modifiedenvsin the postrun command. Sonow, the command will only be run for environments that actually updated
a module.
This also updates the logic for running
puppet generate typestofollow the same pattern: it is only run for environments that actually
had an updated module.
Note this does not affect environment deploys.