From f372165f699a0d9b930922fb910c358885e50435 Mon Sep 17 00:00:00 2001 From: Michael Bridgen Date: Mon, 17 Dec 2018 12:57:50 +0000 Subject: [PATCH 1/2] Move the sync tag only when it's changed Upstream git hosts (e.g., GitHub) can treat each tag push as a fresh event for the purpose of their own notifications; and since fluxd pushes the sync tag, every sync, unconditionally, this means there are many many events. To avoid most of these spurious events, we can push the tag only when it looks like it's changed. That is, when the HEAD of the branch points at a different revision to the sync tag. This was already the condition used to guard _logging_ the sync event. You might question whether it's safe to push the tag conditionally -- could it result in an invalid state? - in a correct configuration, the sync loop in a single fluxd will be the only process moving the tag -- so the view in that process is the only one that matters; - in an _incorrect_ configuration (e.g., more than one fluxd using the same sync tag), we might fail to push the tag if some other process has already shifted it to the revision we're looking at. In that case, we wouldn't want to move it anyway. (And if someone has shifted the tag to a different revision, well we would have pushed it before ..) --- daemon/loop.go | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/daemon/loop.go b/daemon/loop.go index daaf02851..386baebd7 100644 --- a/daemon/loop.go +++ b/daemon/loop.go @@ -321,7 +321,7 @@ func (d *Daemon) doSync(logger log.Logger) (retErr error) { Error: n.Result.Error(), }, Spec: event.ReleaseSpec{ - Type: event.ReleaseContainersSpecType, + Type: event.ReleaseContainersSpecType, ReleaseContainersSpec: &spec, }, Cause: n.Spec.Cause, @@ -411,23 +411,23 @@ func (d *Daemon) doSync(logger log.Logger) (retErr error) { } // Move the tag and push it so we know how far we've gotten. - { - ctx, cancel := context.WithTimeout(ctx, gitOpTimeout) - err := working.MoveSyncTagAndPush(ctx, newTagRev, "Sync pointer") - cancel() - if err != nil { - return err - } - } - if oldTagRev != newTagRev { + { + ctx, cancel := context.WithTimeout(ctx, gitOpTimeout) + err := working.MoveSyncTagAndPush(ctx, newTagRev, "Sync pointer") + cancel() + if err != nil { + return err + } + } logger.Log("tag", d.GitConfig.SyncTag, "old", oldTagRev, "new", newTagRev) - ctx, cancel := context.WithTimeout(ctx, gitOpTimeout) - err := d.Repo.Refresh(ctx) - cancel() - return err + { + ctx, cancel := context.WithTimeout(ctx, gitOpTimeout) + err := d.Repo.Refresh(ctx) + cancel() + return err + } } - return nil } From 3c903fa289bee0a9db9c6f16f1d1dc3e91b9ce2c Mon Sep 17 00:00:00 2001 From: Michael Bridgen Date: Mon, 17 Dec 2018 13:28:20 +0000 Subject: [PATCH 2/2] Reindent test file --- daemon/daemon_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/daemon/daemon_test.go b/daemon/daemon_test.go index 5ea954b49..6cdaca4b6 100644 --- a/daemon/daemon_test.go +++ b/daemon/daemon_test.go @@ -290,7 +290,7 @@ func TestDaemon_ListImagesWithOptions(t *testing.T) { { name: "Override container field selection", opts: v10.ListImagesOptions{ - Spec: specAll, + Spec: specAll, OverrideContainerFields: []string{"Name", "Current", "NewAvailableImagesCount"}, }, expectedImages: []v6.ImageStatus{ @@ -320,7 +320,7 @@ func TestDaemon_ListImagesWithOptions(t *testing.T) { { name: "Override container field selection with invalid field", opts: v10.ListImagesOptions{ - Spec: specAll, + Spec: specAll, OverrideContainerFields: []string{"InvalidField"}, }, expectedImages: nil,