From 3b79692f2eb49206684a6387c3cfb6f175034e07 Mon Sep 17 00:00:00 2001 From: Alfonso Acosta Date: Wed, 30 Jan 2019 15:35:11 +0100 Subject: [PATCH 1/3] Warn users about external changes in sync tag Multiple users have reported an unexpectedly high number of changes in the git respository's sync tag. This has been attributed to multiple fluxd instances, running in separate clusters, using the same git repository and sync tag. In addition, multiple fluxd instances using the same tag is a bad idea in general, since it will lead to commits being missed by fluxd. This change tracks the value of the sync tag and prints a warning when detecting an external change. --- daemon/daemon.go | 24 +++++++++++++----------- daemon/loop.go | 9 +++++++++ 2 files changed, 22 insertions(+), 11 deletions(-) diff --git a/daemon/daemon.go b/daemon/daemon.go index 4884a11bf..7723cf980 100644 --- a/daemon/daemon.go +++ b/daemon/daemon.go @@ -42,17 +42,19 @@ const ( // Daemon is the fully-functional state of a daemon (compare to // `NotReadyDaemon`). type Daemon struct { - V string - Cluster cluster.Cluster - Manifests cluster.Manifests - Registry registry.Registry - ImageRefresh chan image.Name - Repo *git.Repo - GitConfig git.Config - Jobs *job.Queue - JobStatusCache *job.StatusCache - EventWriter event.EventWriter - Logger log.Logger + V string + Cluster cluster.Cluster + Manifests cluster.Manifests + Registry registry.Registry + ImageRefresh chan image.Name + Repo *git.Repo + GitConfig git.Config + lastKnownSyncTagRev string + warnedAboutSyncTagChange bool + Jobs *job.Queue + JobStatusCache *job.StatusCache + EventWriter event.EventWriter + Logger log.Logger // bookkeeping *LoopVars } diff --git a/daemon/loop.go b/daemon/loop.go index 386baebd7..564033cdf 100644 --- a/daemon/loop.go +++ b/daemon/loop.go @@ -179,6 +179,14 @@ func (d *Daemon) doSync(logger log.Logger) (retErr error) { if err != nil && !isUnknownRevision(err) { return err } + // Check if something other than the current instance of fluxd changed the sync tag. + // This is likely to be caused by another fluxd instance using the same tag. + // Having multiple instances fighting for the same tag can lead to fluxd missing manifest changes. + if d.lastKnownSyncTagRev != "" && oldTagRev != d.lastKnownSyncTagRev && !d.warnedAboutSyncTagChange { + logger.Log("warning", + "detected external change in git sync tag; the sync tag should not be shared by fluxd instances") + d.warnedAboutSyncTagChange = true + } newTagRev, err := working.HeadRevision(ctx) if err != nil { @@ -419,6 +427,7 @@ func (d *Daemon) doSync(logger log.Logger) (retErr error) { if err != nil { return err } + d.lastKnownSyncTagRev = newTagRev } logger.Log("tag", d.GitConfig.SyncTag, "old", oldTagRev, "new", newTagRev) { From 9635f3cacdabaa395d6b59ded99b540a1e103b4a Mon Sep 17 00:00:00 2001 From: Alfonso Acosta Date: Thu, 31 Jan 2019 00:00:05 +0100 Subject: [PATCH 2/3] Use variables instead of members --- daemon/daemon.go | 24 +++++++++++------------- daemon/loop.go | 13 ++++++++----- 2 files changed, 19 insertions(+), 18 deletions(-) diff --git a/daemon/daemon.go b/daemon/daemon.go index 7723cf980..4884a11bf 100644 --- a/daemon/daemon.go +++ b/daemon/daemon.go @@ -42,19 +42,17 @@ const ( // Daemon is the fully-functional state of a daemon (compare to // `NotReadyDaemon`). type Daemon struct { - V string - Cluster cluster.Cluster - Manifests cluster.Manifests - Registry registry.Registry - ImageRefresh chan image.Name - Repo *git.Repo - GitConfig git.Config - lastKnownSyncTagRev string - warnedAboutSyncTagChange bool - Jobs *job.Queue - JobStatusCache *job.StatusCache - EventWriter event.EventWriter - Logger log.Logger + V string + Cluster cluster.Cluster + Manifests cluster.Manifests + Registry registry.Registry + ImageRefresh chan image.Name + Repo *git.Repo + GitConfig git.Config + Jobs *job.Queue + JobStatusCache *job.StatusCache + EventWriter event.EventWriter + Logger log.Logger // bookkeeping *LoopVars } diff --git a/daemon/loop.go b/daemon/loop.go index 564033cdf..4a057dbde 100644 --- a/daemon/loop.go +++ b/daemon/loop.go @@ -60,7 +60,10 @@ func (d *Daemon) Loop(stop chan struct{}, wg *sync.WaitGroup, logger log.Logger) // Ask for a sync, and to poll images, straight away d.AskForSync() d.AskForImagePoll() + for { + var lastKnownSyncTagRev string + var warnedAboutSyncTagChange bool select { case <-stop: logger.Log("stopping", "true") @@ -83,7 +86,7 @@ func (d *Daemon) Loop(stop chan struct{}, wg *sync.WaitGroup, logger log.Logger) default: } } - if err := d.doSync(logger); err != nil { + if err := d.doSync(logger, &lastKnownSyncTagRev, &warnedAboutSyncTagChange); err != nil { logger.Log("err", err) } syncTimer.Reset(d.SyncInterval) @@ -149,7 +152,7 @@ func (d *LoopVars) AskForImagePoll() { // -- extra bits the loop needs -func (d *Daemon) doSync(logger log.Logger) (retErr error) { +func (d *Daemon) doSync(logger log.Logger, lastKnownSyncTagRev *string, warnedAboutSyncTagChange *bool) (retErr error) { started := time.Now().UTC() defer func() { syncDuration.With( @@ -182,10 +185,10 @@ func (d *Daemon) doSync(logger log.Logger) (retErr error) { // Check if something other than the current instance of fluxd changed the sync tag. // This is likely to be caused by another fluxd instance using the same tag. // Having multiple instances fighting for the same tag can lead to fluxd missing manifest changes. - if d.lastKnownSyncTagRev != "" && oldTagRev != d.lastKnownSyncTagRev && !d.warnedAboutSyncTagChange { + if *lastKnownSyncTagRev != "" && oldTagRev != *lastKnownSyncTagRev && !*warnedAboutSyncTagChange { logger.Log("warning", "detected external change in git sync tag; the sync tag should not be shared by fluxd instances") - d.warnedAboutSyncTagChange = true + *warnedAboutSyncTagChange = true } newTagRev, err := working.HeadRevision(ctx) @@ -427,7 +430,7 @@ func (d *Daemon) doSync(logger log.Logger) (retErr error) { if err != nil { return err } - d.lastKnownSyncTagRev = newTagRev + *lastKnownSyncTagRev = newTagRev } logger.Log("tag", d.GitConfig.SyncTag, "old", oldTagRev, "new", newTagRev) { From 52235cc51735a07a3201379dd7909b489123c84b Mon Sep 17 00:00:00 2001 From: Alfonso Acosta Date: Fri, 1 Feb 2019 11:47:38 +0100 Subject: [PATCH 3/3] Fix tests --- daemon/loop.go | 6 ++++-- daemon/loop_test.go | 24 ++++++++++++++++++------ 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/daemon/loop.go b/daemon/loop.go index 4a057dbde..dd0bc8823 100644 --- a/daemon/loop.go +++ b/daemon/loop.go @@ -62,8 +62,10 @@ func (d *Daemon) Loop(stop chan struct{}, wg *sync.WaitGroup, logger log.Logger) d.AskForImagePoll() for { - var lastKnownSyncTagRev string - var warnedAboutSyncTagChange bool + var ( + lastKnownSyncTagRev string + warnedAboutSyncTagChange bool + ) select { case <-stop: logger.Log("stopping", "true") diff --git a/daemon/loop_test.go b/daemon/loop_test.go index 58db17078..547928a43 100644 --- a/daemon/loop_test.go +++ b/daemon/loop_test.go @@ -105,8 +105,12 @@ func TestPullAndSync_InitialSync(t *testing.T) { syncDef = &def return nil } - - d.doSync(log.NewLogfmtLogger(ioutil.Discard)) + var ( + logger = log.NewLogfmtLogger(ioutil.Discard) + lastKnownSyncTagRev string + warnedAboutSyncTagChange bool + ) + d.doSync(logger, &lastKnownSyncTagRev, &warnedAboutSyncTagChange) // It applies everything if syncCalled != 1 { @@ -174,8 +178,12 @@ func TestDoSync_NoNewCommits(t *testing.T) { syncDef = &def return nil } - - if err := d.doSync(log.NewLogfmtLogger(ioutil.Discard)); err != nil { + var ( + logger = log.NewLogfmtLogger(ioutil.Discard) + lastKnownSyncTagRev string + warnedAboutSyncTagChange bool + ) + if err := d.doSync(logger, &lastKnownSyncTagRev, &warnedAboutSyncTagChange); err != nil { t.Error(err) } @@ -268,8 +276,12 @@ func TestDoSync_WithNewCommit(t *testing.T) { syncDef = &def return nil } - - d.doSync(log.NewLogfmtLogger(ioutil.Discard)) + var ( + logger = log.NewLogfmtLogger(ioutil.Discard) + lastKnownSyncTagRev string + warnedAboutSyncTagChange bool + ) + d.doSync(logger, &lastKnownSyncTagRev, &warnedAboutSyncTagChange) // It applies everything if syncCalled != 1 {