Skip to content

Conversation

@cfiehe
Copy link
Contributor

@cfiehe cfiehe commented Apr 14, 2025

Fixes #1435.

Description of the Change

This PR fixes an issue in case of S3 storage that causes unchanged packages to be uploaded again on a source update of a published repository. The storage.prefix was added twice and the return paths from the storage did not contain the pool directory as first segment. That was the reason, why the code came to the conclusion, that the package does not exist in the bucket at the target location and uploaded the file again.

Signed-off-by: Christoph Fiehe <[email protected]>
@cfiehe cfiehe force-pushed the bugfix/issue-1435-fix-s3-upload-unchanged-package branch from 41ffe14 to 67bd154 Compare April 14, 2025 11:40
@cfiehe cfiehe requested a review from neolynx April 14, 2025 13:27
@neolynx neolynx force-pushed the bugfix/issue-1435-fix-s3-upload-unchanged-package branch from d54a1df to 9b716d4 Compare April 19, 2025 11:21
@neolynx neolynx force-pushed the bugfix/issue-1435-fix-s3-upload-unchanged-package branch from 9b716d4 to 9ef217b Compare April 20, 2025 09:38
@codecov
Copy link

codecov bot commented Apr 20, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 75.03%. Comparing base (ab18da3) to head (e447fc0).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1440   +/-   ##
=======================================
  Coverage   75.03%   75.03%           
=======================================
  Files         158      158           
  Lines       18356    18356           
=======================================
  Hits        13773    13773           
  Misses       3445     3445           
  Partials     1138     1138           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@neolynx neolynx force-pushed the bugfix/issue-1435-fix-s3-upload-unchanged-package branch from 3ad5d24 to e062df6 Compare April 20, 2025 18:33
@neolynx
Copy link
Member

neolynx commented Apr 20, 2025

pipeline fixed.
gocovmerge needed newer go verison, golangci-lint needed a newer version to be compatible ...

@neolynx neolynx force-pushed the bugfix/issue-1435-fix-s3-upload-unchanged-package branch 2 times, most recently from d4fc670 to bf5e700 Compare April 21, 2025 09:37
@neolynx neolynx force-pushed the bugfix/issue-1435-fix-s3-upload-unchanged-package branch from bf5e700 to e447fc0 Compare April 21, 2025 10:01
@neolynx
Copy link
Member

neolynx commented Apr 21, 2025

@neolynx
Copy link
Member

neolynx commented Apr 21, 2025

@cfiehe I wonder if we can somehow test this behavioe in a system test. maybe logging all uploads in debug mode, and the test would check the output ?

@cfiehe
Copy link
Contributor Author

cfiehe commented Apr 23, 2025

@cfiehe I wonder if we can somehow test this behavioe in a system test. maybe logging all uploads in debug mode, and the test would check the output ?

The question is: What should be covered by the test case? It would be the best, if you can somehow check, that only the expected files are uploaded to the S3 storage and validate afterwards that the files are present in the S3. I must admit, that I have not looked at the existing S3 test cases.

@neolynx
Copy link
Member

neolynx commented Apr 23, 2025

I managed to get this fix tested with the existing s3 tests, by adding

  • initializing the logging for commands
  • adding debug logs, which go to the system tests output

https://github.com/aptly-dev/aptly/actions/runs/14593800326/job/40934817222#step:10:754

+DBG S3: LinkFromPool 'pool/main/b/boost-defaults/libboost-program-options-dev_1.49.0.1_i386.deb'

With your fix reverted, the tests fail because the already uploaded package above is uploaded again.
See: #1444

will cleanup and integrate here...

neolynx added 2 commits April 24, 2025 12:13
* initialize zerolog for commands
* Change default log format: remote colors and timestamp
sort both aptly output and gold file. output original output for
debugging on failure.

* Makefile: enable CAPTURE=1 env variable for capturing gold files
* docker-system-test: use AWS env vars for S3 tests
* fix system tests timing issue with order of gpg logs in publish tests
@cfiehe
Copy link
Contributor Author

cfiehe commented Apr 25, 2025

I managed to get this fix tested with the existing s3 tests, by adding

* initializing the logging for commands

* adding debug logs, which go to the system tests output

https://github.com/aptly-dev/aptly/actions/runs/14593800326/job/40934817222#step:10:754

+DBG S3: LinkFromPool 'pool/main/b/boost-defaults/libboost-program-options-dev_1.49.0.1_i386.deb'

With your fix reverted, the tests fail because the already uploaded package above is uploaded again. See: #1444

will cleanup and integrate here...

Ah, cool. That sounds good 👍.

@neolynx neolynx merged commit c05068c into master Apr 25, 2025
89 of 91 checks passed
@neolynx neolynx deleted the bugfix/issue-1435-fix-s3-upload-unchanged-package branch April 25, 2025 11:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

publish switch to S3 uploading unchanged files starting in version 1.6

3 participants