Skip to content

Commit c98acdb

Browse files
Merge pull request #2093 from JoelSpeed/codegen-info-warn
Update schema checker and output info/warning level messages visually
2 parents ec9bf3f + fec2140 commit c98acdb

File tree

18 files changed

+353
-75
lines changed

18 files changed

+353
-75
lines changed

tools/codegen/cmd/root.go

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ func init() {
8080
// Subsequent groups will continue to generate.
8181
func executeGenerators(genCtx generation.Context, generators ...generation.Generator) error {
8282
errs := []error{}
83+
resultsByGroup := map[string][]generation.Result{}
8384

8485
for _, group := range genCtx.APIGroups {
8586
klog.Infof("Running generators for %s", group.Name)
@@ -90,15 +91,19 @@ func executeGenerators(genCtx generation.Context, generators ...generation.Gener
9091
g = g.ApplyConfig(group.Config)
9192
}
9293

93-
if err := g.GenGroup(group); err != nil {
94+
results, err := g.GenGroup(group)
95+
if err != nil {
9496
errs = append(errs, fmt.Errorf("error running generator %s on group %s: %w", gen.Name(), group.Name, err))
95-
96-
// Don't run any later generators for this group.
97-
break
9897
}
98+
99+
resultsByGroup[group.Name] = append(resultsByGroup[group.Name], results...)
99100
}
100101
}
101102

103+
if err := utils.PrintResults(resultsByGroup); err != nil {
104+
errs = append(errs, err)
105+
}
106+
102107
if len(errs) > 0 {
103108
return utils.NewAggregatePrinter(kerrors.NewAggregate(errs))
104109
}

tools/codegen/pkg/compatibility/generator.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -52,10 +52,10 @@ func (g *generator) Name() string {
5252
}
5353

5454
// GenGroup runs the compatibility generator against the given group context.
55-
func (g *generator) GenGroup(groupCtx generation.APIGroupContext) error {
55+
func (g *generator) GenGroup(groupCtx generation.APIGroupContext) ([]generation.Result, error) {
5656
if g.disabled {
5757
klog.V(2).Infof("Skipping compatibility generation for %s", groupCtx.Name)
58-
return nil
58+
return nil, nil
5959
}
6060

6161
for _, version := range groupCtx.Versions {
@@ -67,9 +67,9 @@ func (g *generator) GenGroup(groupCtx generation.APIGroupContext) error {
6767
klog.V(1).Infof("%s compatibility level comments for %s/%s", action, groupCtx.Name, version.Name)
6868

6969
if err := insertCompatibilityLevelComments(version.Path, g.verify); err != nil {
70-
return fmt.Errorf("could not insert compatibility level comments for %s/%s: %w", groupCtx.Name, version.Name, err)
70+
return nil, fmt.Errorf("could not insert compatibility level comments for %s/%s: %w", groupCtx.Name, version.Name, err)
7171
}
7272
}
7373

74-
return nil
74+
return nil, nil
7575
}

tools/codegen/pkg/deepcopy/generator.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -78,18 +78,18 @@ func (g *generator) Name() string {
7878
}
7979

8080
// GenGroup runs the deepcopy generator against the given group context.
81-
func (g *generator) GenGroup(groupCtx generation.APIGroupContext) error {
81+
func (g *generator) GenGroup(groupCtx generation.APIGroupContext) ([]generation.Result, error) {
8282
if g.disabled {
8383
klog.V(2).Infof("Skipping deepcopy generation for %s", groupCtx.Name)
84-
return nil
84+
return nil, nil
8585
}
8686

8787
// If there is no header file, create an empty file and pass that through.
8888
headerFilePath := g.headerFilePath
8989
if headerFilePath == "" {
9090
tmpFile, err := os.CreateTemp("", "deepcopy-header-*.txt")
9191
if err != nil {
92-
return fmt.Errorf("failed to create temporary file: %w", err)
92+
return nil, fmt.Errorf("failed to create temporary file: %w", err)
9393
}
9494
tmpFile.Close()
9595

@@ -107,9 +107,9 @@ func (g *generator) GenGroup(groupCtx generation.APIGroupContext) error {
107107
klog.V(1).Infof("%s deepcopy functions for for %s/%s", action, groupCtx.Name, version.Name)
108108

109109
if err := generateDeepcopyFunctions(version.Path, version.PackagePath, g.outputBaseFileName, headerFilePath, g.verify); err != nil {
110-
return fmt.Errorf("could not generate deepcopy functions for %s/%s: %w", groupCtx.Name, version.Name, err)
110+
return nil, fmt.Errorf("could not generate deepcopy functions for %s/%s: %w", groupCtx.Name, version.Name, err)
111111
}
112112
}
113113

114-
return nil
114+
return nil, nil
115115
}

tools/codegen/pkg/emptypartialschemas/generator.go

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,13 @@ package emptypartialschemas
22

33
import (
44
"fmt"
5+
"os"
6+
"path/filepath"
7+
58
"github.com/openshift/api/tools/codegen/pkg/generation"
69
"github.com/openshift/api/tools/codegen/pkg/utils"
710
"k8s.io/gengo/args"
811
"k8s.io/klog/v2"
9-
"os"
10-
"path/filepath"
1112
)
1213

1314
// Options contains the configuration required for the compatibility generator.
@@ -68,10 +69,10 @@ func (g *generator) Name() string {
6869
}
6970

7071
// GenGroup runs the empty-partial-schemas generator against the given group context.
71-
func (g *generator) GenGroup(groupCtx generation.APIGroupContext) error {
72+
func (g *generator) GenGroup(groupCtx generation.APIGroupContext) ([]generation.Result, error) {
7273
if g.disabled {
7374
klog.V(2).Infof("Skipping %q generation for %s", g.Name(), groupCtx.Name)
74-
return nil
75+
return nil, nil
7576
}
7677

7778
for _, version := range groupCtx.Versions {
@@ -83,11 +84,11 @@ func (g *generator) GenGroup(groupCtx generation.APIGroupContext) error {
8384
klog.Infof("%s %q functions for for %s/%s", action, g.Name(), groupCtx.Name, version.Name)
8485

8586
if err := g.generatePartialSchemaFiles(version.Path, version.PackagePath, g.verify); err != nil {
86-
return fmt.Errorf("could not generate %v functions for %s/%s: %w", g.Name(), groupCtx.Name, version.Name, err)
87+
return nil, fmt.Errorf("could not generate %v functions for %s/%s: %w", g.Name(), groupCtx.Name, version.Name, err)
8788
}
8889
}
8990

90-
return nil
91+
return nil, nil
9192
}
9293

9394
// generatePartialSchemaFiles generates the DeepCopy functions for the given API package paths.

tools/codegen/pkg/generation/generator.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ type Generator interface {
66
Name() string
77

88
// GenGroup runs the generator against the given APIGroupContext.
9-
GenGroup(APIGroupContext) error
9+
GenGroup(APIGroupContext) ([]Result, error)
1010

1111
// ApplyConfig creates a new generator instance with the given configuration.
1212
ApplyConfig(*Config) Generator
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
package generation
2+
3+
type Result struct {
4+
// Generator is the name of the generator that produced this result.
5+
Generator string
6+
7+
// Group is the name of the API group that was processed.
8+
Group string
9+
10+
// Version is the version of the API group that was processed.
11+
Version string
12+
13+
// Manifest is the path to the file the was processed.
14+
Manifest string
15+
16+
// Info contains informational messages from the generator.
17+
Info []string
18+
19+
// Warnings contains warning messages from the generator.
20+
Warnings []string
21+
22+
// Errors contains error messages from the generator.
23+
Errors []error
24+
}

tools/codegen/pkg/manifestmerge/generator.go

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,17 @@ import (
55
_ "embed"
66
"encoding/json"
77
"fmt"
8-
"github.com/google/go-cmp/cmp"
9-
"k8s.io/apimachinery/pkg/api/equality"
10-
"k8s.io/apimachinery/pkg/util/sets"
118
"math/rand"
129
"os"
1310
"strings"
1411
"sync"
1512

13+
"github.com/google/go-cmp/cmp"
14+
"k8s.io/apimachinery/pkg/api/equality"
15+
"k8s.io/apimachinery/pkg/util/sets"
16+
17+
"path/filepath"
18+
1619
"github.com/openshift/api/tools/codegen/pkg/generation"
1720
"github.com/openshift/api/tools/codegen/pkg/utils"
1821
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
@@ -25,7 +28,6 @@ import (
2528
"k8s.io/kube-openapi/pkg/spec3"
2629
"k8s.io/kube-openapi/pkg/validation/spec"
2730
"k8s.io/utils/pointer"
28-
"path/filepath"
2931
kyaml "sigs.k8s.io/yaml"
3032
)
3133

@@ -104,10 +106,10 @@ func (g *generator) Name() string {
104106
}
105107

106108
// GenGroup runs the schemapatch generator against the given group context.
107-
func (g *generator) GenGroup(groupCtx generation.APIGroupContext) error {
109+
func (g *generator) GenGroup(groupCtx generation.APIGroupContext) ([]generation.Result, error) {
108110
if g.disabled {
109111
klog.V(2).Infof("Skipping %q for %s", g.Name(), groupCtx.Name)
110-
return nil
112+
return nil, nil
111113
}
112114

113115
versionPaths := allVersionPaths(groupCtx.Versions)
@@ -128,10 +130,10 @@ func (g *generator) GenGroup(groupCtx generation.APIGroupContext) error {
128130
}
129131

130132
if len(errs) > 0 {
131-
return kerrors.NewAggregate(errs)
133+
return nil, kerrors.NewAggregate(errs)
132134
}
133135

134-
return nil
136+
return nil, nil
135137
}
136138

137139
// genGroupVersion runs the schemapatch generator against a particular version of the API group.

tools/codegen/pkg/schemacheck/generator.go

Lines changed: 32 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package schemacheck
22

33
import (
4+
"errors"
45
"fmt"
56
"os"
67
"path/filepath"
@@ -79,10 +80,10 @@ func (g *generator) Name() string {
7980
}
8081

8182
// GenGroup runs the schemacheck generator against the given group context.
82-
func (g *generator) GenGroup(groupCtx generation.APIGroupContext) error {
83+
func (g *generator) GenGroup(groupCtx generation.APIGroupContext) ([]generation.Result, error) {
8384
if g.disabled {
8485
klog.V(2).Infof("Skipping API schema check for %s", groupCtx.Name)
85-
return nil
86+
return nil, nil
8687
}
8788

8889
errs := []error{}
@@ -92,73 +93,80 @@ func (g *generator) GenGroup(groupCtx generation.APIGroupContext) error {
9293
comparatorOptions.DisabledComparators = g.disabledComparators
9394

9495
if err := comparatorOptions.Validate(); err != nil {
95-
return fmt.Errorf("could not validate comparator options: %w", err)
96+
return nil, fmt.Errorf("could not validate comparator options: %w", err)
9697
}
9798

9899
comparatorConfig, err := comparatorOptions.Complete()
99100
if err != nil {
100-
return fmt.Errorf("could not complete comparator options: %w", err)
101+
return nil, fmt.Errorf("could not complete comparator options: %w", err)
101102
}
102103

104+
var results []generation.Result
105+
103106
for _, version := range groupCtx.Versions {
104107
klog.V(1).Infof("Verifying API schema for for %s/%s", groupCtx.Name, version.Name)
105108

106-
if err := g.genGroupVersion(groupCtx.Name, version, comparatorConfig); err != nil {
109+
r, err := g.genGroupVersion(groupCtx.Name, version, comparatorConfig)
110+
if err != nil {
107111
errs = append(errs, fmt.Errorf("could not run schemacheck generator for group/version %s/%s: %w", groupCtx.Name, version.Name, err))
108112
}
113+
114+
results = append(results, r...)
109115
}
110116

111117
if len(errs) > 0 {
112-
return kerrors.NewAggregate(errs)
118+
return results, kerrors.NewAggregate(errs)
113119
}
114120

115-
return nil
121+
return results, nil
116122
}
117123

118124
// genGroupVersion runs the schemacheck generator against a particular version of the API group.
119-
func (g *generator) genGroupVersion(group string, version generation.APIVersionContext, comparatorConfig *options.ComparatorConfig) error {
125+
func (g *generator) genGroupVersion(group string, version generation.APIVersionContext, comparatorConfig *options.ComparatorConfig) ([]generation.Result, error) {
120126
contexts, err := loadSchemaCheckGenerationContextsForVersion(version, g.comparisonBase)
121127
if err != nil {
122-
return fmt.Errorf("could not load schema check generation contexts for group/version %s/%s: %w", group, version.Name, err)
128+
return nil, fmt.Errorf("could not load schema check generation contexts for group/version %s/%s: %w", group, version.Name, err)
123129
}
124130

125131
if len(contexts) == 0 {
126132
klog.V(1).Infof("No CRD manifests found for %s/%s", group, version.Name)
127-
return nil
133+
return nil, nil
128134
}
129135

130136
var manifestErrs []error
137+
var results []generation.Result
131138

132139
for _, context := range contexts {
133140
klog.V(1).Infof("Verifying schema for %s\n", context.manifestName)
134141
comparisonResults, errs := comparatorConfig.ComparatorRegistry.Compare(context.oldCRD, context.manifestCRD, comparatorConfig.ComparatorNames...)
135-
if len(errs) > 0 {
136-
return fmt.Errorf("could not compare manifests for %s: %w", context.manifestName, kerrors.NewAggregate(errs))
142+
143+
result := generation.Result{
144+
Generator: g.Name(),
145+
Group: group,
146+
Version: version.Name,
147+
Manifest: context.manifestName,
148+
Errors: errs,
137149
}
138150

151+
manifestErrs = append(manifestErrs, errs...)
152+
139153
for _, comparisonResult := range comparisonResults {
140154
for _, msg := range comparisonResult.Errors {
141-
manifestErrs = append(manifestErrs, fmt.Errorf("error in %s: %s: %v", context.manifestName, comparisonResult.Name, msg))
155+
manifestErrs = append(manifestErrs, errors.New(msg))
156+
result.Errors = append(result.Errors, errors.New(msg))
142157
}
143158
}
144-
145159
for _, comparisonResult := range comparisonResults {
146-
for _, msg := range comparisonResult.Warnings {
147-
klog.Warningf("warning in %s: %s: %v", context.manifestName, comparisonResult.Name, msg)
148-
}
160+
result.Warnings = append(result.Warnings, comparisonResult.Warnings...)
149161
}
150162
for _, comparisonResult := range comparisonResults {
151-
for _, msg := range comparisonResult.Infos {
152-
klog.Infof("info in %s: %s: %v", context.manifestName, comparisonResult.Name, msg)
153-
}
163+
result.Info = append(result.Info, comparisonResult.Infos...)
154164
}
155-
}
156165

157-
if len(manifestErrs) > 0 {
158-
return kerrors.NewAggregate(manifestErrs)
166+
results = append(results, result)
159167
}
160168

161-
return nil
169+
return results, kerrors.NewAggregate(manifestErrs)
162170
}
163171

164172
// schemaCheckGenerationContext contains the context required to verify the schema for a particular

tools/codegen/pkg/schemapatch/generator.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -84,10 +84,10 @@ func (g *generator) Name() string {
8484
}
8585

8686
// GenGroup runs the schemapatch generator against the given group context.
87-
func (g *generator) GenGroup(groupCtx generation.APIGroupContext) error {
87+
func (g *generator) GenGroup(groupCtx generation.APIGroupContext) ([]generation.Result, error) {
8888
if g.disabled {
8989
klog.V(2).Infof("Skipping API schema generation for %s", groupCtx.Name)
90-
return nil
90+
return nil, nil
9191
}
9292

9393
versionPaths := allVersionPaths(groupCtx.Versions)
@@ -97,7 +97,7 @@ func (g *generator) GenGroup(groupCtx generation.APIGroupContext) error {
9797
for _, version := range groupCtx.Versions {
9898
versionRequired, err := shouldProcessGroupVersion(version, g.requiredFeatureSets)
9999
if err != nil {
100-
return fmt.Errorf("could not determine if version %s is required: %w", version.Name, err)
100+
return nil, fmt.Errorf("could not determine if version %s is required: %w", version.Name, err)
101101
}
102102

103103
if !versionRequired {
@@ -117,10 +117,10 @@ func (g *generator) GenGroup(groupCtx generation.APIGroupContext) error {
117117
}
118118

119119
if len(errs) > 0 {
120-
return kerrors.NewAggregate(errs)
120+
return nil, kerrors.NewAggregate(errs)
121121
}
122122

123-
return nil
123+
return nil, nil
124124
}
125125

126126
// genGroupVersion runs the schemapatch generator against a particular version of the API group.

0 commit comments

Comments
 (0)