From 33a7770459e1b3e44922582a1fa3cea543d5585b Mon Sep 17 00:00:00 2001 From: Andrea Luzzardi Date: Fri, 2 Apr 2021 16:38:27 -0700 Subject: [PATCH] store: support multiple deployments per path - Add support for multiple deployments per path in the Store - Add a bunch of tests - Change the Lookup deployment API - Add disambiguation in the CLI commands Fixes #231 Signed-off-by: Andrea Luzzardi --- cmd/dagger/cmd/common/common.go | 21 +++++++++++++++- dagger/store.go | 38 +++++++++++++++++----------- dagger/store_test.go | 44 +++++++++++++++++++++++++-------- 3 files changed, 77 insertions(+), 26 deletions(-) diff --git a/cmd/dagger/cmd/common/common.go b/cmd/dagger/cmd/common/common.go index a812ffd6..2014c224 100644 --- a/cmd/dagger/cmd/common/common.go +++ b/cmd/dagger/cmd/common/common.go @@ -38,7 +38,26 @@ func GetCurrentDeploymentState(ctx context.Context, store *dagger.Store) *dagger Str("deploymentPath", wd). Msg("failed to lookup deployment by path") } - return st + if len(st) == 0 { + lg. + Fatal(). + Err(err). + Str("deploymentPath", wd). + Msg("no deployments match the current directory") + } + if len(st) > 1 { + deployments := []string{} + for _, s := range st { + deployments = append(deployments, s.Name) + } + lg. + Fatal(). + Err(err). + Str("deploymentPath", wd). + Strs("deployments", deployments). + Msg("multiple deployments match the current directory, select one with `--deployment`") + } + return st[0] } // Re-compute a deployment (equivalent to `dagger up`). diff --git a/dagger/store.go b/dagger/store.go index 3847d362..903ea9e6 100644 --- a/dagger/store.go +++ b/dagger/store.go @@ -29,18 +29,18 @@ type Store struct { deployments map[string]*DeploymentState // Various indices for fast lookups - deploymentsByName map[string]*DeploymentState - deploymentsByPath map[string]*DeploymentState - pathsByDeployment map[string][]string + deploymentsByName map[string]*DeploymentState + deploymentsByPath map[string][]*DeploymentState + pathsByDeploymentID map[string][]string } func NewStore(root string) (*Store, error) { store := &Store{ - root: root, - deployments: make(map[string]*DeploymentState), - deploymentsByName: make(map[string]*DeploymentState), - deploymentsByPath: make(map[string]*DeploymentState), - pathsByDeployment: make(map[string][]string), + root: root, + deployments: make(map[string]*DeploymentState), + deploymentsByName: make(map[string]*DeploymentState), + deploymentsByPath: make(map[string][]*DeploymentState), + pathsByDeploymentID: make(map[string][]string), } return store, store.loadAll() } @@ -120,8 +120,8 @@ func (s *Store) indexDeployment(r *DeploymentState) { if i.Type != InputTypeDir { return } - s.deploymentsByPath[i.Dir.Path] = r - s.pathsByDeployment[r.ID] = append(s.pathsByDeployment[r.ID], i.Dir.Path) + s.deploymentsByPath[i.Dir.Path] = append(s.deploymentsByPath[i.Dir.Path], r) + s.pathsByDeploymentID[r.ID] = append(s.pathsByDeploymentID[r.ID], i.Dir.Path) } mapPath(r.PlanSource) @@ -138,10 +138,18 @@ func (s *Store) deindexDeployment(id string) { delete(s.deployments, r.ID) delete(s.deploymentsByName, r.Name) - for _, p := range s.pathsByDeployment[r.ID] { - delete(s.deploymentsByPath, p) + for _, p := range s.pathsByDeploymentID[r.ID] { + // Remove this deployments from the path->deployment mapping + deployments := []*DeploymentState{} + for _, d := range s.deploymentsByPath[p] { + if d.ID == r.ID { + continue + } + deployments = append(deployments, d) + } + s.deploymentsByPath[p] = deployments } - delete(s.pathsByDeployment, r.ID) + delete(s.pathsByDeploymentID, r.ID) } func (s *Store) reindexDeployment(r *DeploymentState) { @@ -205,13 +213,13 @@ func (s *Store) LookupDeploymentByName(ctx context.Context, name string) (*Deplo return st, nil } -func (s *Store) LookupDeploymentByPath(ctx context.Context, path string) (*DeploymentState, error) { +func (s *Store) LookupDeploymentByPath(ctx context.Context, path string) ([]*DeploymentState, error) { s.l.RLock() defer s.l.RUnlock() st, ok := s.deploymentsByPath[path] if !ok { - return nil, fmt.Errorf("%s: %w", path, ErrDeploymentNotExist) + return []*DeploymentState{}, nil } return st, nil } diff --git a/dagger/store_test.go b/dagger/store_test.go index abe7cf33..e46ec121 100644 --- a/dagger/store_test.go +++ b/dagger/store_test.go @@ -66,34 +66,58 @@ func TestStoreLookupByPath(t *testing.T) { require.NoError(t, store.CreateDeployment(ctx, st)) // Lookup by path - r, err := store.LookupDeploymentByPath(ctx, "/test/path") + deployments, err := store.LookupDeploymentByPath(ctx, "/test/path") require.NoError(t, err) - require.NotNil(t, r) - require.Equal(t, st.ID, r.ID) + require.Len(t, deployments, 1) + require.Equal(t, st.ID, deployments[0].ID) // Add a new path require.NoError(t, st.AddInput("bar", DirInput("/test/anotherpath", []string{}))) require.NoError(t, store.UpdateDeployment(ctx, st, nil)) // Lookup by the previous path - r, err = store.LookupDeploymentByPath(ctx, "/test/path") + deployments, err = store.LookupDeploymentByPath(ctx, "/test/path") require.NoError(t, err) - require.Equal(t, st.ID, r.ID) + require.Len(t, deployments, 1) + require.Equal(t, st.ID, deployments[0].ID) // Lookup by the new path - r, err = store.LookupDeploymentByPath(ctx, "/test/anotherpath") + deployments, err = store.LookupDeploymentByPath(ctx, "/test/anotherpath") require.NoError(t, err) - require.Equal(t, st.ID, r.ID) + require.Len(t, deployments, 1) + require.Equal(t, st.ID, deployments[0].ID) // Remove a path require.NoError(t, st.RemoveInputs("foo")) require.NoError(t, store.UpdateDeployment(ctx, st, nil)) // Lookup by the removed path should fail - _, err = store.LookupDeploymentByPath(ctx, "/test/path") - require.Error(t, err) + deployments, err = store.LookupDeploymentByPath(ctx, "/test/path") + require.NoError(t, err) + require.Len(t, deployments, 0) // Lookup by the other path should still work - _, err = store.LookupDeploymentByPath(ctx, "/test/anotherpath") + deployments, err = store.LookupDeploymentByPath(ctx, "/test/anotherpath") require.NoError(t, err) + require.Len(t, deployments, 1) + + // Add another deployment using the same path + otherSt := &DeploymentState{ + Name: "test2", + } + require.NoError(t, otherSt.AddInput("foo", DirInput("/test/anotherpath", []string{}))) + require.NoError(t, store.CreateDeployment(ctx, otherSt)) + + // Lookup by path should return both deployments + deployments, err = store.LookupDeploymentByPath(ctx, "/test/anotherpath") + require.NoError(t, err) + require.Len(t, deployments, 2) + + // Remove the first deployment. Lookup by path should still return the + // second deployment. + require.NoError(t, store.DeleteDeployment(ctx, st, nil)) + deployments, err = store.LookupDeploymentByPath(ctx, "/test/anotherpath") + require.NoError(t, err) + require.Len(t, deployments, 1) + require.Equal(t, otherSt.ID, deployments[0].ID) }