Skip to content

Commit c250963

Browse files
committed
Remove reconcile context in main controller
1 parent 180b557 commit c250963

8 files changed

Lines changed: 48 additions & 47 deletions

File tree

common/secret.go

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,9 @@ func (sm SecretMap) Get(key string) ([]byte, bool) {
3232
return []byte{}, false
3333
}
3434

35-
func (lockedBufferSecret LockedBufferSecret) Destroy() {
36-
if lockedBufferSecret.LockedData != nil {
37-
lockedBufferSecret.LockedData.Destroy()
35+
func (lockedSecret LockedBufferSecret) Destroy() {
36+
if lockedSecret.LockedData != nil {
37+
lockedSecret.LockedData.Destroy()
3838
}
3939
}
4040

@@ -51,19 +51,27 @@ func GetSecret(client client.Client, name string, ns string) (*LockedBufferSecre
5151
return nil, err
5252
}
5353

54-
lockedBufferSecret := &LockedBufferSecret{}
55-
lockedBufferSecret.TypeMeta = secret.TypeMeta
56-
lockedBufferSecret.ObjectMeta = secret.ObjectMeta
57-
for secretKey, secretValue := range secret.Data {
58-
lockedBufferSecret.LockedData[secretKey] = memguard.NewBufferFromBytes(secretValue)
54+
lockedSecret := &LockedBufferSecret{}
55+
lockedSecret.TypeMeta = secret.TypeMeta
56+
lockedSecret.ObjectMeta = secret.ObjectMeta
57+
if lockedSecret.LockedData == nil {
58+
lockedSecret.LockedData = SecretMap{}
5959
}
60-
return lockedBufferSecret, nil
60+
if secret.Data != nil {
61+
for secretKey, secretValue := range secret.Data {
62+
lockedSecret.LockedData[secretKey] = memguard.NewBufferFromBytes(secretValue)
63+
}
64+
}
65+
return lockedSecret, nil
6166
}
6267

6368
// Copies a Locked Buffer Secret into a core Secret with a corresponding cleanup func
6469
func CopySecret(in *LockedBufferSecret, out *corev1.Secret) func() {
6570
out.TypeMeta = in.TypeMeta
6671
out.ObjectMeta = in.ObjectMeta
72+
if out.Data == nil {
73+
out.Data = map[string][]byte{}
74+
}
6775
for key, buf := range in.LockedData {
6876
out.Data[key] = buf.Bytes()
6977
}

internal/controller/runtimecomponent_controller.go

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -101,11 +101,6 @@ func (r *RuntimeComponentReconciler) Reconcile(ctx context.Context, req ctrl.Req
101101
ns = r.watchNamespaces[0]
102102
}
103103

104-
// This reconcile context is cancelled right before the Reconcile function terminates or when the parent context ctx is cancelled (such as in operator leader election), whichever happens first.
105-
// Resources that require cleanup may use this context to hook into program execution before a function return.
106-
recCtx, cancel := context.WithCancel(ctx)
107-
defer cancel()
108-
109104
configMap, err := r.GetOpConfigMap(OperatorName, ns)
110105
if err != nil {
111106
reqLogger.Info("Failed to find runtime-component-operator config map")
@@ -243,7 +238,7 @@ func (r *RuntimeComponentReconciler) Reconcile(ctx context.Context, req ctrl.Req
243238
if serviceAccountName == "" {
244239
serviceAccount := &corev1.ServiceAccount{ObjectMeta: defaultMeta}
245240
err = r.CreateOrUpdate(serviceAccount, instance, func() error {
246-
return appstacksutils.CustomizeServiceAccount(recCtx, serviceAccount, instance, r.GetClient())
241+
return appstacksutils.CustomizeServiceAccount(serviceAccount, instance, r.GetClient())
247242
})
248243
if err != nil {
249244
reqLogger.Error(err, "Failed to reconcile ServiceAccount")
@@ -262,7 +257,7 @@ func (r *RuntimeComponentReconciler) Reconcile(ctx context.Context, req ctrl.Req
262257

263258
// Check if the ServiceAccount has a valid pull secret before creating the deployment/statefulset
264259
// or setting up knative. Otherwise the pods can go into an ImagePullBackOff loop
265-
saErr := appstacksutils.ServiceAccountPullSecretExists(recCtx, instance, r.GetClient())
260+
saErr := appstacksutils.ServiceAccountPullSecretExists(instance, r.GetClient())
266261
if saErr != nil {
267262
return r.ManageError(saErr, common.StatusConditionTypeReconciled, instance)
268263
}
@@ -324,7 +319,7 @@ func (r *RuntimeComponentReconciler) Reconcile(ctx context.Context, req ctrl.Req
324319
}
325320
}
326321

327-
useCertmanager, err := r.GenerateSvcCertSecret(recCtx, ba, "rco", "Runtime Component Operator", "runtime-component-operator")
322+
useCertmanager, err := r.GenerateSvcCertSecret(ba, "rco", "Runtime Component Operator", "runtime-component-operator")
328323
if err != nil {
329324
reqLogger.Error(err, "Failed to reconcile CertManager Certificate")
330325
return r.ManageError(err, common.StatusConditionTypeReconciled, instance)
@@ -370,7 +365,7 @@ func (r *RuntimeComponentReconciler) Reconcile(ctx context.Context, req ctrl.Req
370365
}
371366
}
372367

373-
err = r.ReconcileBindings(recCtx, instance)
368+
err = r.ReconcileBindings(instance)
374369
if err != nil {
375370
return r.ManageError(err, common.StatusConditionTypeReconciled, ba)
376371
}
@@ -400,7 +395,7 @@ func (r *RuntimeComponentReconciler) Reconcile(ctx context.Context, req ctrl.Req
400395
err = r.CreateOrUpdate(statefulSet, instance, func() error {
401396
appstacksutils.CustomizeStatefulSet(statefulSet, instance)
402397
appstacksutils.CustomizePodSpec(&statefulSet.Spec.Template, instance)
403-
if err := appstacksutils.CustomizePodWithSVCCertificate(recCtx, &statefulSet.Spec.Template, instance, r.GetClient()); err != nil {
398+
if err := appstacksutils.CustomizePodWithSVCCertificate(&statefulSet.Spec.Template, instance, r.GetClient()); err != nil {
404399
return err
405400
}
406401
appstacksutils.CustomizePersistence(statefulSet, instance)
@@ -432,7 +427,7 @@ func (r *RuntimeComponentReconciler) Reconcile(ctx context.Context, req ctrl.Req
432427
err = r.CreateOrUpdate(deploy, instance, func() error {
433428
appstacksutils.CustomizeDeployment(deploy, instance)
434429
appstacksutils.CustomizePodSpec(&deploy.Spec.Template, instance)
435-
if err := appstacksutils.CustomizePodWithSVCCertificate(recCtx, &deploy.Spec.Template, instance, r.GetClient()); err != nil {
430+
if err := appstacksutils.CustomizePodWithSVCCertificate(&deploy.Spec.Template, instance, r.GetClient()); err != nil {
436431
return err
437432
}
438433
return nil
@@ -534,7 +529,7 @@ func (r *RuntimeComponentReconciler) Reconcile(ctx context.Context, req ctrl.Req
534529
} else if ok {
535530
if instance.Spec.Monitoring != nil && (instance.Spec.CreateKnativeService == nil || !*instance.Spec.CreateKnativeService) {
536531
// Validate the monitoring endpoints' configuration before creating/updating the ServiceMonitor
537-
if err := appstacksutils.ValidatePrometheusMonitoringEndpoints(recCtx, instance, r.GetClient(), instance.GetNamespace()); err != nil {
532+
if err := appstacksutils.ValidatePrometheusMonitoringEndpoints(instance, r.GetClient(), instance.GetNamespace()); err != nil {
538533
return r.ManageError(err, common.StatusConditionTypeReconciled, instance)
539534
}
540535
sm := &prometheusv1.ServiceMonitor{ObjectMeta: defaultMeta}

utils/hash.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@ import (
1111
"lukechampine.com/blake3"
1212
)
1313

14-
func HashSecretMapData(data common.SecretMap) string {
15-
return hash(data, serializeSecretMapData)
14+
func HashLockedData(data common.SecretMap) string {
15+
return hash(data, serializeLockedData)
1616
}
1717

1818
func HashData(data map[string][]byte) string {
@@ -34,7 +34,7 @@ func hash(data any, serializer func(any) []byte) string {
3434
return hex.EncodeToString(hash)
3535
}
3636

37-
func serializeSecretMapData(data any) []byte {
37+
func serializeLockedData(data any) []byte {
3838
if _, ok := data.(common.SecretMap); !ok {
3939
return []byte{}
4040
}

utils/reconciler.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -653,7 +653,7 @@ func (r *ReconcilerBase) checkIssuerReady(issuer *certmanagerv1.Issuer) error {
653653
return nil
654654
}
655655

656-
func (r *ReconcilerBase) GenerateCMIssuer(recCtx context.Context, namespace string, prefix string, CACommonName string, operatorName string) error {
656+
func (r *ReconcilerBase) GenerateCMIssuer(namespace string, prefix string, CACommonName string, operatorName string) error {
657657
if ok, err := r.IsGroupVersionSupported(certmanagerv1.SchemeGroupVersion.String(), "Issuer"); err != nil {
658658
return err
659659
} else if !ok {
@@ -750,7 +750,7 @@ func (r *ReconcilerBase) GenerateCMIssuer(recCtx context.Context, namespace stri
750750
return nil
751751
}
752752

753-
func (r *ReconcilerBase) GenerateSvcCertSecret(recCtx context.Context, ba common.BaseComponent, prefix string, CACommonName string, operatorName string) (bool, error) {
753+
func (r *ReconcilerBase) GenerateSvcCertSecret(ba common.BaseComponent, prefix string, CACommonName string, operatorName string) (bool, error) {
754754
delete(ba.GetStatus().GetReferences(), common.StatusReferenceCertSecretName)
755755
cleanup := func() {
756756
if ok, err := r.IsGroupVersionSupported(certmanagerv1.SchemeGroupVersion.String(), "Certificate"); err != nil {
@@ -791,7 +791,7 @@ func (r *ReconcilerBase) GenerateSvcCertSecret(recCtx context.Context, ba common
791791
} else if ok {
792792
bao := ba.(metav1.Object)
793793

794-
err = r.GenerateCMIssuer(recCtx, bao.GetNamespace(), prefix, CACommonName, operatorName)
794+
err = r.GenerateCMIssuer(bao.GetNamespace(), prefix, CACommonName, operatorName)
795795
if err != nil {
796796
if errors.Is(err, APIVersionNotFoundError) {
797797
return false, nil
@@ -854,7 +854,7 @@ func (r *ReconcilerBase) GenerateSvcCertSecret(recCtx context.Context, ba common
854854
svcCert.Spec.IssuerRef.Name = customIssuer.Name
855855
}
856856

857-
rVersion, _ := GetIssuerResourceVersion(recCtx, r.client, svcCert)
857+
rVersion, _ := GetIssuerResourceVersion(r.client, svcCert)
858858
if svcCert.Spec.SecretTemplate == nil {
859859
svcCert.Spec.SecretTemplate = &certmanagerv1.CertificateSecretTemplate{
860860
Annotations: map[string]string{},

utils/reconciler_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,7 @@ func TestCreateOrUpdate(t *testing.T) {
196196
r := NewReconcilerBase(rcl, cl, s, &rest.Config{}, record.NewFakeRecorder(10))
197197

198198
err := r.CreateOrUpdate(serviceAccount, runtimecomponent, func() error {
199-
CustomizeServiceAccount(context.TODO(), serviceAccount, runtimecomponent, r.GetClient())
199+
CustomizeServiceAccount(serviceAccount, runtimecomponent, r.GetClient())
200200
return nil
201201
})
202202

utils/service_binding_reconciler.go

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package utils
22

33
import (
4-
"context"
54
"fmt"
65
"strconv"
76
"strings"
@@ -20,14 +19,14 @@ const (
2019
)
2120

2221
// ReconcileBindings goes through the reconcile logic for service binding
23-
func (r *ReconcilerBase) ReconcileBindings(recCtx context.Context, ba common.BaseComponent) error {
24-
if err := r.reconcileExpose(recCtx, ba); err != nil {
22+
func (r *ReconcilerBase) ReconcileBindings(ba common.BaseComponent) error {
23+
if err := r.reconcileExpose(ba); err != nil {
2524
return err
2625
}
2726
return nil
2827
}
2928

30-
func (r *ReconcilerBase) reconcileExpose(recCtx context.Context, ba common.BaseComponent) error {
29+
func (r *ReconcilerBase) reconcileExpose(ba common.BaseComponent) error {
3130
mObj := ba.(metav1.Object)
3231
bindingSecret, err := common.GetSecret(r.GetClient(), getExposeBindingSecretName(ba), mObj.GetNamespace())
3332
defer bindingSecret.Destroy()
@@ -47,7 +46,7 @@ func (r *ReconcilerBase) reconcileExpose(recCtx context.Context, ba common.BaseC
4746
bindingSecret.LockedData = customSecret.LockedData
4847
customSecret.LockedData = nil
4948
// Apply default values to the override secret if certain values are not set
50-
r.applyDefaultValuesToExpose(recCtx, bindingSecret, ba)
49+
r.applyDefaultValuesToExpose(bindingSecret, ba)
5150

5251
if err := r.CreateOrUpdateSecret(bindingSecret, mObj); err != nil {
5352
return err
@@ -79,7 +78,7 @@ func (r *ReconcilerBase) getCustomValuesToExpose(secret *common.LockedBufferSecr
7978
return nil
8079
}
8180

82-
func (r *ReconcilerBase) applyDefaultValuesToExpose(recCtx context.Context, secret *common.LockedBufferSecret, ba common.BaseComponent) {
81+
func (r *ReconcilerBase) applyDefaultValuesToExpose(secret *common.LockedBufferSecret, ba common.BaseComponent) {
8382
mObj := ba.(metav1.Object)
8483
secret.Labels = ba.GetLabels()
8584
secret.Annotations = MergeMaps(secret.Annotations, ba.GetAnnotations())

utils/utils.go

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -846,7 +846,7 @@ func CustomizePersistence(statefulSet *appsv1.StatefulSet, ba common.BaseCompone
846846
}
847847

848848
// CustomizeServiceAccount ...
849-
func CustomizeServiceAccount(recCtx context.Context, sa *corev1.ServiceAccount, ba common.BaseComponent, client client.Client) error {
849+
func CustomizeServiceAccount(sa *corev1.ServiceAccount, ba common.BaseComponent, client client.Client) error {
850850
sa.Labels = ba.GetLabels()
851851
sa.Annotations = MergeMaps(sa.Annotations, ba.GetAnnotations())
852852

@@ -1111,7 +1111,7 @@ func secretShouldExist(secretKeySelector corev1.SecretKeySelector) bool {
11111111
}
11121112

11131113
// Returns an error if any user specified non-optional Secret or ConfigMap for Prometheus monitoring does not exist, otherwise return nil.
1114-
func ValidatePrometheusMonitoringEndpoints(recCtx context.Context, ba common.BaseComponent, client client.Client, namespace string) error {
1114+
func ValidatePrometheusMonitoringEndpoints(ba common.BaseComponent, client client.Client, namespace string) error {
11151115
var basicAuth *prometheusv1.BasicAuth
11161116
var oauth2 *prometheusv1.OAuth2
11171117
var bearerTokenSecret corev1.SecretKeySelector
@@ -1621,7 +1621,7 @@ func (r *ReconcilerBase) toJSONFromRaw(content *runtime.RawExtension) (map[strin
16211621
// Looks for a pull secret in the service account retrieved from the component
16221622
// Returns nil if there is at least one image pull secret, otherwise an error
16231623
// Will always return nil if 'skipPullSecretValidation' is specified in the CR
1624-
func ServiceAccountPullSecretExists(recCtx context.Context, ba common.BaseComponent, client client.Client) error {
1624+
func ServiceAccountPullSecretExists(ba common.BaseComponent, client client.Client) error {
16251625
obj := ba.(metav1.Object)
16261626
ns := obj.GetNamespace()
16271627
saName := obj.GetName()
@@ -1767,15 +1767,15 @@ func CustomizePodWithSVCCertificate(pts *corev1.PodTemplateSpec, ba common.BaseC
17671767
}
17681768

17691769
func addSecretHashAsAnnotation(pts *corev1.PodTemplateSpec, object metav1.Object, client client.Client, secretName string, groupName string) error {
1770-
secret := &corev1.Secret{}
1771-
err := client.Get(context.Background(), types.NamespacedName{Name: secretName, Namespace: object.GetNamespace()}, secret)
1770+
secret, err := common.GetSecret(client, secretName, object.GetNamespace())
1771+
defer secret.Destroy()
17721772
if err != nil {
17731773
return fmt.Errorf("Secret %q was not found in namespace %q, %w", secretName, object.GetNamespace(), err)
17741774
}
17751775
if pts.ObjectMeta.Annotations == nil {
17761776
pts.ObjectMeta.Annotations = make(map[string]string)
17771777
}
1778-
pts.ObjectMeta.Annotations[groupName+"/secret-"+secretName] = HashData(secret.Data)
1778+
pts.ObjectMeta.Annotations[groupName+"/secret-"+secretName] = HashLockedData(secret.LockedData)
17791779
return nil
17801780
}
17811781

@@ -1845,7 +1845,7 @@ func UpdateConfigMap(configMap *corev1.ConfigMap, key string) {
18451845
data[key] = common.LoadFromConfig(common.Config, key)
18461846
}
18471847

1848-
func GetIssuerResourceVersion(recCtx context.Context, client client.Client, certificate *certmanagerv1.Certificate) (string, error) {
1848+
func GetIssuerResourceVersion(client client.Client, certificate *certmanagerv1.Certificate) (string, error) {
18491849
issuer := &certmanagerv1.Issuer{}
18501850
err := client.Get(context.Background(), types.NamespacedName{Name: certificate.Spec.IssuerRef.Name,
18511851
Namespace: certificate.Namespace}, issuer)
@@ -1854,11 +1854,11 @@ func GetIssuerResourceVersion(recCtx context.Context, client client.Client, cert
18541854
}
18551855
if issuer.Spec.CA != nil {
18561856
caSecret, err := common.GetSecret(client, issuer.Spec.CA.SecretName, certificate.Namespace)
1857-
caSecret.LockedData.Destroy()
1857+
defer caSecret.Destroy()
18581858
if err != nil {
18591859
return "", err
18601860
} else {
1861-
return issuer.ResourceVersion + "," + HashData(caSecret.Data), nil
1861+
return issuer.ResourceVersion + "," + HashLockedData(caSecret.LockedData), nil
18621862
}
18631863
} else {
18641864
return issuer.ResourceVersion, nil

utils/utils_test.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package utils
22

33
import (
4-
"context"
54
"os"
65
"reflect"
76
"strconv"
@@ -536,13 +535,13 @@ func TestCustomizeServiceAccount(t *testing.T) {
536535

537536
spec := appstacksv1.RuntimeComponentSpec{PullSecret: &pullSecret}
538537
sa, runtime := &corev1.ServiceAccount{}, createRuntimeComponent(name, namespace, spec)
539-
CustomizeServiceAccount(context.TODO(), sa, runtime, fcl)
538+
CustomizeServiceAccount(sa, runtime, fcl)
540539
emptySAIPS := sa.ImagePullSecrets[0].Name
541540

542541
newSecret := "my-new-secret"
543542
spec = appstacksv1.RuntimeComponentSpec{PullSecret: &newSecret}
544543
runtime = createRuntimeComponent(name, namespace, spec)
545-
CustomizeServiceAccount(context.TODO(), sa, runtime, fcl)
544+
CustomizeServiceAccount(sa, runtime, fcl)
546545

547546
testCSA := []Test{
548547
{"ServiceAccount image pull secrets is empty", pullSecret, emptySAIPS},

0 commit comments

Comments
 (0)