Skip to content

Commit b334aad

Browse files
bryan-coxbryan-cox
committed
wip
Signed-off-by: Bryan Cox <[email protected]>
1 parent 5969417 commit b334aad

File tree

3 files changed

+122
-18
lines changed

3 files changed

+122
-18
lines changed

‎pkg/resource/generator.go

Lines changed: 102 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,14 @@ import (
1515
"k8s.io/klog/v2"
1616

1717
imageregistryv1 "github.com/openshift/api/imageregistry/v1"
18-
"github.com/openshift/library-go/pkg/operator/configobserver/featuregates"
19-
"github.com/openshift/library-go/pkg/operator/events"
2018

2119
"github.com/openshift/cluster-image-registry-operator/pkg/client"
2220
"github.com/openshift/cluster-image-registry-operator/pkg/defaults"
2321
"github.com/openshift/cluster-image-registry-operator/pkg/metrics"
2422
"github.com/openshift/cluster-image-registry-operator/pkg/resource/object"
2523
"github.com/openshift/cluster-image-registry-operator/pkg/storage"
24+
"github.com/openshift/library-go/pkg/operator/configobserver/featuregates"
25+
"github.com/openshift/library-go/pkg/operator/events"
2626
)
2727

2828
func ApplyMutator(gen Mutator) error {
@@ -82,6 +82,8 @@ type Generator struct {
8282
listers *client.Listers
8383
clients *client.Clients
8484
featureGateAccessor featuregates.FeatureGateAccess
85+
driverCache storage.Driver // Cache the storage driver to avoid repeated credential loading
86+
driverCacheKey string // Cache key to track when driver needs refresh
8587
}
8688

8789
func (g *Generator) listRoutes(cr *imageregistryv1.Config) []Mutator {
@@ -97,19 +99,72 @@ func (g *Generator) listRoutes(cr *imageregistryv1.Config) []Mutator {
9799
return mutators
98100
}
99101

102+
// getDriverCacheKey generates a cache key based on credential-related configuration only
103+
func (g *Generator) getDriverCacheKey(cr *imageregistryv1.Config) string {
104+
// Only cache for Azure storage since that's where the credential loading issue occurs
105+
if cr.Spec.Storage.Azure != nil {
106+
// For Azure, we want to cache the driver to avoid repeated credential loading
107+
// The credentials come from secrets/environment variables, not the storage config
108+
// So we only care if Azure storage is configured
109+
stableConfig := "azure"
110+
cacheKey := fmt.Sprintf("%x", hash(stableConfig))
111+
klog.V(2).Infof("Cache key generated: %s (Azure storage configured)", cacheKey)
112+
return cacheKey
113+
}
114+
115+
// For non-Azure storage, return empty string to indicate no caching needed
116+
return ""
117+
}
118+
119+
// hash creates a simple hash of a string
120+
func hash(s string) uint32 {
121+
h := uint32(0)
122+
for i := 0; i < len(s); i++ {
123+
h = 31*h + uint32(s[i])
124+
}
125+
return h
126+
}
127+
100128
func (g *Generator) List(cr *imageregistryv1.Config) ([]Mutator, error) {
101-
driver, err := storage.NewDriver(&cr.Spec.Storage, g.kubeconfig, &g.listers.StorageListers, g.featureGateAccessor)
102-
if err != nil && err != storage.ErrStorageNotConfigured {
103-
return nil, err
104-
} else if err == storage.ErrStorageNotConfigured {
105-
klog.V(6).Info("storage not configured, some mutators might not work.")
129+
// Check if we need to refresh the cached driver (only for Azure)
130+
cacheKey := g.getDriverCacheKey(cr)
131+
if cacheKey != "" {
132+
// Only cache for Azure storage
133+
if g.driverCache == nil || g.driverCacheKey != cacheKey {
134+
klog.V(2).Infof("Creating new storage driver (cache miss or config changed). Old key: %s, New key: %s", g.driverCacheKey, cacheKey)
135+
driver, err := storage.NewDriver(&cr.Spec.Storage, g.kubeconfig, &g.listers.StorageListers, g.featureGateAccessor)
136+
if err != nil && err != storage.ErrStorageNotConfigured {
137+
return nil, err
138+
} else if err == storage.ErrStorageNotConfigured {
139+
klog.V(6).Info("storage not configured, some mutators might not work.")
140+
}
141+
g.driverCache = driver
142+
g.driverCacheKey = cacheKey
143+
} else {
144+
klog.V(2).Infof("Reusing cached storage driver (cache key: %s)", cacheKey)
145+
}
106146
}
107147

108148
var mutators []Mutator
109149
mutators = append(mutators, newGeneratorClusterRole(g.listers.ClusterRoles, g.clients.RBAC))
110150
mutators = append(mutators, newGeneratorClusterRoleBinding(g.listers.ClusterRoleBindings, g.clients.RBAC))
111151
mutators = append(mutators, newGeneratorServiceAccount(g.listers.ServiceAccounts, g.clients.Core))
112152
mutators = append(mutators, newGeneratorPullSecret(g.clients.Core))
153+
154+
// Use cached driver for Azure, create new one for others
155+
var driver storage.Driver
156+
if cacheKey != "" && g.driverCache != nil {
157+
driver = g.driverCache
158+
} else {
159+
var err error
160+
driver, err = storage.NewDriver(&cr.Spec.Storage, g.kubeconfig, &g.listers.StorageListers, g.featureGateAccessor)
161+
if err != nil && err != storage.ErrStorageNotConfigured {
162+
return nil, err
163+
} else if err == storage.ErrStorageNotConfigured {
164+
klog.V(6).Info("storage not configured, some mutators might not work.")
165+
}
166+
}
167+
113168
mutators = append(mutators, newGeneratorSecret(g.listers.Secrets, g.clients.Core, driver))
114169
mutators = append(mutators, newGeneratorService(g.listers.Services, g.clients.Core))
115170
mutators = append(mutators, newGeneratorDeployment(g.eventRecorder, g.listers.Deployments, g.listers.ConfigMaps, g.listers.Secrets, g.listers.ProxyConfigs, g.clients.Core, g.clients.Apps, driver, cr))
@@ -127,17 +182,48 @@ func (g *Generator) List(cr *imageregistryv1.Config) ([]Mutator, error) {
127182
// b.) see if we need to try to create the new storage
128183
func (g *Generator) syncStorage(cr *imageregistryv1.Config) error {
129184
var runCreate bool
130-
// Create a driver with the current configuration
131-
driver, err := storage.NewDriver(&cr.Spec.Storage, g.kubeconfig, &g.listers.StorageListers, g.featureGateAccessor)
132-
if err == storage.ErrStorageNotConfigured {
133-
cr.Spec.Storage, _, err = storage.GetPlatformStorage(&g.listers.StorageListers)
134-
if err != nil {
135-
return fmt.Errorf("unable to get storage configuration from cluster install config: %s", err)
185+
186+
// Check if we need to refresh the cached driver (only for Azure)
187+
cacheKey := g.getDriverCacheKey(cr)
188+
var driver storage.Driver
189+
190+
if cacheKey != "" {
191+
// Only cache for Azure storage
192+
if g.driverCache == nil || g.driverCacheKey != cacheKey {
193+
klog.V(2).Infof("Creating new storage driver for syncStorage (cache miss or config changed). Old key: %s, New key: %s", g.driverCacheKey, cacheKey)
194+
var err error
195+
driver, err = storage.NewDriver(&cr.Spec.Storage, g.kubeconfig, &g.listers.StorageListers, g.featureGateAccessor)
196+
if err == storage.ErrStorageNotConfigured {
197+
cr.Spec.Storage, _, err = storage.GetPlatformStorage(&g.listers.StorageListers)
198+
if err != nil {
199+
return fmt.Errorf("unable to get storage configuration from cluster install config: %s", err)
200+
}
201+
driver, err = storage.NewDriver(&cr.Spec.Storage, g.kubeconfig, &g.listers.StorageListers, g.featureGateAccessor)
202+
}
203+
if err != nil {
204+
return err
205+
}
206+
g.driverCache = driver
207+
g.driverCacheKey = cacheKey
208+
} else {
209+
klog.V(2).Infof("Reusing cached storage driver for syncStorage (cache key: %s)", cacheKey)
210+
driver = g.driverCache
136211
}
212+
} else {
213+
// For non-Azure storage, create new driver (no caching needed)
214+
klog.V(2).Infof("Creating new storage driver for syncStorage (non-Azure storage)")
215+
var err error
137216
driver, err = storage.NewDriver(&cr.Spec.Storage, g.kubeconfig, &g.listers.StorageListers, g.featureGateAccessor)
138-
}
139-
if err != nil {
140-
return err
217+
if err == storage.ErrStorageNotConfigured {
218+
cr.Spec.Storage, _, err = storage.GetPlatformStorage(&g.listers.StorageListers)
219+
if err != nil {
220+
return fmt.Errorf("unable to get storage configuration from cluster install config: %s", err)
221+
}
222+
driver, err = storage.NewDriver(&cr.Spec.Storage, g.kubeconfig, &g.listers.StorageListers, g.featureGateAccessor)
223+
}
224+
if err != nil {
225+
return err
226+
}
141227
}
142228

143229
if driver.StorageChanged(cr) {

‎pkg/storage/azure/azure.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -334,6 +334,8 @@ func NewDriver(ctx context.Context, c *imageregistryv1.ImageRegistryConfigStorag
334334
}
335335

336336
func (d *driver) newAzClient(cfg *Azure, environment autorestazure.Environment, tagset map[string]*string) (*azureclient.Client, error) {
337+
klog.V(2).Infof("Creating new azureclient with shared credential cache: %p", &d.azureCredentials)
338+
337339
client, err := azureclient.New(&azureclient.Options{
338340
Environment: environment,
339341
TenantID: cfg.TenantID,
@@ -343,6 +345,7 @@ func (d *driver) newAzClient(cfg *Azure, environment autorestazure.Environment,
343345
SubscriptionID: cfg.SubscriptionID,
344346
TagSet: tagset,
345347
Policies: d.policies,
348+
CredentialCache: &d.azureCredentials, // Share the driver's credential cache
346349
})
347350
if err != nil {
348351
return nil, err

‎pkg/storage/azure/azureclient/azureclient.go

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ type Options struct {
5151
TagSet map[string]*string
5252
Policies []policy.Policy
5353
Creds azcore.TokenCredential
54+
CredentialCache *sync.Map // Optional external credential cache to share across instances
5455
}
5556

5657
type PrivateEndpointCreateOptions struct {
@@ -97,6 +98,13 @@ func New(opts *Options) (*Client, error) {
9798
}, nil
9899
}
99100

101+
func (c *Client) getCredentialCache() *sync.Map {
102+
if c.opts.CredentialCache != nil {
103+
return c.opts.CredentialCache
104+
}
105+
return &c.azureCredentials
106+
}
107+
100108
func (c *Client) getCreds(ctx context.Context) (azcore.TokenCredential, error) {
101109
if c.creds != nil {
102110
return c.creds, nil
@@ -110,9 +118,14 @@ func (c *Client) getCreds(ctx context.Context) (azcore.TokenCredential, error) {
110118
if userAssignedIdentityCredentialsFilePath != "" {
111119
var ok bool
112120

121+
// Use shared credential cache if available
122+
credCache := c.getCredentialCache()
123+
klog.V(2).Infof("Using credential cache: %p", credCache)
124+
113125
// We need to only store the Azure credentials once and reuse them after that.
114-
storedCreds, found := c.azureCredentials.Load(azureCredentialsKey)
126+
storedCreds, found := credCache.Load(azureCredentialsKey)
115127
if !found {
128+
klog.V(2).Infof("Cache miss - creating new credentials")
116129
klog.V(2).Info("Using UserAssignedIdentityCredentials for Azure authentication for managed Azure HCP")
117130
clientOptions := azcore.ClientOptions{
118131
Cloud: c.clientOpts.Cloud,
@@ -121,8 +134,10 @@ func (c *Client) getCreds(ctx context.Context) (azcore.TokenCredential, error) {
121134
if err != nil {
122135
return nil, err
123136
}
124-
c.azureCredentials.Store(azureCredentialsKey, creds)
137+
credCache.Store(azureCredentialsKey, creds)
138+
klog.V(2).Infof("Stored credentials in cache: %p", creds)
125139
} else {
140+
klog.V(2).Infof("Cache hit - reusing existing credentials")
126141
creds, ok = storedCreds.(azcore.TokenCredential)
127142
if !ok {
128143
return nil, fmt.Errorf("expected %T to be a TokenCredential", storedCreds)

0 commit comments

Comments
 (0)