fix(images): reduce list complexity and make rollback output testable
- commit
- 1c3348b5b680bb2fd2640e81fb55e616f7422bcd
- author
- bnema · · 2026-02-11 20:45 UTC
- parent
- 0e0baca
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
3 files · +44 -10
M internal/adapters/in/cli/rollback.go +7 -6
| @@ -3,6 +3,7 @@ package cli | |||
| 3 | 3 | import ( | |
| 4 | 4 | "context" | |
| 5 | 5 | "fmt" | |
| 6 | + | "io" | |
| 6 | 7 | "os" | |
| 7 | 8 | "sort" | |
| 8 | 9 | "strings" | |
| @@ -46,7 +47,7 @@ func newRollbackListCmd() *cobra.Command { | |||
| 46 | 47 | Short: "List available rollback tags", | |
| 47 | 48 | Args: cobra.ExactArgs(1), | |
| 48 | 49 | RunE: func(cmd *cobra.Command, args []string) error { | |
| 49 | - | return runRollbackList(cmd.Context(), args[0]) | |
| 50 | + | return runRollbackList(cmd.Context(), cmd.OutOrStdout(), args[0]) | |
| 50 | 51 | }, | |
| 51 | 52 | } | |
| 52 | 53 | } | |
| @@ -86,7 +87,7 @@ func runRollback(ctx context.Context, rollbackDomain, targetTag string) error { | |||
| 86 | 87 | return deploySelectedTag(ctx, handle.plane, route, rollbackDomain, imageName, selectedTag) | |
| 87 | 88 | } | |
| 88 | 89 | ||
| 89 | - | func runRollbackList(ctx context.Context, rollbackDomain string) error { | |
| 90 | + | func runRollbackList(ctx context.Context, w io.Writer, rollbackDomain string) error { | |
| 90 | 91 | handle, err := resolveControlPlane(configPath) | |
| 91 | 92 | if err != nil { | |
| 92 | 93 | return err | |
| @@ -108,18 +109,18 @@ func runRollbackList(ctx context.Context, rollbackDomain string) error { | |||
| 108 | 109 | return err | |
| 109 | 110 | } | |
| 110 | 111 | ||
| 111 | - | printRollbackTags(rollbackDomain, currentTag, tags) | |
| 112 | + | printRollbackTags(w, rollbackDomain, currentTag, tags) | |
| 112 | 113 | return nil | |
| 113 | 114 | } | |
| 114 | 115 | ||
| 115 | - | func printRollbackTags(rollbackDomain, currentTag string, tags []string) { | |
| 116 | - | fmt.Printf("Available tags for %s:\n", styles.Theme.Bold.Render(rollbackDomain)) | |
| 116 | + | func printRollbackTags(w io.Writer, rollbackDomain, currentTag string, tags []string) { | |
| 117 | + | fmt.Fprintf(w, "Available tags for %s:\n", styles.Theme.Bold.Render(rollbackDomain)) | |
| 117 | 118 | for _, tag := range tags { | |
| 118 | 119 | suffix := "" | |
| 119 | 120 | if tag == currentTag { | |
| 120 | 121 | suffix = " (current)" | |
| 121 | 122 | } | |
| 122 | - | fmt.Printf("- %s%s\n", tag, suffix) | |
| 123 | + | fmt.Fprintf(w, "- %s%s\n", tag, suffix) | |
| 123 | 124 | } | |
| 124 | 125 | } | |
| 125 | 126 | ||
| @@ -3,6 +3,7 @@ package cli | |||||
| 3 | import ( | 3 | import ( | ||
| 4 | "context" | 4 | "context" | ||
| 5 | "fmt" | 5 | "fmt" | ||
| 6 | + | "io" | |||
| 6 | "os" | 7 | "os" | ||
| 7 | "sort" | 8 | "sort" | ||
| 8 | "strings" | 9 | "strings" | ||
| @@ -46,7 +47,7 @@ func newRollbackListCmd() *cobra.Command { | |||||
| 46 | Short: "List available rollback tags", | 47 | Short: "List available rollback tags", | ||
| 47 | Args: cobra.ExactArgs(1), | 48 | Args: cobra.ExactArgs(1), | ||
| 48 | RunE: func(cmd *cobra.Command, args []string) error { | 49 | RunE: func(cmd *cobra.Command, args []string) error { | ||
| 49 | - | return runRollbackList(cmd.Context(), args[0]) | 50 | + | return runRollbackList(cmd.Context(), cmd.OutOrStdout(), args[0]) |
| 50 | }, | 51 | }, | ||
| 51 | } | 52 | } | ||
| 52 | } | 53 | } | ||
| @@ -86,7 +87,7 @@ func runRollback(ctx context.Context, rollbackDomain, targetTag string) error { | |||||
| 86 | return deploySelectedTag(ctx, handle.plane, route, rollbackDomain, imageName, selectedTag) | 87 | return deploySelectedTag(ctx, handle.plane, route, rollbackDomain, imageName, selectedTag) | ||
| 87 | } | 88 | } | ||
| 88 | 89 | ||||
| 89 | - | func runRollbackList(ctx context.Context, rollbackDomain string) error { | 90 | + | func runRollbackList(ctx context.Context, w io.Writer, rollbackDomain string) error { |
| 90 | handle, err := resolveControlPlane(configPath) | 91 | handle, err := resolveControlPlane(configPath) | ||
| 91 | if err != nil { | 92 | if err != nil { | ||
| 92 | return err | 93 | return err | ||
| @@ -108,18 +109,18 @@ func runRollbackList(ctx context.Context, rollbackDomain string) error { | |||||
| 108 | return err | 109 | return err | ||
| 109 | } | 110 | } | ||
| 110 | 111 | ||||
| 111 | - | printRollbackTags(rollbackDomain, currentTag, tags) | 112 | + | printRollbackTags(w, rollbackDomain, currentTag, tags) |
| 112 | return nil | 113 | return nil | ||
| 113 | } | 114 | } | ||
| 114 | 115 | ||||
| 115 | - | func printRollbackTags(rollbackDomain, currentTag string, tags []string) { | 116 | + | func printRollbackTags(w io.Writer, rollbackDomain, currentTag string, tags []string) { |
| 116 | - | fmt.Printf("Available tags for %s:\n", styles.Theme.Bold.Render(rollbackDomain)) | 117 | + | fmt.Fprintf(w, "Available tags for %s:\n", styles.Theme.Bold.Render(rollbackDomain)) |
| 117 | for _, tag := range tags { | 118 | for _, tag := range tags { | ||
| 118 | suffix := "" | 119 | suffix := "" | ||
| 119 | if tag == currentTag { | 120 | if tag == currentTag { | ||
| 120 | suffix = " (current)" | 121 | suffix = " (current)" | ||
| 121 | } | 122 | } | ||
| 122 | - | fmt.Printf("- %s%s\n", tag, suffix) | 123 | + | fmt.Fprintf(w, "- %s%s\n", tag, suffix) |
| 123 | } | 124 | } | ||
| 124 | } | 125 | } | ||
| 125 | 126 | ||||
M internal/adapters/in/cli/rollback_test.go +13 -0
| @@ -1,6 +1,7 @@ | |||
| 1 | 1 | package cli | |
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | + | "bytes" | |
| 4 | 5 | "testing" | |
| 5 | 6 | ||
| 6 | 7 | "github.com/stretchr/testify/assert" | |
| @@ -51,3 +52,15 @@ func TestValidateTagExists(t *testing.T) { | |||
| 51 | 52 | assert.True(t, validateTagExists("v1.0.0", []string{"v1.0.0", "v2.0.0"})) | |
| 52 | 53 | assert.False(t, validateTagExists("v3.0.0", []string{"v1.0.0", "v2.0.0"})) | |
| 53 | 54 | } | |
| 55 | + | ||
| 56 | + | func TestPrintRollbackTags_WritesToProvidedWriter(t *testing.T) { | |
| 57 | + | var buf bytes.Buffer | |
| 58 | + | ||
| 59 | + | printRollbackTags(&buf, "myapp.example.com", "v1.2.0", []string{"v1.2.0", "v1.1.0"}) | |
| 60 | + | ||
| 61 | + | output := buf.String() | |
| 62 | + | assert.Contains(t, output, "Available tags for") | |
| 63 | + | assert.Contains(t, output, "myapp.example.com") | |
| 64 | + | assert.Contains(t, output, "- v1.2.0 (current)") | |
| 65 | + | assert.Contains(t, output, "- v1.1.0") | |
| 66 | + | } | |
| @@ -1,6 +1,7 @@ | |||||
| 1 | package cli | 1 | package cli | ||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | + | "bytes" | |||
| 4 | "testing" | 5 | "testing" | ||
| 5 | 6 | ||||
| 6 | "github.com/stretchr/testify/assert" | 7 | "github.com/stretchr/testify/assert" | ||
| @@ -51,3 +52,15 @@ func TestValidateTagExists(t *testing.T) { | |||||
| 51 | assert.True(t, validateTagExists("v1.0.0", []string{"v1.0.0", "v2.0.0"})) | 52 | assert.True(t, validateTagExists("v1.0.0", []string{"v1.0.0", "v2.0.0"})) | ||
| 52 | assert.False(t, validateTagExists("v3.0.0", []string{"v1.0.0", "v2.0.0"})) | 53 | assert.False(t, validateTagExists("v3.0.0", []string{"v1.0.0", "v2.0.0"})) | ||
| 53 | } | 54 | } | ||
| 55 | + | ||||
| 56 | + | func TestPrintRollbackTags_WritesToProvidedWriter(t *testing.T) { | |||
| 57 | + | var buf bytes.Buffer | |||
| 58 | + | ||||
| 59 | + | printRollbackTags(&buf, "myapp.example.com", "v1.2.0", []string{"v1.2.0", "v1.1.0"}) | |||
| 60 | + | ||||
| 61 | + | output := buf.String() | |||
| 62 | + | assert.Contains(t, output, "Available tags for") | |||
| 63 | + | assert.Contains(t, output, "myapp.example.com") | |||
| 64 | + | assert.Contains(t, output, "- v1.2.0 (current)") | |||
| 65 | + | assert.Contains(t, output, "- v1.1.0") | |||
| 66 | + | } | |||
M internal/usecase/images/service.go +24 -4
| @@ -57,9 +57,24 @@ func (s *Service) ListImages(ctx context.Context) ([]domain.ImageInfo, error) { | |||
| 57 | 57 | return nil, log.WrapErr(err, "failed to list images") | |
| 58 | 58 | } | |
| 59 | 59 | ||
| 60 | + | images, seenRepoTags, repoDisplayByNormalized := buildRuntimeImageIndex(details) | |
| 61 | + | images, err = s.appendRegistryImages(log, images, seenRepoTags, repoDisplayByNormalized) | |
| 62 | + | if err != nil { | |
| 63 | + | return nil, err | |
| 64 | + | } | |
| 65 | + | ||
| 66 | + | sort.SliceStable(images, func(i, j int) bool { | |
| 67 | + | return lessImageInfo(images[i], images[j]) | |
| 68 | + | }) | |
| 69 | + | ||
| 70 | + | return images, nil | |
| 71 | + | } | |
| 72 | + | ||
| 73 | + | func buildRuntimeImageIndex(details []runtime.ImageDetail) ([]domain.ImageInfo, map[string]struct{}, map[string]string) { | |
| 60 | 74 | images := make([]domain.ImageInfo, 0, len(details)) | |
| 61 | 75 | seenRepoTags := make(map[string]struct{}, len(details)) | |
| 62 | 76 | repoDisplayByNormalized := make(map[string]string) | |
| 77 | + | ||
| 63 | 78 | for _, detail := range details { | |
| 64 | 79 | if isDanglingImage(detail.RepoTags) { | |
| 65 | 80 | images = append(images, domain.ImageInfo{ | |
| @@ -94,6 +109,15 @@ func (s *Service) ListImages(ctx context.Context) ([]domain.ImageInfo, error) { | |||
| 94 | 109 | } | |
| 95 | 110 | } | |
| 96 | 111 | ||
| 112 | + | return images, seenRepoTags, repoDisplayByNormalized | |
| 113 | + | } | |
| 114 | + | ||
| 115 | + | func (s *Service) appendRegistryImages( | |
| 116 | + | log zerowrap.Logger, | |
| 117 | + | images []domain.ImageInfo, | |
| 118 | + | seenRepoTags map[string]struct{}, | |
| 119 | + | repoDisplayByNormalized map[string]string, | |
| 120 | + | ) ([]domain.ImageInfo, error) { | |
| 97 | 121 | repositories, err := s.manifestStorage.ListRepositories() | |
| 98 | 122 | if err != nil { | |
| 99 | 123 | return nil, log.WrapErr(err, "failed to list repositories") | |
| @@ -141,10 +165,6 @@ func (s *Service) ListImages(ctx context.Context) ([]domain.ImageInfo, error) { | |||
| 141 | 165 | } | |
| 142 | 166 | } | |
| 143 | 167 | ||
| 144 | - | sort.SliceStable(images, func(i, j int) bool { | |
| 145 | - | return lessImageInfo(images[i], images[j]) | |
| 146 | - | }) | |
| 147 | - | ||
| 148 | 168 | return images, nil | |
| 149 | 169 | } | |
| 150 | 170 | ||
| @@ -57,9 +57,24 @@ func (s *Service) ListImages(ctx context.Context) ([]domain.ImageInfo, error) { | |||||
| 57 | return nil, log.WrapErr(err, "failed to list images") | 57 | return nil, log.WrapErr(err, "failed to list images") | ||
| 58 | } | 58 | } | ||
| 59 | 59 | ||||
| 60 | + | images, seenRepoTags, repoDisplayByNormalized := buildRuntimeImageIndex(details) | |||
| 61 | + | images, err = s.appendRegistryImages(log, images, seenRepoTags, repoDisplayByNormalized) | |||
| 62 | + | if err != nil { | |||
| 63 | + | return nil, err | |||
| 64 | + | } | |||
| 65 | + | ||||
| 66 | + | sort.SliceStable(images, func(i, j int) bool { | |||
| 67 | + | return lessImageInfo(images[i], images[j]) | |||
| 68 | + | }) | |||
| 69 | + | ||||
| 70 | + | return images, nil | |||
| 71 | + | } | |||
| 72 | + | ||||
| 73 | + | func buildRuntimeImageIndex(details []runtime.ImageDetail) ([]domain.ImageInfo, map[string]struct{}, map[string]string) { | |||
| 60 | images := make([]domain.ImageInfo, 0, len(details)) | 74 | images := make([]domain.ImageInfo, 0, len(details)) | ||
| 61 | seenRepoTags := make(map[string]struct{}, len(details)) | 75 | seenRepoTags := make(map[string]struct{}, len(details)) | ||
| 62 | repoDisplayByNormalized := make(map[string]string) | 76 | repoDisplayByNormalized := make(map[string]string) | ||
| 77 | + | ||||
| 63 | for _, detail := range details { | 78 | for _, detail := range details { | ||
| 64 | if isDanglingImage(detail.RepoTags) { | 79 | if isDanglingImage(detail.RepoTags) { | ||
| 65 | images = append(images, domain.ImageInfo{ | 80 | images = append(images, domain.ImageInfo{ | ||
| @@ -94,6 +109,15 @@ func (s *Service) ListImages(ctx context.Context) ([]domain.ImageInfo, error) { | |||||
| 94 | } | 109 | } | ||
| 95 | } | 110 | } | ||
| 96 | 111 | ||||
| 112 | + | return images, seenRepoTags, repoDisplayByNormalized | |||
| 113 | + | } | |||
| 114 | + | ||||
| 115 | + | func (s *Service) appendRegistryImages( | |||
| 116 | + | log zerowrap.Logger, | |||
| 117 | + | images []domain.ImageInfo, | |||
| 118 | + | seenRepoTags map[string]struct{}, | |||
| 119 | + | repoDisplayByNormalized map[string]string, | |||
| 120 | + | ) ([]domain.ImageInfo, error) { | |||
| 97 | repositories, err := s.manifestStorage.ListRepositories() | 121 | repositories, err := s.manifestStorage.ListRepositories() | ||
| 98 | if err != nil { | 122 | if err != nil { | ||
| 99 | return nil, log.WrapErr(err, "failed to list repositories") | 123 | return nil, log.WrapErr(err, "failed to list repositories") | ||
| @@ -141,10 +165,6 @@ func (s *Service) ListImages(ctx context.Context) ([]domain.ImageInfo, error) { | |||||
| 141 | } | 165 | } | ||
| 142 | } | 166 | } | ||
| 143 | 167 | ||||
| 144 | - | sort.SliceStable(images, func(i, j int) bool { | |||
| 145 | - | return lessImageInfo(images[i], images[j]) | |||
| 146 | - | }) | |||
| 147 | - | ||||
| 148 | return images, nil | 168 | return images, nil | ||
| 149 | } | 169 | } | ||
| 150 | 170 | ||||