From 42911cf14300a1570aa1765b21a1c93c20444b2f Mon Sep 17 00:00:00 2001 From: Elena Morozova Date: Thu, 25 Oct 2018 15:23:33 -0700 Subject: [PATCH 1/4] Add events for container release For ReleaseEventMetadata use spec with both types: ReleaseImageSpec and ReleaseContainersSpec Need this to fix https://github.com/weaveworks/service/issues/2277 --- cmd/fluxctl/release_cmd.go | 6 ++-- daemon/loop.go | 29 ++++++++++++++++-- event/event.go | 30 +++++++++++++++---- http/daemon/server.go | 2 +- remote/mock.go | 2 +- remote/rpc/compat.go | 2 +- .../{containers.go => release_containers.go} | 18 +++++------ update/{release.go => release_image.go} | 18 +++++------ update/spec.go | 4 +-- 9 files changed, 77 insertions(+), 34 deletions(-) rename update/{containers.go => release_containers.go} (86%) rename update/{release.go => release_image.go} (91%) diff --git a/cmd/fluxctl/release_cmd.go b/cmd/fluxctl/release_cmd.go index 1509eec47..51b8d1c45 100644 --- a/cmd/fluxctl/release_cmd.go +++ b/cmd/fluxctl/release_cmd.go @@ -137,7 +137,7 @@ func (opts *controllerReleaseOpts) RunE(cmd *cobra.Command, args []string) error } ctx := context.Background() - spec := update.ReleaseSpec{ + spec := update.ReleaseImageSpec{ ServiceSpecs: controllers, ImageSpec: image, Kind: kind, @@ -182,10 +182,10 @@ func (opts *controllerReleaseOpts) RunE(cmd *cobra.Command, args []string) error return await(ctx, cmd.OutOrStdout(), cmd.OutOrStderr(), opts.API, jobID, !opts.dryRun, opts.verbosity) } -func promptSpec(out io.Writer, result job.Result, verbosity int) (update.ContainerSpecs, error) { +func promptSpec(out io.Writer, result job.Result, verbosity int) (update.ReleaseContainersSpec, error) { menu := update.NewMenu(out, result.Result, verbosity) containerSpecs, err := menu.Run() - return update.ContainerSpecs{ + return update.ReleaseContainersSpec{ Kind: update.ReleaseKindExecute, ContainerSpecs: containerSpecs, SkipMismatches: false, diff --git a/daemon/loop.go b/daemon/loop.go index 87d954959..e5f69efd5 100644 --- a/daemon/loop.go +++ b/daemon/loop.go @@ -306,8 +306,30 @@ func (d *Daemon) doSync(logger log.Logger) (retErr error) { // Interpret some notes as events to send to the upstream switch n.Spec.Type { + case update.Containers: + spec := n.Spec.Spec.(update.ReleaseContainersSpec) + noteEvents = append(noteEvents, event.Event{ + ServiceIDs: n.Result.AffectedResources(), + Type: event.EventRelease, + StartedAt: started, + EndedAt: time.Now().UTC(), + LogLevel: event.LogLevelInfo, + Metadata: &event.ReleaseEventMetadata{ + ReleaseEventCommon: event.ReleaseEventCommon{ + Revision: commits[i].Revision, + Result: n.Result, + Error: n.Result.Error(), + }, + Spec: event.ReleaseSpec{ + Type: event.ReleaseContainerSpecType, + ReleaseContainerSpec: &spec, + }, + Cause: n.Spec.Cause, + }, + }) + includes[event.EventRelease] = true case update.Images: - spec := n.Spec.Spec.(update.ReleaseSpec) + spec := n.Spec.Spec.(update.ReleaseImageSpec) noteEvents = append(noteEvents, event.Event{ ServiceIDs: n.Result.AffectedResources(), Type: event.EventRelease, @@ -320,7 +342,10 @@ func (d *Daemon) doSync(logger log.Logger) (retErr error) { Result: n.Result, Error: n.Result.Error(), }, - Spec: spec, + Spec: event.ReleaseSpec{ + Type: event.ReleaseImageSpecType, + ReleaseImageSpec: &spec, + }, Cause: n.Spec.Cause, }, }) diff --git a/event/event.go b/event/event.go index 2b963bb59..281f8004b 100644 --- a/event/event.go +++ b/event/event.go @@ -95,10 +95,12 @@ func (e Event) String() string { if len(strImageIDs) == 0 { strImageIDs = []string{"no image changes"} } - for _, spec := range metadata.Spec.ServiceSpecs { - if spec == update.ResourceSpecAll { - strServiceIDs = []string{"all services"} - break + if metadata.Spec.Type == ReleaseImageSpecType { + for _, spec := range metadata.Spec.ReleaseImageSpec.ServiceSpecs { + if spec == update.ResourceSpecAll { + strServiceIDs = []string{"all services"} + break + } } } if len(strServiceIDs) == 0 { @@ -239,11 +241,27 @@ type ReleaseEventCommon struct { Error string `json:"error,omitempty"` } +const ( + // ReleaseImageSpecType is a type of release spec when there are update.Images + ReleaseImageSpecType = "releaseImageSpecType" + // ReleaseContainerSpecType is a type of release spec when there are update.Containers + ReleaseContainerSpecType = "releaseContainerSpecType" +) + +// ReleaseSpec is a spec for images and containers release +type ReleaseSpec struct { + // Type is ReleaseImagesSpecType or ReleaseContainersSpecType + // if empty (for previous version), then use ReleaseImagesSpecType + Type string + ReleaseImageSpec *update.ReleaseImageSpec + ReleaseContainerSpec *update.ReleaseContainersSpec +} + // ReleaseEventMetadata is the metadata for when service(s) are released type ReleaseEventMetadata struct { ReleaseEventCommon - Spec update.ReleaseSpec `json:"spec"` - Cause update.Cause `json:"cause"` + Spec ReleaseSpec `json:"spec"` + Cause update.Cause `json:"cause"` } // AutoReleaseEventMetadata is for when service(s) are released diff --git a/http/daemon/server.go b/http/daemon/server.go index 732e56349..aa7a152ab 100644 --- a/http/daemon/server.go +++ b/http/daemon/server.go @@ -227,7 +227,7 @@ func (s HTTPServer) UpdateImages(w http.ResponseWriter, r *http.Request) { excludes = append(excludes, s) } - spec := update.ReleaseSpec{ + spec := update.ReleaseImageSpec{ ServiceSpecs: serviceSpecs, ImageSpec: imageSpec, Kind: releaseKind, diff --git a/remote/mock.go b/remote/mock.go index bed6701aa..8f4e5a9bd 100644 --- a/remote/mock.go +++ b/remote/mock.go @@ -160,7 +160,7 @@ func ServerTestBattery(t *testing.T, wrap func(mock api.UpstreamServer) api.Upst updateSpec := update.Spec{ Type: update.Images, - Spec: update.ReleaseSpec{ + Spec: update.ReleaseImageSpec{ ServiceSpecs: []update.ResourceSpec{ update.ResourceSpecAll, }, diff --git a/remote/rpc/compat.go b/remote/rpc/compat.go index d9498d5ec..686c4b7d5 100644 --- a/remote/rpc/compat.go +++ b/remote/rpc/compat.go @@ -47,7 +47,7 @@ func requireSpecKinds(s update.Spec, kinds []string) error { return fmt.Errorf("Unsupported resource kind: %s", kind) } } - case update.ReleaseSpec: + case update.ReleaseImageSpec: for _, ss := range s.ServiceSpecs { if err := requireServiceSpecKinds(ss, kinds); err != nil { return err diff --git a/update/containers.go b/update/release_containers.go similarity index 86% rename from update/containers.go rename to update/release_containers.go index 624a3c2f3..1db06d0dd 100644 --- a/update/containers.go +++ b/update/release_containers.go @@ -15,8 +15,8 @@ import ( var zeroImageRef = image.Ref{} -// ContainerSpecs defines the spec for a `containers` manifest update. -type ContainerSpecs struct { +// ReleaseContainersSpec defines the spec for a `containers` manifest update. +type ReleaseContainersSpec struct { Kind ReleaseKind ContainerSpecs map[flux.ResourceID][]ContainerUpdate SkipMismatches bool @@ -25,7 +25,7 @@ type ContainerSpecs struct { // CalculateRelease computes required controller updates to satisfy this specification. // It returns an error if any spec calculation fails unless `SkipMismatches` is true. -func (s ContainerSpecs) CalculateRelease(rc ReleaseContext, logger log.Logger) ([]*ControllerUpdate, Result, error) { +func (s ReleaseContainersSpec) CalculateRelease(rc ReleaseContext, logger log.Logger) ([]*ControllerUpdate, Result, error) { results := Result{} prefilter, postfilter := s.filters() all, err := rc.SelectServices(results, prefilter, postfilter) @@ -36,7 +36,7 @@ func (s ContainerSpecs) CalculateRelease(rc ReleaseContext, logger log.Logger) ( return updates, results, s.resultsError(results) } -func (s ContainerSpecs) resultsError(results Result) error { +func (s ReleaseContainersSpec) resultsError(results Result) error { failures := 0 successes := 0 for _, res := range results { @@ -56,7 +56,7 @@ func (s ContainerSpecs) resultsError(results Result) error { return nil } -func (s ContainerSpecs) filters() ([]ControllerFilter, []ControllerFilter) { +func (s ReleaseContainersSpec) filters() ([]ControllerFilter, []ControllerFilter) { var rids []flux.ResourceID for rid := range s.ContainerSpecs { rids = append(rids, rid) @@ -69,7 +69,7 @@ func (s ContainerSpecs) filters() ([]ControllerFilter, []ControllerFilter) { return pre, []ControllerFilter{} } -func (s ContainerSpecs) controllerUpdates(results Result, all []*ControllerUpdate) []*ControllerUpdate { +func (s ReleaseContainersSpec) controllerUpdates(results Result, all []*ControllerUpdate) []*ControllerUpdate { var updates []*ControllerUpdate for _, u := range all { cs, err := u.Controller.ContainersOrError() @@ -157,15 +157,15 @@ func (s ContainerSpecs) controllerUpdates(results Result, all []*ControllerUpdat return updates } -func (s ContainerSpecs) ReleaseKind() ReleaseKind { +func (s ReleaseContainersSpec) ReleaseKind() ReleaseKind { return s.Kind } -func (s ContainerSpecs) ReleaseType() ReleaseType { +func (s ReleaseContainersSpec) ReleaseType() ReleaseType { return "containers" } -func (s ContainerSpecs) CommitMessage(result Result) string { +func (s ReleaseContainersSpec) CommitMessage(result Result) string { var workloads []string body := &bytes.Buffer{} for _, res := range result.AffectedResources() { diff --git a/update/release.go b/update/release_image.go similarity index 91% rename from update/release.go rename to update/release_image.go index 863da1d5f..d97e435ec 100644 --- a/update/release.go +++ b/update/release_image.go @@ -51,7 +51,7 @@ type ReleaseContext interface { // NB: these get sent from fluxctl, so we have to maintain the json format of // this. Eugh. -type ReleaseSpec struct { +type ReleaseImageSpec struct { ServiceSpecs []ResourceSpec ImageSpec ImageSpec Kind ReleaseKind @@ -61,7 +61,7 @@ type ReleaseSpec struct { // ReleaseType gives a one-word description of the release, mainly // useful for labelling metrics or log messages. -func (s ReleaseSpec) ReleaseType() ReleaseType { +func (s ReleaseImageSpec) ReleaseType() ReleaseType { switch { case s.ImageSpec == ImageSpecLatest: return "latest_images" @@ -70,7 +70,7 @@ func (s ReleaseSpec) ReleaseType() ReleaseType { } } -func (s ReleaseSpec) CalculateRelease(rc ReleaseContext, logger log.Logger) ([]*ControllerUpdate, Result, error) { +func (s ReleaseImageSpec) CalculateRelease(rc ReleaseContext, logger log.Logger) ([]*ControllerUpdate, Result, error) { results := Result{} timer := NewStageTimer("select_services") updates, err := s.selectServices(rc, results) @@ -89,11 +89,11 @@ func (s ReleaseSpec) CalculateRelease(rc ReleaseContext, logger log.Logger) ([]* return updates, results, nil } -func (s ReleaseSpec) ReleaseKind() ReleaseKind { +func (s ReleaseImageSpec) ReleaseKind() ReleaseKind { return s.Kind } -func (s ReleaseSpec) CommitMessage(result Result) string { +func (s ReleaseImageSpec) CommitMessage(result Result) string { image := strings.Trim(s.ImageSpec.String(), "<>") var services []string for _, spec := range s.ServiceSpecs { @@ -105,7 +105,7 @@ func (s ReleaseSpec) CommitMessage(result Result) string { // Take the spec given in the job, and figure out which services are // in question based on the running services and those defined in the // repo. Fill in the release results along the way. -func (s ReleaseSpec) selectServices(rc ReleaseContext, results Result) ([]*ControllerUpdate, error) { +func (s ReleaseImageSpec) selectServices(rc ReleaseContext, results Result) ([]*ControllerUpdate, error) { // Build list of filters prefilters, postfilters, err := s.filters(rc) if err != nil { @@ -115,7 +115,7 @@ func (s ReleaseSpec) selectServices(rc ReleaseContext, results Result) ([]*Contr return rc.SelectServices(results, prefilters, postfilters) } -func (s ReleaseSpec) filters(rc ReleaseContext) ([]ControllerFilter, []ControllerFilter, error) { +func (s ReleaseImageSpec) filters(rc ReleaseContext) ([]ControllerFilter, []ControllerFilter, error) { var prefilters, postfilters []ControllerFilter ids := []flux.ResourceID{} @@ -157,7 +157,7 @@ func (s ReleaseSpec) filters(rc ReleaseContext) ([]ControllerFilter, []Controlle return prefilters, postfilters, nil } -func (s ReleaseSpec) markSkipped(results Result) { +func (s ReleaseImageSpec) markSkipped(results Result) { for _, v := range s.ServiceSpecs { if v == ResourceSpecAll { continue @@ -181,7 +181,7 @@ func (s ReleaseSpec) markSkipped(results Result) { // however we do want to see if we *can* do the replacements, because // 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) { +func (s ReleaseImageSpec) calculateImageUpdates(rc ReleaseContext, candidates []*ControllerUpdate, results Result, logger log.Logger) ([]*ControllerUpdate, error) { // Compile an `ImageRepos` of all relevant images var imageRepos ImageRepos var singleRepo image.CanonicalName diff --git a/update/spec.go b/update/spec.go index f1c01f011..a84ef5f20 100644 --- a/update/spec.go +++ b/update/spec.go @@ -49,7 +49,7 @@ func (spec *Spec) UnmarshalJSON(in []byte) error { } spec.Spec = update case Images: - var update ReleaseSpec + var update ReleaseImageSpec if err := json.Unmarshal(wire.SpecBytes, &update); err != nil { return err } @@ -67,7 +67,7 @@ func (spec *Spec) UnmarshalJSON(in []byte) error { } spec.Spec = update case Containers: - var update ContainerSpecs + var update ReleaseContainersSpec if err := json.Unmarshal(wire.SpecBytes, &update); err != nil { return err } From 2b6039702015f3c800cddbf63eadd21bdcb5bbd0 Mon Sep 17 00:00:00 2001 From: Elena Morozova Date: Thu, 25 Oct 2018 16:07:12 -0700 Subject: [PATCH 2/4] Fix tests --- cmd/fluxctl/release_cmd_test.go | 12 ++++---- daemon/daemon_test.go | 11 ++++--- daemon/loop.go | 4 +-- event/event.go | 16 +++++----- event/event_test.go | 9 ++++-- release/releaser_test.go | 52 ++++++++++++++++----------------- 6 files changed, 53 insertions(+), 51 deletions(-) diff --git a/cmd/fluxctl/release_cmd_test.go b/cmd/fluxctl/release_cmd_test.go index b0acd5a0a..015ec893b 100644 --- a/cmd/fluxctl/release_cmd_test.go +++ b/cmd/fluxctl/release_cmd_test.go @@ -12,29 +12,29 @@ import ( func TestReleaseCommand_CLIConversion(t *testing.T) { for _, v := range []struct { args []string - expectedSpec update.ReleaseSpec + expectedSpec update.ReleaseImageSpec }{ - {[]string{"--update-all-images", "--all"}, update.ReleaseSpec{ + {[]string{"--update-all-images", "--all"}, update.ReleaseImageSpec{ ServiceSpecs: []update.ResourceSpec{update.ResourceSpecAll}, ImageSpec: update.ImageSpecLatest, Kind: update.ReleaseKindExecute, }}, - {[]string{"--update-all-images", "--all", "--dry-run"}, update.ReleaseSpec{ + {[]string{"--update-all-images", "--all", "--dry-run"}, update.ReleaseImageSpec{ ServiceSpecs: []update.ResourceSpec{update.ResourceSpecAll}, ImageSpec: update.ImageSpecLatest, Kind: update.ReleaseKindPlan, }}, - {[]string{"--update-image=alpine:latest", "--all"}, update.ReleaseSpec{ + {[]string{"--update-image=alpine:latest", "--all"}, update.ReleaseImageSpec{ ServiceSpecs: []update.ResourceSpec{update.ResourceSpecAll}, ImageSpec: "alpine:latest", Kind: update.ReleaseKindExecute, }}, - {[]string{"--update-all-images", "--controller=deployment/flux"}, update.ReleaseSpec{ + {[]string{"--update-all-images", "--controller=deployment/flux"}, update.ReleaseImageSpec{ ServiceSpecs: []update.ResourceSpec{"default:deployment/flux"}, ImageSpec: update.ImageSpecLatest, Kind: update.ReleaseKindExecute, }}, - {[]string{"--update-all-images", "--all", "--exclude=deployment/test,deployment/yeah"}, update.ReleaseSpec{ + {[]string{"--update-all-images", "--all", "--exclude=deployment/test,deployment/yeah"}, update.ReleaseImageSpec{ ServiceSpecs: []update.ResourceSpec{update.ResourceSpecAll}, ImageSpec: update.ImageSpecLatest, Kind: update.ReleaseKindExecute, diff --git a/daemon/daemon_test.go b/daemon/daemon_test.go index be0d55565..5ea954b49 100644 --- a/daemon/daemon_test.go +++ b/daemon/daemon_test.go @@ -144,7 +144,7 @@ func TestDaemon_ListServicesWithOptions(t *testing.T) { ctx := context.Background() - t.Run("no filter", func (t *testing.T) { + t.Run("no filter", func(t *testing.T) { s, err := d.ListServicesWithOptions(ctx, v11.ListServicesOptions{}) if err != nil { t.Fatalf("Error: %s", err.Error()) @@ -153,7 +153,7 @@ func TestDaemon_ListServicesWithOptions(t *testing.T) { t.Fatalf("Expected %v but got %v", 2, len(s)) } }) - t.Run("filter id", func (t *testing.T) { + t.Run("filter id", func(t *testing.T) { s, err := d.ListServicesWithOptions(ctx, v11.ListServicesOptions{ Namespace: "", Services: []flux.ResourceID{flux.MustParseResourceID(svc)}}) @@ -165,7 +165,7 @@ func TestDaemon_ListServicesWithOptions(t *testing.T) { } }) - t.Run("filter id and namespace", func (t *testing.T) { + t.Run("filter id and namespace", func(t *testing.T) { _, err := d.ListServicesWithOptions(ctx, v11.ListServicesOptions{ Namespace: "foo", Services: []flux.ResourceID{flux.MustParseResourceID(svc)}}) @@ -174,7 +174,7 @@ func TestDaemon_ListServicesWithOptions(t *testing.T) { } }) - t.Run("filter unsupported id kind", func (t *testing.T) { + t.Run("filter unsupported id kind", func(t *testing.T) { _, err := d.ListServicesWithOptions(ctx, v11.ListServicesOptions{ Namespace: "foo", Services: []flux.ResourceID{flux.MustParseResourceID("default:unsupportedkind/goodbyeworld")}}) @@ -184,7 +184,6 @@ func TestDaemon_ListServicesWithOptions(t *testing.T) { }) } - // When I call list images for a service, it should return images func TestDaemon_ListImagesWithOptions(t *testing.T) { d, start, clean, _, _, _ := mockDaemon(t) @@ -848,7 +847,7 @@ func (w *wait) ForImageTag(t *testing.T, d *Daemon, service, container, tag stri func updateImage(ctx context.Context, d *Daemon, t *testing.T) job.ID { return updateManifest(ctx, t, d, update.Spec{ Type: update.Images, - Spec: update.ReleaseSpec{ + Spec: update.ReleaseImageSpec{ Kind: update.ReleaseKindExecute, ServiceSpecs: []update.ResourceSpec{update.ResourceSpecAll}, ImageSpec: newHelloImage, diff --git a/daemon/loop.go b/daemon/loop.go index e5f69efd5..daaf02851 100644 --- a/daemon/loop.go +++ b/daemon/loop.go @@ -321,8 +321,8 @@ func (d *Daemon) doSync(logger log.Logger) (retErr error) { Error: n.Result.Error(), }, Spec: event.ReleaseSpec{ - Type: event.ReleaseContainerSpecType, - ReleaseContainerSpec: &spec, + Type: event.ReleaseContainersSpecType, + ReleaseContainersSpec: &spec, }, Cause: n.Spec.Cause, }, diff --git a/event/event.go b/event/event.go index 281f8004b..6521e56e4 100644 --- a/event/event.go +++ b/event/event.go @@ -95,7 +95,7 @@ func (e Event) String() string { if len(strImageIDs) == 0 { strImageIDs = []string{"no image changes"} } - if metadata.Spec.Type == ReleaseImageSpecType { + if metadata.Spec.Type == "" || metadata.Spec.Type == ReleaseImageSpecType { for _, spec := range metadata.Spec.ReleaseImageSpec.ServiceSpecs { if spec == update.ResourceSpecAll { strServiceIDs = []string{"all services"} @@ -244,17 +244,17 @@ type ReleaseEventCommon struct { const ( // ReleaseImageSpecType is a type of release spec when there are update.Images ReleaseImageSpecType = "releaseImageSpecType" - // ReleaseContainerSpecType is a type of release spec when there are update.Containers - ReleaseContainerSpecType = "releaseContainerSpecType" + // ReleaseContainersSpecType is a type of release spec when there are update.Containers + ReleaseContainersSpecType = "releaseContainersSpecType" ) // ReleaseSpec is a spec for images and containers release type ReleaseSpec struct { - // Type is ReleaseImagesSpecType or ReleaseContainersSpecType - // if empty (for previous version), then use ReleaseImagesSpecType - Type string - ReleaseImageSpec *update.ReleaseImageSpec - ReleaseContainerSpec *update.ReleaseContainersSpec + // Type is ReleaseImageSpecType or ReleaseContainersSpecType + // if empty (for previous version), then use ReleaseImageSpecType + Type string + ReleaseImageSpec *update.ReleaseImageSpec + ReleaseContainersSpec *update.ReleaseContainersSpec } // ReleaseEventMetadata is the metadata for when service(s) are released diff --git a/event/event_test.go b/event/event_test.go index f6737e157..626ecd93c 100644 --- a/event/event_test.go +++ b/event/event_test.go @@ -8,7 +8,7 @@ import ( ) var ( - spec = update.ReleaseSpec{ + spec = update.ReleaseImageSpec{ ImageSpec: update.ImageSpecLatest, } cause = update.Cause{ @@ -22,7 +22,10 @@ func TestEvent_ParseReleaseMetaData(t *testing.T) { Type: EventRelease, Metadata: &ReleaseEventMetadata{ Cause: cause, - Spec: spec, + Spec: ReleaseSpec{ + Type: ReleaseImageSpecType, + ReleaseImageSpec: &spec, + }, }, } @@ -35,7 +38,7 @@ func TestEvent_ParseReleaseMetaData(t *testing.T) { } switch r := e.Metadata.(type) { case *ReleaseEventMetadata: - if r.Spec.ImageSpec != spec.ImageSpec || + if r.Spec.ReleaseImageSpec.ImageSpec != spec.ImageSpec || r.Cause != cause { t.Fatal("Release event wasn't marshalled/unmarshalled") } diff --git a/release/releaser_test.go b/release/releaser_test.go index 449d1ba60..faa9e7520 100644 --- a/release/releaser_test.go +++ b/release/releaser_test.go @@ -243,7 +243,7 @@ func Test_InitContainer(t *testing.T) { } initSpec, _ := update.ParseResourceSpec(initWorkloadID.String()) - spec := update.ReleaseSpec{ + spec := update.ReleaseImageSpec{ ServiceSpecs: []update.ResourceSpec{initSpec}, ImageSpec: update.ImageSpecLatest, Kind: update.ReleaseKindExecute, @@ -268,13 +268,13 @@ func Test_FilterLogic(t *testing.T) { notInRepoSpec, _ := update.ParseResourceSpec(notInRepoService) for _, tst := range []struct { Name string - Spec update.ReleaseSpec + Spec update.ReleaseImageSpec Expected expected }{ // ignored if: excluded OR not included OR not correct image. { Name: "include specific service", - Spec: update.ReleaseSpec{ + Spec: update.ReleaseImageSpec{ ServiceSpecs: []update.ResourceSpec{hwSvcSpec}, ImageSpec: update.ImageSpecLatest, Kind: update.ReleaseKindExecute, @@ -302,7 +302,7 @@ func Test_FilterLogic(t *testing.T) { }, }, { Name: "exclude specific service", - Spec: update.ReleaseSpec{ + Spec: update.ReleaseImageSpec{ ServiceSpecs: []update.ResourceSpec{update.ResourceSpecAll}, ImageSpec: update.ImageSpecLatest, Kind: update.ReleaseKindExecute, @@ -334,7 +334,7 @@ func Test_FilterLogic(t *testing.T) { }, }, { Name: "update specific image", - Spec: update.ReleaseSpec{ + Spec: update.ReleaseImageSpec{ ServiceSpecs: []update.ResourceSpec{update.ResourceSpecAll}, ImageSpec: update.ImageSpecFromRef(newHwRef), Kind: update.ReleaseKindExecute, @@ -363,7 +363,7 @@ func Test_FilterLogic(t *testing.T) { // skipped if: not ignored AND (locked or not found in cluster) // else: service is pending. Name: "skipped & service is pending", - Spec: update.ReleaseSpec{ + Spec: update.ReleaseImageSpec{ ServiceSpecs: []update.ResourceSpec{update.ResourceSpecAll}, ImageSpec: update.ImageSpecLatest, Kind: update.ReleaseKindExecute, @@ -395,7 +395,7 @@ func Test_FilterLogic(t *testing.T) { }, }, { Name: "all overrides spec", - Spec: update.ReleaseSpec{ + Spec: update.ReleaseImageSpec{ ServiceSpecs: []update.ResourceSpec{hwSvcSpec, update.ResourceSpecAll}, ImageSpec: update.ImageSpecLatest, Kind: update.ReleaseKindExecute, @@ -427,7 +427,7 @@ func Test_FilterLogic(t *testing.T) { }, }, { Name: "service not in repo", - Spec: update.ReleaseSpec{ + Spec: update.ReleaseImageSpec{ ServiceSpecs: []update.ResourceSpec{notInRepoSpec}, ImageSpec: update.ImageSpecLatest, Kind: update.ReleaseKindExecute, @@ -468,12 +468,12 @@ func Test_Force_lockedController(t *testing.T) { } for _, tst := range []struct { Name string - Spec update.ReleaseSpec + Spec update.ReleaseImageSpec Expected expected }{ { Name: "force ignores service lock (--controller --update-image)", - Spec: update.ReleaseSpec{ + Spec: update.ReleaseImageSpec{ ServiceSpecs: []update.ResourceSpec{lockedSvcSpec}, ImageSpec: update.ImageSpecFromRef(newLockedRef), Kind: update.ReleaseKindExecute, @@ -488,7 +488,7 @@ func Test_Force_lockedController(t *testing.T) { }, }, { Name: "force does not ignore lock if updating all controllers (--all --update-image)", - Spec: update.ReleaseSpec{ + Spec: update.ReleaseImageSpec{ ServiceSpecs: []update.ResourceSpec{update.ResourceSpecAll}, ImageSpec: update.ImageSpecFromRef(newLockedRef), Kind: update.ReleaseKindExecute, @@ -504,7 +504,7 @@ func Test_Force_lockedController(t *testing.T) { }, { Name: "force ignores service lock (--controller --update-all-images)", - Spec: update.ReleaseSpec{ + Spec: update.ReleaseImageSpec{ ServiceSpecs: []update.ResourceSpec{lockedSvcSpec}, ImageSpec: update.ImageSpecLatest, Kind: update.ReleaseKindExecute, @@ -519,7 +519,7 @@ func Test_Force_lockedController(t *testing.T) { }, }, { Name: "force does not ignore lock if updating all controllers (--all --update-all-images)", - Spec: update.ReleaseSpec{ + Spec: update.ReleaseImageSpec{ ServiceSpecs: []update.ResourceSpec{update.ResourceSpecAll}, ImageSpec: update.ImageSpecLatest, Kind: update.ReleaseKindExecute, @@ -571,12 +571,12 @@ func Test_Force_filteredContainer(t *testing.T) { } for _, tst := range []struct { Name string - Spec update.ReleaseSpec + Spec update.ReleaseImageSpec Expected expected }{ { Name: "force ignores container tag pattern (--controller --update-image)", - Spec: update.ReleaseSpec{ + Spec: update.ReleaseImageSpec{ ServiceSpecs: []update.ResourceSpec{semverSvcSpec}, ImageSpec: update.ImageSpecFromRef(newHwRef), // does not match filter Kind: update.ReleaseKindExecute, @@ -592,7 +592,7 @@ func Test_Force_filteredContainer(t *testing.T) { }, { Name: "force ignores container tag pattern (--all --update-image)", - Spec: update.ReleaseSpec{ + Spec: update.ReleaseImageSpec{ ServiceSpecs: []update.ResourceSpec{update.ResourceSpecAll}, ImageSpec: update.ImageSpecFromRef(newHwRef), // does not match filter Kind: update.ReleaseKindExecute, @@ -608,7 +608,7 @@ func Test_Force_filteredContainer(t *testing.T) { }, { Name: "force complies with semver when updating all images (--controller --update-all-image)", - Spec: update.ReleaseSpec{ + Spec: update.ReleaseImageSpec{ ServiceSpecs: []update.ResourceSpec{semverSvcSpec}, ImageSpec: update.ImageSpecLatest, // will filter images by semver and pick newest version Kind: update.ReleaseKindExecute, @@ -624,7 +624,7 @@ func Test_Force_filteredContainer(t *testing.T) { }, { Name: "force complies with semver when updating all images (--all --update-all-image)", - Spec: update.ReleaseSpec{ + Spec: update.ReleaseImageSpec{ ServiceSpecs: []update.ResourceSpec{update.ResourceSpecAll}, ImageSpec: update.ImageSpecLatest, Kind: update.ReleaseKindExecute, @@ -670,12 +670,12 @@ func Test_ImageStatus(t *testing.T) { testSvcSpec, _ := update.ParseResourceSpec(testSvc.ID.String()) for _, tst := range []struct { Name string - Spec update.ReleaseSpec + Spec update.ReleaseImageSpec Expected expected }{ { Name: "image not found", - Spec: update.ReleaseSpec{ + Spec: update.ReleaseImageSpec{ ServiceSpecs: []update.ResourceSpec{testSvcSpec}, ImageSpec: update.ImageSpecLatest, Kind: update.ReleaseKindExecute, @@ -692,7 +692,7 @@ func Test_ImageStatus(t *testing.T) { }, }, { Name: "image up to date", - Spec: update.ReleaseSpec{ + Spec: update.ReleaseImageSpec{ ServiceSpecs: []update.ResourceSpec{hwSvcSpec}, ImageSpec: update.ImageSpecLatest, Kind: update.ReleaseKindExecute, @@ -746,7 +746,7 @@ func Test_UpdateMultidoc(t *testing.T) { repo: checkout, registry: mockRegistry, } - spec := update.ReleaseSpec{ + spec := update.ReleaseImageSpec{ ServiceSpecs: []update.ResourceSpec{"default:deployment/multi-deploy"}, ImageSpec: update.ImageSpecLatest, Kind: update.ReleaseKindExecute, @@ -794,7 +794,7 @@ func Test_UpdateList(t *testing.T) { repo: checkout, registry: mockRegistry, } - spec := update.ReleaseSpec{ + spec := update.ReleaseImageSpec{ ServiceSpecs: []update.ResourceSpec{"default:deployment/list-deploy"}, ImageSpec: update.ImageSpecLatest, Kind: update.ReleaseKindExecute, @@ -1014,7 +1014,7 @@ func Test_UpdateContainers(t *testing.T) { }, }, } { - specs := update.ContainerSpecs{ + specs := update.ReleaseContainersSpec{ ContainerSpecs: map[flux.ResourceID][]update.ContainerUpdate{tst.WorkloadID: tst.Spec}, Kind: update.ReleaseKindExecute, } @@ -1040,7 +1040,7 @@ func Test_UpdateContainers(t *testing.T) { } } -func testRelease(t *testing.T, ctx *ReleaseContext, spec update.ReleaseSpec, expected update.Result) { +func testRelease(t *testing.T, ctx *ReleaseContext, spec update.ReleaseImageSpec, expected update.Result) { results, err := Release(ctx, spec, log.NewNopLogger()) assert.NoError(t, err) assert.Equal(t, expected, results) @@ -1059,7 +1059,7 @@ func (m *badManifests) UpdateImage(def []byte, resourceID flux.ResourceID, conta func Test_BadRelease(t *testing.T) { cluster := mockCluster(hwSvc) - spec := update.ReleaseSpec{ + spec := update.ReleaseImageSpec{ ServiceSpecs: []update.ResourceSpec{update.ResourceSpecAll}, ImageSpec: update.ImageSpecFromRef(newHwRef), Kind: update.ReleaseKindExecute, From 641d52d5cadcc15bbf075cca74bba9ef8ad3e26f Mon Sep 17 00:00:00 2001 From: Elena Morozova Date: Thu, 1 Nov 2018 15:35:00 -0700 Subject: [PATCH 3/4] Add UnmarshalJSON method to support old version of spec --- event/event.go | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/event/event.go b/event/event.go index 6521e56e4..d675aa311 100644 --- a/event/event.go +++ b/event/event.go @@ -257,6 +257,31 @@ type ReleaseSpec struct { ReleaseContainersSpec *update.ReleaseContainersSpec } +// UnmarshalJSON for old version of spec (update.ReleaseImageSpec) where Type is empty +func (s *ReleaseSpec) UnmarshalJSON(b []byte) error { + type T ReleaseSpec + t := (*T)(s) + if err := json.Unmarshal(b, t); err != nil { + return err + } + + switch t.Type { + case "": + r := &update.ReleaseImageSpec{} + if err := json.Unmarshal(b, r); err != nil { + return err + } + s.Type = ReleaseImageSpecType + s.ReleaseImageSpec = r + + case ReleaseImageSpecType, ReleaseContainersSpecType: + // all good + default: + return errors.New("unknown ReleaseSpec type") + } + return nil +} + // ReleaseEventMetadata is the metadata for when service(s) are released type ReleaseEventMetadata struct { ReleaseEventCommon From 0489573959c71de2d5029748602963655a05bb48 Mon Sep 17 00:00:00 2001 From: Michael Bridgen Date: Thu, 8 Nov 2018 12:40:16 +0000 Subject: [PATCH 4/4] Add test for parsing old event format --- event/event_test.go | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/event/event_test.go b/event/event_test.go index 626ecd93c..86e3188e4 100644 --- a/event/event_test.go +++ b/event/event_test.go @@ -63,3 +63,35 @@ func TestEvent_ParseNoMetadata(t *testing.T) { t.Fatal("Hasn't been unmarshalled properly") } } + +// TestEvent_ParseOldReleaseMetaData makes sure the parsing code can +// handle the older format events recorded against commits. +func TestEvent_ParseOldReleaseMetaData(t *testing.T) { + // A minimal example of an old-style ReleaseEventMetadata. NB it + // must have at least an entry for "spec", since otherwise the + // JSON unmarshaller will not attempt to unparse the spec and + // thereby invoke the specialised UnmarshalJSON. + oldData := ` +{ + "spec": { + "serviceSpecs": [""] + } +} +` + var eventData ReleaseEventMetadata + if err := json.Unmarshal([]byte(oldData), &eventData); err != nil { + t.Fatal(err) + } + if eventData.Spec.Type != ReleaseImageSpecType { + t.Error("did not set spec type to ReleaseImageSpecType") + } + if eventData.Spec.ReleaseImageSpec == nil { + t.Error("did not set .ReleaseImageSpec as expected") + } + if eventData.Spec.ReleaseContainersSpec != nil { + t.Error("unexpectedly set .ReleaseContainersSpec") + } + if len(eventData.Spec.ReleaseImageSpec.ServiceSpecs) != 1 { + t.Error("expected service specs of len 1") + } +}