Skip to content

Commit

Permalink
Add postrequisite support to PipelineStage (#1541)
Browse files Browse the repository at this point in the history
* Add postrequisite support to PipelineStage

* Update comment based on PR feedback
  • Loading branch information
theunrepentantgeek authored Jun 8, 2021
1 parent 30046f6 commit 37c7965
Show file tree
Hide file tree
Showing 3 changed files with 104 additions and 4 deletions.
18 changes: 17 additions & 1 deletion hack/generator/pkg/codegen/code_generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -206,15 +206,31 @@ func (generator *CodeGenerator) Generate(ctx context.Context) error {
func (generator *CodeGenerator) verifyPipeline() error {
var errs []error

// Set of stages that we've already seen, used to confirm prerequisites
stagesSeen := make(map[string]struct{})

// Set of stages we expect to see, each associated with a slice containing the earlier stages that expected each
stagesExpected := make(map[string][]string)

for _, stage := range generator.pipeline {
for _, prereq := range stage.prerequisites {
if _, ok := stagesSeen[prereq]; !ok {
errs = append(errs, errors.Errorf("prerequisite '%s' of stage '%s' not satisfied.", prereq, stage.id))
errs = append(errs, errors.Errorf("prerequisite %q of stage %q not satisfied.", prereq, stage.id))
}
}

for _, postreq := range stage.postrequisites {
stagesExpected[postreq] = append(stagesExpected[postreq], stage.id)
}

stagesSeen[stage.id] = struct{}{}
delete(stagesExpected, stage.id)
}

for required, requiredBy := range stagesExpected {
for _, stageId := range requiredBy {
errs = append(errs, errors.Errorf("postrequisite %q of stage %q not satisfied", required, stageId))
}
}

return kerrors.NewAggregate(errs)
Expand Down
67 changes: 65 additions & 2 deletions hack/generator/pkg/codegen/code_generator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,10 @@ func TestVerifyPipeline_GivenUnsatisfiedPrerequisites_ReturnsError(t *testing.T)
},
}

g.Expect(gen.verifyPipeline()).NotTo(BeNil())
err := gen.verifyPipeline()
g.Expect(err).NotTo(BeNil())
g.Expect(err.Error()).To(ContainSubstring(stage.id))
g.Expect(err.Error()).To(ContainSubstring(barStage.id))
}

func TestVerifyPipeline_GivenOutOfOrderPrerequisites_ReturnsError(t *testing.T) {
Expand All @@ -212,5 +215,65 @@ func TestVerifyPipeline_GivenOutOfOrderPrerequisites_ReturnsError(t *testing.T)
},
}

g.Expect(gen.verifyPipeline()).NotTo(BeNil())
err := gen.verifyPipeline()
g.Expect(err).NotTo(BeNil())
g.Expect(err.Error()).To(ContainSubstring(stage.id))
g.Expect(err.Error()).To(ContainSubstring(barStage.id))
}

func TestVerifyPipeline_GivenSatisfiedPostrequisites_ReturnsNoError(t *testing.T) {
g := NewGomegaWithT(t)

stage := MakeFakePipelineStage("stage").RequiresPostrequisiteStages(barStage.id)

gen := &CodeGenerator{
pipeline: []PipelineStage{
fooStage,
stage,
barStage,
bazStage,
},
}

err := gen.verifyPipeline()
g.Expect(err).To(BeNil())
}

func TestVerifyPipeline_GivenUnsatisfiedPostrequisites_ReturnsError(t *testing.T) {
g := NewGomegaWithT(t)

stage := MakeFakePipelineStage("stage").RequiresPrerequisiteStages(barStage.id)

gen := &CodeGenerator{
pipeline: []PipelineStage{
fooStage,
stage,
bazStage,
},
}

err := gen.verifyPipeline()
g.Expect(err).NotTo(BeNil())
g.Expect(err.Error()).To(ContainSubstring(stage.id))
g.Expect(err.Error()).To(ContainSubstring(barStage.id))
}

func TestVerifyPipeline_GivenOutOfOrderPostrequisites_ReturnsError(t *testing.T) {
g := NewGomegaWithT(t)

stage := MakeFakePipelineStage("stage").RequiresPostrequisiteStages(barStage.id)

gen := &CodeGenerator{
pipeline: []PipelineStage{
fooStage,
barStage,
stage,
bazStage,
},
}

err := gen.verifyPipeline()
g.Expect(err).NotTo(BeNil())
g.Expect(err.Error()).To(ContainSubstring(stage.id))
g.Expect(err.Error()).To(ContainSubstring(barStage.id))
}
23 changes: 22 additions & 1 deletion hack/generator/pkg/codegen/pipeline_stage.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,10 @@ type PipelineStage struct {
action func(context.Context, astmodel.Types) (astmodel.Types, error)
// Tag used for filtering
targets []PipelineTarget
// Identifiers for other stages that must be completed first
// Identifiers for other stages that must be completed before this one
prerequisites []string
// Identifiers for other stages that must be completed after this one
postrequisites []string
}

// MakePipelineStage creates a new pipeline stage that's ready for execution
Expand All @@ -45,6 +47,7 @@ func (stage *PipelineStage) HasId(id string) bool {
return stage.id == id
}

// RequiresPrerequisiteStages declares which stages must have completed before this one is executed
func (stage PipelineStage) RequiresPrerequisiteStages(prerequisites ...string) PipelineStage {
if len(stage.prerequisites) > 0 {
panic(fmt.Sprintf(
Expand All @@ -59,6 +62,24 @@ func (stage PipelineStage) RequiresPrerequisiteStages(prerequisites ...string) P
return stage
}

// RequiresPostrequisiteStages declares which stages must be executed after this one has completed
// This is not completely isomorphic with RequiresPrerequisiteStages as there may be supporting stages that are
// sometimes omitted from execution when targeting different outcomes. Having both pre- and post-requisites allows the
// dependencies to drop out cleanly when different stages are present.
func (stage PipelineStage) RequiresPostrequisiteStages(postrequisites ...string) PipelineStage {
if len(stage.postrequisites) > 0 {
panic(fmt.Sprintf(
"Postrequisites of stage '%s' already set to '%s'; cannot modify to '%s'.",
stage.id,
strings.Join(stage.postrequisites, "; "),
strings.Join(postrequisites, "; ")))
}

stage.postrequisites = postrequisites

return stage
}

// UsedFor specifies that this stage should be used for only the specified targets
func (stage PipelineStage) UsedFor(targets ...PipelineTarget) PipelineStage {
stage.targets = targets
Expand Down

0 comments on commit 37c7965

Please sign in to comment.