From 64be046f745abed8a4e213a26710296ab5750441 Mon Sep 17 00:00:00 2001 From: Aaron Kirkbride Date: Fri, 8 Jun 2018 16:33:41 +0100 Subject: [PATCH] Polyfill container fields filter from v10 to v6 --- api/v6/api.go | 17 ----- api/v6/container.go | 115 ++++++++++++++++++++++++++++++++++ api/v6/container_test.go | 130 +++++++++++++++++++++++++++++++++++++++ daemon/daemon.go | 69 ++------------------- daemon/daemon_test.go | 1 - daemon/images.go | 15 +---- daemon/images_test.go | 58 ----------------- policy/policy.go | 11 ++++ policy/policy_test.go | 52 ++++++++++++++++ remote/logging.go | 2 +- remote/rpc/clientV10.go | 4 +- remote/rpc/clientV6.go | 41 +++++++++++- 12 files changed, 355 insertions(+), 160 deletions(-) create mode 100644 api/v6/container.go create mode 100644 api/v6/container_test.go delete mode 100644 daemon/images_test.go diff --git a/api/v6/api.go b/api/v6/api.go index 42f4ad78dd..1a9349b8b4 100644 --- a/api/v6/api.go +++ b/api/v6/api.go @@ -5,7 +5,6 @@ import ( "github.com/weaveworks/flux" "github.com/weaveworks/flux/git" - "github.com/weaveworks/flux/image" "github.com/weaveworks/flux/job" "github.com/weaveworks/flux/ssh" "github.com/weaveworks/flux/update" @@ -42,22 +41,6 @@ type ControllerStatus struct { Policies map[string]string } -type Container struct { - 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 type GitRemoteConfig struct { diff --git a/api/v6/container.go b/api/v6/container.go new file mode 100644 index 0000000000..b09bdda8cf --- /dev/null +++ b/api/v6/container.go @@ -0,0 +1,115 @@ +package v6 + +import ( + "github.com/pkg/errors" + "github.com/weaveworks/flux/image" + "github.com/weaveworks/flux/registry" + "github.com/weaveworks/flux/update" +) + +// Container describes an individual container including current image info and +// available images. +type Container struct { + 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"` +} + +// NewContainer creates a Container given a list of images and the current image +func NewContainer(name string, images update.ImageInfos, currentImage image.Info, tagPattern string, fields []string) (Container, error) { + // All images + imagesCount := len(images) + imagesErr := "" + if images == nil { + imagesErr = registry.ErrNoImageData.Error() + } + 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) + latestFiltered, _ := filteredImages.Latest() + + container := Container{ + Name: name, + Current: currentImage, + LatestFiltered: latestFiltered, + + Available: images, + AvailableError: imagesErr, + AvailableImagesCount: imagesCount, + NewAvailableImagesCount: newImagesCount, + FilteredImagesCount: filteredImagesCount, + NewFilteredImagesCount: newFilteredImagesCount, + } + return filterContainerFields(container, fields) +} + +// filterContainerFields returns a new container with only the fields specified. If not fields are specified, +// a list of default fields is used. +func filterContainerFields(container Container, fields []string) (Container, error) { + // Default fields + if len(fields) == 0 { + fields = []string{ + "Name", + "Current", + "LatestFiltered", + "Available", + "AvailableError", + "AvailableImagesCount", + "NewAvailableImagesCount", + "FilteredImagesCount", + "NewFilteredImagesCount", + } + } + + var c Container + for _, field := range fields { + switch field { + case "Name": + c.Name = container.Name + case "Current": + c.Current = container.Current + case "LatestFiltered": + c.LatestFiltered = container.LatestFiltered + case "Available": + c.Available = container.Available + case "AvailableError": + c.AvailableError = container.AvailableError + case "AvailableImagesCount": + c.AvailableImagesCount = container.AvailableImagesCount + case "NewAvailableImagesCount": + c.NewAvailableImagesCount = container.NewAvailableImagesCount + case "FilteredImagesCount": + c.FilteredImagesCount = container.FilteredImagesCount + case "NewFilteredImagesCount": + c.NewFilteredImagesCount = container.NewFilteredImagesCount + default: + return c, errors.Errorf("%s is an invalid field", field) + } + } + return c, nil +} diff --git a/api/v6/container_test.go b/api/v6/container_test.go new file mode 100644 index 0000000000..57ebaaa5df --- /dev/null +++ b/api/v6/container_test.go @@ -0,0 +1,130 @@ +package v6 + +import ( + "reflect" + "testing" + + "github.com/weaveworks/flux/image" + "github.com/weaveworks/flux/update" +) + +func TestNewContainer(t *testing.T) { + + testImage := image.Info{ImageID: "test"} + + type args struct { + name string + images update.ImageInfos + currentImage image.Info + tagPattern string + fields []string + } + tests := []struct { + name string + args args + want Container + wantErr bool + }{ + { + name: "Simple", + args: args{ + name: "container1", + images: update.ImageInfos{testImage}, + currentImage: testImage, + tagPattern: "*", + }, + want: Container{ + Name: "container1", + Current: testImage, + LatestFiltered: testImage, + Available: []image.Info{testImage}, + AvailableImagesCount: 1, + NewAvailableImagesCount: 0, + FilteredImagesCount: 1, + NewFilteredImagesCount: 0, + }, + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := NewContainer(tt.args.name, tt.args.images, tt.args.currentImage, tt.args.tagPattern, tt.args.fields) + if (err != nil) != tt.wantErr { + t.Errorf("NewContainer() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("NewContainer() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestFilterContainerFields(t *testing.T) { + testContainer := Container{ + Name: "test", + Current: image.Info{ImageID: "123"}, + LatestFiltered: image.Info{ImageID: "123"}, + Available: []image.Info{{ImageID: "123"}}, + AvailableError: "test", + AvailableImagesCount: 1, + NewAvailableImagesCount: 2, + FilteredImagesCount: 3, + NewFilteredImagesCount: 4, + } + + type args struct { + container Container + fields []string + } + tests := []struct { + name string + args args + want Container + wantErr bool + }{ + { + name: "Default fields", + args: args{ + container: testContainer, + }, + want: testContainer, + wantErr: false, + }, + { + name: "Filter", + args: args{ + container: testContainer, + fields: []string{"Name", "Available", "NewAvailableImagesCount", "NewFilteredImagesCount"}, + }, + want: Container{ + Name: "test", + Available: []image.Info{{ImageID: "123"}}, + NewAvailableImagesCount: 2, + NewFilteredImagesCount: 4, + }, + wantErr: false, + }, + { + name: "Invalid field", + args: args{ + container: testContainer, + fields: []string{"Invalid"}, + }, + want: Container{}, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := filterContainerFields(tt.args.container, tt.args.fields) + if (err != nil) != tt.wantErr { + t.Errorf("FilterContainerFields() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("FilterContainerFields() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/daemon/daemon.go b/daemon/daemon.go index 06b250d314..82fdacfcfd 100644 --- a/daemon/daemon.go +++ b/daemon/daemon.go @@ -572,77 +572,16 @@ func containers2containers(cs []resource.Container) []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() { - var container v6.Container - imageRepo := c.Image.Name - tagPattern := getTagPattern(policyResourceMap, service.ID, c.Name) + tagPattern := policy.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() - } - 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) - } + container, err := v6.NewContainer(c.Name, images, currentImage, tagPattern, fields) + if err != nil { + return res, err } res = append(res, container) } diff --git a/daemon/daemon_test.go b/daemon/daemon_test.go index 4590edd059..5341a9ee85 100644 --- a/daemon/daemon_test.go +++ b/daemon/daemon_test.go @@ -13,7 +13,6 @@ import ( "github.com/go-kit/kit/log" "github.com/stretchr/testify/assert" - "github.com/weaveworks/flux" "github.com/weaveworks/flux/api/v10" "github.com/weaveworks/flux/api/v6" diff --git a/daemon/images.go b/daemon/images.go index eb933b5c47..e72aa6f633 100644 --- a/daemon/images.go +++ b/daemon/images.go @@ -2,12 +2,10 @@ package daemon import ( "context" - "strings" "github.com/go-kit/kit/log" "github.com/pkg/errors" - "github.com/weaveworks/flux" "github.com/weaveworks/flux/git" "github.com/weaveworks/flux/policy" "github.com/weaveworks/flux/update" @@ -51,7 +49,7 @@ func (d *Daemon) pollForNewImages(logger log.Logger) { continue } - pattern := getTagPattern(candidateServicesPolicyMap, service.ID, container.Name) + pattern := policy.GetTagPattern(candidateServicesPolicyMap, service.ID, container.Name) repo := currentImageID.Name logger.Log("repo", repo, "pattern", pattern) @@ -74,17 +72,6 @@ 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:") - } - return "*" -} - // 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 diff --git a/daemon/images_test.go b/daemon/images_test.go deleted file mode 100644 index cc2da131c8..0000000000 --- a/daemon/images_test.go +++ /dev/null @@ -1,58 +0,0 @@ -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/policy/policy.go b/policy/policy.go index 856fd1b021..105294fb7d 100644 --- a/policy/policy.go +++ b/policy/policy.go @@ -36,6 +36,17 @@ func Tag(policy Policy) bool { return strings.HasPrefix(string(policy), "tag.") } +func GetTagPattern(services ResourceMap, service flux.ResourceID, container string) string { + if services == nil { + return "*" + } + policies := services[service] + if pattern, ok := policies.Get(TagPrefix(container)); ok { + return strings.TrimPrefix(pattern, "glob:") + } + return "*" +} + type Updates map[flux.ResourceID]Update type Update struct { diff --git a/policy/policy_test.go b/policy/policy_test.go index 914cd6c951..ac8e2863a9 100644 --- a/policy/policy_test.go +++ b/policy/policy_test.go @@ -2,8 +2,12 @@ package policy import ( "encoding/json" + "fmt" "reflect" "testing" + + "github.com/stretchr/testify/assert" + "github.com/weaveworks/flux" ) func TestJSON(t *testing.T) { @@ -45,3 +49,51 @@ func TestJSON(t *testing.T) { t.Errorf("Parsing equivalent list did not preserve policy. Expected:\n%#v\nGot:\n%#v\n", policy, policy2) } } + +func Test_GetTagPattern(t *testing.T) { + resourceID, err := flux.ParseResourceID("default:deployment/helloworld") + assert.NoError(t, err) + container := "helloContainer" + + type args struct { + services 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: ResourceMap{}}, + want: "*", + }, + { + name: "Match", + args: args{ + services: ResourceMap{ + resourceID: Set{ + 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/remote/logging.go b/remote/logging.go index bc63fe455b..ef64c41663 100644 --- a/remote/logging.go +++ b/remote/logging.go @@ -56,7 +56,7 @@ func (p *ErrorLoggingServer) ListImages(ctx context.Context, spec update.Resourc func (p *ErrorLoggingServer) ListImagesWithOptions(ctx context.Context, opts v10.ListImagesOptions) (_ []v6.ImageStatus, err error) { defer func() { if err != nil { - p.logger.Log("method", "ListImages", "error", err) + p.logger.Log("method", "ListImagesWithOptions", "error", err) } }() return p.server.ListImagesWithOptions(ctx, opts) diff --git a/remote/rpc/clientV10.go b/remote/rpc/clientV10.go index c30b20b540..76bafc6265 100644 --- a/remote/rpc/clientV10.go +++ b/remote/rpc/clientV10.go @@ -11,8 +11,8 @@ import ( ) // RPCClientV10 is the rpc-backed implementation of a server, for -// talking to remote daemons. Version 8 has the same methods, but -// supports a different set of resource kinds to earlier versions. +// talking to remote daemons. This version introduces methods which accept an +// options struct as the first argument. e.g. ListImagesWithOptions type RPCClientV10 struct { *RPCClientV9 } diff --git a/remote/rpc/clientV6.go b/remote/rpc/clientV6.go index 905df0feba..2846c052c0 100644 --- a/remote/rpc/clientV6.go +++ b/remote/rpc/clientV6.go @@ -7,6 +7,7 @@ import ( "net/rpc/jsonrpc" "github.com/weaveworks/flux/api/v10" + "github.com/weaveworks/flux/policy" "github.com/weaveworks/flux/api/v6" fluxerr "github.com/weaveworks/flux/errors" @@ -118,8 +119,44 @@ func (p *RPCClientV6) ListImages(ctx context.Context, spec update.ResourceSpec) } func (p *RPCClientV6) ListImagesWithOptions(ctx context.Context, opts v10.ListImagesOptions) ([]v6.ImageStatus, error) { - // TODO: Backfill list image summary fields from v10 - return p.ListImages(ctx, opts.Spec) + images, err := p.ListImages(ctx, opts.Spec) + if err != nil { + return images, err + } + + var ns string + if opts.Spec != update.ResourceSpecAll { + resourceID, err := opts.Spec.AsID() + if err != nil { + return images, err + } + ns, _, _ = resourceID.Components() + } + services, err := p.ListServices(ctx, ns) + + policyMap := make(policy.ResourceMap) + for _, service := range services { + var s policy.Set + for k, v := range service.Policies { + s[policy.Policy(k)] = v + } + policyMap[service.ID] = s + } + + // Polyfill container fields from v10 + for i, image := range images { + for j, container := range image.Containers { + tagPattern := policy.GetTagPattern(policyMap, image.ID, container.Name) + // Create a new container using the same function used in v10 + newContainer, err := v6.NewContainer(container.Name, container.Available, container.Current, tagPattern, opts.OverrideContainerFields) + if err != nil { + return images, err + } + images[i].Containers[j] = newContainer + } + } + + return images, nil } func (p *RPCClientV6) UpdateManifests(ctx context.Context, u update.Spec) (job.ID, error) {