balena-io-modules / balena-preload

Script for preloading containers onto balena device images
https://www.balena.io/
Apache License 2.0
35 stars 8 forks source link

Avoid hardcoded registry2 URL #244

Closed klutchell closed 3 years ago

klutchell commented 3 years ago

Instead we can extract the registry URL from the image location as we do for the manifest and layer endpoints.

Resolves: https://github.com/balena-io-modules/balena-preload/issues/236 Change-type: patch Signed-off-by: Kyle Harding kyle@balena.io

klutchell commented 3 years ago

I'll format with prettier and push again but the current editorconfig does not match the 4-space tabs currently used in this file.

https://github.com/balena-io-modules/balena-preload/blob/29411dfe11dc26f23695860a5d4c771f5b9fce4b/.editorconfig#L6-L12

pdcastro commented 3 years ago

OK 👍  (If you need to prevent the PR from auto merging, you can add the versionbot/pr-draft label.)

pdcastro commented 3 years ago

Nice 👍   @klutchell, did you get to test this PR with openBalena, to confirm that the registry2 error happens before this PR, and does not happen after this PR? If not, could you test it? To make this kind of testing easier, I've put together these instructions and scripts: https://github.com/pdcastro/ob_multipass (I haven't shared it more widely because it's very opinionated about the use of multipass and everything else, and also because it is not sufficient to produce an installation of openBalena to which devices can be added. But I found it handy to test CLI issues, and in this case, preload.) In my past testing, I was using "open.balena" as the domain name. It works better than a domain name ending with .local.

klutchell commented 3 years ago

I've only tested with balena-cloud.com so far, just to ensure the URL parsing was working. I'll give it a shot with your instructions above and then reopen this PR.

klutchell commented 3 years ago

I can't seem to reproduce the original issue with OB 3.2.1 and CLI 12.37.0.

It seems to always use registry.open.balena and succeed.

ubuntu@localmachine:~$ balena-cli/balena settings 
balenaUrl:            open.balena
apiUrl:               https://api.open.balena
vpnUrl:               vpn.open.balena
registryUrl:          registry.open.balena
registry2Url:         registry2.open.balena
imageMakerUrl:        https://img.open.balena
deltaUrl:             https://delta.open.balena
dashboardUrl:         https://dashboard.open.balena
proxyUrl:             devices.open.balena
dataDirectory:        /home/ubuntu/.balena
projectsDirectory:    /home/ubuntu/BalenaProjects
cacheDirectory:       /home/ubuntu/.balena/cache
binDirectory:         /home/ubuntu/.balena/bin
imageCacheTime:       604800000
tokenRefreshInterval: 3600000
apiKeyVariable:       BALENA_API_KEY

ubuntu@localmachine:~$ balena-cli/balena version
12.37.0

Still investigating how this is possible...

pdcastro commented 3 years ago

It seems to always use registry.open.balena and succeed.

Just some informed guesswork:

klutchell commented 3 years ago

but somehow that resolves to something that works?

I already confirmed that registry2.open.balena does not resolve to anything via curl and ping.

perhaps if cached images already exist locally to the docker engine, the pull is somewhat skipped

Good thought, but I cleared all locally cached images and layers between tests.

pdcastro commented 3 years ago

It seems to always use registry.open.balena and succeed.

So if you add the following:

    console.error(`preload settings:\n${JSON.stringify(balenaSettings, null, 4)}`);

to this line: https://github.com/balena-io-modules/balena-preload/blob/v10.4.6/lib/preload.js#L853

... you're saying that you get this:

    "registryUrl": "registry.open.balena",
    "registry2Url": "registry.open.balena",

instead of this?

    "registryUrl": "registry.open.balena",
    "registry2Url": "registry2.open.balena",

If the changes in this PR still work with openBalena and balenaCloud, we can merge the PR, produce a new CLI release and ask the forum users to test it (explaining we're unsure of the solution because we couldn't reproduce the error) and let us know whether it works for them.

klutchell commented 3 years ago

The settings are what we expect, different values for registry and registry2, but I still can't reproduce the failure. I'm going to add more debugging around my changes before we go further.

preload settings:
{
    "balenaUrl": "open.balena",
    "apiUrl": "https://api.open.balena",
    "vpnUrl": "vpn.open.balena",
    "registryUrl": "registry.open.balena",
    "registry2Url": "registry2.open.balena",
    "imageMakerUrl": "https://img.open.balena",
    "deltaUrl": "https://delta.open.balena",
    "dashboardUrl": "https://dashboard.open.balena",
    "proxyUrl": "devices.open.balena",
    "tunnelUrl": "tunnel.open.balena",
    "dataDirectory": "/home/ubuntu/.balena",
    "projectsDirectory": "/home/ubuntu/BalenaProjects",
    "cacheDirectory": "/home/ubuntu/.balena/cache",
    "binDirectory": "/home/ubuntu/.balena/bin",
    "imageCacheTime": 604800000,
    "tokenRefreshInterval": 3600000,
    "apiKeyVariable": "BALENA_API_KEY"
}
klutchell commented 3 years ago

I found that _getApplicationSize and thus all of my changes (except _getRegistryToken) are only called when multiple images are part of the release being preloaded. I couldn't reproduce because I was testing with a single image release.

I switched to a multi-container example and reproduced the issue, and confirmed the fix in this PR.