Skip to content

Commit

Permalink
Introduce Pipeline State (#1644)
Browse files Browse the repository at this point in the history
  • Loading branch information
theunrepentantgeek authored Jul 14, 2021
1 parent e8a90cd commit 9954b93
Show file tree
Hide file tree
Showing 54 changed files with 204 additions and 110 deletions.
18 changes: 7 additions & 11 deletions hack/generator/pkg/codegen/code_generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import (

"github.com/Azure/azure-service-operator/hack/generator/pkg/astmodel"
"github.com/Azure/azure-service-operator/hack/generator/pkg/codegen/pipeline"
"github.com/Azure/azure-service-operator/hack/generator/pkg/codegen/storage"
"github.com/Azure/azure-service-operator/hack/generator/pkg/config"
)

Expand Down Expand Up @@ -79,9 +78,6 @@ func NewCodeGeneratorFromConfig(configuration *config.Configuration, idFactory a
}

func createAllPipelineStages(idFactory astmodel.IdentifierFactory, configuration *config.Configuration) []pipeline.Stage {
// graph keeps track of the conversions we need between different API & Storage versions
graph := storage.NewConversionGraph()

return []pipeline.Stage{

pipeline.LoadSchemaIntoTypes(idFactory, configuration, pipeline.DefaultSchemaLoader),
Expand Down Expand Up @@ -158,9 +154,9 @@ func createAllPipelineStages(idFactory astmodel.IdentifierFactory, configuration
// Create Storage types
// TODO: For now only used for ARM
pipeline.InjectOriginalVersionFunction(idFactory).UsedFor(pipeline.ARMTarget),
pipeline.CreateStorageTypes(graph).UsedFor(pipeline.ARMTarget),
pipeline.CreateStorageTypes().UsedFor(pipeline.ARMTarget),
pipeline.InjectOriginalVersionProperty().UsedFor(pipeline.ARMTarget),
pipeline.InjectPropertyAssignmentFunctions(graph, idFactory).UsedFor(pipeline.ARMTarget),
pipeline.InjectPropertyAssignmentFunctions(idFactory).UsedFor(pipeline.ARMTarget),
pipeline.InjectOriginalGVKFunction(idFactory).UsedFor(pipeline.ARMTarget),

pipeline.SimplifyDefinitions(),
Expand Down Expand Up @@ -194,17 +190,17 @@ func createAllPipelineStages(idFactory astmodel.IdentifierFactory, configuration
func (generator *CodeGenerator) Generate(ctx context.Context) error {
klog.V(1).Infof("Generator version: %v", pipeline.CombinedVersion())

defs := make(astmodel.Types)
state := pipeline.NewState()
for i, stage := range generator.pipeline {
klog.V(0).Infof("%d/%d: %s", i+1, len(generator.pipeline), stage.Description())
// Defensive copy (in case the pipeline modifies its inputs) so that we can compare types in vs out
defsOut, err := stage.Run(ctx, defs.Copy())
stateOut, err := stage.Run(ctx, state)
if err != nil {
return errors.Wrapf(err, "failed during pipeline stage %d/%d: %s", i+1, len(generator.pipeline), stage.Description())
}

defsAdded := defsOut.Except(defs)
defsRemoved := defs.Except(defsOut)
defsAdded := stateOut.Types().Except(state.Types())
defsRemoved := state.Types().Except(stateOut.Types())

if len(defsAdded) > 0 && len(defsRemoved) > 0 {
klog.V(1).Infof("Added %d, removed %d type definitions", len(defsAdded), len(defsRemoved))
Expand All @@ -214,7 +210,7 @@ func (generator *CodeGenerator) Generate(ctx context.Context) error {
klog.V(1).Infof("Removed %d type definitions", len(defsRemoved))
}

defs = defsOut
state = stateOut
}

klog.Info("Finished")
Expand Down
2 changes: 1 addition & 1 deletion hack/generator/pkg/codegen/code_generator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ func TestRemoveStages_PanicsForUnknownStage(t *testing.T) {
}

func MakeFakePipelineStage(id string) pipeline.Stage {
return pipeline.MakeStage(
return pipeline.MakeLegacyStage(
id, "Stage "+id, func(ctx context.Context, types astmodel.Types) (astmodel.Types, error) {
return types, nil
})
Expand Down
10 changes: 5 additions & 5 deletions hack/generator/pkg/codegen/golden_files_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ func makeEmbeddedTestTypeDefinition() astmodel.TypeDefinition {
}

func injectEmbeddedStructType() pipeline.Stage {
return pipeline.MakeStage(
return pipeline.MakeLegacyStage(
"injectEmbeddedStructType",
"Injects an embedded struct into each object",
func(ctx context.Context, defs astmodel.Types) (astmodel.Types, error) {
Expand Down Expand Up @@ -194,7 +194,7 @@ func loadTestSchemaIntoTypes(
path string) pipeline.Stage {
source := configuration.SchemaURL

return pipeline.MakeStage(
return pipeline.MakeLegacyStage(
"loadTestSchema",
"Load and walk schema (test)",
func(ctx context.Context, types astmodel.Types) (astmodel.Types, error) {
Expand Down Expand Up @@ -227,7 +227,7 @@ func loadTestSchemaIntoTypes(
func exportPackagesTestPipelineStage(t *testing.T, testName string) pipeline.Stage {
g := goldie.New(t)

return pipeline.MakeStage(
return pipeline.MakeLegacyStage(
"exportTestPackages",
"Export packages for test",
func(ctx context.Context, defs astmodel.Types) (astmodel.Types, error) {
Expand Down Expand Up @@ -272,7 +272,7 @@ func exportPackagesTestPipelineStage(t *testing.T, testName string) pipeline.Sta
}

func stripUnusedTypesPipelineStage() pipeline.Stage {
return pipeline.MakeStage(
return pipeline.MakeLegacyStage(
"stripUnused",
"Strip unused types for test",
func(ctx context.Context, defs astmodel.Types) (astmodel.Types, error) {
Expand All @@ -295,7 +295,7 @@ func stripUnusedTypesPipelineStage() pipeline.Stage {
// TODO: we're hard-coding references, and even if we were sourcing them from Swagger
// TODO: we have no way to give Swagger to the golden files tests currently.
func addCrossResourceReferencesForTest(idFactory astmodel.IdentifierFactory) pipeline.Stage {
return pipeline.MakeStage(
return pipeline.MakeLegacyStage(
"addCrossResourceReferences",
"Add cross resource references for test",
func(ctx context.Context, defs astmodel.Types) (astmodel.Types, error) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import (
// to all Kubernetes types.
// The genruntime.ARMTransformer interface is used to convert from the Kubernetes type to the corresponding ARM type and back.
func ApplyARMConversionInterface(idFactory astmodel.IdentifierFactory) Stage {
return MakeStage(
return MakeLegacyStage(
"applyArmConversionInterface",
"Add ARM conversion interfaces to Kubernetes types",
func(ctx context.Context, definitions astmodel.Types) (astmodel.Types, error) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ var armIDDescriptionRegex = regexp.MustCompile("(?i).*/subscriptions/.*?/resourc

// AddCrossResourceReferences replaces cross resource references with genruntime.ResourceReference.
func AddCrossResourceReferences(configuration *config.Configuration, idFactory astmodel.IdentifierFactory) Stage {
return MakeStage(
return MakeLegacyStage(
"addCrossResourceReferences",
"Replace cross-resource references with genruntime.ResourceReference",
func(ctx context.Context, definitions astmodel.Types) (astmodel.Types, error) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import (

// ApplyExportFilters creates a Stage to reduce our set of types for export
func ApplyExportFilters(configuration *config.Configuration) Stage {
return MakeStage(
return MakeLegacyStage(
"filterTypes",
"Apply export filters to reduce the number of generated types",
func(ctx context.Context, types astmodel.Types) (astmodel.Types, error) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import (
// ApplyKubernetesResourceInterface ensures that every Resource implements the KubernetesResource interface
func ApplyKubernetesResourceInterface(idFactory astmodel.IdentifierFactory) Stage {

return MakeStage(
return MakeLegacyStage(
"applyKubernetesResourceInterface",
"Add the KubernetesResource interface to every resource",
func(ctx context.Context, types astmodel.Types) (astmodel.Types, error) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import (
// been "lowered" to objects.
func ApplyPropertyRewrites(config *config.Configuration) Stage {

return MakeStage(
return MakeLegacyStage(
"propertyRewrites",
"Modify property types using configured transforms",
func(ctx context.Context, types astmodel.Types) (astmodel.Types, error) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import (
// has TypeName's that are all reachable as well. This check fails if there is any TypeName that refers to a type that doesn't
// exist.
func AssertTypesCollectionValid() Stage {
return MakeStage(
return MakeLegacyStage(
"assertTypesStructureValid",
"Verify that all local TypeNames refer to a type",
func(ctx context.Context, types astmodel.Types) (astmodel.Types, error) {
Expand Down
2 changes: 1 addition & 1 deletion hack/generator/pkg/codegen/pipeline/check_for_anytype.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ func checkForAnyType(description string, packages []string) Stage {
expectedPackages[p] = struct{}{}
}

return MakeStage(
return MakeLegacyStage(
"rogueCheck",
description,
func(ctx context.Context, defs astmodel.Types) (astmodel.Types, error) {
Expand Down
20 changes: 14 additions & 6 deletions hack/generator/pkg/codegen/pipeline/check_for_anytype_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ func TestFindsAnyTypes(t *testing.T) {
add := func(p astmodel.PackageReference, n string, t astmodel.Type) {
defs.Add(astmodel.MakeTypeDefinition(astmodel.MakeTypeName(p, n), t))
}

// A couple of types in the same package...
add(p1, "A", astmodel.AnyType)
add(p1, "B", astmodel.NewObjectType().WithProperties(
Expand All @@ -38,9 +39,11 @@ func TestFindsAnyTypes(t *testing.T) {
// One that's fine.
add(p3, "C", astmodel.NewArrayType(astmodel.IntType))

results, err := FilterOutDefinitionsUsingAnyType(nil).action(context.Background(), defs)
state := NewState().WithTypes(defs)
stage := FilterOutDefinitionsUsingAnyType(nil)
finalState, err := stage.Run(context.Background(), state)

g.Expect(results).To(HaveLen(0))
g.Expect(finalState).To(BeNil())
g.Expect(err).To(MatchError("AnyTypes found - add exclusions for: horo.logy/v20200730, road.train/v20200730"))
}

Expand All @@ -66,14 +69,16 @@ func TestIgnoresExpectedAnyTypePackages(t *testing.T) {
add(p3, "C", astmodel.NewArrayType(astmodel.IntType))

exclusions := []string{"horo.logy/v20200730", "road.train/v20200730"}
results, err := FilterOutDefinitionsUsingAnyType(exclusions).action(context.Background(), defs)

state := NewState().WithTypes(defs)
finalState, err := FilterOutDefinitionsUsingAnyType(exclusions).action(context.Background(), state)
g.Expect(err).To(BeNil())

expected := make(astmodel.Types)
expected.Add(astmodel.MakeTypeDefinition(
astmodel.MakeTypeName(p3, "C"), astmodel.NewArrayType(astmodel.IntType),
))
g.Expect(results).To(Equal(expected))
g.Expect(finalState.Types()).To(Equal(expected))
}

func TestComplainsAboutUnneededExclusions(t *testing.T) {
Expand Down Expand Up @@ -103,7 +108,10 @@ func TestComplainsAboutUnneededExclusions(t *testing.T) {
"gamma.knife/v20200821",
"road.train/v20200730",
}
results, err := FilterOutDefinitionsUsingAnyType(exclusions).action(context.Background(), defs)
g.Expect(results).To(HaveLen(0))

state := NewState().WithTypes(defs)
stage := FilterOutDefinitionsUsingAnyType(exclusions)
finalState, err := stage.Run(context.Background(), state)
g.Expect(finalState).To(BeNil())
g.Expect(errors.Cause(err)).To(MatchError("no AnyTypes found in: gamma.knife/v20200821, people.vultures/20200821"))
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import (
// CollapseCrossGroupReferences finds and removes references between API groups. This isn't particularly common
// but does occur in a few instances, for example from Microsoft.Compute -> Microsoft.Compute.Extensions.
func CollapseCrossGroupReferences() Stage {
return MakeStage(
return MakeLegacyStage(
"collapseCrossGroupReferences",
"Finds and removes cross group references",
func(ctx context.Context, types astmodel.Types) (astmodel.Types, error) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ var (

// ConvertAllOfAndOneOfToObjects reduces the AllOfType and OneOfType to ObjectType
func ConvertAllOfAndOneOfToObjects(idFactory astmodel.IdentifierFactory) Stage {
return MakeStage(
return MakeLegacyStage(
"allof-anyof-objects",
"Convert allOf and oneOf to object types",
func(ctx context.Context, defs astmodel.Types) (astmodel.Types, error) {
Expand Down
2 changes: 1 addition & 1 deletion hack/generator/pkg/codegen/pipeline/create_arm_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import (
// CreateARMTypes walks the type graph and builds new types for communicating
// with ARM
func CreateARMTypes(idFactory astmodel.IdentifierFactory) Stage {
return MakeStage(
return MakeLegacyStage(
"createArmTypes",
"Create types for interaction with ARM",
func(ctx context.Context, definitions astmodel.Types) (astmodel.Types, error) {
Expand Down
16 changes: 9 additions & 7 deletions hack/generator/pkg/codegen/pipeline/create_storage_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,11 @@ const CreateStorageTypesStageId = "createStorageTypes"
// CreateStorageTypes returns a pipeline stage that creates dedicated storage types for each resource and nested object.
// Storage versions are created for *all* API versions to allow users of older versions of the operator to easily
// upgrade. This is of course a bit odd for the first release, but defining the approach from day one is useful.
func CreateStorageTypes(conversionGraph *storage.ConversionGraph) Stage {
func CreateStorageTypes() Stage {
result := MakeStage(
CreateStorageTypesStageId,
"Create storage versions of CRD types",
func(ctx context.Context, types astmodel.Types) (astmodel.Types, error) {
func(ctx context.Context, state *State) (*State, error) {

// Predicate to isolate both resources and complex objects
isPropertyContainer := func(def astmodel.TypeDefinition) bool {
Expand All @@ -37,12 +37,13 @@ func CreateStorageTypes(conversionGraph *storage.ConversionGraph) Stage {
}

// Filter to the types we want to process
typesToConvert := types.Where(isPropertyContainer).Where(isNotARMType)
typesToConvert := state.Types().Where(isPropertyContainer).Where(isNotARMType)

storageTypes := make(astmodel.Types)
typeConverter := storage.NewTypeConverter(types)
typeConverter := storage.NewTypeConverter(state.Types())

// Create storage variants
conversionGraph := storage.NewConversionGraph()
for name, def := range typesToConvert {
storageDef, err := typeConverter.ConvertDefinition(def)
if err != nil {
Expand All @@ -53,9 +54,10 @@ func CreateStorageTypes(conversionGraph *storage.ConversionGraph) Stage {
conversionGraph.AddLink(name.PackageReference, storageDef.Name().PackageReference)
}

result := types.Copy()
result.AddTypes(storageTypes)
return result, nil
types := state.Types().Copy()
types.AddTypes(storageTypes)

return state.WithTypes(types).WithConversionGraph(conversionGraph), nil
})

result.RequiresPrerequisiteStages(injectOriginalVersionFunctionStageId)
Expand Down
10 changes: 4 additions & 6 deletions hack/generator/pkg/codegen/pipeline/create_storage_types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (
. "github.com/onsi/gomega"

"github.com/Azure/azure-service-operator/hack/generator/pkg/astmodel"
"github.com/Azure/azure-service-operator/hack/generator/pkg/codegen/storage"
"github.com/Azure/azure-service-operator/hack/generator/pkg/test"
)

Expand Down Expand Up @@ -42,14 +41,13 @@ func TestCreateStorageTypes(t *testing.T) {
types.AddAll(resourceV1, specV1, statusV1, resourceV2, specV2, statusV2, address2021)

// Run the stage

graph := storage.NewConversionGraph()
createStorageTypes := CreateStorageTypes(graph)
createStorageTypes := CreateStorageTypes()

// Don't need a context when testing
finalTypes, err := createStorageTypes.Run(context.TODO(), types)
state := NewState()
finalState, err := createStorageTypes.Run(context.TODO(), state)

g.Expect(err).To(Succeed())

test.AssertPackagesGenerateExpectedCode(t, finalTypes, t.Name())
test.AssertPackagesGenerateExpectedCode(t, finalState.Types(), t.Name())
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import (
// AddCrossplaneAtProvider adds an "AtProvider" property as the sole property in every resource status
func AddCrossplaneAtProvider(idFactory astmodel.IdentifierFactory) Stage {

return MakeStage(
return MakeLegacyStage(
"addCrossplaneAtProviderProperty",
"Add an 'AtProvider' property on every status",
func(ctx context.Context, types astmodel.Types) (astmodel.Types, error) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ var CrossplaneRuntimeV1Alpha1Package = astmodel.MakeExternalPackageReference("gi
// AddCrossplaneEmbeddedResourceSpec puts an embedded runtimev1alpha1.ResourceSpec on every spec type
func AddCrossplaneEmbeddedResourceSpec(idFactory astmodel.IdentifierFactory) Stage {

return MakeStage(
return MakeLegacyStage(
"addCrossplaneEmbeddedResourceSpec",
"Add an embedded runtimev1alpha1.ResourceSpec to every spec type",
func(ctx context.Context, types astmodel.Types) (astmodel.Types, error) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import (
// AddCrossplaneEmbeddedResourceStatus puts an embedded runtimev1alpha1.ResourceStatus on every spec type
func AddCrossplaneEmbeddedResourceStatus(idFactory astmodel.IdentifierFactory) Stage {

return MakeStage(
return MakeLegacyStage(
"addCrossplaneEmbeddedResourceStatus",
"Add an embedded runtimev1alpha1.ResourceStatus to every status type",
func(ctx context.Context, types astmodel.Types) (astmodel.Types, error) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import (
// and moves everything that was at the spec level down a level into the ForProvider type
func AddCrossplaneForProvider(idFactory astmodel.IdentifierFactory) Stage {

return MakeStage(
return MakeLegacyStage(
"addCrossplaneForProviderProperty",
"Add a 'ForProvider' property on every spec",
func(ctx context.Context, types astmodel.Types) (astmodel.Types, error) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import (
// AddCrossplaneOwnerProperties adds the 3-tuple of (xName, xNameRef, xNameSelector) for each owning resource
func AddCrossplaneOwnerProperties(idFactory astmodel.IdentifierFactory) Stage {

return MakeStage(
return MakeLegacyStage(
"addCrossplaneOwnerProperties",
"Add the 3-tuple of (xName, xNameRef, xNameSelector) for each owning resource",
func(ctx context.Context, types astmodel.Types) (astmodel.Types, error) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import (

// DeleteGeneratedCode creates a pipeline stage for cleanup of our output folder prior to generating files
func DeleteGeneratedCode(outputFolder string) Stage {
return MakeStage(
return MakeLegacyStage(
"deleteGenerated",
"Delete generated code from "+outputFolder,
func(ctx context.Context, types astmodel.Types) (astmodel.Types, error) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import (
const resourcesPropertyName = astmodel.PropertyName("Resources")

func DetermineResourceOwnership(configuration *config.Configuration) Stage {
return MakeStage(
return MakeLegacyStage(
"determineResourceOwnership",
"Determine ARM resource relationships",
func(ctx context.Context, types astmodel.Types) (astmodel.Types, error) {
Expand Down
Loading

0 comments on commit 9954b93

Please sign in to comment.