Merge pull request #1297 from weaveworks/issue/1267-multiple-paths

Support multiple `-git-path` arguments

cluster/kubernetes/manifests.go

@@ -10,8 +10,8 @@ import (
 type Manifests struct {
 }
 
-func (c *Manifests) LoadManifests(base, first string, rest ...string) (map[string]resource.Resource, error) {
-	return kresource.Load(base, first, rest...)
+func (c *Manifests) LoadManifests(base string, paths []string) (map[string]resource.Resource, error) {
+	return kresource.Load(base, paths)
 }
 
 func (c *Manifests) ParseManifests(allDefs []byte) (map[string]resource.Resource, error) {

cluster/kubernetes/resource/load.go

@@ -15,14 +15,13 @@ import (
 // Load takes paths to directories or files, and creates an object set
 // based on the file(s) therein. Resources are named according to the
 // file content, rather than the file name of directory structure.
-func Load(base, atLeastOne string, more ...string) (map[string]resource.Resource, error) {
-	roots := append([]string{atLeastOne}, more...)
+func Load(base string, paths []string) (map[string]resource.Resource, error) {
 	objs := map[string]resource.Resource{}
 	charts, err := newChartTracker(base)
 	if err != nil {
 		return nil, errors.Wrapf(err, "walking %q for chartdirs", base)
 	}
-	for _, root := range roots {
+	for _, root := range paths {
 		err := filepath.Walk(root, func(path string, info os.FileInfo, err error) error {
 			if err != nil {
 				return errors.Wrapf(err, "walking %q for yamels", path)

cluster/kubernetes/resource/load_test.go

@@ -200,7 +200,7 @@ func TestLoadSome(t *testing.T) {
 	if err := testfiles.WriteTestFiles(dir); err != nil {
 		t.Fatal(err)
 	}
-	objs, err := Load(dir, dir)
+	objs, err := Load(dir, []string{dir})
 	if err != nil {
 		t.Error(err)
 	}
@@ -231,7 +231,7 @@ func TestChartTracker(t *testing.T) {
 		if f == "garbage" {
 			continue
 		}
-		if m, err := Load(dir, fq); err != nil || len(m) == 0 {
+		if m, err := Load(dir, []string{fq}); err != nil || len(m) == 0 {
 			t.Errorf("Load returned 0 objs, err=%v", err)
 		}
 	}
@@ -250,7 +250,7 @@ func TestChartTracker(t *testing.T) {
 	}
 	for _, f := range chartfiles {
 		fq := filepath.Join(dir, f)
-		if m, err := Load(dir, fq); err != nil || len(m) != 0 {
+		if m, err := Load(dir, []string{fq}); err != nil || len(m) != 0 {
 			t.Errorf("%q not ignored as a chart should be", f)
 		}
 	}

cluster/manifests.go

@@ -26,11 +26,11 @@ func ErrResourceNotFound(name string) error {
 type Manifests interface {
 	// Update the image in a manifest's bytes to that given
 	UpdateImage(def []byte, resourceID flux.ResourceID, container string, newImageID image.Ref) ([]byte, error)
-	// Load all the resource manifests under the path given. `baseDir`
-	// is used to relativise the paths, which are supplied as absolute
-	// paths to directories or files; at least one path must be
-	// supplied.
-	LoadManifests(baseDir, first string, rest ...string) (map[string]resource.Resource, error)
+	// Load all the resource manifests under the paths
+	// given. `baseDir` is used to relativise the paths, which are
+	// supplied as absolute paths to directories or files; at least
+	// one path should be supplied, even if it is the same as `baseDir`.
+	LoadManifests(baseDir string, paths []string) (map[string]resource.Resource, error)
 	// Parse the manifests given in an exported blob
 	ParseManifests([]byte) (map[string]resource.Resource, error)
 	// UpdatePolicies modifies a manifest to apply the policy update specified
@@ -40,8 +40,8 @@ type Manifests interface {
 // UpdateManifest looks for the manifest for the identified resource,
 // reads its contents, applies f(contents), and writes the results
 // back to the file.
-func UpdateManifest(m Manifests, root string, id flux.ResourceID, f func(manifest []byte) ([]byte, error)) error {
-	resources, err := m.LoadManifests(root, root)
+func UpdateManifest(m Manifests, root string, paths []string, id flux.ResourceID, f func(manifest []byte) ([]byte, error)) error {
+	resources, err := m.LoadManifests(root, paths)
 	if err != nil {
 		return err
 	}

cluster/mock.go

@@ -17,7 +17,7 @@ type Mock struct {
 	SyncFunc           func(SyncDef) error
 	PublicSSHKeyFunc   func(regenerate bool) (ssh.PublicKey, error)
 	UpdateImageFunc    func(def []byte, id flux.ResourceID, container string, newImageID image.Ref) ([]byte, error)
-	LoadManifestsFunc  func(base, first string, rest ...string) (map[string]resource.Resource, error)
+	LoadManifestsFunc  func(base string, paths []string) (map[string]resource.Resource, error)
 	ParseManifestsFunc func([]byte) (map[string]resource.Resource, error)
 	UpdateManifestFunc func(path, resourceID string, f func(def []byte) ([]byte, error)) error
 	UpdatePoliciesFunc func([]byte, flux.ResourceID, policy.Update) ([]byte, error)
@@ -51,8 +51,8 @@ func (m *Mock) UpdateImage(def []byte, id flux.ResourceID, container string, new
 	return m.UpdateImageFunc(def, id, container, newImageID)
 }
 
-func (m *Mock) LoadManifests(base, first string, rest ...string) (map[string]resource.Resource, error) {
-	return m.LoadManifestsFunc(base, first, rest...)
+func (m *Mock) LoadManifests(base string, paths []string) (map[string]resource.Resource, error) {
+	return m.LoadManifestsFunc(base, paths)
 }
 
 func (m *Mock) ParseManifests(def []byte) (map[string]resource.Resource, error) {

cmd/fluxd/main.go

@@ -79,7 +79,7 @@ func main() {
 		// Git repo & key etc.
 		gitURL       = fs.String("git-url", "", "URL of git repo with Kubernetes manifests; e.g., [email protected]:weaveworks/flux-example")
 		gitBranch    = fs.String("git-branch", "master", "branch of git repo to use for Kubernetes manifests")
-		gitPath      = fs.String("git-path", "", "path within git repo to locate Kubernetes manifests (relative path)")
+		gitPath      = fs.StringSlice("git-path", []string{}, "relative paths within the git repo to locate Kubernetes manifests")
 		gitUser      = fs.String("git-user", "Weave Flux", "username to use as git committer")
 		gitEmail     = fs.String("git-email", "[email protected]", "email to use as git committer")
 		gitSetAuthor = fs.Bool("git-set-author", false, "If set, the author of git commits will reflect the user who initiated the commit and will differ from the git committer.")
@@ -161,9 +161,11 @@ func main() {
 		*gitSkipMessage = defaultGitSkipMessage
 	}
 
-	if len(*gitPath) > 0 && (*gitPath)[0] == '/' {
-		logger.Log("err", "git subdirectory (--git-path) should not have leading forward slash")
-		os.Exit(1)
+	for _, path := range *gitPath {
+		if len(path) > 0 && path[0] == '/' {
+			logger.Log("err", "subdirectory given as --git-path should not have leading forward slash")
+			os.Exit(1)
+		}
 	}
 
 	if *sshKeygenDir == "" {
@@ -353,7 +355,7 @@ func main() {
 
 	gitRemote := git.Remote{URL: *gitURL}
 	gitConfig := git.Config{
-		Path:        *gitPath,
+		Paths:       *gitPath,
 		Branch:      *gitBranch,
 		SyncTag:     *gitSyncTag,
 		NotesRef:    *gitNotesRef,

daemon/daemon.go

@@ -5,6 +5,7 @@ import (
 	"context"
 	"fmt"
 	"sort"
+	"strings"
 	"time"
 
 	"github.com/go-kit/kit/log"
@@ -75,7 +76,7 @@ func (d *Daemon) getResources(ctx context.Context) (map[string]resource.Resource
 	var globalReadOnly v6.ReadOnlyReason
 	err := d.WithClone(ctx, func(checkout *git.Checkout) error {
 		var err error
-		resources, err = d.Manifests.LoadManifests(checkout.Dir(), checkout.ManifestDir())
+		resources, err = d.Manifests.LoadManifests(checkout.Dir(), checkout.ManifestDirs())
 		return err
 	})
 
@@ -349,7 +350,7 @@ func (d *Daemon) updatePolicy(spec update.Spec, updates policy.Updates) updateFu
 				anythingAutomated = true
 			}
 			// find the service manifest
-			err := cluster.UpdateManifest(d.Manifests, working.ManifestDir(), serviceID, func(def []byte) ([]byte, error) {
+			err := cluster.UpdateManifest(d.Manifests, working.Dir(), working.ManifestDirs(), serviceID, func(def []byte) ([]byte, error) {
 				newDef, err := d.Manifests.UpdatePolicies(def, serviceID, u)
 				if err != nil {
 					result.Result[serviceID] = update.ControllerResult{
@@ -496,7 +497,7 @@ func (d *Daemon) JobStatus(ctx context.Context, jobID job.ID) (job.Status, error
 		if err != nil {
 			return errors.Wrap(err, "enumerating commit notes")
 		}
-		commits, err := d.Repo.CommitsBefore(ctx, "HEAD", d.GitConfig.Path)
+		commits, err := d.Repo.CommitsBefore(ctx, "HEAD", d.GitConfig.Paths...)
 		if err != nil {
 			return errors.Wrap(err, "checking revisions for status")
 		}
@@ -529,7 +530,7 @@ func (d *Daemon) JobStatus(ctx context.Context, jobID job.ID) (job.Status, error
 // 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)
+	commits, err := d.Repo.CommitsBetween(ctx, d.GitConfig.SyncTag, commitRef, d.GitConfig.Paths...)
 	if err != nil {
 		return nil, err
 	}
@@ -550,11 +551,15 @@ func (d *Daemon) GitRepoConfig(ctx context.Context, regenerate bool) (v6.GitConf
 
 	origin := d.Repo.Origin()
 	status, _ := d.Repo.Status()
+	path := ""
+	if len(d.GitConfig.Paths) > 0 {
+		path = strings.Join(d.GitConfig.Paths, ",")
+	}
 	return v6.GitConfig{
 		Remote: v6.GitRemoteConfig{
 			URL:    origin.URL,
 			Branch: d.GitConfig.Branch,
-			Path:   d.GitConfig.Path,
+			Path:   path,
 		},
 		PublicSSHKey: publicSSHKey,
 		Status:       status,

daemon/daemon_test.go

@@ -386,7 +386,8 @@ func TestDaemon_Release(t *testing.T) {
 		}
 		defer co.Clean()
 		// open a file
-		if file, err := os.Open(filepath.Join(co.ManifestDir(), "helloworld-deploy.yaml")); err == nil {
+		dirs := co.ManifestDirs()
+		if file, err := os.Open(filepath.Join(dirs[0], "helloworld-deploy.yaml")); err == nil {
 
 			// make sure it gets closed
 			defer file.Close()
@@ -430,7 +431,8 @@ func TestDaemon_PolicyUpdate(t *testing.T) {
 			return false
 		}
 		defer co.Clean()
-		m, err := d.Manifests.LoadManifests(co.Dir(), co.ManifestDir())
+		dirs := co.ManifestDirs()
+		m, err := d.Manifests.LoadManifests(co.Dir(), dirs)
 		if err != nil {
 			t.Fatalf("Error: %s", err.Error())
 		}
@@ -774,7 +776,8 @@ func (w *wait) ForImageTag(t *testing.T, d *Daemon, service, container, tag stri
 		}
 		defer co.Clean()
 
-		m, err := d.Manifests.LoadManifests(co.Dir(), co.ManifestDir())
+		dirs := co.ManifestDirs()
+		m, err := d.Manifests.LoadManifests(co.Dir(), dirs)
 		assert.NoError(t, err)
 
 		resources, err := d.Manifests.ParseManifests(m[service].Bytes())

daemon/images.go

@@ -96,6 +96,7 @@ func (r resources) IDs() (ids []flux.ResourceID) {
 }
 
 // getUnlockedAutomatedServices returns all the resources that are
+// both automated, and not locked.
 func (d *Daemon) getUnlockedAutomatedResources(ctx context.Context) (resources, error) {
 	resources, _, err := d.getResources(ctx)
 	if err != nil {

daemon/loop.go

@@ -186,7 +186,7 @@ func (d *Daemon) doSync(logger log.Logger) (retErr error) {
 	}
 
 	// Get a map of all resources defined in the repo
-	allResources, err := d.Manifests.LoadManifests(working.Dir(), working.ManifestDir())
+	allResources, err := d.Manifests.LoadManifests(working.Dir(), working.ManifestDirs())
 	if err != nil {
 		return errors.Wrap(err, "loading resources from repo")
 	}
@@ -217,10 +217,10 @@ func (d *Daemon) doSync(logger log.Logger) (retErr error) {
 		var err error
 		ctx, cancel := context.WithTimeout(ctx, gitOpTimeout)
 		if oldTagRev != "" {
-			commits, err = d.Repo.CommitsBetween(ctx, oldTagRev, newTagRev, d.GitConfig.Path)
+			commits, err = d.Repo.CommitsBetween(ctx, oldTagRev, newTagRev, d.GitConfig.Paths...)
 		} else {
 			initialSync = true
-			commits, err = d.Repo.CommitsBefore(ctx, newTagRev, d.GitConfig.Path)
+			commits, err = d.Repo.CommitsBefore(ctx, newTagRev, d.GitConfig.Paths...)
 		}
 		cancel()
 		if err != nil {
@@ -240,7 +240,7 @@ func (d *Daemon) doSync(logger log.Logger) (retErr error) {
 		if err == nil && len(changedFiles) > 0 {
 			// We had some changed files, we're syncing a diff
 			// FIXME(michael): this won't be accurate when a file can have more than one resource
-			changedResources, err = d.Manifests.LoadManifests(working.Dir(), changedFiles[0], changedFiles[1:]...)
+			changedResources, err = d.Manifests.LoadManifests(working.Dir(), changedFiles)
 		}
 		cancel()
 		if err != nil {

daemon/loop_test.go

@@ -135,7 +135,7 @@ func TestPullAndSync_InitialSync(t *testing.T) {
 	// It creates the tag at HEAD
 	if err := d.Repo.Refresh(context.Background()); err != nil {
 		t.Errorf("pulling sync tag: %v", err)
-	} else if revs, err := d.Repo.CommitsBefore(context.Background(), gitSyncTag, gitPath); err != nil {
+	} else if revs, err := d.Repo.CommitsBefore(context.Background(), gitSyncTag); err != nil {
 		t.Errorf("finding revisions before sync tag: %v", err)
 	} else if len(revs) <= 0 {
 		t.Errorf("Found no revisions before the sync tag")
@@ -197,12 +197,12 @@ func TestDoSync_NoNewCommits(t *testing.T) {
 	}
 
 	// It doesn't move the tag
-	oldRevs, err := d.Repo.CommitsBefore(ctx, gitSyncTag, gitPath)
+	oldRevs, err := d.Repo.CommitsBefore(ctx, gitSyncTag)
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	if revs, err := d.Repo.CommitsBefore(ctx, gitSyncTag, gitPath); err != nil {
+	if revs, err := d.Repo.CommitsBefore(ctx, gitSyncTag); err != nil {
 		t.Errorf("finding revisions before sync tag: %v", err)
 	} else if !reflect.DeepEqual(revs, oldRevs) {
 		t.Errorf("Should have kept the sync tag at HEAD")
@@ -230,7 +230,8 @@ func TestDoSync_WithNewCommit(t *testing.T) {
 			return err
 		}
 		// Push some new changes
-		err = cluster.UpdateManifest(k8s, checkout.ManifestDir(), flux.MustParseResourceID("default:deployment/helloworld"), func(def []byte) ([]byte, error) {
+		dirs := checkout.ManifestDirs()
+		err = cluster.UpdateManifest(k8s, checkout.Dir(), dirs, flux.MustParseResourceID("default:deployment/helloworld"), func(def []byte) ([]byte, error) {
 			// A simple modification so we have changes to push
 			return []byte(strings.Replace(string(def), "replicas: 5", "replicas: 4", -1)), nil
 		})
@@ -300,7 +301,7 @@ func TestDoSync_WithNewCommit(t *testing.T) {
 	defer cancel()
 	if err := d.Repo.Refresh(ctx); err != nil {
 		t.Errorf("pulling sync tag: %v", err)
-	} else if revs, err := d.Repo.CommitsBetween(ctx, oldRevision, gitSyncTag, gitPath); err != nil {
+	} else if revs, err := d.Repo.CommitsBetween(ctx, oldRevision, gitSyncTag); err != nil {
 		t.Errorf("finding revisions before sync tag: %v", err)
 	} else if len(revs) <= 0 {
 		t.Errorf("Should have moved sync tag forward")

git/gittest/repo_test.go

@@ -25,7 +25,8 @@ func TestCommit(t *testing.T) {
 	defer cleanup()
 
 	for file, _ := range testfiles.Files {
-		path := filepath.Join(checkout.ManifestDir(), file)
+		dirs := checkout.ManifestDirs()
+		path := filepath.Join(dirs[0], file)
 		if err := ioutil.WriteFile(path, []byte("FIRST CHANGE"), 0666); err != nil {
 			t.Fatal(err)
 		}
@@ -45,7 +46,7 @@ func TestCommit(t *testing.T) {
 		t.Error(err)
 	}
 
-	commits, err := repo.CommitsBefore(ctx, "HEAD", "")
+	commits, err := repo.CommitsBefore(ctx, "HEAD")
 
 	if err != nil {
 		t.Fatal(err)
@@ -107,8 +108,9 @@ func TestCheckout(t *testing.T) {
 	}
 
 	changedFile := ""
+	dirs := checkout.ManifestDirs()
 	for file, _ := range testfiles.Files {
-		path := filepath.Join(checkout.ManifestDir(), file)
+		path := filepath.Join(dirs[0], file)
 		if err := ioutil.WriteFile(path, []byte("FIRST CHANGE"), 0666); err != nil {
 			t.Fatal(err)
 		}
@@ -120,7 +122,7 @@ func TestCheckout(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	path := filepath.Join(checkout.ManifestDir(), changedFile)
+	path := filepath.Join(dirs[0], changedFile)
 	if err := ioutil.WriteFile(path, []byte("SECOND CHANGE"), 0666); err != nil {
 		t.Fatal(err)
 	}
@@ -135,7 +137,7 @@ func TestCheckout(t *testing.T) {
 	}
 
 	check := func(c *git.Checkout) {
-		contents, err := ioutil.ReadFile(filepath.Join(c.ManifestDir(), changedFile))
+		contents, err := ioutil.ReadFile(filepath.Join(dirs[0], changedFile))
 		if err != nil {
 			t.Fatal(err)
 		}