fixup

cmd/fluxd/main.go

@@ -144,6 +144,11 @@ func main() {
 		}
 	}
 
+	if len(*gitPath) > 0 && (*gitPath)[0] == '/' {
+		logger.Log("err", "git subdirectory (--git-path) should not have leading forward slash")
+		os.Exit(1)
+	}
+
 	// Cluster component.
 	var clusterVersion string
 	var sshKeyRing ssh.KeyRing
@@ -349,8 +354,6 @@ func main() {
 	var checker *checkpoint.Checker
 	updateCheckLogger := log.With(logger, "component", "checkpoint")
 
-	// There's a standard set of refs we'll want to sync; the notes
-	// ref, the sync tag, and the branch we were asked to sync to.
 	gitRemote := git.Remote{URL: *gitURL}
 	gitConfig := git.Config{
 		Path:      *gitPath,

daemon/daemon.go

@@ -323,7 +323,7 @@ func (d *Daemon) release(spec update.Spec, c release.Changes) DaemonJobFunc {
 				// On the chance pushing failed because it was not
 				// possible to fast-forward, ask the repo to fetch
 				// from upstream ASAP, so the next attempt is more
-				// likely to suceed.
+				// likely to succeed.
 				d.Repo.Notify()
 				return nil, err
 			}
@@ -389,7 +389,6 @@ func (d *Daemon) JobStatus(ctx context.Context, jobID job.ID) (job.Status, error
 
 		for _, commit := range commits {
 			if _, ok := notes[commit.Revision]; ok {
-				// FIXME(michael) Why ignore the error here? Might be context timeout
 				note, _ := working.GetNote(ctx, commit.Revision)
 				if note != nil && note.JobID == jobID {
 					status = job.Status{
@@ -409,12 +408,11 @@ func (d *Daemon) JobStatus(ctx context.Context, jobID job.ID) (job.Status, error
 	return status, err
 }
 
-// Ask the daemon how far it's got applying things; in particular, is
-// it past the given commit? Return the list of commits between where
-// we have applied (the sync tag) and the ref given, inclusive. E.g.,
-// if you send HEAD, you'll get all the commits yet to be applied. If
-// you send a hash and it's applied at or _past_ it, you'll get an
-// empty list.
+// Ask the daemon how far it's got applying things; in particular, is it
+// past the given commit? Return the list of commits between where
+// we have applied (the sync tag) and the ref given, inclusive. E.g., if you send HEAD,
+// you'll get all the commits yet to be applied. If you send a hash
+// and it's applied at or _past_ it, you'll get an empty list.
 func (d *Daemon) SyncStatus(ctx context.Context, commitRef string) ([]string, error) {
 	commits, err := d.Repo.CommitsBetween(ctx, d.GitConfig.SyncTag, commitRef, d.GitConfig.Path)
 	if err != nil {
@@ -436,14 +434,15 @@ func (d *Daemon) GitRepoConfig(ctx context.Context, regenerate bool) (flux.GitCo
 	}
 
 	origin := d.Repo.Origin()
+	status, _ := d.Repo.Status()
 	return flux.GitConfig{
 		Remote: flux.GitRemoteConfig{
 			URL:    origin.URL,
 			Branch: d.GitConfig.Branch,
 			Path:   d.GitConfig.Path,
 		},
 		PublicSSHKey: publicSSHKey,
-		Status:       flux.RepoReady, // TODO(michael): get from repo mirror itself
+		Status:       status,
 	}, nil
 }
 

daemon/images.go

@@ -77,7 +77,6 @@ func getTagPattern(services policy.ResourceMap, service flux.ResourceID, contain
 	return "*"
 }
 
-// FIXME(michael) consider rewriting this stuff, it's a bit convulated.
 func (d *Daemon) unlockedAutomatedServices(ctx context.Context) (policy.ResourceMap, error) {
 	var services policy.ResourceMap
 	err := d.WithClone(ctx, func(checkout *git.Checkout) error {

flux.go

@@ -271,18 +271,6 @@ type Container struct {
 
 // --- config types
 
-// FIXME(michael): remove when known to be unnecessary
-// func NewGitRemoteConfig(url, branch, path string) (GitRemoteConfig, error) {
-// 	if len(path) > 0 && path[0] == '/' {
-// 		return GitRemoteConfig{}, errors.New("git subdirectory (--git-path) should not have leading forward slash")
-// 	}
-// 	return GitRemoteConfig{
-// 		URL:    url,
-// 		Branch: branch,
-// 		Path:   path,
-// 	}, nil
-// }
-
 type GitRemoteConfig struct {
 	URL    string `json:"url"`
 	Branch string `json:"branch"`

git/repo.go

@@ -13,7 +13,8 @@ import (
 )
 
 const (
-	interval = 5 * time.Minute
+	interval  = 5 * time.Minute
+	opTimeout = 20 * time.Second
 
 	DefaultCloneTimeout = 2 * time.Minute
 	CheckPushTag        = "flux-write-check"
@@ -133,26 +134,32 @@ func (r *Repo) Start(shutdown <-chan struct{}, done *sync.WaitGroup) error {
 		status := r.status
 		r.mu.RUnlock()
 
+		bg := context.Background()
+
 		switch status {
 
-		// FIXME(michael): I don't think this is a real status; perhaps have a no-op repo instead.
+		// TODO(michael): I don't think this is a real status; perhaps
+		// have a no-op repo instead.
 		case flux.RepoNoConfig:
 			// this is not going to change in the lifetime of this
 			// process
 			return ErrNoConfig
 		case flux.RepoNew:
 
-			ctx := context.Background()
 			rootdir, err := ioutil.TempDir(os.TempDir(), "flux-gitclone")
 			if err != nil {
 				return err
 			}
 
+			ctx, cancel := context.WithTimeout(bg, opTimeout)
 			dir, err = mirror(ctx, rootdir, url)
+			cancel()
 			if err == nil {
 				r.mu.Lock()
 				r.dir = dir
+				ctx, cancel := context.WithTimeout(bg, opTimeout)
 				err = r.fetch(ctx)
+				cancel()
 				r.mu.Unlock()
 			}
 			if err == nil {
@@ -164,8 +171,9 @@ func (r *Repo) Start(shutdown <-chan struct{}, done *sync.WaitGroup) error {
 			r.setStatus(flux.RepoNew, err)
 
 		case flux.RepoCloned:
-			// FIXME(michael) context.Backgorund() -> context.WithTimeout(...)? Here, /passim/
-			err := checkPush(context.Background(), dir, url)
+			ctx, cancel := context.WithTimeout(bg, opTimeout)
+			err := checkPush(ctx, dir, url)
+			cancel()
 			if err == nil {
 				r.setStatus(flux.RepoReady, nil)
 				continue // with new status, skipping timer