Skip to content

Commit d3e5099

Browse files
tjamettjamet
committed
Merge config and spec hash
1 parent 288451f commit d3e5099

File tree

2 files changed

+69
-69
lines changed

2 files changed

+69
-69
lines changed

‎internal/controller/genericprovider_controller.go‎

Lines changed: 32 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
"encoding/json"
2323
"errors"
2424
"fmt"
25+
"hash"
2526

2627
corev1 "k8s.io/api/core/v1"
2728
apierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -50,8 +51,7 @@ type GenericProviderReconciler struct {
5051
}
5152

5253
const (
53-
appliedSpecHashAnnotation = "operator.cluster.x-k8s.io/applied-spec-hash"
54-
appliedConfigHashAnnotation = "operator.cluster.x-k8s.io/applied-config-hash"
54+
appliedSpecHashAnnotation = "operator.cluster.x-k8s.io/applied-spec-hash"
5555
)
5656

5757
func (r *GenericProviderReconciler) SetupWithManager(mgr ctrl.Manager, options controller.Options) error {
@@ -114,23 +114,12 @@ func (r *GenericProviderReconciler) Reconcile(ctx context.Context, req reconcile
114114
}
115115

116116
// Check if spec hash stays the same and don't go further in this case.
117-
specHash, err := calculateHash(r.Provider.GetSpec())
117+
specHash, err := calculateHash(ctx, r.Client, r.Provider)
118118
if err != nil {
119119
return ctrl.Result{}, err
120120
}
121121

122-
upTodate := r.Provider.GetAnnotations()[appliedSpecHashAnnotation] == specHash
123-
124-
configHash, err := r.getProviderConfigSecretHash(ctx)
125-
if err != nil {
126-
return ctrl.Result{}, err
127-
}
128-
129-
if r.Provider.GetAnnotations()[appliedConfigHashAnnotation] != configHash {
130-
upTodate = false
131-
}
132-
133-
if upTodate {
122+
if r.Provider.GetAnnotations()[appliedSpecHashAnnotation] == specHash {
134123
log.Info("No changes detected, skipping further steps")
135124
return ctrl.Result{}, nil
136125
}
@@ -145,25 +134,14 @@ func (r *GenericProviderReconciler) Reconcile(ctx context.Context, req reconcile
145134
// Set the spec hash annotation if reconciliation was successful or reset it otherwise.
146135
if res.IsZero() && err == nil {
147136
// Recalculate spec hash in case it was changed during reconciliation process.
148-
specHash, err = calculateHash(r.Provider.GetSpec())
137+
specHash, err := calculateHash(ctx, r.Client, r.Provider)
149138
if err != nil {
150139
return ctrl.Result{}, err
151140
}
152141

153142
annotations[appliedSpecHashAnnotation] = specHash
154-
155-
configHash, err := r.getProviderConfigSecretHash(ctx)
156-
if err != nil {
157-
return ctrl.Result{}, err
158-
}
159-
if configHash != "" {
160-
annotations[appliedConfigHashAnnotation] = configHash
161-
} else {
162-
delete(annotations, appliedConfigHashAnnotation)
163-
}
164143
} else {
165144
annotations[appliedSpecHashAnnotation] = ""
166-
delete(annotations, appliedConfigHashAnnotation)
167145
}
168146

169147
r.Provider.SetAnnotations(annotations)
@@ -251,40 +229,52 @@ func (r *GenericProviderReconciler) reconcileDelete(ctx context.Context, provide
251229
return res, nil
252230
}
253231

254-
func (r *GenericProviderReconciler) getProviderConfigSecretHash(ctx context.Context) (string, error) {
255-
if r.Provider.GetSpec().ConfigSecret != nil {
232+
func addConfigSecretToHash(ctx context.Context, k8sClient client.Client, hash hash.Hash, provider genericprovider.GenericProvider) error {
233+
if provider.GetSpec().ConfigSecret != nil {
256234
secret := &corev1.Secret{
257235
ObjectMeta: metav1.ObjectMeta{
258-
Namespace: r.Provider.GetSpec().ConfigSecret.Namespace,
259-
Name: r.Provider.GetSpec().ConfigSecret.Name,
236+
Namespace: provider.GetSpec().ConfigSecret.Namespace,
237+
Name: provider.GetSpec().ConfigSecret.Name,
260238
},
261239
}
262240
if secret.Namespace == "" {
263-
secret.Namespace = r.Provider.GetNamespace()
241+
secret.Namespace = provider.GetNamespace()
264242
}
265-
err := r.Client.Get(ctx, client.ObjectKeyFromObject(secret), secret)
243+
err := k8sClient.Get(ctx, client.ObjectKeyFromObject(secret), secret)
266244
if err != nil {
267-
return "", err
245+
return err
268246
}
269-
configHash, err := calculateHash(secret.Data)
247+
err = addObjectToHash(hash, secret.Data)
270248
if err != nil {
271-
return "", err
249+
return err
272250
}
273-
return configHash, nil
251+
return nil
274252
}
275-
return "", nil
253+
return nil
276254
}
277255

278-
func calculateHash(object interface{}) (string, error) {
256+
func addObjectToHash(hash hash.Hash, object interface{}) error {
279257
jsonData, err := json.Marshal(object)
280258
if err != nil {
281-
return "", fmt.Errorf("cannot parse provider spec: %w", err)
259+
return fmt.Errorf("cannot marshal object: %w", err)
260+
}
261+
262+
if _, err = hash.Write(jsonData); err != nil {
263+
return fmt.Errorf("cannot calculate object hash: %w", err)
282264
}
265+
return nil
266+
}
283267

268+
func calculateHash(ctx context.Context, k8sClient client.Client, provider genericprovider.GenericProvider) (string, error) {
284269
hash := sha256.New()
285270

286-
if _, err = hash.Write(jsonData); err != nil {
287-
return "", fmt.Errorf("cannot calculate provider spec hash: %w", err)
271+
err := addObjectToHash(hash, provider.GetSpec())
272+
if err != nil {
273+
return "", err
274+
}
275+
276+
if err := addConfigSecretToHash(ctx, k8sClient, hash, provider); err != nil {
277+
return "", err
288278
}
289279

290280
return fmt.Sprintf("%x", hash.Sum(nil)), nil

‎internal/controller/genericprovider_controller_test.go‎

Lines changed: 37 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ limitations under the License.
1717
package controller
1818

1919
import (
20+
"context"
2021
"testing"
2122
"time"
2223

@@ -536,7 +537,7 @@ func TestProviderConfigSecretChanges(t *testing.T) {
536537
expectSameHash bool
537538
}{
538539
{
539-
name: "With the same configmap data, the hash annotation doesn't change",
540+
name: "With the same config secret data, the hash annotation doesn't change",
540541
cmData: map[string][]byte{
541542
"some-key": []byte("some data"),
542543
"another-key": []byte("another data"),
@@ -548,7 +549,7 @@ func TestProviderConfigSecretChanges(t *testing.T) {
548549
expectSameHash: true,
549550
},
550551
{
551-
name: "With the same configmap data, the hash annotation doesn't change",
552+
name: "With different config secret data, the hash annotation changes",
552553
cmData: map[string][]byte{
553554
"some-key": []byte("some data"),
554555
"another-key": []byte("another data"),
@@ -569,18 +570,6 @@ func TestProviderConfigSecretChanges(t *testing.T) {
569570
g.Expect(env.CleanupAndWait(ctx, objs...)).To(Succeed())
570571
}()
571572

572-
dataHash, err := calculateHash(tc.cmData)
573-
g.Expect(err).ToNot(HaveOccurred())
574-
575-
updatedDataHash, err := calculateHash(tc.updatedCMData)
576-
g.Expect(err).ToNot(HaveOccurred())
577-
578-
if tc.expectSameHash {
579-
g.Expect(updatedDataHash).To(Equal(dataHash))
580-
} else {
581-
g.Expect(updatedDataHash).ToNot(Equal(dataHash))
582-
}
583-
584573
cmSecretName := "test-config"
585574

586575
provider := &operatorv1.CoreProvider{
@@ -630,11 +619,14 @@ func TestProviderConfigSecretChanges(t *testing.T) {
630619
g.Expect(env.CreateAndWait(ctx, secret.DeepCopy())).To(Succeed())
631620
objs = append(objs, secret)
632621

622+
initialHash, err := calculateHash(ctx, env.Client, provider)
623+
g.Expect(err).ToNot(HaveOccurred())
624+
633625
t.Log("creating test provider", provider.GetName())
634626
g.Expect(env.CreateAndWait(ctx, provider.DeepCopy())).To(Succeed())
635627
objs = append(objs, provider)
636628

637-
g.Eventually(generateExpectedResultChecker(provider, appliedConfigHashAnnotation, dataHash, corev1.ConditionTrue), timeout).Should(BeEquivalentTo(true))
629+
g.Eventually(generateExpectedResultChecker(provider, initialHash, corev1.ConditionTrue), timeout).Should(BeEquivalentTo(true))
638630

639631
g.Eventually(func() error {
640632
if err := env.Client.Get(ctx, client.ObjectKeyFromObject(secret), secret); err != nil {
@@ -647,6 +639,21 @@ func TestProviderConfigSecretChanges(t *testing.T) {
647639
return env.Client.Update(ctx, secret)
648640
}).Should(Succeed())
649641

642+
var updatedDataHash string
643+
if tc.expectSameHash {
644+
g.Eventually(func() string {
645+
updatedDataHash, err = calculateHash(ctx, env.Client, provider)
646+
g.Expect(err).ToNot(HaveOccurred())
647+
return updatedDataHash
648+
}, 15*time.Second).Should(Equal(initialHash))
649+
} else {
650+
g.Eventually(func() string {
651+
updatedDataHash, err = calculateHash(ctx, env.Client, provider)
652+
g.Expect(err).ToNot(HaveOccurred())
653+
return updatedDataHash
654+
}, 15*time.Second).ShouldNot(Equal(initialHash))
655+
}
656+
650657
g.Eventually(func() error {
651658
if err := env.Client.Get(ctx, client.ObjectKeyFromObject(provider), provider); err != nil {
652659
return err
@@ -664,7 +671,7 @@ func TestProviderConfigSecretChanges(t *testing.T) {
664671
return env.Client.Update(ctx, provider)
665672
}).Should(Succeed())
666673

667-
g.Eventually(generateExpectedResultChecker(provider, appliedConfigHashAnnotation, updatedDataHash, corev1.ConditionTrue), timeout).Should(BeEquivalentTo(true))
674+
g.Eventually(generateExpectedResultChecker(provider, updatedDataHash, corev1.ConditionTrue), timeout).Should(BeEquivalentTo(true))
668675
})
669676
}
670677
}
@@ -758,12 +765,6 @@ func TestProviderSpecChanges(t *testing.T) {
758765
t.Run(tc.name, func(t *testing.T) {
759766
g := NewWithT(t)
760767

761-
specHash, err := calculateHash(tc.spec)
762-
g.Expect(err).ToNot(HaveOccurred())
763-
764-
updatedSpecHash, err := calculateHash(tc.updatedSpec)
765-
g.Expect(err).ToNot(HaveOccurred())
766-
767768
provider := &operatorv1.CoreProvider{
768769
ObjectMeta: metav1.ObjectMeta{
769770
Name: "cluster-api",
@@ -773,6 +774,15 @@ func TestProviderSpecChanges(t *testing.T) {
773774
},
774775
}
775776

777+
updatedProvider := provider.DeepCopy()
778+
updatedProvider.SetSpec(tc.updatedSpec)
779+
780+
specHash, err := calculateHash(context.Background(), env.Client, provider)
781+
g.Expect(err).ToNot(HaveOccurred())
782+
783+
updatedSpecHash, err := calculateHash(context.Background(), env.Client, updatedProvider)
784+
g.Expect(err).ToNot(HaveOccurred())
785+
776786
namespace := "test-provider-spec-changes"
777787

778788
ns, err := env.CreateNamespace(ctx, namespace)
@@ -785,7 +795,7 @@ func TestProviderSpecChanges(t *testing.T) {
785795
t.Log("creating test provider", provider.GetName())
786796
g.Expect(env.CreateAndWait(ctx, provider.DeepCopy())).To(Succeed())
787797

788-
g.Eventually(generateExpectedResultChecker(provider, appliedSpecHashAnnotation, specHash, corev1.ConditionTrue), timeout).Should(BeEquivalentTo(true))
798+
g.Eventually(generateExpectedResultChecker(provider, specHash, corev1.ConditionTrue), timeout).Should(BeEquivalentTo(true))
789799

790800
g.Eventually(func() error {
791801
if err := env.Client.Get(ctx, client.ObjectKeyFromObject(provider), provider); err != nil {
@@ -808,9 +818,9 @@ func TestProviderSpecChanges(t *testing.T) {
808818
}).Should(Succeed())
809819

810820
if !tc.expectError {
811-
g.Eventually(generateExpectedResultChecker(provider, appliedSpecHashAnnotation, updatedSpecHash, corev1.ConditionTrue), timeout).Should(BeEquivalentTo(true))
821+
g.Eventually(generateExpectedResultChecker(provider, updatedSpecHash, corev1.ConditionTrue), timeout).Should(BeEquivalentTo(true))
812822
} else {
813-
g.Eventually(generateExpectedResultChecker(provider, appliedSpecHashAnnotation, "", corev1.ConditionFalse), timeout).Should(BeEquivalentTo(true))
823+
g.Eventually(generateExpectedResultChecker(provider, "", corev1.ConditionFalse), timeout).Should(BeEquivalentTo(true))
814824
}
815825

816826
// Clean up
@@ -819,14 +829,14 @@ func TestProviderSpecChanges(t *testing.T) {
819829
}
820830
}
821831

822-
func generateExpectedResultChecker(provider genericprovider.GenericProvider, hashKey, specHash string, condStatus corev1.ConditionStatus) func() bool {
832+
func generateExpectedResultChecker(provider genericprovider.GenericProvider, specHash string, condStatus corev1.ConditionStatus) func() bool {
823833
return func() bool {
824834
if err := env.Get(ctx, client.ObjectKeyFromObject(provider), provider); err != nil {
825835
return false
826836
}
827837

828838
// In case of error we don't want the spec annotation to be updated
829-
if provider.GetAnnotations()[hashKey] != specHash {
839+
if provider.GetAnnotations()[appliedSpecHashAnnotation] != specHash {
830840
return false
831841
}
832842

0 commit comments

Comments
 (0)