Merge branch 'master' of github.com:grammarly/rocker-compose

src/compose/action.go

@@ -36,8 +36,16 @@ func NewStepAction(async bool, actions ...Action) Action {
 		return actions[0]
 	}
 
+	//filter NoAction elements
+	acts := []Action{}
+	for _, a := range actions {
+		if a != NoAction{
+			acts = append(acts, a)
+		}
+	}
+
 	return &stepAction{
-		actions: actions,
+		actions: acts,
 		async:   async,
 	}
 }

src/compose/diff.go

@@ -8,79 +8,77 @@ type Diff interface {
 	Diff(ns string, expected []*Container, actual []*Container) ([]Action, error)
 }
 
-type comparator struct {}
-
-type dependencyGraph struct {
+// graph with container dependencies
+type graph struct {
 	dependencies map[*Container][]*dependency
 }
 
+// single dependency (external - means in other namespace)
 type dependency struct {
 	container *Container
 	external  bool
 }
 
 func NewDiff() Diff {
-	return &comparator{}
+	return &graph{
+		dependencies:    make(map[*Container][]*dependency),
+	}
 }
 
-func (c *comparator) Diff(ns string, expected []*Container, actual []*Container) (res []Action, err error) {
-	var depGraph *dependencyGraph
-	depGraph, err = buildDependencyGraph(ns, expected, actual)
+func (g *graph) Diff(ns string, expected []*Container, actual []*Container) (res []Action, err error) {
+	//filling dependency graph
+	err = g.buildDependencyGraph(ns, expected, actual)
 	if err != nil {
 		res = []Action{}
 		return
 	}
 
-	if depGraph.hasCycles() {
-		err = fmt.Errorf("Dependencies have cycles")
+	//check for cycles in configuration
+	if g.hasCycles() {
+		err = fmt.Errorf("Dependencies have cycles, check links and volumes-from")
 		return
 	}
 
-	res = getContainersToRemove(ns, expected, actual)
-	res = append(res, depGraph.buildExecutionPlan(actual)...)
-
+	res = listContainersToRemove(ns, expected, actual)
+	res = append(res, g.buildExecutionPlan(actual)...)
 	return
 }
 
-func buildDependencyGraph(ns string, expected []*Container, actual []*Container) (*dependencyGraph, error) {
-	dg := dependencyGraph{
-		dependencies: make(map[*Container][]*dependency),
-	}
-
+func (g *graph) buildDependencyGraph(ns string, expected []*Container, actual []*Container) error {
 	for _, c := range expected {
-		dg.dependencies[c] = []*dependency{}
-		dependencies, err := getDependencies(ns, expected, actual, c)
+		g.dependencies[c] = []*dependency{}
+		dependencies, err := resolveDependencies(ns, expected, actual, c)
 		if err != nil {
-			return nil, err
+			return err
 		}
-		dg.dependencies[c] = append(dg.dependencies[c], dependencies...)
+		g.dependencies[c] = append(g.dependencies[c], dependencies...)
 	}
-
-	return &dg, nil
+	return nil
 }
 
-func getDependencies(ns string, expected []*Container, actual []*Container, target *Container) (resolved []*dependency, err error) {
+func resolveDependencies(ns string, expected []*Container, actual []*Container, target *Container) (resolved []*dependency, err error) {
 	resolved = []*dependency{}
-	toResolve := append(target.Config.VolumesFrom, target.Config.Links...)
+	var toResolve []ContainerName = append(target.Config.VolumesFrom, target.Config.Links...)
 	for _, dep := range toResolve {
+		// in case of the same namespace, we should find dependency
+		// in given configuration
 		if dep.Namespace == ns {
 			if d := find(expected, &dep); d != nil {
 				resolved = append(resolved, &dependency{container: d, external: false})
-			} else {
-				err = fmt.Errorf("Cannot find internal dependency at config %s", dep.String())
+				continue
 			}
 		} else {
 			if d := find(actual, &dep); d != nil {
 				resolved = append(resolved, &dependency{container: d, external: true})
-			} else {
-				err = fmt.Errorf("Cannot find extenal dependency %s at target system", dep.String())
+				continue
 			}
 		}
+		err = fmt.Errorf("Cannot resolve dependency at config %s", dep)
 	}
 	return
 }
 
-func getContainersToRemove(ns string, expected []*Container, actual []*Container) (res []Action) {
+func listContainersToRemove(ns string, expected []*Container, actual []*Container) (res []Action) {
 	for _, a := range actual {
 		if a.Name.Namespace == ns {
 			var found bool
@@ -95,69 +93,81 @@ func getContainersToRemove(ns string, expected []*Container, actual []*Container
 	return
 }
 
-func (dg *dependencyGraph) buildExecutionPlan(actual []*Container) (res []Action) {
+func (dg *graph) buildExecutionPlan(actual []*Container) (res []Action) {
 	visited := map[*Container]bool{}
 	restarted := map[*Container]struct {}{}
 
+	// while number of visited deps less than number of
+	// dependencies which should be visited - loop
 	for len(visited) < len(dg.dependencies) {
 		var step []Action = []Action{}
 
-		nextDep:
+		nextDependency:
 		for container, deps := range dg.dependencies {
+			// if dependency is already visited - skip it
 			if _, contains := visited[container]; contains {
 				continue
 			}
 
 			var ensures []Action = []Action{}
 			var restart bool
 
+			// check transitive dependencies of current dependency
 			for _, dependency := range deps {
+
+				// for all external dependencies (in other namespace), ensure that it exists
 				if dependency.external {
 					ensures = append(ensures, NewEnsureContainerExistAction(dependency.container))
+
+					// if any of dependencies not initialized yet, iterate to next one
 				} else if finalized, contains := visited[dependency.container]; !contains || !finalized {
-					continue nextDep
+					continue nextDependency
 				}
+				// if dependency should be restarted - we should restart current one
 				_, contains := restarted[dependency.container]
 				restart = restart || contains
 			}
 
+			// predefine flag / set false to prevent getting into the same operation
 			visited[container] = false
 
+			// comparing dependency with current state
 			for _, actualContainer := range actual {
 				if container.IsSameKind(actualContainer) {
+					//in configuration was changed or restart forced by dependency - recreate container
 					if !container.IsEqualTo(actualContainer) || restart {
-						if len(ensures) > 0 {
-							step = append(step, NewStepAction(false,
-								NewStepAction(true, ensures...),
-								NewRemoveContainerAction(actualContainer),
-								NewRunContainerAction(container),
-							))
-						}else {
-							step = append(step, NewStepAction(false,
-								NewRemoveContainerAction(actualContainer),
-								NewRunContainerAction(container),
-							))
-						}
+						step = append(step, NewStepAction(false,
+							NewStepAction(true, ensures...),
+							NewRemoveContainerAction(actualContainer),
+							NewRunContainerAction(container),
+						))
+
+						// mark container as recreated
 						restarted[container] = struct {}{}
-						continue nextDep
+						continue nextDependency
 					}
+
+					// adding ensure action if applicable
 					step = append(step, NewStepAction(true, ensures...))
-					continue nextDep
+					continue nextDependency
 				}
 			}
 
+			// container is not exi
 			step = append(step, NewStepAction(false,
 				NewStepAction(true, ensures...),
 				NewRunContainerAction(container),
 			))
 		}
 
 		//finalize step
-		for k, v := range visited {
-			if !v {
-				visited[k] = true
+		for container, visit := range visited {
+			if !visit {
+				visited[container] = true
 			}
 		}
+
+		// adding step to result (step actions will be run concurrently)
 		res = append(res, NewStepAction(true, step...))
 	}
 	return
@@ -172,7 +182,7 @@ func find(containers []*Container, name *ContainerName) *Container {
 	return nil
 }
 
-func (dg *dependencyGraph) hasCycles() bool {
+func (dg *graph) hasCycles() bool {
 	for k, _ := range dg.dependencies {
 		if dg.hasCycles0([]*Container{k}, k) {
 			return true
@@ -181,7 +191,7 @@ func (dg *dependencyGraph) hasCycles() bool {
 	return false
 }
 
-func (dg *dependencyGraph) hasCycles0(path []*Container, curr *Container) bool {
+func (dg *graph) hasCycles0(path []*Container, curr *Container) bool {
 	for _, c := range path[:len(path)-1] {
 		if c.IsSameKind(curr) {
 			return true

src/compose/runner.go

@@ -1,6 +1,8 @@
 package compose
 
-import log "github.com/Sirupsen/logrus"
+import (
+	log "github.com/Sirupsen/logrus"
+)
 
 type Runner interface {
 	Run([]Action) error
@@ -33,7 +35,7 @@ func (r *dockerClientRunner) Run(actions []Action) (err error) {
 
 func (r *dryRunner) Run(actions []Action) error {
 	for _, a := range actions {
-		log.Infof("[DRY] Running: %s", a.String())
+		log.Infof("[DRY] Running: %s", a)
 	}
 	return nil
 }