feat: add `--untagged-only` for independent manifest cleanup by balcsida · Pull Request #489 · Azure/acr-cli

balcsida · 2025-08-08T21:22:40Z

Purpose of the PR

This PR introduces a new --untagged-only flag to the purge command, enabling users to clean up untagged manifests independently without affecting tagged manifests. This addresses the confusion and limitations reported in various issues and provides a comprehensive solution for dangling manifest cleanup.

Key Features Added

New --untagged-only flag: Exclusively targets untagged manifests without deleting any tags first
Optional --ago and --keep support: When used with --untagged-only, these flags become optional and provide additional filtering capabilities:
- --ago: Filter untagged manifests by age (delete only manifests older than specified duration)
- --keep: Preserve the most recent N untagged manifests
Enhanced documentation: Comprehensive help text and examples clearly distinguish between --untagged and --untagged-only

Implementation Details

Flag validation: Improved validation logic with better UX (validation happens before authentication)
Mutual exclusivity: --untagged and --untagged-only are mutually exclusive to prevent confusion
Age filtering: Added manifest age filtering support in purgeDanglingManifests function
Keep logic: Added support to preserve the most recent untagged manifests when --keep is specified
Comprehensive testing: Added extensive unit tests and integration test scripts

Enhanced User Experience

Clear flag descriptions: Updated help text clearly explains when to use each flag
Organized examples: Help examples are categorized into:
- TAG DELETION EXAMPLES (traditional workflow)
- DANGLING MANIFEST CLEANUP EXAMPLES (new --untagged-only workflow)
- ADVANCED OPTIONS
Better error messages: Improved validation with clearer error messaging

Usage Examples

# Basic dangling manifest cleanup
acr purge -r example --untagged-only

# Clean up dangling manifests in specific repository
acr purge -r example --filter "hello-world:.*" --untagged-only

# Clean up old dangling manifests (older than 3 days), keeping 5 most recent
acr purge -r example --untagged-only --ago 3d --keep 5

# Traditional workflow: delete tags then clean up resulting dangling manifests
acr purge -r example --filter "hello-world:\w*test\w*" --ago 5d --untagged

Key Differences Between Flags

--untagged: Used as a cleanup step after tag deletion. Requires --filter and --ago. Deletes tags first, then cleans up untagged manifests left behind.
--untagged-only: Primary tool for dangling manifest cleanup. Works independently without deleting tags. Optional --filter, --ago, and --keep for targeted cleanup.

Breaking Changes
None. The existing --untagged flag behavior remains unchanged for backward compatibility.

Testing

All existing tests pass
Added comprehensive unit tests for new functionality
Added integration test scripts for real Azure registry testing
Fixed edge cases and improved error handling

Fixes #64
Fixes #83
Fixes #95
Fixes #480
Fixes #486

balcsida · 2025-08-14T00:49:08Z

@estebanreyl, as the --untagged flag caused so much confusion in the past, do you think it would make sense to deprecate it and have some new name for this flag to prevent any accidental manifest deletion?

estebanreyl · 2025-08-26T18:58:01Z

I like the change, but I am little hesitant when it comes to this type of usability change scenarios, so I messaged a couple of PMs to take a look and will see what they say and will update. I am sorry its taken so long

balcsida · 2025-08-26T19:14:28Z

Again, no worries at all, @estebanreyl! 🙇

FeynmanZhou · 2025-09-09T21:15:00Z

Hey @balcsida ,

Thanks for making several PRs to acr purge. I am wondering if retention policy is a good fit for your scenario? When a retention policy is enabled, untagged manifests in the registry are automatically deleted after a number of days you set.

balcsida · 2025-09-10T09:07:00Z

@FeynmanZhou Thanks for the suggestion! I'm familiar with retention policies, but they don't address the specific issues this PR solves:

Organizational constraints

Some organizations (like ours) face significant bureaucratic hurdles when implementing policies (especially preview ones). A scheduled cleanup pipeline using existing CLI tools is often much more practical to deploy and manage.

User experience issues

