diff --git a/hack/generate-lib-resources.py b/hack/generate-lib-resources.py index f18a1dcc1f..1161f1b0e6 100755 --- a/hack/generate-lib-resources.py +++ b/hack/generate-lib-resources.py @@ -32,7 +32,7 @@ def generate_resourceread(directory, types): short_name = os.path.basename(base) imports[package] = '\t{}{} "{}"'.format(short_name, version, package) - lines.extend([import_line for _, import_line in sorted(imports.items(), key=lambda package_line: package_line[0])]) + lines.extend(group_imports(imports)) lines.extend([ ')', '', @@ -90,6 +90,7 @@ def generate_resourcebuilder(directory, types, clients, modifiers, health_checks 'import (', '\t"context"', '\t"fmt"', + '\t"sync"', '', ] @@ -128,7 +129,7 @@ def generate_resourcebuilder(directory, types, clients, modifiers, health_checks 'protobuf': client['package'].startswith('k8s.io/') and 'kube-aggregator' not in client['package'], } - lines.extend([import_line for _, import_line in sorted(imports.items(), key=lambda package_line: package_line[0])]) + lines.extend(group_imports(imports)) longest_property = max(len(prop_name) for prop_name in client_properties.keys()) @@ -150,19 +151,91 @@ def generate_resourcebuilder(directory, types, clients, modifiers, health_checks lines.extend([ '}', '', - 'func newBuilder(config *rest.Config, m manifest.Manifest) Interface {', - '\treturn &builder{', - '\t\traw: m.Raw,', + '// clientSet holds cached typed clients for resource building.', + 'type clientSet struct {', + ]) + lines.extend([ + '\t{:{width}} {}'.format(prop_name, data['type'], width=longest_property) + for prop_name, data in sorted(client_properties.items()) + ]) + lines.extend([ + '}', '', + 'func newClientSet(config *rest.Config) (*clientSet, error) {', ]) for prop_name, data in sorted(client_properties.items()): new_client_arg = 'config' if data.get('protobuf'): new_client_arg = 'withProtobuf({})'.format(new_client_arg) - lines.append('\t\t{:{width}} {}.NewForConfigOrDie({}),'.format(prop_name + ':', data['client_short_name'], new_client_arg, width=longest_property+1)) - + var_name = prop_name[0].lower() + prop_name[1:] + lines.extend([ + '\t{}, err := {}.NewForConfig({})'.format(var_name, data['client_short_name'], new_client_arg), + '\tif err != nil {', + '\t\treturn nil, err', + '\t}', + ]) + lines.extend([ + '\treturn &clientSet{', + ]) + for prop_name, data in sorted(client_properties.items()): + var_name = prop_name[0].lower() + prop_name[1:] + lines.append('\t\t{:{width}} {},'.format(prop_name + ':', var_name, width=longest_property+1)) lines.extend([ + '\t}, nil', + '}', + '', + '// clientSetSlot pairs a rest.Config pointer with its lazily-created clientSet.', + 'type clientSetSlot struct {', + '\tconfig *rest.Config', + '\tclients *clientSet', + '}', + '', + '// clientSetSlots caches clientSets for the two rest.Config pointers used by the', + '// CVO: one for default-QPS and one for burst-QPS during initialization. The', + '// fixed-size array makes the two-client constraint visible in the code and will', + '// produce a clear error if a future change introduces a third config.', + 'var (', + '\tclientSetMu sync.Mutex', + '\tclientSetSlots [2]clientSetSlot', + ')', + '', + 'func getOrCreateClientSet(config *rest.Config) (*clientSet, error) {', + '\tif config == nil {', + '\t\treturn nil, fmt.Errorf("cannot create client set from nil rest.Config")', + '\t}', + '\tclientSetMu.Lock()', + '\tdefer clientSetMu.Unlock()', + '\tfor i := range clientSetSlots {', + '\t\tif clientSetSlots[i].config == config {', + '\t\t\treturn clientSetSlots[i].clients, nil', + '\t\t}', + '\t}', + '\tcs, err := newClientSet(config)', + '\tif err != nil {', + '\t\treturn nil, err', + '\t}', + '\tfor i := range clientSetSlots {', + '\t\tif clientSetSlots[i].config == nil {', + '\t\t\tclientSetSlots[i] = clientSetSlot{config: config, clients: cs}', + '\t\t\treturn cs, nil', + '\t\t}', + '\t}', + '\treturn nil, fmt.Errorf("exceeded %d cached client sets, which is the maximum supported by the CVO", len(clientSetSlots))', + '}', + '', + 'func newBuilder(config *rest.Config, m manifest.Manifest) (Interface, error) {', + '\tcs, err := getOrCreateClientSet(config)', + '\tif err != nil {', + '\t\treturn nil, err', '\t}', + '\treturn &builder{', + '\t\traw: m.Raw,', + '', + ]) + for prop_name, data in sorted(client_properties.items()): + lines.append('\t\t{:{width}} cs.{},'.format(prop_name + ':', prop_name, width=longest_property+1)) + lines.extend([ + '\t}, nil', '}', '', 'func (b *builder) WithMode(m Mode) Interface {', @@ -263,6 +336,33 @@ def generate_resourcebuilder(directory, types, clients, modifiers, health_checks f.write('\n'.join(lines)) +def group_imports(imports): + """Group import lines by gci category: default, k8s.io, github.com/openshift, localmodule.""" + local_module = 'github.com/openshift/cluster-version-operator' + groups = { + 'default': [], + 'k8s.io': [], + 'openshift': [], + 'localmodule': [], + } + for package, line in sorted(imports.items(), key=lambda package_line: package_line[0]): + if package.startswith(local_module): + groups['localmodule'].append(line) + elif package.startswith('github.com/openshift'): + groups['openshift'].append(line) + elif package.startswith('k8s.io'): + groups['k8s.io'].append(line) + else: + groups['default'].append(line) + result = [] + for key in ['default', 'k8s.io', 'openshift', 'localmodule']: + if groups[key]: + if result: + result.append('') + result.extend(groups[key]) + return result + + def scheme_group_versions(types): sgvs = {} for package, type_names in types.items(): diff --git a/lib/resourcebuilder/interface.go b/lib/resourcebuilder/interface.go index 3cea33944e..dba60c25d6 100644 --- a/lib/resourcebuilder/interface.go +++ b/lib/resourcebuilder/interface.go @@ -58,10 +58,10 @@ func NewResourceMapper() *ResourceMapper { type MetaV1ObjectModifierFunc func(metav1.Object) -// NewInterfaceFunc returns an Interface. +// NewInterfaceFunc returns an Interface or an error. // It requires rest Config that can be used to create a client // and the Manifest. -type NewInterfaceFunc func(rest *rest.Config, m manifest.Manifest) Interface +type NewInterfaceFunc func(rest *rest.Config, m manifest.Manifest) (Interface, error) // Mode is how this builder is being used. type Mode int @@ -85,5 +85,5 @@ func New(mapper *ResourceMapper, rest *rest.Config, m manifest.Manifest) (Interf if !ok { return nil, fmt.Errorf("no mapping found for gvk: %v", m.GVK) } - return f(rest, m), nil + return f(rest, m) } diff --git a/lib/resourcebuilder/resourcebuilder.go b/lib/resourcebuilder/resourcebuilder.go index 87e1c52b35..da8fadfc60 100644 --- a/lib/resourcebuilder/resourcebuilder.go +++ b/lib/resourcebuilder/resourcebuilder.go @@ -6,6 +6,7 @@ package resourcebuilder import ( "context" "fmt" + "sync" operatorsv1 "github.com/operator-framework/api/pkg/operators/v1" operatorsclientv1 "github.com/operator-framework/operator-lifecycle-manager/pkg/api/client/clientset/versioned/typed/operators/v1" @@ -56,22 +57,140 @@ type builder struct { securityClientv1 securityclientv1.SecurityV1Interface } -func newBuilder(config *rest.Config, m manifest.Manifest) Interface { +// clientSet holds cached typed clients for resource building. +type clientSet struct { + admissionregistrationClientv1 admissionregistrationclientv1.AdmissionregistrationV1Interface + apiextensionsClientv1 apiextensionsclientv1.ApiextensionsV1Interface + apiregistrationClientv1 apiregistrationclientv1.ApiregistrationV1Interface + appsClientv1 appsclientv1.AppsV1Interface + batchClientv1 batchclientv1.BatchV1Interface + configClientv1 configclientv1.ConfigV1Interface + coreClientv1 coreclientv1.CoreV1Interface + imageClientv1 imageclientv1.ImageV1Interface + operatorsClientv1 operatorsclientv1.OperatorsV1Interface + rbacClientv1 rbacclientv1.RbacV1Interface + securityClientv1 securityclientv1.SecurityV1Interface +} + +func newClientSet(config *rest.Config) (*clientSet, error) { + admissionregistrationClientv1, err := admissionregistrationclientv1.NewForConfig(withProtobuf(config)) + if err != nil { + return nil, err + } + apiextensionsClientv1, err := apiextensionsclientv1.NewForConfig(withProtobuf(config)) + if err != nil { + return nil, err + } + apiregistrationClientv1, err := apiregistrationclientv1.NewForConfig(config) + if err != nil { + return nil, err + } + appsClientv1, err := appsclientv1.NewForConfig(withProtobuf(config)) + if err != nil { + return nil, err + } + batchClientv1, err := batchclientv1.NewForConfig(withProtobuf(config)) + if err != nil { + return nil, err + } + configClientv1, err := configclientv1.NewForConfig(config) + if err != nil { + return nil, err + } + coreClientv1, err := coreclientv1.NewForConfig(withProtobuf(config)) + if err != nil { + return nil, err + } + imageClientv1, err := imageclientv1.NewForConfig(config) + if err != nil { + return nil, err + } + operatorsClientv1, err := operatorsclientv1.NewForConfig(config) + if err != nil { + return nil, err + } + rbacClientv1, err := rbacclientv1.NewForConfig(withProtobuf(config)) + if err != nil { + return nil, err + } + securityClientv1, err := securityclientv1.NewForConfig(config) + if err != nil { + return nil, err + } + return &clientSet{ + admissionregistrationClientv1: admissionregistrationClientv1, + apiextensionsClientv1: apiextensionsClientv1, + apiregistrationClientv1: apiregistrationClientv1, + appsClientv1: appsClientv1, + batchClientv1: batchClientv1, + configClientv1: configClientv1, + coreClientv1: coreClientv1, + imageClientv1: imageClientv1, + operatorsClientv1: operatorsClientv1, + rbacClientv1: rbacClientv1, + securityClientv1: securityClientv1, + }, nil +} + +// clientSetSlot pairs a rest.Config pointer with its lazily-created clientSet. +type clientSetSlot struct { + config *rest.Config + clients *clientSet +} + +// clientSetSlots caches clientSets for the two rest.Config pointers used by the +// CVO: one for default-QPS and one for burst-QPS during initialization. The +// fixed-size array makes the two-client constraint visible in the code and will +// produce a clear error if a future change introduces a third config. +var ( + clientSetMu sync.Mutex + clientSetSlots [2]clientSetSlot +) + +func getOrCreateClientSet(config *rest.Config) (*clientSet, error) { + if config == nil { + return nil, fmt.Errorf("cannot create client set from nil rest.Config") + } + clientSetMu.Lock() + defer clientSetMu.Unlock() + for i := range clientSetSlots { + if clientSetSlots[i].config == config { + return clientSetSlots[i].clients, nil + } + } + cs, err := newClientSet(config) + if err != nil { + return nil, err + } + for i := range clientSetSlots { + if clientSetSlots[i].config == nil { + clientSetSlots[i] = clientSetSlot{config: config, clients: cs} + return cs, nil + } + } + return nil, fmt.Errorf("exceeded %d cached client sets, which is the maximum supported by the CVO", len(clientSetSlots)) +} + +func newBuilder(config *rest.Config, m manifest.Manifest) (Interface, error) { + cs, err := getOrCreateClientSet(config) + if err != nil { + return nil, err + } return &builder{ raw: m.Raw, - admissionregistrationClientv1: admissionregistrationclientv1.NewForConfigOrDie(withProtobuf(config)), - apiextensionsClientv1: apiextensionsclientv1.NewForConfigOrDie(withProtobuf(config)), - apiregistrationClientv1: apiregistrationclientv1.NewForConfigOrDie(config), - appsClientv1: appsclientv1.NewForConfigOrDie(withProtobuf(config)), - batchClientv1: batchclientv1.NewForConfigOrDie(withProtobuf(config)), - configClientv1: configclientv1.NewForConfigOrDie(config), - coreClientv1: coreclientv1.NewForConfigOrDie(withProtobuf(config)), - imageClientv1: imageclientv1.NewForConfigOrDie(config), - operatorsClientv1: operatorsclientv1.NewForConfigOrDie(config), - rbacClientv1: rbacclientv1.NewForConfigOrDie(withProtobuf(config)), - securityClientv1: securityclientv1.NewForConfigOrDie(config), - } + admissionregistrationClientv1: cs.admissionregistrationClientv1, + apiextensionsClientv1: cs.apiextensionsClientv1, + apiregistrationClientv1: cs.apiregistrationClientv1, + appsClientv1: cs.appsClientv1, + batchClientv1: cs.batchClientv1, + configClientv1: cs.configClientv1, + coreClientv1: cs.coreClientv1, + imageClientv1: cs.imageClientv1, + operatorsClientv1: cs.operatorsClientv1, + rbacClientv1: cs.rbacClientv1, + securityClientv1: cs.securityClientv1, + }, nil } func (b *builder) WithMode(m Mode) Interface { diff --git a/pkg/cvo/internal/operatorstatus.go b/pkg/cvo/internal/operatorstatus.go index b2342825a7..174a748031 100644 --- a/pkg/cvo/internal/operatorstatus.go +++ b/pkg/cvo/internal/operatorstatus.go @@ -56,9 +56,13 @@ type clusterOperatorBuilder struct { mode resourcebuilder.Mode } -func newClusterOperatorBuilder(config *rest.Config, m manifest.Manifest) resourcebuilder.Interface { - client := configclientv1.NewForConfigOrDie(config).ClusterOperators() - return NewClusterOperatorBuilder(clientClusterOperatorsGetter{getter: client}, client, m) +func newClusterOperatorBuilder(config *rest.Config, m manifest.Manifest) (resourcebuilder.Interface, error) { + configClient, err := configclientv1.NewForConfig(config) + if err != nil { + return nil, err + } + client := configClient.ClusterOperators() + return NewClusterOperatorBuilder(clientClusterOperatorsGetter{getter: client}, client, m), nil } // ClusterOperatorsGetter abstracts object access with a client or a cache lister. diff --git a/pkg/cvo/sync_test.go b/pkg/cvo/sync_test.go index 4894dd7729..91735745a5 100644 --- a/pkg/cvo/sync_test.go +++ b/pkg/cvo/sync_test.go @@ -446,8 +446,8 @@ func (t *testBuilder) Do(_ context.Context) error { } func newTestBuilder(r *recorder, rts map[action]error) resourcebuilder.NewInterfaceFunc { - return func(_ *rest.Config, m manifest.Manifest) resourcebuilder.Interface { - return &testBuilder{recorder: r, reactors: rts, m: &m} + return func(_ *rest.Config, m manifest.Manifest) (resourcebuilder.Interface, error) { + return &testBuilder{recorder: r, reactors: rts, m: &m}, nil } }