diff --git a/codegen/cmd/injection-gen/generators/comment_parser.go b/codegen/cmd/injection-gen/generators/comment_parser.go index ada0dff3ec..68ac8b4cba 100644 --- a/codegen/cmd/injection-gen/generators/comment_parser.go +++ b/codegen/cmd/injection-gen/generators/comment_parser.go @@ -19,25 +19,41 @@ import "strings" // Adapted from the k8s.io comment parser https://github.com/kubernetes/gengo/blob/master/types/comments.go +// CommentsTags maps marker prefixes to a set of tags containing keys and values +type CommentTags map[string]CommentTag + +// CommentTags maps keys to a list of values +type CommentTag map[string][]string + // ExtractCommentTags parses comments for lines of the form: // -// 'marker' + ':' "key=value,key2=value2". +// "marker" + "prefix" + ':' + "key=value,key2=value2". // -// Values are optional; empty map is the default. A tag can be specified more than +// In the following example the marker is '+' and the prefix is 'foo': +// +foo:key=value1,key2=value2,key=value3 +// +// Values are optional; empty map is the default. A tag can be specified more than // one time and all values are returned. If the resulting map has an entry for // a key, the value (a slice) is guaranteed to have at least 1 element. // // Example: if you pass "+" for 'marker', and the following lines are in // the comments: -// +foo:key=value1,key2=value2 +// +foo:key=value1,key2=value2,key=value3 // +bar // // Then this function will return: -// map[string]map[string]string{"foo":{"key":value1","key2":"value2"}, "bar": nil} +// map[string]map[string]string{ +// "foo":{ +// "key": []string{"value1", "value3"}, +// "key2": []string{"value2"} +// }, +// "bar": {}, +// } // // Users are not expected to repeat values. -func ExtractCommentTags(marker string, lines []string) map[string]map[string]string { - out := map[string]map[string]string{} +func ExtractCommentTags(marker string, lines []string) CommentTags { + out := CommentTags{} + for _, line := range lines { line = strings.TrimSpace(line) if len(line) == 0 || !strings.HasPrefix(line, marker) { @@ -45,29 +61,32 @@ func ExtractCommentTags(marker string, lines []string) map[string]map[string]str } options := strings.SplitN(line[len(marker):], ":", 2) + prefix := options[0] + if len(options) == 2 { vals := strings.Split(options[1], ",") - opts := out[options[0]] + opts := out[prefix] if opts == nil { - opts = make(map[string]string, len(vals)) + opts = make(CommentTag, len(vals)) + out[prefix] = opts } for _, pair := range vals { - if kv := strings.SplitN(pair, "=", 2); len(kv) == 2 { - opts[kv[0]] = kv[1] - } else if kv[0] != "" { - opts[kv[0]] = "" + kv := strings.SplitN(pair, "=", 2) + if len(kv) == 1 && kv[0] == "" { + continue + } + if _, ok := opts[kv[0]]; !ok { + opts[kv[0]] = []string{} + } + if len(kv) == 2 { + opts[kv[0]] = append(opts[kv[0]], kv[1]) } - } - if len(opts) == 0 { - out[options[0]] = nil - } else { - out[options[0]] = opts } } else if len(options) == 1 && options[0] != "" { - if _, has := out[options[0]]; !has { - out[options[0]] = nil + if _, has := out[prefix]; !has { + out[prefix] = CommentTag{} } } } diff --git a/codegen/cmd/injection-gen/generators/comment_parser_test.go b/codegen/cmd/injection-gen/generators/comment_parser_test.go index e45dd30046..bbe5fab45c 100644 --- a/codegen/cmd/injection-gen/generators/comment_parser_test.go +++ b/codegen/cmd/injection-gen/generators/comment_parser_test.go @@ -15,7 +15,11 @@ limitations under the License. */ package generators -import "testing" +import ( + "testing" + + "github.com/google/go-cmp/cmp" +) func TestParseComments(t *testing.T) { comment := []string{ @@ -26,28 +30,31 @@ func TestParseComments(t *testing.T) { "+with:option", "+pair:key=value", "+manypairs:key1=value1,key2=value2", + "+duplicate:key1=value1,key1=value2", } extracted := ExtractCommentTags("+", comment) - if val, ok := extracted["foo"]; !ok || val != nil { - t.Errorf("Failed to extract single key got=%t,%v want=true,nil", ok, val) - } - - if val, ok := extracted["bar"]; !ok || val != nil { - t.Errorf("Failed to extract single key got=%t,%v want=true,nil", ok, val) + expected := CommentTags{ + "foo": {}, + "bar": {}, + "with": { + "option": {}, + }, + "pair": { + "key": []string{"value"}, + }, + "manypairs": { + "key1": []string{"value1"}, + "key2": []string{"value2"}, + }, + "duplicate": { + "key1": []string{"value1", "value2"}, + }, } - if val, ok := extracted["with"]; !ok || val["option"] != "" { - t.Errorf(`Failed to extract single key got=%t,%v want=true,{"option":""}`, ok, val) - } - - if val, ok := extracted["pair"]; !ok || val["key"] != "value" { - t.Errorf(`Failed to extract single key got=%t,%v want=true,{"key":"value"}`, ok, val) - } - - if val, ok := extracted["manypairs"]; !ok || val["key1"] != "value1" || val["key2"] != "value2" { - t.Errorf(`Failed to extract single key got=%t,%v want=true,{"key":"value"}`, ok, val) + if diff := cmp.Diff(expected, extracted); diff != "" { + t.Error("diff (-want, +got): ", diff) } } @@ -61,24 +68,25 @@ func TestMergeDuplicates(t *testing.T) { "+bar", "+manypairs:key1=value1", "+manypairs:key2=value2", + "+manypairs:key1=value3", "+oops:,,,", } extracted := ExtractCommentTags("+", comment) - if val, ok := extracted["foo"]; !ok || val != nil { - t.Errorf("Failed to extract single key got=%t,%v want=true,nil", ok, val) - } - - if val, ok := extracted["bar"]; !ok || val["key"] != "value" { - t.Errorf(`Failed to extract single key got=%t,%v want=true,{"key":"value"}`, ok, val) - } - - if val, ok := extracted["manypairs"]; !ok || val["key1"] != "value1" || val["key2"] != "value2" { - t.Errorf(`Failed to extract single key got=%t,%v want=true,{"key":"value"}`, ok, val) + expected := CommentTags{ + "foo": {}, + "bar": { + "key": []string{"value"}, + }, + "manypairs": { + "key1": []string{"value1", "value3"}, + "key2": []string{"value2"}, + }, + "oops": {}, } - if val, ok := extracted["oops"]; !ok || val != nil { - t.Errorf(`Failed to extract single key got=%t,%v want=true,{"oops":nil}`, ok, val) + if diff := cmp.Diff(expected, extracted); diff != "" { + t.Error("diff (-want, +got): ", diff) } } diff --git a/codegen/cmd/injection-gen/generators/packages.go b/codegen/cmd/injection-gen/generators/packages.go index 1900802fad..f79339824b 100644 --- a/codegen/cmd/injection-gen/generators/packages.go +++ b/codegen/cmd/injection-gen/generators/packages.go @@ -193,29 +193,36 @@ func MustParseClientGenTags(lines []string) Tags { return ret } -func extractCommentTags(t *types.Type) map[string]map[string]string { +func extractCommentTags(t *types.Type) CommentTags { comments := append(append([]string{}, t.SecondClosestCommentLines...), t.CommentLines...) return ExtractCommentTags("+", comments) } -func extractReconcilerClassTag(tags map[string]map[string]string) (string, bool) { +func extractReconcilerClassesTag(tags CommentTags) ([]string, bool) { vals, ok := tags["genreconciler"] if !ok { - return "", false + return nil, false } - classname, has := vals["class"] - return classname, has + classnames, has := vals["class"] + if has && len(classnames) == 0 { + return nil, false + } + return classnames, has } -func isKRShaped(tags map[string]map[string]string) bool { +func isKRShaped(tags CommentTags) bool { vals, has := tags["genreconciler"] if !has { return false } - return vals["krshapedlogic"] != "false" + stringVals, has := vals["krshapedlogic"] + if !has || len(vals) == 0 { + return true // Default is true + } + return stringVals[0] != "false" } -func isNonNamespaced(tags map[string]map[string]string) bool { +func isNonNamespaced(tags CommentTags) bool { vals, has := tags["genclient"] if !has { return false @@ -224,7 +231,7 @@ func isNonNamespaced(tags map[string]map[string]string) bool { return has } -func stubs(tags map[string]map[string]string) bool { +func stubs(tags CommentTags) bool { vals, has := tags["genreconciler"] if !has { return false @@ -545,7 +552,7 @@ func reconcilerPackages(basePackage string, groupPkgName string, gv clientgentyp // Fix for golang iterator bug. t := t extracted := extractCommentTags(t) - reconcilerClass, hasReconcilerClass := extractReconcilerClassTag(extracted) + reconcilerClasses, hasReconcilerClass := extractReconcilerClassesTag(extracted) nonNamespaced := isNonNamespaced(extracted) isKRShaped := isKRShaped(extracted) stubs := stubs(extracted) @@ -573,7 +580,7 @@ func reconcilerPackages(basePackage string, groupPkgName string, gv clientgentyp clientPkg: clientPackagePath, informerPackagePath: informerPackagePath, schemePkg: filepath.Join(customArgs.VersionedClientSetPackage, "scheme"), - reconcilerClass: reconcilerClass, + reconcilerClasses: reconcilerClasses, hasReconcilerClass: hasReconcilerClass, hasStatus: hasStatus(t), }) @@ -602,7 +609,7 @@ func reconcilerPackages(basePackage string, groupPkgName string, gv clientgentyp outputPackage: filepath.Join(packagePath, "stub"), imports: generator.NewImportTracker(), informerPackagePath: informerPackagePath, - reconcilerClass: reconcilerClass, + reconcilerClasses: reconcilerClasses, hasReconcilerClass: hasReconcilerClass, }) return generators @@ -633,7 +640,7 @@ func reconcilerPackages(basePackage string, groupPkgName string, gv clientgentyp listerPkg: listerPackagePath, groupGoName: groupGoName, groupVersion: gv, - reconcilerClass: reconcilerClass, + reconcilerClasses: reconcilerClasses, hasReconcilerClass: hasReconcilerClass, nonNamespaced: nonNamespaced, isKRShaped: isKRShaped, diff --git a/codegen/cmd/injection-gen/generators/reconciler_controller.go b/codegen/cmd/injection-gen/generators/reconciler_controller.go index a624275a50..63556dec60 100644 --- a/codegen/cmd/injection-gen/generators/reconciler_controller.go +++ b/codegen/cmd/injection-gen/generators/reconciler_controller.go @@ -38,7 +38,7 @@ type reconcilerControllerGenerator struct { schemePkg string informerPackagePath string - reconcilerClass string + reconcilerClasses []string hasReconcilerClass bool hasStatus bool } @@ -69,7 +69,7 @@ func (g *reconcilerControllerGenerator) GenerateType(c *generator.Context, t *ty m := map[string]interface{}{ "type": t, "group": g.groupName, - "class": g.reconcilerClass, + "classes": g.reconcilerClasses, "hasClass": g.hasReconcilerClass, "hasStatus": g.hasStatus, "controllerImpl": c.Universe.Type(types.Name{ @@ -195,12 +195,24 @@ var reconcilerControllerNewImpl = ` const ( defaultControllerAgentName = "{{.type|lowercaseSingular}}-controller" defaultFinalizerName = "{{.type|allLowercasePlural}}.{{.group}}" - {{if .hasClass}} + {{if .hasClass }} // ClassAnnotationKey points to the annotation for the class of this resource. - ClassAnnotationKey = "{{ .class }}" + {{if gt (.classes | len) 1 }}// Deprecated: Use ClassAnnotationKeys given multiple keys exist + {{end -}} + ClassAnnotationKey = "{{ index .classes 0 }}" {{end}} ) +{{if gt (.classes | len) 1 }} +var ( + // ClassAnnotationKeys points to the annotation for the class of this resource. + ClassAnnotationKeys = []string{ + {{range $class := .classes}}"{{$class}}", + {{end}} + } +) +{{end}} + // NewImpl returns a {{.controllerImpl|raw}} that handles queuing and feeding work from // the queue through an implementation of {{.controllerReconciler|raw}}, delegating to // the provided Interface and optional Finalizer methods. OptionsFn is used to return diff --git a/codegen/cmd/injection-gen/generators/reconciler_controller_stub.go b/codegen/cmd/injection-gen/generators/reconciler_controller_stub.go index 37db246c35..13dff4d516 100644 --- a/codegen/cmd/injection-gen/generators/reconciler_controller_stub.go +++ b/codegen/cmd/injection-gen/generators/reconciler_controller_stub.go @@ -35,7 +35,7 @@ type reconcilerControllerStubGenerator struct { reconcilerPkg string informerPackagePath string - reconcilerClass string + reconcilerClasses []string hasReconcilerClass bool } @@ -64,7 +64,7 @@ func (g *reconcilerControllerStubGenerator) GenerateType(c *generator.Context, t m := map[string]interface{}{ "type": t, - "class": g.reconcilerClass, + "classes": g.reconcilerClasses, "hasClass": g.hasReconcilerClass, "informerGet": c.Universe.Function(types.Name{ Package: g.informerPackagePath, @@ -91,10 +91,18 @@ func (g *reconcilerControllerStubGenerator) GenerateType(c *generator.Context, t Package: g.reconcilerPkg, Name: "ClassAnnotationKey", }), + "classAnnotationKeys": c.Universe.Variable(types.Name{ + Package: g.reconcilerPkg, + Name: "ClassAnnotationKeys", + }), "annotationFilterFunc": c.Universe.Function(types.Name{ Package: "knative.dev/pkg/reconciler", Name: "AnnotationFilterFunc", }), + "orFilterFunc": c.Universe.Function(types.Name{ + Package: "knative.dev/pkg/reconciler", + Name: "Or", + }), "filterHandler": c.Universe.Type(types.Name{ Package: "k8s.io/client-go/tools/cache", Name: "FilteringResourceEventHandler", @@ -120,7 +128,17 @@ func NewController( {{if .hasClass}} classValue := "default" // TODO: update this to the appropriate value. + {{if len .classes | eq 1 }} classFilter := {{.annotationFilterFunc|raw}}({{.classAnnotationKey|raw}}, classValue, false /*allowUnset*/) + {{else if gt (len .classes) 1 }} + filterFuncs := make([]func(interface{}) bool, 0, len({{.classAnnotationKeys|raw}})) + + for _, key := range {{.classAnnotationKeys|raw}} { + filterFuncs = append(filterFuncs, {{.annotationFilterFunc|raw}}(key, classValue, false /*allowUnset*/)) + } + + classFilter := {{.orFilterFunc|raw}}(filterFuncs...) + {{end}} {{end}} // TODO: setup additional informers here. diff --git a/codegen/cmd/injection-gen/generators/reconciler_reconciler.go b/codegen/cmd/injection-gen/generators/reconciler_reconciler.go index e787096b3f..5e3f23faf4 100644 --- a/codegen/cmd/injection-gen/generators/reconciler_reconciler.go +++ b/codegen/cmd/injection-gen/generators/reconciler_reconciler.go @@ -37,7 +37,7 @@ type reconcilerReconcilerGenerator struct { listerName string listerPkg string - reconcilerClass string + reconcilerClasses []string hasReconcilerClass bool nonNamespaced bool isKRShaped bool @@ -74,7 +74,7 @@ func (g *reconcilerReconcilerGenerator) GenerateType(c *generator.Context, t *ty "type": t, "group": namer.IC(g.groupGoName), "version": namer.IC(g.groupVersion.Version.String()), - "class": g.reconcilerClass, + "classes": g.reconcilerClasses, "hasClass": g.hasReconcilerClass, "isKRShaped": g.isKRShaped, "hasStatus": g.hasStatus, @@ -219,6 +219,9 @@ func (g *reconcilerReconcilerGenerator) GenerateType(c *generator.Context, t *ty sw.Do(reconcilerInterfaceFactory, m) sw.Do(reconcilerNewReconciler, m) sw.Do(reconcilerImplFactory, m) + if len(g.reconcilerClasses) > 1 { + sw.Do(reconcilerLookupClass, m) + } if g.hasStatus { sw.Do(reconcilerStatusFactory, m) } @@ -227,6 +230,17 @@ func (g *reconcilerReconcilerGenerator) GenerateType(c *generator.Context, t *ty return sw.Error() } +var reconcilerLookupClass = ` +func lookupClass(annotations map[string]string) (string, bool) { + for _, key := range ClassAnnotationKeys { + if val, ok := annotations[key]; ok { + return val, true + } + } + return "", false +} +` + var reconcilerInterfaceFactory = ` // Interface defines the strongly typed interfaces to be implemented by a // controller reconciling {{.type|raw}}. @@ -293,8 +307,15 @@ type reconcilerImpl struct { skipStatusUpdates bool {{end}} - {{if .hasClass}} - // classValue is the resource annotation[{{ .class }}] instance value this reconciler instance filters on. + {{if len .classes | eq 1 }} + // classValue is the resource annotation[{{ index .classes 0 }}] instance value this reconciler instance filters on. + classValue string + {{else if gt (len .classes) 1 }} + // classValue is the resource annotation instance value this reconciler instance filters on. + // The annotations key are: + {{- range $class := .classes}} + // {{$class}} + {{- end}} classValue string {{end}} } @@ -416,14 +437,22 @@ func (r *reconcilerImpl) Reconcile(ctx {{.contextContext|raw}}, key string) erro } else if err != nil { return err } - {{if .hasClass}} + +{{if len .classes | eq 1 }} if classValue, found := original.GetAnnotations()[ClassAnnotationKey]; !found || classValue != r.classValue { logger.Debugw("Skip reconciling resource, class annotation value does not match reconciler instance value.", zap.String("classKey", ClassAnnotationKey), zap.String("issue", classValue+"!="+r.classValue)) return nil } - {{end}} +{{else if gt (len .classes) 1 }} + if classValue, found := lookupClass(original.GetAnnotations()); !found || classValue != r.classValue { + logger.Debugw("Skip reconciling resource, class annotation value does not match reconciler instance value.", + zap.Strings("classKeys", ClassAnnotationKeys), + zap.String("issue", classValue+"!="+r.classValue)) + return nil + } +{{end}} // Don't modify the informers copy. resource := original.DeepCopy() diff --git a/reconciler/filter.go b/reconciler/filter.go index 36c86c4d37..9f7d2fa364 100644 --- a/reconciler/filter.go +++ b/reconciler/filter.go @@ -77,6 +77,18 @@ func Not(f func(interface{}) bool) func(interface{}) bool { } } +// Or creates a FilterFunc which performs an OR of the passed in FilterFuncs +func Or(funcs ...func(interface{}) bool) func(interface{}) bool { + return func(obj interface{}) bool { + for _, f := range funcs { + if f(obj) { + return true + } + } + return false + } +} + // ChainFilterFuncs creates a FilterFunc which performs an AND of the passed FilterFuncs. func ChainFilterFuncs(funcs ...func(interface{}) bool) func(interface{}) bool { return func(obj interface{}) bool {