OCPBUGS-77042: use NewFromConfig instead of NewFromConfigOrDie in builder

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():

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)
 }

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 {

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.

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
 	}
 }