Skip to content

Commit

Permalink
chore: refactor run.Sort stack ordering (#1413)
Browse files Browse the repository at this point in the history
<!--  Thanks for sending a pull request!  Here are some tips for you:

1. If this is your first time, please read our contributor guidelines:
https://github.com/terramate-io/terramate/blob/main/CONTRIBUTING.md
2. If the PR is unfinished, mark it as draft:
https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/changing-the-stage-of-a-pull-request
3. Please update the PR title using the Conventional Commits convention:
https://www.conventionalcommits.org/en/v1.0.0/
    Example: feat: add support for XYZ.
-->

## What this PR does / why we need it:

The list of stacks that needs to be ordered for `script run` execution
uses different data types than for `run`, which makes it difficult to
re-use. This PR makes the sorting function more flexible so it can be
used with both data types.

## Which issue(s) this PR fixes:
<!--
*Automatically closes linked issue when PR is merged.
Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.
_If PR is about `failing-tests or flakes`, please post the related
issues/tests in a comment and do not use `Fixes`_*
Keep it empty if not applicable.
-->

## Special notes for your reviewer:
* The interface of the sort function changes from returning the sorted
list as a new list, to sorting in-place. However, even before the passed
list was modified with `sort.Sort()`, so this is at least obvious now.

## Does this PR introduce a user-facing change?
<!--
If no, just write "no" in the block below.
If yes, please explain the change and update documentation and the
CHANGELOG.md file accordingly.
-->
```
no
```
  • Loading branch information
snakster committed Feb 1, 2024
2 parents 4decf4a + 19e56dc commit 017241e
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 62 deletions.
8 changes: 5 additions & 3 deletions cmd/terramate/cli/cli.go
Original file line number Diff line number Diff line change
Expand Up @@ -1471,7 +1471,8 @@ func (c *cli) printStacksList(allStacks []stack.Entry, why bool, runOrder bool)
if runOrder {
var failReason string
var err error
stacks, failReason, err = run.Sort(c.cfg(), stacks)
failReason, err = run.Sort(c.cfg(), stacks,
func(s *config.SortableStack) *config.Stack { return s.Stack })
if err != nil {
c.fatal("Invalid stack configuration", errors.E(err, failReason))
}
Expand Down Expand Up @@ -1671,7 +1672,8 @@ func (c *cli) printRunOrder(friendlyFmt bool) {
}

logger.Debug().Msg("Get run order.")
orderedStacks, reason, err := run.Sort(c.cfg(), stacks)
reason, err := run.Sort(c.cfg(), stacks,
func(s *config.SortableStack) *config.Stack { return s.Stack })
if err != nil {
if errors.IsKind(err, dag.ErrCycleDetected) {
c.fatal("Invalid stack configuration", errors.E(err, reason))
Expand All @@ -1680,7 +1682,7 @@ func (c *cli) printRunOrder(friendlyFmt bool) {
}
}

for _, s := range orderedStacks {
for _, s := range stacks {
dir := s.Dir().String()
if !friendlyFmt {
printer.Stdout.Println(dir)
Expand Down
9 changes: 5 additions & 4 deletions cmd/terramate/cli/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,8 @@ func (c *cli) runOnStacks() {
}
}

orderedStacks, reason, err := runutil.Sort(c.cfg(), stacks)
reason, err := runutil.Sort(c.cfg(), stacks,
func(s *config.SortableStack) *config.Stack { return s.Stack })
if err != nil {
if errors.IsKind(err, dag.ErrCycleDetected) {
fatal(err, "cycle detected: %s", reason)
Expand All @@ -104,7 +105,7 @@ func (c *cli) runOnStacks() {
}

if c.parsedArgs.Run.Reverse {
config.ReverseStacks(orderedStacks)
config.ReverseStacks(stacks)
}

if c.parsedArgs.Run.CloudSyncDeployment && c.parsedArgs.Run.CloudSyncDriftStatus {
Expand All @@ -121,7 +122,7 @@ func (c *cli) runOnStacks() {
if !c.prj.isRepo {
fatal(errors.E("cloud features requires a git repository"))
}
c.ensureAllStackHaveIDs(orderedStacks)
c.ensureAllStackHaveIDs(stacks)
c.detectCloudMetadata()
}

Expand All @@ -130,7 +131,7 @@ func (c *cli) runOnStacks() {
}

var runs []runContext
for _, st := range orderedStacks {
for _, st := range stacks {
run := runContext{
Stack: st.Stack,
Cmd: c.parsedArgs.Run.Command,
Expand Down
24 changes: 8 additions & 16 deletions cmd/terramate/cli/script_run.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ func (c *cli) runScript() {

for jobIdx, job := range evalScript.Jobs {
for cmdIdx, cmd := range job.Commands() {
exc := runContext{
run := runContext{
Stack: st.Stack,
Cmd: cmd.Args,
ScriptIdx: scriptIdx,
Expand All @@ -101,17 +101,18 @@ func (c *cli) runScript() {
}

if cmd.Options != nil {
exc.CloudSyncDeployment = cmd.Options.CloudSyncDeployment
exc.CloudSyncTerraformPlanFile = cmd.Options.CloudSyncTerraformPlan
run.CloudSyncDeployment = cmd.Options.CloudSyncDeployment
run.CloudSyncTerraformPlanFile = cmd.Options.CloudSyncTerraformPlan
}

runs = append(runs, exc)
runs = append(runs, run)
}
}
}
}

orderedStacks, reason, err := runutil.Sort(c.cfg(), stacks)
reason, err := runutil.Sort(c.cfg(), runs,
func(run runContext) *config.Stack { return run.Stack })
if err != nil {
if errors.IsKind(err, dag.ErrCycleDetected) {
fatal(err, "cycle detected: %s", reason)
Expand All @@ -120,22 +121,13 @@ func (c *cli) runScript() {
}
}

var orderedRuns []runContext
for _, st := range orderedStacks {
for _, r := range runs {
if r.Stack.Dir.String() == st.Dir().String() {
orderedRuns = append(orderedRuns, r)
}
}
}

c.prepareScriptCloudDeploymentSync(orderedRuns)
c.prepareScriptCloudDeploymentSync(runs)

isSuccessExit := func(exitCode int) bool {
return exitCode == 0
}

err = c.runAll(orderedRuns, isSuccessExit, runAllOptions{
err = c.runAll(runs, isSuccessExit, runAllOptions{
Quiet: c.parsedArgs.Quiet,
DryRun: c.parsedArgs.Script.Run.DryRun,
ScriptRun: true,
Expand Down
79 changes: 40 additions & 39 deletions run/order.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,20 +8,20 @@ import (
"os"
"path"
"path/filepath"
"sort"
"strings"

"github.com/rs/zerolog/log"
"github.com/terramate-io/terramate/config"
"github.com/terramate-io/terramate/errors"
"github.com/terramate-io/terramate/printer"
"github.com/terramate-io/terramate/run/dag"
"golang.org/x/exp/slices"
)

// Sort computes the final execution order for the given list of stacks.
// In the case of multiple possible orders, it returns the lexicographic sorted
// path.
func Sort(root *config.Root, stacks config.List[*config.SortableStack]) (config.List[*config.SortableStack], string, error) {
func Sort[S ~[]E, E any](root *config.Root, items S, getStack func(E) *config.Stack) (string, error) {
d := dag.New()

logger := log.With().
Expand All @@ -33,35 +33,42 @@ func Sort(root *config.Root, stacks config.List[*config.SortableStack]) (config.
return s1.Dir.HasPrefix(s2.Dir.String() + "/")
}

sort.Sort(stacks)
for _, stackElem := range stacks {
for _, otherElem := range stacks {
if stackElem.Dir() == otherElem.Dir() {
getStackDir := func(s E) string {
return getStack(s).Dir.String()
}

slices.SortFunc(items, func(a, b E) int {
return strings.Compare(getStack(a).Dir.String(), getStack(b).Dir.String())
})

for _, a := range items {
for _, b := range items {
if getStack(a).Dir == getStack(b).Dir {
continue
}

if isParentStack(stackElem.Stack, otherElem.Stack) {
logger.Debug().Msgf("stack %q runs before %q since it is its parent", otherElem, stackElem)
if isParentStack(getStack(a), getStack(b)) {
logger.Debug().Msgf("stack %q runs before %q since it is its parent", getStackDir(a), getStackDir(b))

otherElem.AppendBefore(stackElem.Dir().String())
getStack(b).AppendBefore(getStack(a).Dir.String())
}
}
}

visited := dag.Visited{}
for _, elem := range stacks {
if _, ok := visited[dag.ID(elem.Dir().String())]; ok {
for _, elem := range items {
if _, ok := visited[dag.ID(getStack(elem).Dir.String())]; ok {
continue
}

logger.Debug().
Stringer("stack", elem.Dir()).
Stringer("stack", getStack(elem).Dir).
Msg("Build DAG.")

err := BuildDAG(
d,
root,
elem.Stack,
getStack(elem),
"before",
func(s config.Stack) []string { return s.Before },
"after",
Expand All @@ -70,46 +77,40 @@ func Sort(root *config.Root, stacks config.List[*config.SortableStack]) (config.
)

if err != nil {
return nil, "", err
return "", err
}
}

reason, err := d.Validate()
if err != nil {
return nil, reason, err
return reason, err
}

order := d.Order()

orderedStacks := make(config.List[*config.SortableStack], 0, len(order))

isSelectedStack := func(s *config.Stack) bool {
// Stacks may be added on the DAG from after/before references
// but they should not be on the final order if they are not part
// of the previously selected stacks passed as a parameter.
// This is important for change detection to work on ordering and
// also for filtering by working dir.
for _, stack := range stacks {
if s.Dir == stack.Dir() {
return true
}
}
return false
}

for _, id := range order {
orderLookup := make(map[string]int, len(order))
for idx, id := range order {
val, err := d.Node(id)
if err != nil {
return nil, "", fmt.Errorf("calculating run-order: %w", err)
return "", fmt.Errorf("calculating run-order: %w", err)
}
s := val.(*config.Stack)
if !isSelectedStack(s) {
continue
}
orderedStacks = append(orderedStacks, s.Sortable())
orderLookup[s.Dir.String()] = idx
}

return orderedStacks, "", nil
slices.SortFunc(items, func(a, b E) int {
// TODO: Replace with cmp.Compare once we upgrade Go version.
i := orderLookup[getStackDir(a)]
j := orderLookup[getStackDir(b)]
if i == j {
return 0
} else if i < j {
return -1
} else {
return 1
}
})

return "", nil
}

// BuildDAG builds a run order DAG for the given stack.
Expand Down

0 comments on commit 017241e

Please sign in to comment.