Skip to content

Commit 09c1e41

Browse files
amoralejopenshift-merge-bot[bot]
amoralej
authored and
openshift-merge-bot[bot]
committed
Move parameters to pointers
When integrating the watcher-operator in the openstack-operator, the defaulting webhooks can interfere with the kubebuilder defaults. The defaulting webhook can set fields to their zero-values. In the case of a string, then "". This happens simply because of the serialization that occurs in the webhooks. Then when the field arrives in the kubebuilder defaulting logic, it has a non-null value, "", so the kubebuilder leaves it alone and it then eventually arrives in the reconcile loop as "". To avoid that, I'm moving to use pointers to all string parameters which has a kubebuilder default value.
1 parent e9d83a7 commit 09c1e41

10 files changed

Lines changed: 112 additions & 82 deletions

‎api/v1beta1/common_types.go‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ type WatcherCommon struct {
3737
// +kubebuilder:validation:Optional
3838
// +kubebuilder:default=watcher
3939
// ServiceUser - optional username used for this service to register in keystone
40-
ServiceUser string `json:"serviceUser"`
40+
ServiceUser *string `json:"serviceUser"`
4141

4242
// +kubebuilder:validation:Optional
4343
// +kubebuilder:default={service: WatcherPassword,}
@@ -47,7 +47,7 @@ type WatcherCommon struct {
4747
// +kubebuilder:validation:Optional
4848
// +kubebuilder:default=memcached
4949
// MemcachedInstance is the name of the Memcached CR that all watcher service will use.
50-
MemcachedInstance string `json:"memcachedInstance"`
50+
MemcachedInstance *string `json:"memcachedInstance"`
5151

5252
// +kubebuilder:validation:Optional
5353
// +kubebuilder:default=false
@@ -68,7 +68,7 @@ type WatcherCommon struct {
6868
// +kubebuilder:validation:Optional
6969
// +kubebuilder:default=metric-storage-prometheus-endpoint
7070
// Secret containing prometheus connection parameters
71-
PrometheusSecret string `json:"prometheusSecret"`
71+
PrometheusSecret *string `json:"prometheusSecret"`
7272

7373
// +kubebuilder:validation:Optional
7474
// TopologyRef to apply the Topology defined by the associated CR referenced
@@ -91,7 +91,7 @@ type WatcherSpecCore struct {
9191
// +kubebuilder:validation:Optional
9292
// +kubebuilder:default=osp-secret
9393
// Secret containing all passwords / keys needed
94-
Secret string `json:"secret"`
94+
Secret *string `json:"secret"`
9595

9696
// +kubebuilder:validation:Required
9797
// MariaDB instance name
@@ -101,7 +101,7 @@ type WatcherSpecCore struct {
101101
// +kubebuilder:validation:Optional
102102
// +kubebuilder:default=watcher
103103
// DatabaseAccount - MariaDBAccount CR name used for watcher DB, defaults to watcher
104-
DatabaseAccount string `json:"databaseAccount"`
104+
DatabaseAccount *string `json:"databaseAccount"`
105105

106106
// +kubebuilder:validation:Required
107107
// +kubebuilder:default={replicas:1}
@@ -128,7 +128,7 @@ type PasswordSelector struct {
128128
// +kubebuilder:validation:Optional
129129
// +kubebuilder:default="WatcherPassword"
130130
// Service - Selector to get the watcher service user password from the Secret
131-
Service string `json:"service"`
131+
Service *string `json:"service"`
132132
}
133133

134134
// WatcherSubCrsCommon

‎api/v1beta1/zz_generated.deepcopy.go‎

Lines changed: 31 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎controllers/watcher_controller.go‎

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -233,9 +233,9 @@ func (r *WatcherReconciler) Reconcile(ctx context.Context, req ctrl.Request) (re
233233
// Top level secret
234234
hash, _, inputSecret, err := ensureSecret(
235235
ctx,
236-
types.NamespacedName{Namespace: instance.Namespace, Name: instance.Spec.Secret},
236+
types.NamespacedName{Namespace: instance.Namespace, Name: *instance.Spec.Secret},
237237
[]string{
238-
instance.Spec.PasswordSelectors.Service,
238+
*instance.Spec.PasswordSelectors.Service,
239239
},
240240
helper.GetClient(),
241241
&instance.Status.Conditions,
@@ -266,7 +266,7 @@ func (r *WatcherReconciler) Reconcile(ctx context.Context, req ctrl.Request) (re
266266

267267
hashPrometheus, _, prometheusSecret, err := ensureSecret(
268268
ctx,
269-
types.NamespacedName{Namespace: instance.Namespace, Name: instance.Spec.PrometheusSecret},
269+
types.NamespacedName{Namespace: instance.Namespace, Name: *instance.Spec.PrometheusSecret},
270270
[]string{
271271
PrometheusHost,
272272
PrometheusPort,
@@ -383,7 +383,7 @@ func (r *WatcherReconciler) Reconcile(ctx context.Context, req ctrl.Request) (re
383383
// running with current database account info
384384
err = mariadbv1.DeleteUnusedMariaDBAccountFinalizers(
385385
ctx, helper, watcher.DatabaseCRName,
386-
instance.Spec.DatabaseAccount, instance.Namespace)
386+
*instance.Spec.DatabaseAccount, instance.Namespace)
387387
if err != nil {
388388
return ctrl.Result{}, err
389389
}
@@ -527,7 +527,7 @@ func (r *WatcherReconciler) ensureDB(
527527
// ensure MariaDBAccount exists without being yet associated with any database.
528528
// This account record may be created by the openstack-operator up front.
529529
_, _, err := mariadbv1.EnsureMariaDBAccount(
530-
ctx, h, instance.Spec.DatabaseAccount,
530+
ctx, h, *instance.Spec.DatabaseAccount,
531531
instance.Namespace, false, watcher.DatabaseUsernamePrefix,
532532
)
533533
if err != nil {
@@ -551,7 +551,7 @@ func (r *WatcherReconciler) ensureDB(
551551
*instance.Spec.DatabaseInstance, // mariadb/galera service to target
552552
watcher.DatabaseName, // name used in CREATE DATABASE in mariadb
553553
watcher.DatabaseCRName, // CR name for MariaDBDatabase
554-
instance.Spec.DatabaseAccount, // CR name for MariaDBAccount
554+
*instance.Spec.DatabaseAccount, // CR name for MariaDBAccount
555555
instance.Namespace, // namespace
556556
)
557557

@@ -678,9 +678,9 @@ func (r *WatcherReconciler) ensureKeystoneSvc(
678678
ServiceName: watcher.ServiceName,
679679
ServiceDescription: "Watcher Service",
680680
Enabled: true,
681-
ServiceUser: instance.Spec.ServiceUser,
682-
Secret: instance.Spec.Secret,
683-
PasswordSelector: instance.Spec.PasswordSelectors.Service,
681+
ServiceUser: *instance.Spec.ServiceUser,
682+
Secret: *instance.Spec.Secret,
683+
PasswordSelector: *instance.Spec.PasswordSelectors.Service,
684684
}
685685

686686
ksSvc := keystonev1.NewKeystoneService(ksSvcSpec, instance.Namespace, serviceLabels, time.Duration(10)*time.Second)
@@ -818,13 +818,13 @@ func (r *WatcherReconciler) createSubLevelSecret(
818818
databaseAccount := db.GetAccount()
819819
databaseSecret := db.GetSecret()
820820
data := map[string]string{
821-
instance.Spec.PasswordSelectors.Service: string(inputSecret.Data[instance.Spec.PasswordSelectors.Service]),
822-
TransportURLSelector: string(transportURLSecret.Data[TransportURLSelector]),
823-
DatabaseAccount: databaseAccount.Name,
824-
DatabaseUsername: databaseAccount.Spec.UserName,
825-
DatabasePassword: string(databaseSecret.Data[mariadbv1.DatabasePasswordSelector]),
826-
DatabaseHostname: db.GetDatabaseHostname(),
827-
watcher.GlobalCustomConfigFileName: instance.Spec.CustomServiceConfig,
821+
*instance.Spec.PasswordSelectors.Service: string(inputSecret.Data[*instance.Spec.PasswordSelectors.Service]),
822+
TransportURLSelector: string(transportURLSecret.Data[TransportURLSelector]),
823+
DatabaseAccount: databaseAccount.Name,
824+
DatabaseUsername: databaseAccount.Spec.UserName,
825+
DatabasePassword: string(databaseSecret.Data[mariadbv1.DatabasePasswordSelector]),
826+
DatabaseHostname: db.GetDatabaseHostname(),
827+
watcher.GlobalCustomConfigFileName: instance.Spec.CustomServiceConfig,
828828
}
829829
secretName := instance.Name
830830

@@ -1130,7 +1130,7 @@ func (r *WatcherReconciler) reconcileDelete(ctx context.Context, instance *watch
11301130
Log.Info(fmt.Sprintf("Reconcile Service '%s' delete started", instance.Name))
11311131

11321132
// remove db finalizer first
1133-
db, err := mariadbv1.GetDatabaseByNameAndAccount(ctx, helper, watcher.DatabaseCRName, instance.Spec.DatabaseAccount, instance.Namespace)
1133+
db, err := mariadbv1.GetDatabaseByNameAndAccount(ctx, helper, watcher.DatabaseCRName, *instance.Spec.DatabaseAccount, instance.Namespace)
11341134
if err != nil && !k8s_errors.IsNotFound(err) {
11351135
return ctrl.Result{}, err
11361136
}
@@ -1161,7 +1161,7 @@ func (r *WatcherReconciler) reconcileDelete(ctx context.Context, instance *watch
11611161
prometheusSecret := &corev1.Secret{}
11621162
reader := helper.GetClient()
11631163
err = reader.Get(ctx,
1164-
types.NamespacedName{Namespace: instance.Namespace, Name: instance.Spec.PrometheusSecret},
1164+
types.NamespacedName{Namespace: instance.Namespace, Name: *instance.Spec.PrometheusSecret},
11651165
prometheusSecret)
11661166

11671167
if err == nil {
@@ -1187,10 +1187,10 @@ func (r *WatcherReconciler) SetupWithManager(mgr ctrl.Manager) error {
11871187
if err := mgr.GetFieldIndexer().IndexField(context.Background(), &watcherv1beta1.Watcher{}, passwordSecretField, func(rawObj client.Object) []string {
11881188
// Extract the secret name from the spec, if one is provided
11891189
cr := rawObj.(*watcherv1beta1.Watcher)
1190-
if cr.Spec.Secret == "" {
1190+
if *cr.Spec.Secret == "" {
11911191
return nil
11921192
}
1193-
return []string{cr.Spec.Secret}
1193+
return []string{*cr.Spec.Secret}
11941194
}); err != nil {
11951195
return err
11961196
}
@@ -1199,10 +1199,10 @@ func (r *WatcherReconciler) SetupWithManager(mgr ctrl.Manager) error {
11991199
if err := mgr.GetFieldIndexer().IndexField(context.Background(), &watcherv1beta1.Watcher{}, prometheusSecretField, func(rawObj client.Object) []string {
12001200
// Extract the secret name from the spec, if one is provided
12011201
cr := rawObj.(*watcherv1beta1.Watcher)
1202-
if cr.Spec.PrometheusSecret == "" {
1202+
if *cr.Spec.PrometheusSecret == "" {
12031203
return nil
12041204
}
1205-
return []string{cr.Spec.PrometheusSecret}
1205+
return []string{*cr.Spec.PrometheusSecret}
12061206
}); err != nil {
12071207
return err
12081208
}

‎controllers/watcherapi_controller.go‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ func (r *WatcherAPIReconciler) Reconcile(ctx context.Context, req ctrl.Request)
166166
ctx,
167167
types.NamespacedName{Namespace: instance.Namespace, Name: instance.Spec.Secret},
168168
[]string{
169-
instance.Spec.PasswordSelectors.Service,
169+
*instance.Spec.PasswordSelectors.Service,
170170
TransportURLSelector,
171171
DatabaseAccount,
172172
DatabaseUsername,
@@ -188,7 +188,7 @@ func (r *WatcherAPIReconciler) Reconcile(ctx context.Context, req ctrl.Request)
188188

189189
hashPrometheus, _, prometheusSecret, err := ensureSecret(
190190
ctx,
191-
types.NamespacedName{Namespace: instance.Namespace, Name: instance.Spec.PrometheusSecret},
191+
types.NamespacedName{Namespace: instance.Namespace, Name: *instance.Spec.PrometheusSecret},
192192
[]string{
193193
PrometheusHost,
194194
PrometheusPort,
@@ -207,12 +207,12 @@ func (r *WatcherAPIReconciler) Reconcile(ctx context.Context, req ctrl.Request)
207207
return ctrl.Result{}, errors.New("error retrieving required data from prometheus secret")
208208
}
209209

210-
configVars[instance.Spec.PrometheusSecret] = env.SetValue(hashPrometheus)
210+
configVars[*instance.Spec.PrometheusSecret] = env.SetValue(hashPrometheus)
211211

212212
// all our input checks out so report InputReady
213213
instance.Status.Conditions.MarkTrue(condition.InputReadyCondition, condition.InputReadyMessage)
214214

215-
memcached, err := ensureMemcached(ctx, helper, instance.Namespace, instance.Spec.MemcachedInstance, &instance.Status.Conditions)
215+
memcached, err := ensureMemcached(ctx, helper, instance.Namespace, *instance.Spec.MemcachedInstance, &instance.Status.Conditions)
216216

217217
if err != nil {
218218
return ctrl.Result{}, err
@@ -452,8 +452,8 @@ func (r *WatcherAPIReconciler) generateServiceConfigs(
452452
watcher.DatabaseName,
453453
),
454454
"KeystoneAuthURL": keystoneInternalURL,
455-
"ServicePassword": string(secret.Data[instance.Spec.PasswordSelectors.Service]),
456-
"ServiceUser": instance.Spec.ServiceUser,
455+
"ServicePassword": string(secret.Data[*instance.Spec.PasswordSelectors.Service]),
456+
"ServiceUser": *instance.Spec.ServiceUser,
457457
"TransportURL": string(secret.Data[TransportURLSelector]),
458458
"MemcachedServers": memcachedInstance.GetMemcachedServerListString(),
459459
"MemcachedServersWithInet": memcachedInstance.GetMemcachedServerListWithInetString(),
@@ -766,7 +766,7 @@ func (r *WatcherAPIReconciler) reconcileDelete(ctx context.Context, instance *wa
766766
}
767767

768768
// Remove our finalizer from Memcached
769-
memcached, err := memcachedv1.GetMemcachedByName(ctx, helper, instance.Spec.MemcachedInstance, instance.Namespace)
769+
memcached, err := memcachedv1.GetMemcachedByName(ctx, helper, *instance.Spec.MemcachedInstance, instance.Namespace)
770770
if err != nil && !k8s_errors.IsNotFound(err) {
771771
return ctrl.Result{}, nil
772772
}
@@ -847,10 +847,10 @@ func (r *WatcherAPIReconciler) SetupWithManager(mgr ctrl.Manager) error {
847847
if err := mgr.GetFieldIndexer().IndexField(context.Background(), &watcherv1beta1.WatcherAPI{}, prometheusSecretField, func(rawObj client.Object) []string {
848848
// Extract the secret name from the spec, if one is provided
849849
cr := rawObj.(*watcherv1beta1.WatcherAPI)
850-
if cr.Spec.PrometheusSecret == "" {
850+
if *cr.Spec.PrometheusSecret == "" {
851851
return nil
852852
}
853-
return []string{cr.Spec.PrometheusSecret}
853+
return []string{*cr.Spec.PrometheusSecret}
854854
}); err != nil {
855855
return err
856856
}

‎controllers/watcherapplier_controller.go‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ func (r *WatcherApplierReconciler) Reconcile(ctx context.Context, req ctrl.Reque
163163
ctx,
164164
types.NamespacedName{Namespace: instance.Namespace, Name: instance.Spec.Secret},
165165
[]string{
166-
instance.Spec.PasswordSelectors.Service,
166+
*instance.Spec.PasswordSelectors.Service,
167167
TransportURLSelector,
168168
DatabaseAccount,
169169
DatabaseUsername,
@@ -184,7 +184,7 @@ func (r *WatcherApplierReconciler) Reconcile(ctx context.Context, req ctrl.Reque
184184
// all our input checks out so report InputReady
185185
instance.Status.Conditions.MarkTrue(condition.InputReadyCondition, condition.InputReadyMessage)
186186

187-
memcached, err := ensureMemcached(ctx, helper, instance.Namespace, instance.Spec.MemcachedInstance, &instance.Status.Conditions)
187+
memcached, err := ensureMemcached(ctx, helper, instance.Namespace, *instance.Spec.MemcachedInstance, &instance.Status.Conditions)
188188

189189
if err != nil {
190190
return ctrl.Result{}, err
@@ -307,7 +307,7 @@ func (r *WatcherApplierReconciler) reconcileDelete(ctx context.Context, instance
307307
Log.Info(fmt.Sprintf("Reconcile Service '%s' delete started", instance.Name))
308308

309309
// Remove our finalizer from Memcached
310-
memcached, err := memcachedv1.GetMemcachedByName(ctx, helper, instance.Spec.MemcachedInstance, instance.Namespace)
310+
memcached, err := memcachedv1.GetMemcachedByName(ctx, helper, *instance.Spec.MemcachedInstance, instance.Namespace)
311311
if err != nil && !k8s_errors.IsNotFound(err) {
312312
return ctrl.Result{}, nil
313313
}
@@ -419,8 +419,8 @@ func (r *WatcherApplierReconciler) generateServiceConfigs(
419419
watcher.DatabaseName,
420420
),
421421
"KeystoneAuthURL": keystoneInternalURL,
422-
"ServicePassword": string(secret.Data[instance.Spec.PasswordSelectors.Service]),
423-
"ServiceUser": instance.Spec.ServiceUser,
422+
"ServicePassword": string(secret.Data[*instance.Spec.PasswordSelectors.Service]),
423+
"ServiceUser": *instance.Spec.ServiceUser,
424424
"TransportURL": string(secret.Data[TransportURLSelector]),
425425
"MemcachedServers": memcachedInstance.GetMemcachedServerListString(),
426426
"MemcachedServersWithInet": memcachedInstance.GetMemcachedServerListWithInetString(),
@@ -449,10 +449,10 @@ func (r *WatcherApplierReconciler) SetupWithManager(mgr ctrl.Manager) error {
449449
if err := mgr.GetFieldIndexer().IndexField(context.Background(), &watcherv1beta1.WatcherApplier{}, prometheusSecretField, func(rawObj client.Object) []string {
450450
// Extract the secret name from the spec, if one is provided
451451
cr := rawObj.(*watcherv1beta1.WatcherApplier)
452-
if cr.Spec.PrometheusSecret == "" {
452+
if *cr.Spec.PrometheusSecret == "" {
453453
return nil
454454
}
455-
return []string{cr.Spec.PrometheusSecret}
455+
return []string{*cr.Spec.PrometheusSecret}
456456
}); err != nil {
457457
return err
458458
}

0 commit comments

Comments
 (0)