Add ability to sync specific architectures. #3128

lukaszraczylo · 2025-04-30T14:42:41Z

What type of PR is this?

Feature

Which issue does this PR fix:

Adds ability to specify the images architecture for mirroring.

What does this PR do / Why do we need it:

Instead of pulling 10s of GB of images for different architectures, you can now pull only architectures you use.

If an issue # is not available please add repro steps and logs showing the issue:

Testing done on this change:

Automation added to e2e:

Will this break upgrades or downgrades?

Meh.

Does this PR introduce any user-facing change?:

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

pkg/extensions/config/sync/config.go

rchincha · 2025-04-30T17:19:38Z

examples/config-sync-architectures.json

+						{
+							"prefix": "/repo4/**",
+							"tags": {
+								"excludeRegex": ".*-(amd64|arm64)$"


tag-based exclusion if platform info is in the image name?

I'd say "meh". Platform info in the image name does not mean anything to be honest. I've seen ( and even accidentally released myself ) quite a few images tagged with one platform declared as part of the tag and image itself / binaries within being different platform.

rchincha · 2025-04-30T17:21:04Z

@lukaszraczylo thanks for your PR. This has been a long standing ask from the community.

lukaszraczylo · 2025-04-30T20:20:38Z

@rchincha I'll work on the suggestions as well, and I'd PR it faster, but I'm using zotregistry for past two days :D

rchincha · 2025-04-30T20:44:44Z

@rchincha I'll work on the suggestions as well, and I'd PR it faster, but I'm using zotregistry for past two days :D

If you able to test locally, you can iterate a lot quicker.
Also do take a look at the CI and failures to understand what would matter to get this PR merged.

lukaszraczylo · 2025-04-30T21:46:21Z

@rchincha sorted. I also have an idea re: resources usage improvement, but I'll leave it for the next PR to not scope creep too much :)

rchincha · 2025-04-30T23:25:34Z

pkg/extensions/sync/destination.go

+
+// ParsePlatform parses a platform string into a Platform struct
+// The string can be in the format "os/arch" or just "arch"
+func ParsePlatform(platform string) Platform {


Ok, instead of a nested map, it is a flat string with a '/' separator

pkg/extensions/config/sync/config.go

pkg/extensions/sync/destination.go

rchincha · 2025-04-30T23:30:11Z

@rchincha sorted. I also have an idea re: resources usage improvement, but I'll leave it for the next PR to not scope creep too much :)

Looking forward to that also.

codecov · 2025-04-30T23:49:21Z

Codecov Report

Attention: Patch coverage is 14.92537% with 171 lines in your changes missing coverage. Please review.

Project coverage is 90.34%. Comparing base (06a0cd5) to head (3eecc29).

Files with missing lines	Patch %	Lines
pkg/extensions/sync/service.go	11.00%	95 Missing and 2 partials ⚠️
pkg/extensions/sync/destination.go	19.56%	71 Missing and 3 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3128      +/-   ##
==========================================
- Coverage   90.79%   90.34%   -0.46%     
==========================================
  Files         172      172              
  Lines       32385    32577     +192     
==========================================
+ Hits        29405    29431      +26     
- Misses       2241     2403     +162     
- Partials      739      743       +4

☔ 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.

lukaszraczylo · 2025-05-01T00:20:06Z

@rchincha I resolved all the issues / suggestions from what I see :) If anything else - I'll pick it up tomorrow

rchincha

Call this config-sync-platforms.json instead

rchincha

test/blackbox tests - just make one file right?

…detection

lukaszraczylo · 2025-05-01T08:26:18Z

@rchincha that's done. Also rewrote the commit messages to adhere to the rules

pkg/extensions/sync/destination.go

andaaron

@eusebiu-constantin-petu-dbk , please review this PR

@rchincha when sync-ing an index, do we want an exact copy, or a new index containing just a subset of the synced manifests? The image would contain invalid references in the 1st case, and digest would change in the 2nd case. There is also a 3rd option of saving the new content with the old digest, but I don't know what the implications would be, since it doesn't respect the content addressability.

andaaron · 2025-05-01T13:31:40Z

pkg/extensions/sync/destination.go

 	tempStoreController storage.StoreController, // temp store controller
 	metaDB mTypes.MetaDB,
 	log log.Logger,
+	config ...*syncconf.RegistryConfig, // optional config for filtering


Why is this optional?

andaaron · 2025-05-01T13:39:02Z

pkg/extensions/sync/destination.go

+				return err
+			}
+
+			manifestContent = updatedContent


Note this will break any referrer pointing to the original index digest.
Unless the same digest is kept in the filename, but a different one would be computed based on the new filename, are we doing this?

andaaron · 2025-05-01T13:41:13Z

pkg/extensions/sync/service.go

+
+	for _, spec := range platformSpecs {
+		parts := strings.Split(spec, "/")
+		if len(parts) == 2 {


Do we need to handle the variant here as well?

Maybe we should have a platform method for matching against a different platform object (including partial matches in case filtering is done just for a subset of platform attributes of course)?

andaaron · 2025-05-01T13:44:26Z

pkg/extensions/sync/service.go

+}
+
+// shouldIncludeArchitecture determines if an architecture should be included in the sync
+// DEPRECATED: Use shouldIncludePlatform instead


Why commit this code if it is deprecated?

andaaron · 2025-05-01T13:46:23Z

pkg/extensions/sync/service.go

+					if !service.shouldIncludePlatform(platform) {
+						platformDesc := platform.Architecture
+						if platform.OS != "" {
+							platformDesc = platform.OS + "/" + platform.Architecture


Do we need to handle variant here?

Maybe we should have a Platform method to return the os/arch/variant string for that instance.

eusebiu-constantin-petu-dbk · 2025-05-15T16:48:00Z

Sorry for being late to the party :D.

Why do you filter platforms in destination.go ? It will commit whatever it finds inside the synced repo, so if you filter upstream there's no need to filter again. Or am I missing something?

Also there is an API in regclient to filter platforms, I'll make a PR based on this one.

rchincha · 2025-05-26T17:42:46Z

@lukaszraczylo more folks are hitting this and complaining.

https://cloud-native.slack.com/archives/C03EGRE4QGH
^ pls join the slack channel.

lukaszraczylo · 2025-06-05T20:04:47Z

@rchincha I may try to sit down on it this weekend although I'm a little bit busy with family and baby on the way so no promises

rchincha · 2025-10-08T16:51:28Z

@lukaszraczylo pls do let us know if you are able to continue work on this PR.

lukaszraczylo · 2025-10-08T23:16:49Z

@rchincha, sure - I can resume the work, although I'm disappointed with the review style and last-minute submissions of the change requests, as they've already been working on the previous comments, making it a bit of a wild goose chase.

rchincha · 2025-10-08T23:53:02Z

@rchincha, sure - I can resume the work, although I'm disappointed with the review style and last-minute submissions of the change requests, as they've already been working on the previous comments, making it a bit of a wild goose chase.

@lukaszraczylo feedback noted. Just so you are aware, the contributors and maintainers of this project work from diverse locations/timezones and mostly contributing their free time, hence feels a little choppy at times - overall the goal is to continue delivering a high quality project.

andaaron · 2025-10-09T05:59:04Z

@rchincha when sync-ing an index, do we want an exact copy, or a new index containing just a subset of the synced manifests? The image would contain invalid references in the 1st case, and digest would change in the 2nd case. There is also a 3rd option of saving the new content with the old digest, but I don't know what the implications would be, since it doesn't respect the content addressability

@rchincha let's sync on this before more effort is put in

rchincha · 2025-10-10T16:31:26Z

For example, upstream repo has:
original_index[manifest_archA, manifest_archB, manifest_archC]

Assuming we want to sync/mirror only archA, we can either make:

original_index[manifest_archA, manifest_archB, manifest_archC] with missing manifest_archB and manifest_archC blobs, but this may require GC fixes potentially. Referrers etc which are downloaded will remain intact.

(OR)

new_index[manifest_archA], but this may have client implications because of digest mismatches potentially.

rchincha · 2025-10-11T05:23:27Z

There appears to be a notion of "sparse index" which is option 1. We should probably implement that. We should fix any GC issues that pop up.
However, let's also ask the question, what happens if configuration is changed to reduce/increase/alter the arch pattern?

rchincha · 2025-10-16T16:43:53Z

opencontainers/distribution-spec#310

andaaron · 2025-10-29T06:11:16Z

Ok, let's go for the sparse index. We'll need to make a list of all cases where we are walking repo content and check if we handle missing references nicely.

andaaron · 2025-11-13T22:12:13Z

@lukaszraczylo I have merged #3503 today.
It should take care of all the issues sparse indexes may cause in other features after they get copied to zot storage.

lukaszraczylo requested review from andaaron and rchincha as code owners April 30, 2025 14:42

lukaszraczylo mentioned this pull request Apr 30, 2025

[Feat]: Suggestion to Add a Configuration Option for System Architecture Pull in Zot Registry #2997

Open

rchincha reviewed Apr 30, 2025

View reviewed changes

pkg/extensions/config/sync/config.go Outdated Show resolved Hide resolved

rchincha reviewed Apr 30, 2025

View reviewed changes

lukaszraczylo force-pushed the add-arch-to-mirror branch from e752310 to 0c1c99f Compare April 30, 2025 21:33

rchincha reviewed Apr 30, 2025

View reviewed changes

pkg/extensions/config/sync/config.go Outdated Show resolved Hide resolved

rchincha reviewed Apr 30, 2025

View reviewed changes

pkg/extensions/sync/destination.go Outdated Show resolved Hide resolved

rchincha reviewed May 1, 2025

View reviewed changes

lukaszraczylo added 6 commits May 1, 2025 09:23

feat(sync): Add ability to sync specific architectures

e74cd8f

feat(sync): Add support for platform/architecture

513920c

fix(build): Fix build issues

cbd61a6

refactor(platform): drop architecture reference in favor of platform …

90ffcb0

…detection

chore: remove deprecated architectures and transition to platforms

0e1e93f

chore: Clean up the bat tests

e34d4e1

lukaszraczylo force-pushed the add-arch-to-mirror branch from 3eecc29 to e34d4e1 Compare May 1, 2025 08:25

andaaron requested a review from eusebiu-constantin-petu-dbk May 1, 2025 11:19

andaaron reviewed May 1, 2025

View reviewed changes

pkg/extensions/sync/destination.go Outdated Show resolved Hide resolved

lukaszraczylo added 2 commits May 1, 2025 13:16

feat(sync): Add support for platform/architecture/variant

dd6a4cf

chore: Additional tests

55cc827

andaaron reviewed May 1, 2025

View reviewed changes

Merge branch 'main' into add-arch-to-mirror

24cacde

stephanme mentioned this pull request Jul 21, 2025

[Bug]: Panic on tag deletion #3273

Closed

Add ability to sync specific architectures. #3128

Are you sure you want to change the base?

Add ability to sync specific architectures. #3128

Uh oh!

Conversation

lukaszraczylo commented Apr 30, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rchincha commented Apr 30, 2025

Uh oh!

lukaszraczylo commented Apr 30, 2025

Uh oh!

rchincha commented Apr 30, 2025

Uh oh!

lukaszraczylo commented Apr 30, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

rchincha commented Apr 30, 2025

Uh oh!

codecov bot commented Apr 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

lukaszraczylo commented May 1, 2025

Uh oh!

rchincha left a comment

Choose a reason for hiding this comment

Uh oh!

rchincha left a comment

Choose a reason for hiding this comment

Uh oh!

lukaszraczylo commented May 1, 2025

Uh oh!

Uh oh!

andaaron left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eusebiu-constantin-petu-dbk commented May 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rchincha commented May 26, 2025

Uh oh!

lukaszraczylo commented Jun 5, 2025

Uh oh!

rchincha commented Oct 8, 2025

Uh oh!

lukaszraczylo commented Oct 8, 2025

Uh oh!

rchincha commented Oct 8, 2025

Uh oh!

andaaron commented Oct 9, 2025

Uh oh!

rchincha commented Oct 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rchincha commented Oct 11, 2025

Uh oh!

rchincha commented Oct 16, 2025

Uh oh!

andaaron commented Oct 29, 2025

Uh oh!

andaaron commented Nov 13, 2025

Uh oh!

codecov bot commented Apr 30, 2025 •

edited

Loading

eusebiu-constantin-petu-dbk commented May 15, 2025 •

edited

Loading

rchincha commented Oct 10, 2025 •

edited

Loading