Skip to content

Commit 4fdc60d

Browse files
RichardChen820RichardChen820
authored
[Fix] Conditional refresh does not work after controller restarting (#121)
* Fix the restart bug * Not update secrets after restart
1 parent 0f6071c commit 4fdc60d

File tree

4 files changed

+68
-62
lines changed

4 files changed

+68
-62
lines changed

‎internal/controller/appconfigurationprovider_controller.go‎

Lines changed: 63 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -134,50 +134,13 @@ func (reconciler *AzureAppConfigurationProviderReconciler) Reconcile(ctx context
134134
}
135135

136136
existingConfigMap := corev1.ConfigMap{}
137-
isExisting := false
138137
_, err = reconciler.verifyTargetObjectExistence(ctx, provider, &existingConfigMap)
139138
if err != nil {
140139
reconciler.logAndSetFailStatus(provider, err)
141140
return reconcile.Result{Requeue: true, RequeueAfter: RequeueReconcileAfter}, nil
142141
}
143142

144-
existingSecrets := make(map[string]corev1.Secret)
145-
var existingSecret corev1.Secret
146-
if provider.Spec.Secret != nil {
147-
existingSecret = corev1.Secret{
148-
ObjectMeta: metav1.ObjectMeta{
149-
Name: provider.Spec.Secret.Target.SecretName,
150-
},
151-
}
152-
isExisting, err = reconciler.verifyTargetObjectExistence(ctx, provider, &existingSecret)
153-
if err != nil {
154-
reconciler.logAndSetFailStatus(provider, err)
155-
return reconcile.Result{Requeue: true, RequeueAfter: RequeueReconcileAfter}, nil
156-
}
157-
if isExisting {
158-
existingSecrets[provider.Spec.Secret.Target.SecretName] = existingSecret
159-
}
160-
}
161-
162-
if reconciler.ProvidersReconcileState[req.NamespacedName] != nil {
163-
for name := range reconciler.ProvidersReconcileState[req.NamespacedName].ExistingK8sSecrets {
164-
if _, ok := existingSecrets[name]; !ok {
165-
existingSecret = corev1.Secret{
166-
ObjectMeta: metav1.ObjectMeta{
167-
Name: name,
168-
},
169-
}
170-
isExisting, err = reconciler.verifyTargetObjectExistence(ctx, provider, &existingSecret)
171-
if err != nil {
172-
reconciler.logAndSetFailStatus(provider, err)
173-
return reconcile.Result{Requeue: true, RequeueAfter: RequeueReconcileAfter}, nil
174-
}
175-
if isExisting {
176-
existingSecrets[name] = existingSecret
177-
}
178-
}
179-
}
180-
} else {
143+
if reconciler.ProvidersReconcileState[req.NamespacedName] == nil {
181144
// Initialize the ReconcileState for the provider
182145
reconciler.ProvidersReconcileState[req.NamespacedName] = &ReconciliationState{
183146
Generation: -1,
@@ -191,6 +154,27 @@ func (reconciler *AzureAppConfigurationProviderReconciler) Reconcile(ctx context
191154
}
192155
}
193156

157+
existingSecrets := make(map[string]corev1.Secret)
158+
if provider.Spec.Secret != nil {
159+
secretNames := []string{provider.Spec.Secret.Target.SecretName}
160+
161+
if reconciler.ProvidersReconcileState[req.NamespacedName].ExistingK8sSecrets != nil {
162+
for secretName := range maps.Keys(reconciler.ProvidersReconcileState[req.NamespacedName].ExistingK8sSecrets) {
163+
secretNames = append(secretNames, secretName)
164+
}
165+
}
166+
167+
for _, secretName := range secretNames {
168+
if _, ok := existingSecrets[secretName]; ok {
169+
continue
170+
}
171+
172+
if err := reconciler.collectExistingSecret(ctx, provider, secretName, existingSecrets); err != nil {
173+
return reconcile.Result{Requeue: true, RequeueAfter: RequeueReconcileAfter}, nil
174+
}
175+
}
176+
}
177+
194178
// Reset the resource version if the configmap or secret was unexpected deleted
195179
if existingConfigMap.Name == "" {
196180
reconciler.ProvidersReconcileState[req.NamespacedName].ConfigMapResourceVersion = nil
@@ -262,17 +246,12 @@ func (reconciler *AzureAppConfigurationProviderReconciler) Reconcile(ctx context
262246
if processor.RefreshOptions.SecretSettingPopulated {
263247
// Verify the existence of the secret which is not owned by the current provider
264248
for name := range processor.Settings.SecretSettings {
265-
if _, ok := existingSecrets[name]; !ok {
266-
_, err := reconciler.verifyTargetObjectExistence(ctx, provider, &corev1.Secret{
267-
ObjectMeta: metav1.ObjectMeta{
268-
Name: name,
269-
},
270-
})
271-
272-
if err != nil {
273-
reconciler.logAndSetFailStatus(provider, err)
274-
return reconcile.Result{Requeue: true, RequeueAfter: RequeueReconcileAfter}, nil
275-
}
249+
if _, ok := existingSecrets[name]; ok {
250+
continue
251+
}
252+
253+
if err := reconciler.collectExistingSecret(ctx, provider, name, existingSecrets); err != nil {
254+
return reconcile.Result{Requeue: true, RequeueAfter: RequeueReconcileAfter}, nil
276255
}
277256
}
278257

@@ -320,6 +299,32 @@ func (reconciler *AzureAppConfigurationProviderReconciler) verifyTargetObjectExi
320299
return true, verifyExistingTargetObject(obj, targetName, provider.Name)
321300
}
322301

302+
// collectExistingSecret verifies the existence of a secret and adds it to the existingSecrets map if it exists
303+
func (reconciler *AzureAppConfigurationProviderReconciler) collectExistingSecret(
304+
ctx context.Context,
305+
provider *acpv1.AzureAppConfigurationProvider,
306+
name string,
307+
existingSecrets map[string]corev1.Secret) error {
308+
309+
existingSecret := corev1.Secret{
310+
ObjectMeta: metav1.ObjectMeta{
311+
Name: name,
312+
},
313+
}
314+
315+
isExisting, err := reconciler.verifyTargetObjectExistence(ctx, provider, &existingSecret)
316+
if err != nil {
317+
reconciler.logAndSetFailStatus(provider, err)
318+
return err
319+
}
320+
321+
if isExisting {
322+
existingSecrets[name] = existingSecret
323+
}
324+
325+
return nil
326+
}
327+
323328
func (reconciler *AzureAppConfigurationProviderReconciler) logAndSetFailStatus(
324329
provider *acpv1.AzureAppConfigurationProvider,
325330
err error) {
@@ -375,7 +380,14 @@ func (reconciler *AzureAppConfigurationProviderReconciler) createOrUpdateConfigM
375380
existingConfigMap *corev1.ConfigMap,
376381
provider *acpv1.AzureAppConfigurationProvider,
377382
settings *loader.TargetKeyValueSettings) (reconcile.Result, error) {
383+
namespacedName := types.NamespacedName{
384+
Name: provider.Name,
385+
Namespace: provider.Namespace,
386+
}
387+
378388
if !shouldCreateOrUpdateConfigMap(existingConfigMap, settings.ConfigMapSettings, provider.Spec.Target.ConfigMapData) {
389+
// Set the resource version of the existing configMap to reconcile state in case reconcile state loses after restart
390+
reconciler.ProvidersReconcileState[namespacedName].ConfigMapResourceVersion = &existingConfigMap.ResourceVersion
379391
klog.V(5).Infof("Skip updating the configMap %q in %q namespace since data is not changed", provider.Spec.Target.ConfigMapName, provider.Namespace)
380392
return reconcile.Result{}, nil
381393
}
@@ -410,10 +422,6 @@ func (reconciler *AzureAppConfigurationProviderReconciler) createOrUpdateConfigM
410422
return reconcile.Result{Requeue: true, RequeueAfter: RequeueReconcileAfter}, err
411423
}
412424

413-
namespacedName := types.NamespacedName{
414-
Name: provider.Name,
415-
Namespace: provider.Namespace,
416-
}
417425
reconciler.ProvidersReconcileState[namespacedName].ConfigMapResourceVersion = &configMapObj.ResourceVersion
418426
klog.V(5).Infof("configMap %q in %q namespace is %s", configMapObj.Name, configMapObj.Namespace, string(operationResult))
419427

@@ -429,16 +437,10 @@ func (reconciler *AzureAppConfigurationProviderReconciler) createOrUpdateSecrets
429437
klog.V(3).Info("No secret settings are fetched from Azure AppConfiguration")
430438
}
431439

432-
namespacedName := types.NamespacedName{
433-
Name: provider.Name,
434-
Namespace: provider.Namespace,
435-
}
436-
437440
for secretName, secret := range processor.Settings.SecretSettings {
438441
if !shouldCreateOrUpdateSecret(processor, secretName, existingSecrets) {
439-
if _, ok := reconciler.ProvidersReconcileState[namespacedName].ExistingK8sSecrets[secretName]; ok {
440-
processor.Settings.K8sSecrets[secretName].SecretResourceVersion = reconciler.ProvidersReconcileState[namespacedName].ExistingK8sSecrets[secretName].SecretResourceVersion
441-
}
442+
// Set the resource version of the existing secret to reconcile state in case reconcile state loses after restart
443+
processor.Settings.K8sSecrets[secretName].SecretResourceVersion = existingSecrets[secretName].ResourceVersion
442444
klog.V(5).Infof("Skip updating the secret %q in %q namespace since data is not changed", secretName, provider.Namespace)
443445
continue
444446
}

‎internal/controller/processor.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -339,6 +339,7 @@ func (processor *AppConfigurationProviderProcessor) Finish() (ctrl.Result, error
339339
processor.Provider.Status.RefreshStatus.LastKeyValueRefreshTime = processor.CurrentTime
340340
}
341341
if processor.RefreshOptions.keyValuePageETagsChanged {
342+
processor.ReconciliationState.KeyValueETags = processor.RefreshOptions.updatedKeyValueETags
342343
processor.Provider.Status.RefreshStatus.LastKeyValueRefreshTime = processor.CurrentTime
343344
}
344345
// Update provider last key vault refresh time

‎internal/loader/configuration_setting_loader.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -360,6 +360,7 @@ func (csl *ConfigurationSettingLoader) CheckPageETags(ctx context.Context, eTags
360360
if settingsClient == nil {
361361
settingsClient = &EtagSettingsClient{
362362
etags: eTags,
363+
refreshInterval: csl.Spec.Configuration.Refresh.Interval,
363364
}
364365
}
365366

‎internal/loader/settings_client.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,8 @@ type SettingsResponse struct {
2222
}
2323

2424
type EtagSettingsClient struct {
25-
etags map[acpv1.Selector][]*azcore.ETag
25+
etags map[acpv1.Selector][]*azcore.ETag
26+
refreshInterval string
2627
}
2728

2829
type SentinelSettingsClient struct {
@@ -83,6 +84,7 @@ func (s *EtagSettingsClient) GetSettings(ctx context.Context, client *azappconfi
8384
}
8485
}
8586

87+
klog.V(3).Infof("There's no change to the selected key values, just exit and revisit them after %s", s.refreshInterval)
8688
// no change in the settings, return nil etags
8789
return settingsResponse, nil
8890
}

0 commit comments

Comments
 (0)