Skip to content

Commit

Permalink
Merge config and spec hash
Browse files Browse the repository at this point in the history
  • Loading branch information
@tjamet
tjamet committed Aug 20, 2024
1 parent c023711 commit c8563e7
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 69 deletions.
74 changes: 32 additions & 42 deletions internal/controller/genericprovider_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"encoding/json"
"errors"
"fmt"
"hash"

corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
Expand Down Expand Up @@ -49,8 +50,7 @@ type GenericProviderReconciler struct {
}

const (
appliedSpecHashAnnotation = "operator.cluster.x-k8s.io/applied-spec-hash"
appliedConfigHashAnnotation = "operator.cluster.x-k8s.io/applied-config-hash"
appliedSpecHashAnnotation = "operator.cluster.x-k8s.io/applied-spec-hash"
)

func (r *GenericProviderReconciler) SetupWithManager(mgr ctrl.Manager, options controller.Options) error {
Expand Down Expand Up @@ -113,23 +113,12 @@ func (r *GenericProviderReconciler) Reconcile(ctx context.Context, req reconcile
}

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

upTodate := r.Provider.GetAnnotations()[appliedSpecHashAnnotation] == specHash

configHash, err := r.getProviderConfigSecretHash(ctx)
if err != nil {
return ctrl.Result{}, err
}

if r.Provider.GetAnnotations()[appliedConfigHashAnnotation] != configHash {
upTodate = false
}

if upTodate {
if r.Provider.GetAnnotations()[appliedSpecHashAnnotation] == specHash {
log.Info("No changes detected, skipping further steps")
return ctrl.Result{}, nil
}
Expand All @@ -144,25 +133,14 @@ func (r *GenericProviderReconciler) Reconcile(ctx context.Context, req reconcile
// Set the spec hash annotation if reconciliation was successful or reset it otherwise.
if res.IsZero() && err == nil {
// Recalculate spec hash in case it was changed during reconciliation process.
specHash, err = calculateHash(r.Provider.GetSpec())
specHash, err := calculateHash(ctx, r.Client, r.Provider)
if err != nil {
return ctrl.Result{}, err
}

annotations[appliedSpecHashAnnotation] = specHash

configHash, err := r.getProviderConfigSecretHash(ctx)
if err != nil {
return ctrl.Result{}, err
}
if configHash != "" {
annotations[appliedConfigHashAnnotation] = configHash
} else {
delete(annotations, appliedConfigHashAnnotation)
}
} else {
annotations[appliedSpecHashAnnotation] = ""
delete(annotations, appliedConfigHashAnnotation)
}

r.Provider.SetAnnotations(annotations)
Expand Down Expand Up @@ -250,40 +228,52 @@ func (r *GenericProviderReconciler) reconcileDelete(ctx context.Context, provide
return res, nil
}

func (r *GenericProviderReconciler) getProviderConfigSecretHash(ctx context.Context) (string, error) {
if r.Provider.GetSpec().ConfigSecret != nil {
func addConfigSecretToHash(ctx context.Context, k8sClient client.Client, hash hash.Hash, provider genericprovider.GenericProvider) error {
if provider.GetSpec().ConfigSecret != nil {
secret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Namespace: r.Provider.GetSpec().ConfigSecret.Namespace,
Name: r.Provider.GetSpec().ConfigSecret.Name,
Namespace: provider.GetSpec().ConfigSecret.Namespace,
Name: provider.GetSpec().ConfigSecret.Name,
},
}
if secret.Namespace == "" {
secret.Namespace = r.Provider.GetNamespace()
secret.Namespace = provider.GetNamespace()
}
err := r.Client.Get(ctx, client.ObjectKeyFromObject(secret), secret)
err := k8sClient.Get(ctx, client.ObjectKeyFromObject(secret), secret)
if err != nil {
return "", err
return err
}
configHash, err := calculateHash(secret.Data)
err = addObjectToHash(hash, secret.Data)
if err != nil {
return "", err
return err
}
return configHash, nil
return nil
}
return "", nil
return nil
}

func calculateHash(object interface{}) (string, error) {
func addObjectToHash(hash hash.Hash, object interface{}) error {
jsonData, err := json.Marshal(object)
if err != nil {
return "", fmt.Errorf("cannot parse provider spec: %w", err)
return fmt.Errorf("cannot marshal object: %w", err)
}

if _, err = hash.Write(jsonData); err != nil {
return fmt.Errorf("cannot calculate object hash: %w", err)
}
return nil
}

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

if _, err = hash.Write(jsonData); err != nil {
return "", fmt.Errorf("cannot calculate provider spec hash: %w", err)
err := addObjectToHash(hash, provider.GetSpec())
if err != nil {
return "", err
}

if err := addConfigSecretToHash(ctx, k8sClient, hash, provider); err != nil {
return "", err
}

return fmt.Sprintf("%x", hash.Sum(nil)), nil
Expand Down
58 changes: 31 additions & 27 deletions internal/controller/genericprovider_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package controller

import (
"context"
"testing"
"time"

Expand Down Expand Up @@ -527,7 +528,7 @@ func TestProviderConfigSecretChanges(t *testing.T) {
expectSameHash bool
}{
{
name: "With the same configmap data, the hash annotation doesn't change",
name: "With the same config secret data, the hash annotation doesn't change",
cmData: map[string][]byte{
"some-key": []byte("some data"),
"another-key": []byte("another data"),
Expand All @@ -539,7 +540,7 @@ func TestProviderConfigSecretChanges(t *testing.T) {
expectSameHash: true,
},
{
name: "With the same configmap data, the hash annotation doesn't change",
name: "With different config secret data, the hash annotation changes",
cmData: map[string][]byte{
"some-key": []byte("some data"),
"another-key": []byte("another data"),
Expand All @@ -556,18 +557,6 @@ func TestProviderConfigSecretChanges(t *testing.T) {
t.Run(tc.name, func(t *testing.T) {
g := NewWithT(t)

dataHash, err := calculateHash(tc.cmData)
g.Expect(err).ToNot(HaveOccurred())

updatedDataHash, err := calculateHash(tc.updatedCMData)
g.Expect(err).ToNot(HaveOccurred())

if tc.expectSameHash {
g.Expect(updatedDataHash).To(Equal(dataHash))
} else {
g.Expect(updatedDataHash).ToNot(Equal(dataHash))
}

cmSecretName := "test-config"

provider := &operatorv1.CoreProvider{
Expand Down Expand Up @@ -614,10 +603,13 @@ func TestProviderConfigSecretChanges(t *testing.T) {

g.Expect(env.CreateAndWait(ctx, secret.DeepCopy())).To(Succeed())

initialHash, err := calculateHash(ctx, env.Client, provider)
g.Expect(err).ToNot(HaveOccurred())

t.Log("creating test provider", provider.GetName())
g.Expect(env.CreateAndWait(ctx, provider.DeepCopy())).To(Succeed())

g.Eventually(generateExpectedResultChecker(provider, appliedConfigHashAnnotation, dataHash, corev1.ConditionTrue), timeout).Should(BeEquivalentTo(true))
g.Eventually(generateExpectedResultChecker(provider, initialHash, corev1.ConditionTrue), timeout).Should(BeEquivalentTo(true))

g.Eventually(func() error {
if err := env.Client.Get(ctx, client.ObjectKeyFromObject(secret), secret); err != nil {
Expand All @@ -630,6 +622,15 @@ func TestProviderConfigSecretChanges(t *testing.T) {
return env.Client.Update(ctx, secret)
}).Should(Succeed())

updatedDataHash, err := calculateHash(ctx, env.Client, provider)
g.Expect(err).ToNot(HaveOccurred())

if tc.expectSameHash {
g.Expect(updatedDataHash).To(Equal(initialHash))
} else {
g.Expect(updatedDataHash).ToNot(Equal(initialHash))
}

g.Eventually(func() error {
if err := env.Client.Get(ctx, client.ObjectKeyFromObject(provider), provider); err != nil {
return err
Expand All @@ -647,7 +648,7 @@ func TestProviderConfigSecretChanges(t *testing.T) {
return env.Client.Update(ctx, provider)
}).Should(Succeed())

g.Eventually(generateExpectedResultChecker(provider, appliedConfigHashAnnotation, updatedDataHash, corev1.ConditionTrue), timeout).Should(BeEquivalentTo(true))
g.Eventually(generateExpectedResultChecker(provider, updatedDataHash, corev1.ConditionTrue), timeout).Should(BeEquivalentTo(true))

// Clean up
g.Expect(env.Cleanup(ctx, provider, secret, providerNamespace, configNamespace)).To(Succeed())
Expand Down Expand Up @@ -744,12 +745,6 @@ func TestProviderSpecChanges(t *testing.T) {
t.Run(tc.name, func(t *testing.T) {
g := NewWithT(t)

specHash, err := calculateHash(tc.spec)
g.Expect(err).ToNot(HaveOccurred())

updatedSpecHash, err := calculateHash(tc.updatedSpec)
g.Expect(err).ToNot(HaveOccurred())

provider := &operatorv1.CoreProvider{
ObjectMeta: metav1.ObjectMeta{
Name: "cluster-api",
Expand All @@ -759,6 +754,15 @@ func TestProviderSpecChanges(t *testing.T) {
},
}

updatedProvider := provider.DeepCopy()
updatedProvider.SetSpec(tc.updatedSpec)

specHash, err := calculateHash(context.Background(), env.Client, provider)
g.Expect(err).ToNot(HaveOccurred())

updatedSpecHash, err := calculateHash(context.Background(), env.Client, updatedProvider)
g.Expect(err).ToNot(HaveOccurred())

namespace := "test-provider-spec-changes"

ns, err := env.CreateNamespace(ctx, namespace)
Expand All @@ -771,7 +775,7 @@ func TestProviderSpecChanges(t *testing.T) {
t.Log("creating test provider", provider.GetName())
g.Expect(env.CreateAndWait(ctx, provider.DeepCopy())).To(Succeed())

g.Eventually(generateExpectedResultChecker(provider, appliedSpecHashAnnotation, specHash, corev1.ConditionTrue), timeout).Should(BeEquivalentTo(true))
g.Eventually(generateExpectedResultChecker(provider, specHash, corev1.ConditionTrue), timeout).Should(BeEquivalentTo(true))

g.Eventually(func() error {
if err := env.Client.Get(ctx, client.ObjectKeyFromObject(provider), provider); err != nil {
Expand All @@ -794,9 +798,9 @@ func TestProviderSpecChanges(t *testing.T) {
}).Should(Succeed())

if !tc.expectError {
g.Eventually(generateExpectedResultChecker(provider, appliedSpecHashAnnotation, updatedSpecHash, corev1.ConditionTrue), timeout).Should(BeEquivalentTo(true))
g.Eventually(generateExpectedResultChecker(provider, updatedSpecHash, corev1.ConditionTrue), timeout).Should(BeEquivalentTo(true))
} else {
g.Eventually(generateExpectedResultChecker(provider, appliedSpecHashAnnotation, "", corev1.ConditionFalse), timeout).Should(BeEquivalentTo(true))
g.Eventually(generateExpectedResultChecker(provider, "", corev1.ConditionFalse), timeout).Should(BeEquivalentTo(true))
}

// Clean up
Expand All @@ -805,14 +809,14 @@ func TestProviderSpecChanges(t *testing.T) {
}
}

func generateExpectedResultChecker(provider genericprovider.GenericProvider, hashKey, specHash string, condStatus corev1.ConditionStatus) func() bool {
func generateExpectedResultChecker(provider genericprovider.GenericProvider, specHash string, condStatus corev1.ConditionStatus) func() bool {
return func() bool {
if err := env.Get(ctx, client.ObjectKeyFromObject(provider), provider); err != nil {
return false
}

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

Expand Down
6 changes: 6 additions & 0 deletions internal/envtest/environment.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,12 @@ func New(uncachedObjs ...client.Object) *Environment {
err = kerrors.NewAggregate([]error{err, env.Stop()})
panic(err)
}
u, _ := env.AddUser(envtest.User{Name: "debug", Groups: []string{"system:masters"}}, env.Config)

cfg, _ := u.KubeConfig()
fd, _ := os.Create("kubeconfig")
fd.Write(cfg)
defer fd.Close()

objs := []client.Object{}
if len(uncachedObjs) > 0 {
Expand Down

0 comments on commit c8563e7

Please sign in to comment.