diff --git a/api/v6/api.go b/api/v6/api.go index 9befabd71..c3c10950a 100644 --- a/api/v6/api.go +++ b/api/v6/api.go @@ -42,10 +42,19 @@ type ControllerStatus struct { } type Container struct { - Name string - Current image.Info - Available []image.Info - AvailableError string `json:",omitempty"` + Name string `json:",omitempty"` + Current image.Info `json:",omitempty"` + LatestFiltered image.Info `json:",omitempty"` + + // All available images (ignoring tag filters) + Available []image.Info `json:",omitempty"` + AvailableError string `json:",omitempty"` + AvailableImagesCount int `json:",omitempty"` + NewAvailableImagesCount int `json:",omitempty"` + + // Filtered available images (matching tag filters) + FilteredImagesCount int `json:",omitempty"` + NewFilteredImagesCount int `json:",omitempty"` } // --- config types @@ -66,13 +75,17 @@ type Deprecated interface { SyncNotify(context.Context) error } +type ListImagesOptions struct { + OverrideContainerFields []string +} + type NotDeprecated interface { // from v5 Export(context.Context) ([]byte, error) // v6 ListServices(ctx context.Context, namespace string) ([]ControllerStatus, error) - ListImages(context.Context, update.ResourceSpec) ([]ImageStatus, error) + ListImages(ctx context.Context, spec update.ResourceSpec, opts ListImagesOptions) ([]ImageStatus, error) UpdateManifests(context.Context, update.Spec) (job.ID, error) SyncStatus(ctx context.Context, ref string) ([]string, error) JobStatus(context.Context, job.ID) (job.Status, error) diff --git a/cmd/fluxctl/list_images_cmd.go b/cmd/fluxctl/list_images_cmd.go index e9bb7a115..138fff09e 100644 --- a/cmd/fluxctl/list_images_cmd.go +++ b/cmd/fluxctl/list_images_cmd.go @@ -67,7 +67,7 @@ func (opts *controllerShowOpts) RunE(cmd *cobra.Command, args []string) error { ctx := context.Background() - controllers, err := opts.API.ListImages(ctx, resourceSpec) + controllers, err := opts.API.ListImages(ctx, resourceSpec, v6.ListImagesOptions{}) if err != nil { return err } diff --git a/daemon/daemon.go b/daemon/daemon.go index b2feeee03..acc0231df 100644 --- a/daemon/daemon.go +++ b/daemon/daemon.go @@ -70,35 +70,43 @@ func (d *Daemon) Export(ctx context.Context) ([]byte, error) { return d.Cluster.Export() } -func (d *Daemon) ListServices(ctx context.Context, namespace string) ([]v6.ControllerStatus, error) { - clusterServices, err := d.Cluster.AllControllers(namespace) - if err != nil { - return nil, errors.Wrap(err, "getting services from cluster") - } - +func (d *Daemon) getPolicyResourceMap(ctx context.Context) (policy.ResourceMap, v6.ReadOnlyReason, error) { var services policy.ResourceMap var globalReadOnly v6.ReadOnlyReason - err = d.WithClone(ctx, func(checkout *git.Checkout) error { + err := d.WithClone(ctx, func(checkout *git.Checkout) error { var err error services, err = d.Manifests.ServicesWithPolicies(checkout.ManifestDir()) return err }) + + // Capture errors related to read-only repositories switch { case err == git.ErrNotReady: globalReadOnly = v6.ReadOnlyNotReady case err == git.ErrNoConfig: globalReadOnly = v6.ReadOnlyNoRepo case err != nil: - return nil, errors.Wrap(err, "getting service policies") + return nil, globalReadOnly, errors.Wrap(err, "getting service policies") + } + + return services, globalReadOnly, nil +} + +func (d *Daemon) ListServices(ctx context.Context, namespace string) ([]v6.ControllerStatus, error) { + clusterServices, err := d.Cluster.AllControllers(namespace) + if err != nil { + return nil, errors.Wrap(err, "getting services from cluster") + } + + policyResourceMap, readOnly, err := d.getPolicyResourceMap(ctx) + if err != nil { + return nil, err } var res []v6.ControllerStatus for _, service := range clusterServices { - var readOnly v6.ReadOnlyReason - policies, ok := services[service.ID] + policies, ok := policyResourceMap[service.ID] switch { - case globalReadOnly != "": - readOnly = globalReadOnly case !ok: readOnly = v6.ReadOnlyMissing case service.IsSystem: @@ -130,7 +138,7 @@ func (cs clusterContainers) Containers(i int) []resource.Container { } // List the images available for set of services -func (d *Daemon) ListImages(ctx context.Context, spec update.ResourceSpec) ([]v6.ImageStatus, error) { +func (d *Daemon) ListImages(ctx context.Context, spec update.ResourceSpec, opts v6.ListImagesOptions) ([]v6.ImageStatus, error) { var services []cluster.Controller var err error if spec == update.ResourceSpecAll { @@ -143,17 +151,25 @@ func (d *Daemon) ListImages(ctx context.Context, spec update.ResourceSpec) ([]v6 services, err = d.Cluster.SomeControllers([]flux.ResourceID{id}) } - images, err := update.CollectAvailableImages(d.Registry, clusterContainers(services), d.Logger) + imageRepos, err := update.FetchImageRepos(d.Registry, clusterContainers(services), d.Logger) if err != nil { return nil, errors.Wrap(err, "getting images for services") } + policyResourceMap, _, err := d.getPolicyResourceMap(ctx) + if err != nil { + return nil, err + } + var res []v6.ImageStatus for _, service := range services { - containers := containersWithAvailable(service, images) + serviceContainers, err := getServiceContainers(service, imageRepos, policyResourceMap, opts.OverrideContainerFields) + if err != nil { + return nil, err + } res = append(res, v6.ImageStatus{ ID: service.ID, - Containers: containers, + Containers: serviceContainers, }) } @@ -544,23 +560,83 @@ func containers2containers(cs []resource.Container) []v6.Container { return res } -func containersWithAvailable(service cluster.Controller, images update.ImageMap) (res []v6.Container) { +func getServiceContainers(service cluster.Controller, imageRepos update.ImageRepos, policyResourceMap policy.ResourceMap, fields []string) (res []v6.Container, err error) { + if len(fields) == 0 { + fields = []string{ + "Name", + "Current", + "LatestFiltered", + "Available", + "AvailableError", + "AvailableImagesCount", + "NewAvailableImagesCount", + "FilteredImagesCount", + "NewFilteredImagesCount", + } + } + for _, c := range service.ContainersOrNil() { - available := images.Available(c.Image.Name) - availableErr := "" - if available == nil { - availableErr = registry.ErrNoImageData.Error() + var container v6.Container + + imageRepo := c.Image.Name + tagPattern := getTagPattern(policyResourceMap, service.ID, c.Name) + + images := imageRepos.GetRepoImages(imageRepo) + currentImage := images.FindWithRef(c.Image) + + // All images + imagesCount := len(images) + imagesErr := "" + if images == nil { + imagesErr = registry.ErrNoImageData.Error() } - res = append(res, v6.Container{ - Name: c.Name, - Current: image.Info{ - ID: c.Image, - }, - Available: available, - AvailableError: availableErr, - }) + var newImages []image.Info + for _, img := range images { + if img.CreatedAt.After(currentImage.CreatedAt) { + newImages = append(newImages, img) + } + } + newImagesCount := len(newImages) + + // Filtered images + filteredImages := images.Filter(tagPattern) + filteredImagesCount := len(filteredImages) + var newFilteredImages []image.Info + for _, img := range filteredImages { + if img.CreatedAt.After(currentImage.CreatedAt) { + newFilteredImages = append(newFilteredImages, img) + } + } + newFilteredImagesCount := len(newFilteredImages) + + for _, field := range fields { + switch field { + case "Name": + container.Name = c.Name + case "Current": + container.Current = currentImage + case "LatestFiltered": + container.LatestFiltered, _ = filteredImages.Latest() + case "Available": + container.Available = images + case "AvailableError": + container.AvailableError = imagesErr + case "AvailableImagesCount": + container.AvailableImagesCount = imagesCount + case "NewAvailableImagesCount": + container.NewAvailableImagesCount = newImagesCount + case "FilteredImagesCount": + container.FilteredImagesCount = filteredImagesCount + case "NewFilteredImagesCount": + container.NewFilteredImagesCount = newFilteredImagesCount + default: + return nil, errors.Errorf("%s is an invalid field", field) + } + } + res = append(res, container) } - return res + + return res, nil } func policyCommitMessage(us policy.Updates, cause update.Cause) string { diff --git a/daemon/daemon_test.go b/daemon/daemon_test.go index 72856d108..f8f9baa2e 100644 --- a/daemon/daemon_test.go +++ b/daemon/daemon_test.go @@ -12,6 +12,7 @@ import ( "time" "github.com/go-kit/kit/log" + "github.com/stretchr/testify/assert" "github.com/weaveworks/flux" "github.com/weaveworks/flux/api/v6" @@ -40,6 +41,10 @@ const ( newHelloImage = "quay.io/weaveworks/helloworld:2" currentHelloImage = "quay.io/weaveworks/helloworld:master-a000001" + anotherSvc = "another:deployment/service" + anotherContainer = "it-doesn't-matter" + anotherImage = "another/service:latest" + invalidNS = "adsajkfldsa" testVersion = "test" ) @@ -137,26 +142,155 @@ func TestDaemon_ListImages(t *testing.T) { ctx := context.Background() - // List all images for services - ss := update.ResourceSpec(update.ResourceSpecAll) - is, err := d.ListImages(ctx, ss) - if err != nil { - t.Fatalf("Error: %s", err.Error()) - } - ids := imageIDs(is) - if 3 != len(ids) { - t.Fatalf("Expected %v but got %v", 3, len(ids)) + specAll := update.ResourceSpec(update.ResourceSpecAll) + + // Service 1 + svcID, err := flux.ParseResourceID(svc) + assert.NoError(t, err) + currentImageRef, err := image.ParseRef(currentHelloImage) + assert.NoError(t, err) + newImageRef, err := image.ParseRef(newHelloImage) + assert.NoError(t, err) + + // Service 2 + anotherSvcID, err := flux.ParseResourceID(anotherSvc) + assert.NoError(t, err) + anotherImageRef, err := image.ParseRef(anotherImage) + assert.NoError(t, err) + + tests := []struct { + name string + spec update.ResourceSpec + opts v6.ListImagesOptions + + expectedImages []v6.ImageStatus + expectedNumImages int + shouldError bool + }{ + { + name: "All services", + spec: specAll, + opts: v6.ListImagesOptions{}, + expectedImages: []v6.ImageStatus{ + { + ID: svcID, + Containers: []v6.Container{ + { + Name: container, + Current: image.Info{ID: currentImageRef}, + LatestFiltered: image.Info{ID: currentImageRef}, + Available: []image.Info{ + {ID: currentImageRef}, + {ID: newImageRef}, + }, + AvailableImagesCount: 2, + NewAvailableImagesCount: 1, + FilteredImagesCount: 2, + NewFilteredImagesCount: 1, + }, + }, + }, + { + ID: anotherSvcID, + Containers: []v6.Container{ + { + Name: anotherContainer, + Current: image.Info{ID: anotherImageRef}, + LatestFiltered: image.Info{}, + Available: []image.Info{ + {ID: anotherImageRef}, + }, + AvailableImagesCount: 1, + NewAvailableImagesCount: 0, + FilteredImagesCount: 0, // Excludes latest + NewFilteredImagesCount: 0, + }, + }, + }, + }, + shouldError: false, + }, + { + name: "Specific service", + spec: update.ResourceSpec(svc), + opts: v6.ListImagesOptions{}, + expectedImages: []v6.ImageStatus{ + { + ID: svcID, + Containers: []v6.Container{ + { + Name: container, + Current: image.Info{ID: currentImageRef}, + LatestFiltered: image.Info{ID: currentImageRef}, + Available: []image.Info{ + {ID: currentImageRef}, + {ID: newImageRef}, + }, + AvailableImagesCount: 2, + NewAvailableImagesCount: 1, + FilteredImagesCount: 2, + NewFilteredImagesCount: 1, + }, + }, + }, + }, + shouldError: false, + }, + { + name: "Override container field selection", + spec: specAll, + opts: v6.ListImagesOptions{OverrideContainerFields: []string{"Name", "Current", "NewAvailableImagesCount"}}, + expectedImages: []v6.ImageStatus{ + { + ID: svcID, + Containers: []v6.Container{ + { + Name: container, + Current: image.Info{ID: currentImageRef}, + NewAvailableImagesCount: 1, + }, + }, + }, + { + ID: anotherSvcID, + Containers: []v6.Container{ + { + Name: anotherContainer, + Current: image.Info{ID: anotherImageRef}, + NewAvailableImagesCount: 0, + }, + }, + }, + }, + shouldError: false, + }, + { + name: "Override container field selection with invalid field", + spec: specAll, + opts: v6.ListImagesOptions{OverrideContainerFields: []string{"InvalidField"}}, + expectedImages: nil, + shouldError: true, + }, } - // List images for specific service - ss = update.ResourceSpec(svc) - is, err = d.ListImages(ctx, ss) - if err != nil { - t.Fatalf("Error: %s", err.Error()) - } - ids = imageIDs(is) - if 2 != len(ids) { - t.Fatalf("Expected %v but got %v", 2, len(ids)) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + is, err := d.ListImages(ctx, tt.spec, tt.opts) + assert.Equal(t, tt.shouldError, err != nil) + + // Clear CreatedAt fields for testing + for ri, r := range is { + for ci, c := range r.Containers { + is[ri].Containers[ci].Current.CreatedAt = time.Time{} + is[ri].Containers[ci].LatestFiltered.CreatedAt = time.Time{} + for ai := range c.Available { + is[ri].Containers[ci].Available[ai].CreatedAt = time.Time{} + } + } + } + + assert.Equal(t, tt.expectedImages, is) + }) } } @@ -558,16 +692,6 @@ func (w *wait) ForSyncStatus(d *Daemon, rev string, expectedNumCommits int) []st return revs } -func imageIDs(status []v6.ImageStatus) []image.Info { - var availableImgs []image.Info - for _, i := range status { - for _, c := range i.Containers { - availableImgs = append(availableImgs, c.Available...) - } - } - return availableImgs -} - func updateImage(ctx context.Context, d *Daemon, t *testing.T) job.ID { return updateManifest(ctx, t, d, update.Spec{ Type: update.Images, diff --git a/daemon/images.go b/daemon/images.go index cddba1f69..eb933b5c4 100644 --- a/daemon/images.go +++ b/daemon/images.go @@ -18,23 +18,23 @@ func (d *Daemon) pollForNewImages(logger log.Logger) { ctx := context.Background() - candidateServices, err := d.unlockedAutomatedServices(ctx) + candidateServicesPolicyMap, err := d.getUnlockedAutomatedServicesPolicyMap(ctx) if err != nil { logger.Log("error", errors.Wrap(err, "getting unlocked automated services")) return } - if len(candidateServices) == 0 { + if len(candidateServicesPolicyMap) == 0 { logger.Log("msg", "no automated services") return } // Find images to check - services, err := d.Cluster.SomeControllers(candidateServices.ToSlice()) + services, err := d.Cluster.SomeControllers(candidateServicesPolicyMap.ToSlice()) if err != nil { logger.Log("error", errors.Wrap(err, "checking services for new images")) return } // Check the latest available image(s) for each service - imageMap, err := update.CollectAvailableImages(d.Registry, clusterContainers(services), logger) + imageRepos, err := update.FetchImageRepos(d.Registry, clusterContainers(services), logger) if err != nil { logger.Log("error", errors.Wrap(err, "fetching image updates")) return @@ -51,11 +51,13 @@ func (d *Daemon) pollForNewImages(logger log.Logger) { continue } - pattern := getTagPattern(candidateServices, service.ID, container.Name) + pattern := getTagPattern(candidateServicesPolicyMap, service.ID, container.Name) repo := currentImageID.Name logger.Log("repo", repo, "pattern", pattern) - if latest, ok := imageMap.LatestImage(repo, pattern); ok && latest.ID != currentImageID { + filteredImages := imageRepos.GetRepoImages(repo).Filter(pattern) + + if latest, ok := filteredImages.Latest(); ok && latest.ID != currentImageID { if latest.ID.Tag == "" { logger.Log("msg", "untagged image in available images", "action", "skip", "available", repo) continue @@ -73,6 +75,9 @@ func (d *Daemon) pollForNewImages(logger log.Logger) { } func getTagPattern(services policy.ResourceMap, service flux.ResourceID, container string) string { + if services == nil { + return "*" + } policies := services[service] if pattern, ok := policies.Get(policy.TagPrefix(container)); ok { return strings.TrimPrefix(pattern, "glob:") @@ -80,7 +85,8 @@ func getTagPattern(services policy.ResourceMap, service flux.ResourceID, contain return "*" } -func (d *Daemon) unlockedAutomatedServices(ctx context.Context) (policy.ResourceMap, error) { +// getUnlockedAutomatedServicesPolicyMap returns a resource policy map for all unlocked automated services +func (d *Daemon) getUnlockedAutomatedServicesPolicyMap(ctx context.Context) (policy.ResourceMap, error) { var services policy.ResourceMap err := d.WithClone(ctx, func(checkout *git.Checkout) error { var err error diff --git a/daemon/images_test.go b/daemon/images_test.go new file mode 100644 index 000000000..cc2da131c --- /dev/null +++ b/daemon/images_test.go @@ -0,0 +1,58 @@ +package daemon + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/weaveworks/flux" + "github.com/weaveworks/flux/policy" +) + +func Test_getTagPattern(t *testing.T) { + resourceID, err := flux.ParseResourceID("default:deployment/helloworld") + assert.NoError(t, err) + container := "helloContainer" + + type args struct { + services policy.ResourceMap + service flux.ResourceID + container string + } + tests := []struct { + name string + args args + want string + }{ + { + name: "Nil policies", + args: args{services: nil}, + want: "*", + }, + { + name: "No match", + args: args{services: policy.ResourceMap{}}, + want: "*", + }, + { + name: "Match", + args: args{ + services: policy.ResourceMap{ + resourceID: policy.Set{ + policy.Policy(fmt.Sprintf("tag.%s", container)): "glob:master-*", + }, + }, + service: resourceID, + container: container, + }, + want: "master-*", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := getTagPattern(tt.args.services, tt.args.service, tt.args.container); got != tt.want { + t.Errorf("getTagPattern() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/http/client/client.go b/http/client/client.go index ccafac535..a4acaea0b 100644 --- a/http/client/client.go +++ b/http/client/client.go @@ -57,9 +57,9 @@ func (c *Client) ListServices(ctx context.Context, namespace string) ([]v6.Contr return res, err } -func (c *Client) ListImages(ctx context.Context, s update.ResourceSpec) ([]v6.ImageStatus, error) { +func (c *Client) ListImages(ctx context.Context, s update.ResourceSpec, opts v6.ListImagesOptions) ([]v6.ImageStatus, error) { var res []v6.ImageStatus - err := c.Get(ctx, &res, transport.ListImages, "service", string(s)) + err := c.Get(ctx, &res, transport.ListImages, "service", string(s), "containerFields", strings.Join(opts.OverrideContainerFields, ",")) return res, err } @@ -210,7 +210,7 @@ func (c *Client) executeRequest(req *http.Request) (*http.Response, error) { if err := json.Unmarshal(body, &niceError); err != nil { return resp, errors.Wrap(err, "decoding response body of error") } - // just in case it's JSON but not one of our own errors + // just in case it's JSON but not one of our own errors if niceError.Err != nil { return resp, &niceError } diff --git a/http/daemon/server.go b/http/daemon/server.go index 5f22231e3..7380272ae 100644 --- a/http/daemon/server.go +++ b/http/daemon/server.go @@ -3,6 +3,7 @@ package daemon import ( "encoding/json" "net/http" + "strings" "github.com/gorilla/mux" "github.com/pkg/errors" @@ -11,6 +12,7 @@ import ( "github.com/weaveworks/flux" "github.com/weaveworks/flux/api" + "github.com/weaveworks/flux/api/v6" transport "github.com/weaveworks/flux/http" "github.com/weaveworks/flux/job" fluxmetrics "github.com/weaveworks/flux/metrics" @@ -91,14 +93,27 @@ func (s HTTPServer) SyncStatus(w http.ResponseWriter, r *http.Request) { } func (s HTTPServer) ListImages(w http.ResponseWriter, r *http.Request) { - service := mux.Vars(r)["service"] + queryValues := r.URL.Query() + + // service - Select services to update. + service := queryValues.Get("service") + if service == "" { + service = string(update.ResourceSpecAll) + } spec, err := update.ParseResourceSpec(service) if err != nil { transport.WriteError(w, r, http.StatusBadRequest, errors.Wrapf(err, "parsing service spec %q", service)) return } - d, err := s.server.ListImages(r.Context(), spec) + // containerFields - Override which fields to return in the container struct. + var opts v6.ListImagesOptions + containerFields := queryValues.Get("containerFields") + if containerFields != "" { + opts.OverrideContainerFields = strings.Split(containerFields, ",") + } + + d, err := s.server.ListImages(r.Context(), spec, opts) if err != nil { transport.ErrorResponse(w, r, err) return @@ -122,7 +137,7 @@ func (s HTTPServer) UpdateManifests(w http.ResponseWriter, r *http.Request) { } func (s HTTPServer) ListServices(w http.ResponseWriter, r *http.Request) { - namespace := mux.Vars(r)["namespace"] + namespace := r.URL.Query().Get("namespace") res, err := s.server.ListServices(r.Context(), namespace) if err != nil { transport.ErrorResponse(w, r, err) diff --git a/http/transport.go b/http/transport.go index 7b9d764c9..74d019519 100644 --- a/http/transport.go +++ b/http/transport.go @@ -29,8 +29,8 @@ func DeprecateVersions(r *mux.Router, versions ...string) { func NewAPIRouter() *mux.Router { r := mux.NewRouter() - r.NewRoute().Name(ListServices).Methods("GET").Path("/v6/services").Queries("namespace", "{namespace}") // optional namespace! - r.NewRoute().Name(ListImages).Methods("GET").Path("/v6/images").Queries("service", "{service}") + r.NewRoute().Name(ListServices).Methods("GET").Path("/v6/services") + r.NewRoute().Name(ListImages).Methods("GET").Path("/v6/images") r.NewRoute().Name(UpdateManifests).Methods("POST").Path("/v9/update-manifests") r.NewRoute().Name(JobStatus).Methods("GET").Path("/v6/jobs").Queries("id", "{id}") diff --git a/image/image.go b/image/image.go index ac283165a..fbb6e179d 100644 --- a/image/image.go +++ b/image/image.go @@ -225,16 +225,16 @@ func (i Ref) WithNewTag(t string) Ref { // from its registry. type Info struct { // the reference to this image; probably a tagged image name - ID Ref + ID Ref `json:",omitempty"` // the digest we got when fetching the metadata, which will be // different each time a manifest is uploaded for the reference - Digest string + Digest string `json:",omitempty"` // an identifier for the *image* this reference points to; this // will be the same for references that point at the same image // (but does not necessarily equal Docker's image ID) - ImageID string + ImageID string `json:",omitempty"` // the time at which the image pointed at was created - CreatedAt time.Time + CreatedAt time.Time `json:",omitempty"` } // MarshalJSON returns the Info value in JSON (as bytes). It is diff --git a/registry/cache/memcached/integration_test.go b/registry/cache/memcached/integration_test.go index 92b9b4ffc..b3e212d9a 100644 --- a/registry/cache/memcached/integration_test.go +++ b/registry/cache/memcached/integration_test.go @@ -66,14 +66,14 @@ Loop: case <-timeout.C: t.Fatal("Cache timeout") case <-tick.C: - _, err := r.GetRepository(id.Name) + _, err := r.GetSortedRepositoryImages(id.Name) if err == nil { break Loop } } } - img, err := r.GetRepository(id.Name) + img, err := r.GetSortedRepositoryImages(id.Name) if err != nil { t.Fatal(err) } diff --git a/registry/cache/registry.go b/registry/cache/registry.go index 9d27e1a7a..ea55ec575 100644 --- a/registry/cache/registry.go +++ b/registry/cache/registry.go @@ -31,9 +31,9 @@ type Cache struct { Reader Reader } -// GetRepository returns the list of image manifests in an image +// GetSortedRepositoryImages returns the list of image manifests in an image // repository (e.g,. at "quay.io/weaveworks/flux") -func (c *Cache) GetRepository(id image.Name) ([]image.Info, error) { +func (c *Cache) GetSortedRepositoryImages(id image.Name) ([]image.Info, error) { repoKey := NewRepositoryKey(id.CanonicalName()) bytes, _, err := c.Reader.GetKey(repoKey) if err != nil { diff --git a/registry/cache/warming_test.go b/registry/cache/warming_test.go index 29159a55d..7ad49c68a 100644 --- a/registry/cache/warming_test.go +++ b/registry/cache/warming_test.go @@ -71,7 +71,7 @@ func TestWarm(t *testing.T) { warmer.warm(context.TODO(), logger, repo, registry.NoCredentials()) registry := &Cache{Reader: c} - repoInfo, err := registry.GetRepository(ref.Name) + repoInfo, err := registry.GetSortedRepositoryImages(ref.Name) if err != nil { t.Error(err) } diff --git a/registry/mock/mock.go b/registry/mock/mock.go index b1c202d63..1f243fec8 100644 --- a/registry/mock/mock.go +++ b/registry/mock/mock.go @@ -40,7 +40,7 @@ type Registry struct { Err error } -func (m *Registry) GetRepository(id image.Name) ([]image.Info, error) { +func (m *Registry) GetSortedRepositoryImages(id image.Name) ([]image.Info, error) { var imgs []image.Info for _, i := range m.Images { // include only if it's the same repository in the same place diff --git a/registry/monitoring.go b/registry/monitoring.go index 2d6be8662..fcdef5444 100644 --- a/registry/monitoring.go +++ b/registry/monitoring.go @@ -46,9 +46,9 @@ func NewInstrumentedRegistry(next Registry) Registry { } } -func (m *instrumentedRegistry) GetRepository(id image.Name) (res []image.Info, err error) { +func (m *instrumentedRegistry) GetSortedRepositoryImages(id image.Name) (res []image.Info, err error) { start := time.Now() - res, err = m.next.GetRepository(id) + res, err = m.next.GetSortedRepositoryImages(id) registryDuration.With( fluxmetrics.LabelSuccess, strconv.FormatBool(err == nil), ).Observe(time.Since(start).Seconds()) diff --git a/registry/registry.go b/registry/registry.go index f0768671f..5794ee503 100644 --- a/registry/registry.go +++ b/registry/registry.go @@ -12,7 +12,7 @@ var ( // Registry is a store of image metadata. type Registry interface { - GetRepository(image.Name) ([]image.Info, error) + GetSortedRepositoryImages(image.Name) ([]image.Info, error) GetImage(image.Ref) (image.Info, error) } diff --git a/remote/logging.go b/remote/logging.go index b2deed8a6..86226bdaa 100644 --- a/remote/logging.go +++ b/remote/logging.go @@ -43,13 +43,13 @@ func (p *ErrorLoggingServer) ListServices(ctx context.Context, maybeNamespace st return p.server.ListServices(ctx, maybeNamespace) } -func (p *ErrorLoggingServer) ListImages(ctx context.Context, spec update.ResourceSpec) (_ []v6.ImageStatus, err error) { +func (p *ErrorLoggingServer) ListImages(ctx context.Context, spec update.ResourceSpec, opts v6.ListImagesOptions) (_ []v6.ImageStatus, err error) { defer func() { if err != nil { p.logger.Log("method", "ListImages", "error", err) } }() - return p.server.ListImages(ctx, spec) + return p.server.ListImages(ctx, spec, opts) } func (p *ErrorLoggingServer) JobStatus(ctx context.Context, jobID job.ID) (_ job.Status, err error) { diff --git a/remote/metrics.go b/remote/metrics.go index 2938f188e..50a98282a 100644 --- a/remote/metrics.go +++ b/remote/metrics.go @@ -56,14 +56,14 @@ func (i *instrumentedServer) ListServices(ctx context.Context, namespace string) return i.s.ListServices(ctx, namespace) } -func (i *instrumentedServer) ListImages(ctx context.Context, spec update.ResourceSpec) (_ []v6.ImageStatus, err error) { +func (i *instrumentedServer) ListImages(ctx context.Context, spec update.ResourceSpec, opts v6.ListImagesOptions) (_ []v6.ImageStatus, err error) { defer func(begin time.Time) { requestDuration.With( fluxmetrics.LabelMethod, "ListImages", fluxmetrics.LabelSuccess, fmt.Sprint(err == nil), ).Observe(time.Since(begin).Seconds()) }(time.Now()) - return i.s.ListImages(ctx, spec) + return i.s.ListImages(ctx, spec, opts) } func (i *instrumentedServer) UpdateManifests(ctx context.Context, spec update.Spec) (_ job.ID, err error) { diff --git a/remote/mock.go b/remote/mock.go index 0b5fac128..e23a41540 100644 --- a/remote/mock.go +++ b/remote/mock.go @@ -65,7 +65,7 @@ func (p *MockServer) ListServices(ctx context.Context, ns string) ([]v6.Controll return p.ListServicesAnswer, p.ListServicesError } -func (p *MockServer) ListImages(context.Context, update.ResourceSpec) ([]v6.ImageStatus, error) { +func (p *MockServer) ListImages(context.Context, update.ResourceSpec, v6.ListImagesOptions) ([]v6.ImageStatus, error) { return p.ListImagesAnswer, p.ListImagesError } @@ -194,7 +194,7 @@ func ServerTestBattery(t *testing.T, wrap func(mock api.UpstreamServer) api.Upst t.Error("expected error from ListServices, got nil") } - ims, err := client.ListImages(ctx, update.ResourceSpecAll) + ims, err := client.ListImages(ctx, update.ResourceSpecAll, v6.ListImagesOptions{}) if err != nil { t.Error(err) } @@ -202,7 +202,7 @@ func ServerTestBattery(t *testing.T, wrap func(mock api.UpstreamServer) api.Upst t.Error(fmt.Errorf("expected:\n%#v\ngot:\n%#v", mock.ListImagesAnswer, ims)) } mock.ListImagesError = fmt.Errorf("list images error") - if _, err = client.ListImages(ctx, update.ResourceSpecAll); err == nil { + if _, err = client.ListImages(ctx, update.ResourceSpecAll, v6.ListImagesOptions{}); err == nil { t.Error("expected error from ListImages, got nil") } diff --git a/remote/rpc/baseclient.go b/remote/rpc/baseclient.go index 9b7dfa4b4..5375d1526 100644 --- a/remote/rpc/baseclient.go +++ b/remote/rpc/baseclient.go @@ -33,7 +33,7 @@ func (bc baseClient) ListServices(context.Context, string) ([]v6.ControllerStatu return nil, remote.UpgradeNeededError(errors.New("ListServices method not implemented")) } -func (bc baseClient) ListImages(context.Context, update.ResourceSpec) ([]v6.ImageStatus, error) { +func (bc baseClient) ListImages(context.Context, update.ResourceSpec, v6.ListImagesOptions) ([]v6.ImageStatus, error) { return nil, remote.UpgradeNeededError(errors.New("ListImages method not implemented")) } diff --git a/remote/rpc/clientV6.go b/remote/rpc/clientV6.go index 1cbfb6b85..d7206af69 100644 --- a/remote/rpc/clientV6.go +++ b/remote/rpc/clientV6.go @@ -99,7 +99,7 @@ func (p *RPCClientV6) ListServices(ctx context.Context, namespace string) ([]v6. return services, err } -func (p *RPCClientV6) ListImages(ctx context.Context, spec update.ResourceSpec) ([]v6.ImageStatus, error) { +func (p *RPCClientV6) ListImages(ctx context.Context, spec update.ResourceSpec, opts v6.ListImagesOptions) ([]v6.ImageStatus, error) { var images []v6.ImageStatus if err := requireServiceSpecKinds(spec, supportedKindsV6); err != nil { return images, remote.UpgradeNeededError(err) diff --git a/remote/rpc/clientV7.go b/remote/rpc/clientV7.go index b87a6c65d..074b5f46d 100644 --- a/remote/rpc/clientV7.go +++ b/remote/rpc/clientV7.go @@ -62,7 +62,7 @@ func (p *RPCClientV7) ListServices(ctx context.Context, namespace string) ([]v6. return resp.Result, err } -func (p *RPCClientV7) ListImages(ctx context.Context, spec update.ResourceSpec) ([]v6.ImageStatus, error) { +func (p *RPCClientV7) ListImages(ctx context.Context, spec update.ResourceSpec, opts v6.ListImagesOptions) ([]v6.ImageStatus, error) { var resp ListImagesResponse if err := requireServiceSpecKinds(spec, supportedKindsV7); err != nil { return resp.Result, remote.UpgradeNeededError(err) diff --git a/remote/rpc/clientV8.go b/remote/rpc/clientV8.go index dddb3eba8..bfcdaf3eb 100644 --- a/remote/rpc/clientV8.go +++ b/remote/rpc/clientV8.go @@ -32,7 +32,7 @@ func NewClientV8(conn io.ReadWriteCloser) *RPCClientV8 { return &RPCClientV8{NewClientV7(conn)} } -func (p *RPCClientV8) ListImages(ctx context.Context, spec update.ResourceSpec) ([]v6.ImageStatus, error) { +func (p *RPCClientV8) ListImages(ctx context.Context, spec update.ResourceSpec, opts v6.ListImagesOptions) ([]v6.ImageStatus, error) { var resp ListImagesResponse if err := requireServiceSpecKinds(spec, supportedKindsV8); err != nil { return resp.Result, remote.UnsupportedResourceKind(err) diff --git a/remote/rpc/server.go b/remote/rpc/server.go index ebe2ba531..c12905d96 100644 --- a/remote/rpc/server.go +++ b/remote/rpc/server.go @@ -89,7 +89,7 @@ type ListImagesResponse struct { } func (p *RPCServer) ListImages(spec update.ResourceSpec, resp *ListImagesResponse) error { - v, err := p.s.ListImages(context.Background(), spec) + v, err := p.s.ListImages(context.Background(), spec, v6.ListImagesOptions{}) resp.Result = v if err != nil { if err, ok := errors.Cause(err).(*fluxerr.Error); ok { diff --git a/update/images.go b/update/images.go index b4d9eae59..42e870a64 100644 --- a/update/images.go +++ b/update/images.go @@ -14,47 +14,67 @@ import ( "github.com/weaveworks/flux/resource" ) -type infoMap map[image.CanonicalName][]image.Info +type imageReposMap map[image.CanonicalName]ImageInfos -type ImageMap struct { - images infoMap +// ImageRepos contains a map of image repositories to their images +type ImageRepos struct { + imageRepos imageReposMap } -// LatestImage returns the latest releasable image for a repository -// for which the tag matches a given pattern. A releasable image is -// one that is not tagged "latest". (Assumes the available images are -// in descending order of latestness.) If no such image exists, -// returns a zero value and `false`, and the caller can decide whether -// that's an error or not. -func (m ImageMap) LatestImage(repo image.Name, tagGlob string) (image.Info, bool) { - for _, available := range m.images[repo.CanonicalName()] { - tag := available.ID.Tag +// GetRepoImages returns image.Info entries for all the images in the +// named image repository. +func (r ImageRepos) GetRepoImages(repo image.Name) ImageInfos { + if canon, ok := r.imageRepos[repo.CanonicalName()]; ok { + infos := make([]image.Info, len(canon)) + for i := range canon { + infos[i] = canon[i] + infos[i].ID = repo.ToRef(infos[i].ID.Tag) + } + return infos + } + return nil +} + +// ImageInfos is a list of image.Info which can be filtered. +type ImageInfos []image.Info + +// Filter returns only the images which match the tagGlob. +func (ii ImageInfos) Filter(tagGlob string) ImageInfos { + var filtered ImageInfos + for _, i := range ii { + tag := i.ID.Tag // Ignore latest if and only if it's not what the user wants. if !strings.EqualFold(tagGlob, "latest") && strings.EqualFold(tag, "latest") { continue } if glob.Glob(tagGlob, tag) { var im image.Info - im = available - im.ID = repo.ToRef(tag) - return im, true + im = i + filtered = append(filtered, im) } } + return filtered +} + +// Latest returns the latest image from ImageInfos. If no such image exists, +// returns a zero value and `false`, and the caller can decide whether +// that's an error or not. +func (ii ImageInfos) Latest() (image.Info, bool) { + if len(ii) > 0 { + return ii[0], true + } return image.Info{}, false } -// Available returns image.Info entries for all the images in the -// named image repository. -func (m ImageMap) Available(repo image.Name) []image.Info { - if canon, ok := m.images[repo.CanonicalName()]; ok { - infos := make([]image.Info, len(canon)) - for i := range canon { - infos[i] = canon[i] - infos[i].ID = repo.ToRef(infos[i].ID.Tag) +// FindWithRef returns image.Info given an image ref. If the image cannot be +// found, it returns the image.Info with the ID provided. +func (ii ImageInfos) FindWithRef(ref image.Ref) image.Info { + for _, img := range ii { + if img.ID == ref { + return img } - return infos } - return nil + return image.Info{ID: ref} } // containers represents a collection of things that have containers @@ -73,50 +93,50 @@ func (cs controllerContainers) Containers(i int) []resource.Container { return cs[i].Controller.ContainersOrNil() } -// collectUpdateImages is a convenient shim to -// `CollectAvailableImages`. -func collectUpdateImages(registry registry.Registry, updateable []*ControllerUpdate, logger log.Logger) (ImageMap, error) { - return CollectAvailableImages(registry, controllerContainers(updateable), logger) +// fetchUpdatableImageRepos is a convenient shim to +// `FetchImageRepos`. +func fetchUpdatableImageRepos(registry registry.Registry, updateable []*ControllerUpdate, logger log.Logger) (ImageRepos, error) { + return FetchImageRepos(registry, controllerContainers(updateable), logger) } -// CollectAvailableImages finds all the known image metadata for +// FetchImageRepos finds all the known image metadata for // containers in the controllers given. -func CollectAvailableImages(reg registry.Registry, cs containers, logger log.Logger) (ImageMap, error) { - images := infoMap{} +func FetchImageRepos(reg registry.Registry, cs containers, logger log.Logger) (ImageRepos, error) { + imageRepos := imageReposMap{} for i := 0; i < cs.Len(); i++ { for _, container := range cs.Containers(i) { - images[container.Image.CanonicalName()] = nil + imageRepos[container.Image.CanonicalName()] = nil } } - for name := range images { - imageRepo, err := reg.GetRepository(name.Name) + for repo := range imageRepos { + sortedRepoImages, err := reg.GetSortedRepositoryImages(repo.Name) if err != nil { // Not an error if missing. Use empty images. if !fluxerr.IsMissing(err) { - logger.Log("err", errors.Wrapf(err, "fetching image metadata for %s", name)) + logger.Log("err", errors.Wrapf(err, "fetching image metadata for %s", repo)) continue } } - images[name] = imageRepo + imageRepos[repo] = sortedRepoImages } - return ImageMap{images}, nil + return ImageRepos{imageRepos}, nil } -// Create a map of images. It will check that each image exists. -func exactImages(reg registry.Registry, images []image.Ref) (ImageMap, error) { - m := infoMap{} +// Create a map of image repos to images. It will check that each image exists. +func exactImageRepos(reg registry.Registry, images []image.Ref) (ImageRepos, error) { + m := imageReposMap{} for _, id := range images { // We must check that the exact images requested actually exist. Otherwise we risk pushing invalid images to git. exist, err := imageExists(reg, id) if err != nil { - return ImageMap{}, errors.Wrap(image.ErrInvalidImageID, err.Error()) + return ImageRepos{}, errors.Wrap(image.ErrInvalidImageID, err.Error()) } if !exist { - return ImageMap{}, errors.Wrap(image.ErrInvalidImageID, fmt.Sprintf("image %q does not exist", id)) + return ImageRepos{}, errors.Wrap(image.ErrInvalidImageID, fmt.Sprintf("image %q does not exist", id)) } m[id.CanonicalName()] = []image.Info{{ID: id}} } - return ImageMap{m}, nil + return ImageRepos{m}, nil } // Checks whether the given image exists in the repository. diff --git a/update/images_test.go b/update/images_test.go index ed2aafaf9..79e3c929e 100644 --- a/update/images_test.go +++ b/update/images_test.go @@ -20,25 +20,27 @@ var ( // names (e.g., `index.docker.io/library/alpine`), but we ask // questions in terms of everyday names (e.g., `alpine`). func TestDecanon(t *testing.T) { - m := ImageMap{infoMap{ + m := ImageRepos{imageReposMap{ name: infos, }} - latest, ok := m.LatestImage(mustParseName("weaveworks/helloworld"), "*") + filteredImages := m.GetRepoImages(mustParseName("weaveworks/helloworld")).Filter("*") + latest, ok := filteredImages.Latest() if !ok { t.Error("did not find latest image") } else if latest.ID.Name != mustParseName("weaveworks/helloworld") { t.Error("name did not match what was asked") } - latest, ok = m.LatestImage(mustParseName("index.docker.io/weaveworks/helloworld"), "*") + filteredImages = m.GetRepoImages(mustParseName("index.docker.io/weaveworks/helloworld")).Filter("*") + latest, ok = filteredImages.Latest() if !ok { t.Error("did not find latest image") } else if latest.ID.Name != mustParseName("index.docker.io/weaveworks/helloworld") { t.Error("name did not match what was asked") } - avail := m.Available(mustParseName("weaveworks/helloworld")) + avail := m.GetRepoImages(mustParseName("weaveworks/helloworld")) if len(avail) != len(infos) { t.Errorf("expected %d available images, got %d", len(infos), len(avail)) } @@ -50,8 +52,8 @@ func TestDecanon(t *testing.T) { } func TestAvail(t *testing.T) { - m := ImageMap{infoMap{name: infos}} - avail := m.Available(mustParseName("weaveworks/goodbyeworld")) + m := ImageRepos{imageReposMap{name: infos}} + avail := m.GetRepoImages(mustParseName("weaveworks/goodbyeworld")) if len(avail) > 0 { t.Errorf("did not expect available images, but got %#v", avail) } diff --git a/update/release.go b/update/release.go index 803d1c829..df52bc574 100644 --- a/update/release.go +++ b/update/release.go @@ -188,20 +188,20 @@ func (s ReleaseSpec) markSkipped(results Result) { // if not, it indicates there's likely some problem with the running // system vs the definitions given in the repo.) func (s ReleaseSpec) calculateImageUpdates(rc ReleaseContext, candidates []*ControllerUpdate, results Result, logger log.Logger) ([]*ControllerUpdate, error) { - // Compile an `ImageMap` of all relevant images - var images ImageMap + // Compile an `ImageRepos` of all relevant images + var imageRepos ImageRepos var singleRepo image.CanonicalName var err error switch s.ImageSpec { case ImageSpecLatest: - images, err = collectUpdateImages(rc.Registry(), candidates, logger) + imageRepos, err = fetchUpdatableImageRepos(rc.Registry(), candidates, logger) default: var ref image.Ref ref, err = s.ImageSpec.AsRef() if err == nil { singleRepo = ref.CanonicalName() - images, err = exactImages(rc.Registry(), []image.Ref{ref}) + imageRepos, err = exactImageRepos(rc.Registry(), []image.Ref{ref}) } } @@ -231,7 +231,8 @@ func (s ReleaseSpec) calculateImageUpdates(rc ReleaseContext, candidates []*Cont for _, container := range containers { currentImageID := container.Image - latestImage, ok := images.LatestImage(currentImageID.Name, "*") + filteredImages := imageRepos.GetRepoImages(currentImageID.Name).Filter("*") + latestImage, ok := filteredImages.Latest() if !ok { if currentImageID.CanonicalName() != singleRepo { ignoredOrSkipped = ReleaseStatusIgnored