The linked issues (#64, #83, #95, #480, #486) demonstrate a clear pattern: users consistently expect the --untagged flag to behave like the proposed --untagged-only flag. The fact that multiple users took the effort to find this repository and report these issues suggests this confusion is causing real problems in production environments.

Complementary solution

Rather than replacing retention policies, this PR provides an alternative approach for teams that need more granular control or face organizational constraints. The --untagged-only flag offers:

Immediate deployment without policy approval processes
Scriptable control with --ago and --keep options
Clear semantics that align with user expectations

This change maintains full backward compatibility while preventing the unintended manifest deletions that several users have reported.

escherstair · 2026-01-22T09:24:43Z

Since this PR has been already merged and it has a breaking change, I think that this one should be merged ASAP too.
Because this one is the clean way to have the untagged images deleted. Otherwise everyone would creates its own workaround, playing aorund with other flags (that exist for other purposes)

balcsida · 2026-01-22T13:38:26Z

Hey @escherstair ,

Apologies for breaking your workflows with my previous PR, that was not my intention.

I'm going to rebase my branch and fix the conflicts to make this PR mergable as soon as I can

- Add unit tests for untagged-only flag validation - Add integration test script for real Azure registry testing - Update main test script with untagged-only test cases - Fix index out of range bug in GetUntaggedManifests - Test scenarios include dry-run, locked manifests, and filters

- Remove GetManifest expectations for untagged manifests (not called) - Fix UpdateAcrManifestAttributes mock to return proper response - Update GetRepositories test to match actual behavior - Update mutual exclusivity test to accept Cobra's error message - All unit tests now passing (100% success rate)

balcsida · 2026-01-31T14:26:02Z

@escherstair, the ASAP happened to be today, thanks for your patience with me

escherstair · 2026-02-01T08:17:47Z

@balcsida don't worry.
Thank you very much for you job.
I really appreciate it.

estebanreyl · 2026-02-02T21:23:07Z

cmd/acr/purge.go


-  - Delete all tags that contain the word test in the tag name and are older than 5 days in the example.azurecr.io registry inside the hello-world 
-    repository, after that, remove the dangling manifests in the same repository
+  - Delete tags containing "test" that are older than 5 days, then clean up any dangling manifests left behind


Ago also applies to the dangling manifests , something clearer would be something like:

"Delete tags containing ‘test’ that are older than 5 days, then delete any dangling (untagged) manifests older than 5 days"

Updated example text to clarify

estebanreyl

Overall LGTM, we are in line with including the changes. I've added some comments on some small items that would be nice to address before merge. Thanks again for the contribution!

estebanreyl · 2026-02-02T21:27:04Z

cmd/acr/purge.go

 		Example: purgeExampleMessage,
 		RunE: func(_ *cobra.Command, _ []string) error {
+			// Validate flag combinations before authentication
+			if !purgeParams.untaggedOnly && !purgeParams.untagged {


Doesn't --untagged still require filter?

True! Added validation for --untagged requiring filter and ago

estebanreyl · 2026-02-02T21:32:44Z

cmd/acr/purge.go

+				}
+				tagFilters = make(map[string]string)
+				for _, repoName := range allRepoNames {
+					tagFilters[repoName] = ".*" // dummy filter that won't be used


If the tag filter won't be used it seems most logical to use a filter string that cannot be matched or adjust the structure. Maybe use ^$ or the empty string?

Changed to empty string

estebanreyl · 2026-02-02T21:33:47Z

cmd/acr/purge.go

 			}

-			deletedTagsCount, deletedManifestsCount, err := purge(ctx, acrClient, loginURL, repoParallelism, purgeParams.ago, purgeParams.keep, purgeParams.filterTimeout, purgeParams.untagged, tagFilters, purgeParams.dryRun, purgeParams.includeLocked)
+			deletedTagsCount, deletedManifestsCount, err := purge(ctx, acrClient, loginURL, repoParallelism, purgeParams.ago, purgeParams.keep, purgeParams.filterTimeout, purgeParams.untagged || purgeParams.untaggedOnly, purgeParams.untaggedOnly, tagFilters, purgeParams.dryRun, purgeParams.includeLocked)


Maybe combine the purgeParams.untagged || purgeParams.untaggedOnly in a variable before and call it supportUntaggedCleanup? Just to make it clear, I see they are marked as mutually exclusive so just makes it clear.

estebanreyl · 2026-02-02T21:34:49Z

cmd/acr/purge.go


-	cmd.Flags().BoolVar(&purgeParams.untagged, "untagged", false, "If the untagged flag is set all the manifests that do not have any tags associated to them will be also purged, except if they belong to a manifest list that contains at least one tag")
+	cmd.Flags().BoolVar(&purgeParams.untagged, "untagged", false, "In addition to deleting tags (based on --filter and --ago), also delete untagged manifests that were left behind after tag deletion. This is typically used as a cleanup step after deleting tags. Note: This requires --filter and --ago to be specified")
+	cmd.Flags().BoolVar(&purgeParams.untaggedOnly, "untagged-only", false, "Clean up dangling manifests: Delete ONLY untagged manifests (manifests without any tags), without deleting any tags first. This is the primary way to clean up dangling manifests in your registry. Optional: Use --ago to delete only old untagged manifests, --keep to preserve recent ones, and --filter to target specific repositories")


Can we note this ignores the tag portion of filter?

Good catch, added note to flag description

estebanreyl · 2026-02-02T21:40:27Z

cmd/acr/purge.go

-	if err != nil {
-		return 0, 0, err
+	// For untagged-only mode without --ago, use 0 duration to delete all untagged manifests
+	var agoDuration time.Duration


This should probably be done as part of the parameter validation above

estebanreyl · 2026-02-02T21:42:14Z

cmd/acr/purge.go

+		var manifestToTagsCountMap map[string]int
+
+		// Skip tag deletion if untagged-only mode is enabled
+		if !untaggedOnly {


Kinda a nit but we should have the default behavior first and avoid the not check so swapping the order like:

if untaggedOnly{
} else {
}

estebanreyl · 2026-02-02T21:45:01Z

cmd/acr/purge.go

+		// If the untagged flag is set or untagged-only mode is enabled, delete manifests
 		if removeUtaggedManifests {
-			singleDeletedManifestsCount, err = purgeDanglingManifests(ctx, acrClient, repoParallelism, loginURL, repoName, agoDuration, manifestToTagsCountMap, dryRun, includeLocked)
+			singleDeletedManifestsCount, err = purgeDanglingManifests(ctx, acrClient, repoParallelism, loginURL, repoName, agoDuration, tagsToKeep, manifestToTagsCountMap, dryRun, includeLocked)


Should this be changed from tagsToKeep to imagesToKeep?

estebanreyl · 2026-02-02T22:05:05Z

cmd/acr/purge.go

+			if manifestsToDelete[i].LastUpdateTime == nil || manifestsToDelete[j].LastUpdateTime == nil {
+				return false
+			}
+			tiI, errI := time.Parse(time.RFC3339Nano, *manifestsToDelete[i].LastUpdateTime)


I don't expect this to happen, but this leads to slightly mysterious/inconsistent behavior in the event that we do fail to parse the last updateTime as cases where less(i, j) == false and less(j, i) == false can happen without the items being equal (because you haven’t actually compared anything). I would suggest establishing a consistent ordering (for example the nil one is always considered older) and maybe moving the keep functionality to its own function so it can be tested independently. I asked copilot to do it and it gave me this:

sort.Slice(manifestsToDelete, func(i, j int) bool { mi := manifestsToDelete[i] mj := manifestsToDelete[j] // Extract strings; nil => invalid var si, sj string if mi.LastUpdateTime != nil { si = *mi.LastUpdateTime } if mj.LastUpdateTime != nil { sj = *mj.LastUpdateTime } ti, errI := time.Parse(time.RFC3339Nano, si) if errI != nil && si != "" { // fallback to RFC3339 if needed if t, err := time.Parse(time.RFC3339, si); err == nil { ti, errI = t, nil } } tj, errJ := time.Parse(time.RFC3339Nano, sj) if errJ != nil && sj != "" { if t, err := time.Parse(time.RFC3339, sj); err == nil { tj, errJ = t, nil } } // Define validity flags vi := (errI == nil) vj := (errJ == nil) // Ordering rules (total order): // 1) Valid timestamps come before invalid (invalid considered oldest). if vi != vj { return vi // true if i valid and j invalid } // 2) Both valid: newest first. if vi && vj { if ti.Equal(tj) { // 3) Tie-breaker for determinism (adjust to your field) return mi.Digest < mj.Digest } return ti.After(tj) // newest first } // 4) Both invalid: tie-breaker for determinism return mi.Digest < mj.Digest })

estebanreyl · 2026-02-02T22:10:39Z

scripts/experimental/test-purge-untagged-only.sh

+    # Test regular purge with --untagged performance
+    echo "Testing regular purge with --untagged performance..."
+    start_time=$(date +%s%N)
+    "$ACR_CLI" purge --registry "$REGISTRY" --filter "$repo:.*" --ago 0d --untagged --dry-run >/dev/null 2>&1


to have a real comparison shouldn't you avoid cleaning up any tags, so use 0^$ here?

balcsida · 2026-02-03T10:06:37Z

Thank you @estebanreyl for the quick review 🙇

estebanreyl · 2026-02-03T22:40:42Z

cmd/acr/purge.go

+			// untagged-only mode: filter and ago are optional (skip validation)
+			// untagged mode: requires filter and ago (it's a cleanup step after tag deletion)
+			// standard mode: requires filter and ago for tag deletion
+			if purgeParams.untagged {


I don't think this first if statement is necessary. It seems like the second else if actually covers all the necessary scenarios. I don't think there is a need for any special message that needs to be output for --untagged vs just regular tag cleanup.

estebanreyl

LGTM, left one final comment but can merge after that, thanks for the contribution! :D

…heck

balcsida force-pushed the feat/untagged-only-flag branch from 1dda8ef to fe3c53c Compare August 13, 2025 10:48

balcsida marked this pull request as ready for review August 13, 2025 11:02

balcsida requested review from Wwwsylvia, estebanreyl, northtyphoon, sajayantony, shizhMSFT and wangxiaoxuan273 as code owners August 13, 2025 11:02

balcsida force-pushed the feat/untagged-only-flag branch from e718249 to a160bef Compare August 19, 2025 12:34

balcsida changed the title ~~feat: add --untagged-only flag for independent untagged manifest cleanup~~ feat: add --untagged-only for independent manifest cleanup Oct 13, 2025

balcsida force-pushed the feat/untagged-only-flag branch from a160bef to d0a7c7a Compare January 31, 2026 14:12

balcsida added 12 commits January 31, 2026 15:14

feat: add --untagged-only flag for independent untagged manifest cleanup

f4413d8

fix: resolve gofmt formatting issue in purge.go

25285ed

fix: move flag validation before authentication for better UX

e4cec03

fix: gofmt

bea78b8

feat: make --ago and --keep flags optional with --untagged-only

9e118dd

docs: clarify --untagged vs --untagged-only flag usage and examples

26a241c

test: update tests for optional --ago and --keep with --untagged-only

c123cfa

fix: remove empty block and fix formatting in purge commands

b922dd6

chore: increase timeout

f745903

test: update test timestamps for 2026 compatibility

4298857

balcsida force-pushed the feat/untagged-only-flag branch from d0a7c7a to 4298857 Compare January 31, 2026 14:24

estebanreyl reviewed Feb 2, 2026

View reviewed changes

fix: address PR review comments for --untagged-only

7e2e6c5

balcsida force-pushed the feat/untagged-only-flag branch from a846786 to 7e2e6c5 Compare February 3, 2026 10:02

estebanreyl reviewed Feb 3, 2026

View reviewed changes

estebanreyl approved these changes Feb 3, 2026

View reviewed changes

refactor: simplify flag validation by removing redundant --untagged c…

3c8abc8

…heck

Conversation

balcsida commented Aug 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

balcsida commented Aug 14, 2025

Uh oh!

estebanreyl commented Aug 26, 2025

Uh oh!

balcsida commented Aug 26, 2025

Uh oh!

FeynmanZhou commented Sep 9, 2025

Uh oh!

balcsida commented Sep 10, 2025

Organizational constraints

User experience issues

Complementary solution

Uh oh!

escherstair commented Jan 22, 2026

Uh oh!

balcsida commented Jan 22, 2026

Uh oh!

balcsida commented Jan 31, 2026

Uh oh!

escherstair commented Feb 1, 2026

Uh oh!

estebanreyl Feb 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

estebanreyl 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!

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!

Choose a reason for hiding this comment

Uh oh!

balcsida commented Feb 3, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

estebanreyl left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

balcsida commented Aug 8, 2025 •

edited

Loading

estebanreyl Feb 2, 2026 •

edited

Loading

estebanreyl left a comment •

edited

Loading