Let syncing work without a writable clone

In principle it is only necessary to have a read-only clone of the git
repo in order to do a sync. Most of the needed methods are on clone,
though, by historical accident. So actually making this the case takes
a bit of shuffling.

daemon/daemon.go

@@ -76,31 +76,21 @@ func (d *Daemon) Export(ctx context.Context) ([]byte, error) {
 
 type repo interface {
 	Dir() string
-	AbsolutePaths() []string
-}
-
-type exportRepo struct {
-	*git.Export
-	paths []string
-}
-
-func (r exportRepo) AbsolutePaths() []string {
-	return git.MakeAbsolutePaths(r, r.paths)
 }
 
 func (d *Daemon) getManifestStore(r repo) (manifests.Store, error) {
+	absPaths := git.MakeAbsolutePaths(r, d.GitConfig.Paths)
 	if d.ManifestGenerationEnabled {
-		return manifests.NewConfigAware(r.Dir(), r.AbsolutePaths(), d.Manifests)
+		return manifests.NewConfigAware(r.Dir(), absPaths, d.Manifests)
 	}
-	return manifests.NewRawFiles(r.Dir(), r.AbsolutePaths(), d.Manifests), nil
+	return manifests.NewRawFiles(r.Dir(), absPaths, d.Manifests), nil
 }
 
 func (d *Daemon) getResources(ctx context.Context) (map[string]resource.Resource, v6.ReadOnlyReason, error) {
 	var resources map[string]resource.Resource
 	var globalReadOnly v6.ReadOnlyReason
 	err := d.WithReadonlyClone(ctx, func(checkout *git.Export) error {
-		r := exportRepo{checkout, d.GitConfig.Paths}
-		cm, err := d.getManifestStore(r)
+		cm, err := d.getManifestStore(checkout)
 		if err != nil {
 			return err
 		}
@@ -580,37 +570,33 @@ func (d *Daemon) JobStatus(ctx context.Context, jobID job.ID) (job.Status, error
 	// Look through the commits for a note referencing this job.  This
 	// means that even if fluxd restarts, we will at least remember
 	// jobs which have pushed a commit.
-	// FIXME(michael): consider looking at the repo for this, since read op
-	err := d.WithWorkingClone(ctx, func(working *git.Checkout) error {
-		notes, err := working.NoteRevList(ctx)
-		if err != nil {
-			return errors.Wrap(err, "enumerating commit notes")
-		}
-		commits, err := d.Repo.CommitsBefore(ctx, "HEAD", d.GitConfig.Paths...)
-		if err != nil {
-			return errors.Wrap(err, "checking revisions for status")
-		}
-
-		for _, commit := range commits {
-			if _, ok := notes[commit.Revision]; ok {
-				var n note
-				ok, err := working.GetNote(ctx, commit.Revision, &n)
-				if ok && err == nil && n.JobID == jobID {
-					status = job.Status{
-						StatusString: job.StatusSucceeded,
-						Result: job.Result{
-							Revision: commit.Revision,
-							Spec:     &n.Spec,
-							Result:   n.Result,
-						},
-					}
-					return nil
+	notes, err := d.Repo.NoteRevList(ctx, d.GitConfig.NotesRef)
+	if err != nil {
+		return status, errors.Wrap(err, "enumerating commit notes")
+	}
+	commits, err := d.Repo.CommitsBefore(ctx, "HEAD", d.GitConfig.Paths...)
+	if err != nil {
+		return status, errors.Wrap(err, "checking revisions for status")
+	}
+
+	for _, commit := range commits {
+		if _, ok := notes[commit.Revision]; ok {
+			var n note
+			ok, err := d.Repo.GetNote(ctx, commit.Revision, d.GitConfig.NotesRef, &n)
+			if ok && err == nil && n.JobID == jobID {
+				status = job.Status{
+					StatusString: job.StatusSucceeded,
+					Result: job.Result{
+						Revision: commit.Revision,
+						Spec:     &n.Spec,
+						Result:   n.Result,
+					},
 				}
+				return status, nil
 			}
 		}
-		return unknownJobError(jobID)
-	})
-	return status, err
+	}
+	return status, unknownJobError(jobID)
 }
 
 // Ask the daemon how far it's got applying things; in particular, is it

daemon/sync.go

@@ -40,20 +40,15 @@ type changeSet struct {
 
 // Sync starts the synchronization of the cluster with git.
 func (d *Daemon) Sync(ctx context.Context, started time.Time, newRevision string, ratchet revisionRatchet) error {
-	// Checkout a working clone used for this sync
+	// Make a read-only clone used for this sync
 	ctxt, cancel := context.WithTimeout(ctx, d.GitTimeout)
-	working, err := d.Repo.Clone(ctxt, d.GitConfig)
+	working, err := d.Repo.Export(ctxt, newRevision)
 	if err != nil {
 		return err
 	}
 	cancel()
 	defer working.Clean()
 
-	// Ensure we are syncing the given revision
-	if err := working.Checkout(ctx, newRevision); err != nil {
-		return err
-	}
-
 	// Retrieve change set of commits we need to sync
 	c, err := getChangeSet(ctx, ratchet, newRevision, d.Repo, d.GitTimeout, d.GitConfig.Paths)
 	if err != nil {
@@ -72,18 +67,18 @@ func (d *Daemon) Sync(ctx context.Context, started time.Time, newRevision string
 	}
 
 	// Determine what resources changed during the sync
-	changedResources, err := getChangedResources(ctx, c, d.GitTimeout, working, resourceStore, resources)
+	changedResources, err := d.getChangedResources(ctx, c, d.GitTimeout, working, resourceStore, resources)
 	serviceIDs := resource.IDSet{}
 	for _, r := range changedResources {
 		serviceIDs.Add([]resource.ID{r.ResourceID()})
 	}
 
 	// Retrieve git notes and collect events from them
-	notes, err := getNotes(ctx, d.GitTimeout, working)
+	notes, err := d.getNotes(ctx, d.GitTimeout)
 	if err != nil {
 		return err
 	}
-	noteEvents, includesEvents, err := collectNoteEvents(ctx, c, notes, d.GitTimeout, working, started, d.Logger)
+	noteEvents, includesEvents, err := d.collectNoteEvents(ctx, c, notes, d.GitTimeout, started, d.Logger)
 	if err != nil {
 		return err
 	}
@@ -169,15 +164,15 @@ func doSync(ctx context.Context, manifestsStore manifests.Store, clus cluster.Cl
 
 // getChangedResources calculates what resources are modified during
 // this sync.
-func getChangedResources(ctx context.Context, c changeSet, timeout time.Duration, working *git.Checkout,
+func (d *Daemon) getChangedResources(ctx context.Context, c changeSet, timeout time.Duration, working *git.Export,
 	manifestsStore manifests.Store, resources map[string]resource.Resource) (map[string]resource.Resource, error) {
 	if c.initialSync {
 		return resources, nil
 	}
 
 	errorf := func(err error) error { return errors.Wrap(err, "loading resources from repo") }
 	ctx, cancel := context.WithTimeout(ctx, timeout)
-	changedFiles, err := working.ChangedFiles(ctx, c.oldTagRev)
+	changedFiles, err := working.ChangedFiles(ctx, c.oldTagRev, d.GitConfig.Paths)
 	if err != nil {
 		return nil, errorf(err)
 	}
@@ -216,9 +211,9 @@ func getChangedResources(ctx context.Context, c changeSet, timeout time.Duration
 }
 
 // getNotes retrieves the git notes from the working clone.
-func getNotes(ctx context.Context, timeout time.Duration, working *git.Checkout) (map[string]struct{}, error) {
+func (d *Daemon) getNotes(ctx context.Context, timeout time.Duration) (map[string]struct{}, error) {
 	ctx, cancel := context.WithTimeout(ctx, timeout)
-	notes, err := working.NoteRevList(ctx)
+	notes, err := d.Repo.NoteRevList(ctx, d.GitConfig.NotesRef)
 	cancel()
 	if err != nil {
 		return nil, errors.Wrap(err, "loading notes from repo")
@@ -231,8 +226,8 @@ func getNotes(ctx context.Context, timeout time.Duration, working *git.Checkout)
 // of what other things this sync includes e.g., releases and
 // autoreleases, that we're already posting as events, so upstream
 // can skip the sync event if it wants to.
-func collectNoteEvents(ctx context.Context, c changeSet, notes map[string]struct{}, timeout time.Duration,
-	working *git.Checkout, started time.Time, logger log.Logger) ([]event.Event, map[string]bool, error) {
+func (d *Daemon) collectNoteEvents(ctx context.Context, c changeSet, notes map[string]struct{}, timeout time.Duration,
+	started time.Time, logger log.Logger) ([]event.Event, map[string]bool, error) {
 	if len(c.commits) == 0 {
 		return nil, nil, nil
 	}
@@ -248,7 +243,7 @@ func collectNoteEvents(ctx context.Context, c changeSet, notes map[string]struct
 		}
 		var n note
 		ctx, cancel := context.WithTimeout(ctx, timeout)
-		ok, err := working.GetNote(ctx, c.commits[i].Revision, &n)
+		ok, err := d.Repo.GetNote(ctx, c.commits[i].Revision, d.GitConfig.NotesRef, &n)
 		cancel()
 		if err != nil {
 			return nil, nil, errors.Wrap(err, "loading notes from repo")

git/export.go

@@ -3,6 +3,7 @@ package git
 import (
 	"context"
 	"os"
+	"path/filepath"
 )
 
 type Export struct {
@@ -30,3 +31,14 @@ func (r *Repo) Export(ctx context.Context, ref string) (*Export, error) {
 	}
 	return &Export{dir}, nil
 }
+
+// ChangedFiles does a git diff listing changed files
+func (c *Export) ChangedFiles(ctx context.Context, sinceRef string, paths []string) ([]string, error) {
+	list, err := changed(ctx, c.Dir(), sinceRef, paths)
+	if err == nil {
+		for i, file := range list {
+			list[i] = filepath.Join(c.Dir(), file)
+		}
+	}
+	return list, err
+}

git/gittest/repo_test.go

@@ -190,7 +190,7 @@ func TestCheckout(t *testing.T) {
 	}
 
 	var note Note
-	ok, err := checkout.GetNote(ctx, head, &note)
+	ok, err := repo.GetNote(ctx, head, params.NotesRef, &note)
 	if err != nil {
 		t.Error(err)
 	}
@@ -241,7 +241,7 @@ func TestCheckout(t *testing.T) {
 		}
 
 		var note Note
-		ok, err := c.GetNote(ctx, rev, &note)
+		ok, err := repo.GetNote(ctx, rev, params.NotesRef, &note)
 		if !ok {
 			t.Error("note not found")
 		}

git/repo.go

@@ -273,6 +273,15 @@ func (r *Repo) DeleteTag(ctx context.Context, tag string) error {
 	return deleteTag(ctx, r.dir, tag, r.origin.URL)
 }
 
+func (r *Repo) NoteRevList(ctx context.Context, notesRef string) (map[string]struct{}, error) {
+	return noteRevList(ctx, r.Dir(), notesRef)
+}
+
+// GetNote gets a note for the revision specified, or nil if there is no such note.
+func (r *Repo) GetNote(ctx context.Context, rev, notesRef string, note interface{}) (bool, error) {
+	return getNote(ctx, r.Dir(), notesRef, rev, note)
+}
+
 // step attempts to advance the repo state machine, and returns `true`
 // if it has made progress, `false` otherwise.
 func (r *Repo) step(bg context.Context) bool {

git/working.go

@@ -164,7 +164,7 @@ func (c *Checkout) CommitAndPush(ctx context.Context, commitAction CommitAction,
 		if err != nil {
 			return err
 		}
-		if err := addNote(ctx, c.Dir(), rev, c.config.NotesRef, note); err != nil {
+		if err := addNote(ctx, c.Dir(), rev, c.realNotesRef, note); err != nil {
 			return err
 		}
 	}
@@ -183,11 +183,6 @@ func (c *Checkout) CommitAndPush(ctx context.Context, commitAction CommitAction,
 	return nil
 }
 
-// GetNote gets a note for the revision specified, or nil if there is no such note.
-func (c *Checkout) GetNote(ctx context.Context, rev string, note interface{}) (bool, error) {
-	return getNote(ctx, c.Dir(), c.realNotesRef, rev, note)
-}
-
 func (c *Checkout) HeadRevision(ctx context.Context) (string, error) {
 	return refRevision(ctx, c.Dir(), "HEAD")
 }
@@ -199,21 +194,6 @@ func (c *Checkout) MoveTagAndPush(ctx context.Context, tagAction TagAction) erro
 	return moveTagAndPush(ctx, c.Dir(), c.upstream.URL, tagAction)
 }
 
-// ChangedFiles does a git diff listing changed files
-func (c *Checkout) ChangedFiles(ctx context.Context, ref string) ([]string, error) {
-	list, err := changed(ctx, c.Dir(), ref, c.config.Paths)
-	if err == nil {
-		for i, file := range list {
-			list[i] = filepath.Join(c.Dir(), file)
-		}
-	}
-	return list, err
-}
-
-func (c *Checkout) NoteRevList(ctx context.Context) (map[string]struct{}, error) {
-	return noteRevList(ctx, c.Dir(), c.realNotesRef)
-}
-
 func (c *Checkout) Checkout(ctx context.Context, rev string) error {
 	return checkout(ctx, c.Dir(), rev)
 }