Improved startup time #250
Conversation
| @@ -191,6 +191,9 @@ def _setup_zap_cli(self): | |||
| """ | |||
| self.zap_cli.extend(self._get_standard_options()) | |||
|
|
|||
| # Addon update has already been done, if enabled. Prevent a new check for update | |||
| self.zap_cli.append("-silent") | |||
There was a problem hiding this comment.
Just wondering if this option will place nice with the -addonupdate option, ie if a user has updateAddons=true in config, will the -silent option prevent this from working, or maybe cause the command to fail?
There was a problem hiding this comment.
It indeed does not play nice with -addonupdate, but this is why I added the option in _setup_zap_cli() and not in _get_standard_options() : unless I missed something in the call tree, the -silent should be added only on the main ZAP command, not the command that deals with installing and updating plugins.
Unless I missed something, it should be fine (it looked fine in my tests)
| @@ -151,7 +151,7 @@ def get_update_command(self): | |||
| *self._get_standard_options(), | |||
| "-cmd", | |||
| ] | |||
| if self.my_conf("miscOptions.updateAddons", default=True): | |||
| if self.my_conf("miscOptions.updateAddons"): | |||
There was a problem hiding this comment.
I'm not entirely sure if I understand correctly, so please bear with me. If updateAddons is not "truthy", get_update_command method will return an empty list. In the _handle_plugins function of zap_podman module, it seems that this would result in the command being ["sh", "-c"] plus the zap_cli command. In this case, would it be better to skip the handle_plugin execution altogether similar to how it's handled in zap_none ?
There was a problem hiding this comment.
yes, in zap_podman, the situation is a little dirty because, afaik, podman can run only a single command, and since we need to run the update separately from the scan, the code needs to cheat a little with a "sh -c '; '".
When there's no need to handle plugins, it's would probably be preferable to avoid this sh -c all together, especially if updateAddons is off by default.
I will improve on that, such that and skip the handle_plugin when possible
There was a problem hiding this comment.
so, I tried to improve this a bit, but: _handle_plugins() can't actually do the same as its zap_none counterpart, because it can't actually run a podman command.
I updated in this way:
_handle_plugins()now simply returns the command to handle the plugins as a list (i.e.: empty list if there's nothing to do)- the caller (
run()) will then either run the regular ZAP command if there are not plugin to be handled (now default), or bundle both the plugin command and the scan command in a singleshwrapper.
Let me know if that's a better solution, or if we should approach it differently.
There was a problem hiding this comment.
This approach looks like a good improvement.
Just to understand better, why are both commands run in a single podman execution? Could they run separately in different podman executions while sharing storage? I’m not suggesting a change, just curious about how it works
There was a problem hiding this comment.
It should be doable, yes
But instead returns either the command to handle plugins as a list. The caller then needs to, if need be, inject that in a `sh` wrapper
3 changes:
updateAddonsfrom True to False (including documentation update)-silentoption)e2e test time statistics (limited to only VAPI test) :
→ original code (updateAddons is overridden to False during the e2e tests):
1 passed, 2 deselected in 215.03s (0:03:35)
1 passed, 2 deselected in 220.85s (0:03:40)
1 passed, 2 deselected in 328.86s (0:05:28)
1 passed, 2 deselected in 368.89s (0:06:08)
→ disable the "fake" run :
1 passed, 2 deselected in 199.90s (0:03:19)
1 passed, 2 deselected in 197.37s (0:03:17)
1 passed, 2 deselected in 191.93s (0:03:11)
→ disable the "fake" run + use
-silent:1 passed, 2 deselected in 64.06s (0:01:04)
1 passed, 2 deselected in 36.08s
1 passed, 2 deselected in 44.79s
1 passed, 2 deselected in 48.89s
NOTE: the time varies from run to run even when using the same image, so these statistic might not be very valuable, but still seem to show improvements.