handle corner cases for StorageID generation

There can be cases where Ceph FSID is not yet initialized or missing
from the server response. In such cases, it is better to error out
early than to proceed with incorrect state.

We also reconcile all blue secrets irrespective of the changes to
ensure that we do not miss out on any updates. Additionally, we
watch CephCluster for FSID changes.

Signed-off-by: Umanga Chapagain <uchapaga@redhat.com>

Author: umangachapagain

addons/blue_secret_controller.go

@@ -2,18 +2,21 @@ package addons
 
 import (
 	"context"
+	"fmt"
 	"log/slog"
 
+	rookv1 "github.com/rook/rook/pkg/apis/ceph.rook.io/v1"
+	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/types"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/builder"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/event"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
-
-	corev1 "k8s.io/api/core/v1"
-	"k8s.io/apimachinery/pkg/api/errors"
-	"k8s.io/apimachinery/pkg/runtime"
+	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 )
 
 // BlueSecretReconciler reconciles a MirrorPeer object
@@ -46,12 +49,50 @@ func (r *BlueSecretReconciler) SetupWithManager(mgr ctrl.Manager) error {
 		},
 	}
 
+	cephClusterPredicate := predicate.Funcs{
+		CreateFunc: func(e event.CreateEvent) bool {
+			c, ok := e.Object.(*rookv1.CephCluster)
+			if !ok {
+				return false
+			}
+			return c.Status.CephStatus.FSID != ""
+		},
+		DeleteFunc: func(e event.DeleteEvent) bool {
+			return false
+		},
+		UpdateFunc: func(e event.UpdateEvent) bool {
+			oc, ok := e.ObjectOld.(*rookv1.CephCluster)
+			if !ok {
+				return false
+			}
+			nc, ok := e.ObjectNew.(*rookv1.CephCluster)
+			if !ok {
+				return false
+			}
+			return oc.Status.CephStatus.FSID != nc.Status.CephStatus.FSID
+		},
+		GenericFunc: func(_ event.GenericEvent) bool {
+			return false
+		},
+	}
+
 	r.Logger.Info("Setting up controller with manager")
 
 	return ctrl.NewControllerManagedBy(mgr).
 		Named("bluesecret_controller").
 		Watches(&corev1.Secret{}, &handler.EnqueueRequestForObject{},
-			builder.WithPredicates(predicate.LabelChangedPredicate{}, blueSecretPredicate)).
+			builder.WithPredicates(blueSecretPredicate)).
+		Watches(&rookv1.CephCluster{}, handler.EnqueueRequestsFromMapFunc(
+			func(_ context.Context, obj client.Object) []reconcile.Request {
+				return []reconcile.Request{
+					{
+						NamespacedName: types.NamespacedName{
+							Name:      fmt.Sprintf("%s-%s", RookDefaultBlueSecretMatchConvergedString, obj.GetName()),
+							Namespace: obj.GetNamespace(),
+						},
+					},
+				}
+			}), builder.WithPredicates(cephClusterPredicate)).
 		Complete(r)
 }
 

controllers/utils/hash_test.go

@@ -110,3 +110,64 @@ func TestCreateUniqueReplicationId(t *testing.T) {
 		})
 	}
 }
+
+func TestCalculateMD5Hash(t *testing.T) {
+	type args struct {
+		value any
+	}
+	tests := []struct {
+		name string
+		args args
+		want string
+	}{
+		{
+			name: "RBD with empty CephFSID",
+			args: args{
+				value: [2]string{"", ""},
+			},
+			want: "2432510f5993984b053e7d74ce53d94c",
+		},
+		{
+			name: "CephFS with empty CephFSID",
+			args: args{
+				value: [2]string{"", "csi"},
+			},
+			want: "793b48b9b17bdad5cd141e6daadfdd4e",
+		},
+		{
+			name: "C1/RBD",
+			args: args{
+				value: [2]string{"a0eaa723-bf87-48db-ad11-17e276edda61", ""},
+			},
+			want: "bb2361459436fd47121419c658d4c79b",
+		},
+		{
+			name: "C1/CephFS",
+			args: args{
+				value: [2]string{"a0eaa723-bf87-48db-ad11-17e276edda61", "csi"},
+			},
+			want: "41711cb35464e85f4446ace583adbaa1",
+		},
+		{
+			name: "C2/RBD",
+			args: args{
+				value: [2]string{"30dc85db-f35f-4c03-bb43-c8592487d8b9", ""},
+			},
+			want: "9e4fbb3d9e52875d6393409a51927243",
+		},
+		{
+			name: "C2/CephFS",
+			args: args{
+				value: [2]string{"30dc85db-f35f-4c03-bb43-c8592487d8b9", "csi"},
+			},
+			want: "4b7e331b508946ca7165f14804c37ffc",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := CalculateMD5Hash(tt.args.value); got != tt.want {
+				t.Errorf("CalculateMD5Hash() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}

controllers/utils/storageclass.go

@@ -121,8 +121,11 @@ func CalculateStorageId(ctx context.Context, c client.Client, sc *storagev1.Stor
 	}
 
 	cephClusterFSID := cephCluster.Status.CephStatus.FSID
-	var storageId string
+	if cephClusterFSID == "" {
+		return "", fmt.Errorf("failed to calculate StorageID. Ceph FSID is empty")
+	}
 
+	var storageId string
 	switch sc.Provisioner {
 	case fmt.Sprintf(RBDProvisionerTemplate, storageClusterNamespacedName.Namespace):
 		storageId = CalculateMD5Hash([2]string{cephClusterFSID, DefaultRadosNamespace})