Our code did not deal with the `assumes:` field in the raw yaml
that we get from the store. This lead to the really nasty bug
that on a refresh the assumes is not checked correctly, see
https://bugs.launchpad.net/snapd/+bug/1940553
* We don't need to create the parent directory of exeInCorePath, since we don't
actually need to create the exeInCorePath file at all for the test.
* We also don't need to create the interpInCorePath parent directory because it
is created by MockCommand() since interpInCorePath is an absolute file path.
* Add a comment about the restore for where the hostXdelta3Cmd is located for
clarity.
* Add a comment about the realism of the test and how it's not a necessity, but
is nice to have anyways.
* Add coreInterpCmd.Restore(), even though it is largely unnecessary.
* Sanity check that the test scenario makes sense, if we didn't want deltas,
then either they should have been disabled explicitly through the environment
variable, or there should not have been both binaries mocked.
Signed-off-by: Ian Johnson <ian.johnson@canonical.com>
* Make the caching of the xdelta3 command a callback function that can be
called instead of a location, since in the case when we are executing from
the system snap, we will have additional arguments we need to pass to the
interpreter for the system snap in addition to calling the interpreter. This
also ensures that the working directory the command is run in and the
environment for the command that is returned from CommandFromSystemSnap is
saved as well if that is needed.
* Mock snapdtool.CommandFromSystemSnap in store package so we avoid the ELF
parsing which is architecture dependent - this is already tested in the
snapdtool package, so testing the real codepath here too is a bit over the
top and would make the test unnecessarily complicated when the test is
already fairly complicated as-is.
Signed-off-by: Ian Johnson <ian.johnson@canonical.com>
Prefer using xdelta3 from the system snap if it is available and we are able to
run the config subcommand successfully. This ensures that we try to use a known
good xdelta3 command which should always work with what the store sends down,
but does still offer the fallback of using xdelta3 from the host system if for
some reason the xdelta3 command from the system snap becomes broken at some
point. In that case, attempt using the xdelta3 command from the host system,
performing the same config check before assuming that it is usable.
This has the effect that on older systems like Centos 7 and Amazon Linux 2,
where for example xdelta3 from the distro does not support LZMA, when a delta
generated using LZMA is encountered, we will first try the system snap provided
xdelta3 which should succeed.
We also cache the result of inspecting the available xdelta3 commands, to
prevent unnecessary commands.
Finally, log more messages, namely that if applying the delta fails, log the
error condition as a message so someone with log access can provide information
as well as log if we find an unsuitable xdelta3 somewhere (either from the
system snap, or from the host system). All of these should aid in debugging
future delta issues if they happen to crop up.
Signed-off-by: Ian Johnson <ian.johnson@canonical.com>
It appears that ResponseHeaderTimeout needs to be set explicitly, no headers response from the server doesn't seem to be covered by any other timeout.
This should fix the download getting stuck issue.
For more context, see the stacktrace obtained from a system where download got stuck: https://pastebin.canonical.com/p/nZkRMTBbv3/
(the affected goroutine seems to be sitting inside for-select loop of /usr/lib/go-1.10/src/net/http/transport.go:2033)
(to play with it comment out ResponseHeaderTimeout: ... from transport.go and modify test timeout from 5 * time.Second to a huge value and it should be stuck)
* Set ResponseHeaderTimeout on the default transport.
* Use 250ms for mocked ResponseHeaderTimeout.
* Bump ResponseHeaderTimeout to 15s.
* Bump test timeout to avoid potential issues with LP builds.
It appears that ResponseHeaderTimeout needs to be set explicitly, no headers response from the server doesn't seem to be covered by any other timeout.
This should fix the download getting stuck issue.
For more context, see the stacktrace obtained from a system where download got stuck: https://pastebin.canonical.com/p/nZkRMTBbv3/
(the affected goroutine seems to be sitting inside for-select loop of /usr/lib/go-1.10/src/net/http/transport.go:2033)
(to play with it comment out ResponseHeaderTimeout: ... from transport.go and modify test timeout from 5 * time.Second to a huge value and it should be stuck)
* Set ResponseHeaderTimeout on the default transport.
* Use 250ms for mocked ResponseHeaderTimeout.
* Bump ResponseHeaderTimeout to 15s.
* Bump test timeout to avoid potential issues with LP builds.
The log appears is visible in the user's terminal when running `snap download`
command.
Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
The log appears is visible in the user's terminal when running `snap download`
command.
Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
The current error when the store.Sections() download fails is:
```
Catalog refresh failed: cannot sections...
```
which is not great. This commit changes it to:
```
Catalog refresh failed: cannot retrieve sections
```
which reads a bit better.
had to make the comment in wrappers/services_test.go a one-liner
otherwise half of it is lost
last set of files needing changing (as per current master)
