Add support for storing the "local" pool on Azure

[refi64]

Requirements

All new code should be covered with tests, documentation should be updated. CI should pass.

Description of the Change

This adds support for storing the "local" package pool on Azure. It includes:

• Misc. test fixes from the Python 3 migration when passing --capture.
• Adding some functional tests for Azure publishing based on the S3 ones, as a foundation for these changes.
• Changes to the Azure endpoint syntax to match the official Azure connection string format.
• Some refactoring of Azure publishing to move any code reused by the package pool to a shared file.
• Tangentially related change to clean up temporary files when mirroring.

I described some of these in further detail in the individual commits.

This does not include changing all references to the "local" package pool, because I'm honestly not sure if that's worthwhile...

Checklist

• [x] unit-test added (if change is algorithm)
• [x] functional test added/updated (if change is functional)
• [x] man page updated (if applicable)
• [ ] ~~bash completion updated (if applicable)~~ (CLI was not touched)
• [ ] ~~documentation updated~~ (I don't believe the changes here would result in changes to the website content)
• [x] author name in AUTHORS

[refi64]

Hmm, the Azure logs seem to have some 500 internal server errors from Azurite, I'll look into it locally.

[refi64]

I can't reproduce the CI failure at all locally, so I added a step to upload Azurite's logs if the tests fail, which should hopefully help clear things up a bit...

[refi64]

Can't use 'tar -xzf' extract archive file: /home/runner/work/_actions/_temp_4c5950e6-f316-4a7c-aa09-7b03239b3a59/3b979214-6f9f-46a7-976a-ed55121cf5e1.tar.gz. return code: 2.

...GitHub Actions issue maybe?

[chuan]

Can't use 'tar -xzf' extract archive file: /home/runner/work/_actions/_temp_4c5950e6-f316-4a7c-aa09-7b03239b3a59/3b979214-6f9f-46a7-976a-ed55121cf5e1.tar.gz. return code: 2.

...GitHub Actions issue maybe?

Where did you see this error?

Since in the CI #137 run only the long test for v1.17 failed, I suspect it is an intermittent issue with Azurite or the runtime environment.

[refi64]

@chuan apologies for the confusion, that was from an earlier run, but I think @randombenj restarted it since then. There was a bug in the way config files were written that I missed, just pushed up a fix.

[chuan]

@refi64 I figured out that the Azurite 500 errors are due to its use of /tmp. The Azure tests in your change should pass the CI if you pull the change here: https://github.com/aptly-dev/aptly/pull/1078.

[refi64]

@chuan thanks! I just pushed a new version including that commit to see how it goes

[chuan]

The latest change looks good to me and glad to see all CI jobs passed!

[refi64]

Just rebased on top of the merged master incl. the Azurite fixes PR.

[randombenj]

As this is quite a big change and I am not too familiar with the source yet, this will probably take some time to review ...

[randombenj]

I am not sure if I understand all the changes yet ... However, I am wondering a bit why the changes are necessary in the first place? Couldn't you just mount the azure blob storage to the local disk and then run aptly as is to this directory? This would be much simpler and aptly wouldn't have to implement all the read write logic?

[chuan]

I am not sure if I understand all the changes yet ... However, I am wondering a bit why the changes are necessary in the first place? Couldn't you just mount the azure blob storage to the local disk and then run aptly as is to this directory? This would be much simpler and aptly wouldn't have to implement all the read write logic?

Mounting Azure blob storage to local disk can usually be done via either Azure Blob NFS support or via FUSE driver, both have their own limitations.

Azure blob NFS protocol only supports limited features of blob storage. For example, users can only use it with a storage account that enables the hierarchical namespace and cannot enable NFS on an existing storage account. More limitations can be found here: https://docs.microsoft.com/en-us/azure/storage/blobs/network-file-system-protocol-known-issues

FUSE implementation (https://github.com/Azure/azure-storage-fuse) uses the same underlying REST APIs as the implementation here. It is targeting more generic use cases, has different requirements, and its implementation is much more complicated because it needs to support the complete set of filesystem APIs. Having our own storage and publish implementation directly based on REST APIs cuts through the middle layer and makes it possible to optimize for our own use cases. For example, the file moving operation is made atomic in the Azure publish support, which is not supported in FUSE.

Similar arguments also apply as to why we need to support publishing to S3 and OpenStack Swift.

[refi64]

As an aside, in addition to the other reasons already detailed above, using FUSE filesystems for stuff like this tends to introduce a decent number of new and exciting bugs, simply because the ways that the filesystems deviate from standard POSIX behavior usually aren't accounted for in various tools.

Quoting Benj Fassbind (2022-06-23 04:17:49)

I am not sure if I understand all the changes yet ... However, I am wondering a bit why the changes are necessary in the first place? Couldn't you just mount the azure blob storage to the local disk and then run aptly as is to this directory? This would be much simpler and aptly wouldn't have to implement all the read write logic?

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: <aptly-dev/aptly @.**>

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 61.22449% with 133 lines in your changes are missing coverage. Please review.

Project coverage is 52.10%. Comparing base (b3d9055) to head (5307c1a). Report is 88 commits behind head on master.

:exclamation: Current head 5307c1a differs from pull request most recent head da0c525. Consider uploading reports for the commit da0c525 to get more accurate results

Files	Patch %	Lines
azure/package_pool.go	64.96%	33 Missing and 15 partials :warning:
utils/config.go	34.37%	19 Missing and 2 partials :warning:
azure/azure.go	76.82%	13 Missing and 6 partials :warning:
context/context.go	0.00%	18 Missing :warning:
api/mirror.go	0.00%	16 Missing :warning:
files/public.go	47.61%	8 Missing and 3 partials :warning:

Additional details and impacted files

@@             Coverage Diff             @@
##           master    #1074       +/-   ##
===========================================
- Coverage   66.30%   52.10%   -14.21%     
===========================================
  Files         141       75       -66     
  Lines       15908    11491     -4417     
===========================================
- Hits        10548     5987     -4561     
- Misses       4610     4920      +310     
+ Partials      750      584      -166     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[neolynx]

could you check if https://github.com/aptly-dev/aptly/pull/1283 works for you ?

especially since a prefix was introduced in azure/public.go ...