diff --git a/deploy/charts/rook-ceph-cluster/values.yaml b/deploy/charts/rook-ceph-cluster/values.yaml index 58a680f01a9f..f8c6baac18a9 100644 --- a/deploy/charts/rook-ceph-cluster/values.yaml +++ b/deploy/charts/rook-ceph-cluster/values.yaml @@ -157,7 +157,7 @@ cephClusterSpec: # the corresponding "backend protocol" annotation(s) for your ingress controller of choice) ssl: true - # Network configuration, see: https://github.com/rook/rook/blob/master/Documentation/CRDs/ceph-cluster-crd.md#network-configuration-settings + # Network configuration, see: https://github.com/rook/rook/blob/master/Documentation/CRDs/Cluster/ceph-cluster-crd.md#network-configuration-settings network: connections: # Whether to encrypt the data in transit across the wire to prevent eavesdropping the data on the network. diff --git a/pkg/operator/ceph/cluster/predicate.go b/pkg/operator/ceph/cluster/predicate.go index 2b19649aaefc..f7f5112a625b 100644 --- a/pkg/operator/ceph/cluster/predicate.go +++ b/pkg/operator/ceph/cluster/predicate.go @@ -21,6 +21,7 @@ import ( "context" "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" cephv1 "github.com/rook/rook/pkg/apis/ceph.rook.io/v1" "github.com/rook/rook/pkg/clusterd" discoverDaemon "github.com/rook/rook/pkg/daemon/discover" @@ -28,12 +29,28 @@ import ( "github.com/rook/rook/pkg/operator/k8sutil" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/event" "sigs.k8s.io/controller-runtime/pkg/predicate" ) +func shouldReconcileChangedNode(objOld, objNew *corev1.Node) bool { + // do not watch node if only resourceversion got changed + resourceQtyComparer := cmpopts.IgnoreFields(v1.ObjectMeta{}, "ResourceVersion") + diff := cmp.Diff(objOld.Spec, objNew.Spec, resourceQtyComparer) + + // do not watch node if only LastHeartbeatTime got changed + resourceQtyComparer = cmpopts.IgnoreFields(corev1.NodeCondition{}, "LastHeartbeatTime") + diff1 := cmp.Diff(objOld.Spec, objNew.Spec, resourceQtyComparer) + + if diff == "" && diff1 == "" { + return false + } + return true +} + // predicateForNodeWatcher is the predicate function to trigger reconcile on Node events func predicateForNodeWatcher(ctx context.Context, client client.Client, context *clusterd.Context) predicate.Funcs { return predicate.Funcs{ @@ -43,8 +60,17 @@ func predicateForNodeWatcher(ctx context.Context, client client.Client, context }, UpdateFunc: func(e event.UpdateEvent) bool { - clientCluster := newClientCluster(client, e.ObjectNew.GetNamespace(), context) - return clientCluster.onK8sNode(ctx, e.ObjectNew) + if objOld, ok := e.ObjectOld.(*corev1.Node); ok { + if objNew, ok := e.ObjectNew.(*corev1.Node); ok { + if !shouldReconcileChangedNode(objOld, objNew) { + return false + } + + clientCluster := newClientCluster(client, e.ObjectNew.GetNamespace(), context) + return clientCluster.onK8sNode(ctx, e.ObjectNew) + } + } + return false }, DeleteFunc: func(e event.DeleteEvent) bool { diff --git a/pkg/operator/ceph/cluster/predicate_test.go b/pkg/operator/ceph/cluster/predicate_test.go index aaec26afa794..ce760d42a2c3 100644 --- a/pkg/operator/ceph/cluster/predicate_test.go +++ b/pkg/operator/ceph/cluster/predicate_test.go @@ -18,10 +18,12 @@ package cluster import ( "testing" + "time" cephv1 "github.com/rook/rook/pkg/apis/ceph.rook.io/v1" "github.com/stretchr/testify/assert" corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) func TestIsHotPlugCM(t *testing.T) { @@ -40,3 +42,28 @@ func TestIsHotPlugCM(t *testing.T) { cm.Labels["app"] = "rook-discover" assert.True(t, isHotPlugCM(cm)) } + +func TestCompareNodes(t *testing.T) { + tests := []struct { + name string + oldobj corev1.Node + newobj corev1.Node + reconcile bool + }{ + {"if no changes", corev1.Node{}, corev1.Node{}, false}, + {"if only Resourceversion got changed", corev1.Node{ObjectMeta: metav1.ObjectMeta{ResourceVersion: "123"}}, corev1.Node{ObjectMeta: metav1.ObjectMeta{ResourceVersion: "145"}}, false}, + {"if only heartbeat got changed", corev1.Node{Status: corev1.NodeStatus{Conditions: []corev1.NodeCondition{{LastHeartbeatTime: metav1.NewTime(time.Now())}}}}, corev1.Node{Status: corev1.NodeStatus{Conditions: []corev1.NodeCondition{{LastHeartbeatTime: metav1.NewTime(<-time.After(5))}}}}, false}, + {"if both Resourceversion and heartbeat change", corev1.Node{ObjectMeta: metav1.ObjectMeta{ResourceVersion: "123"}, Status: corev1.NodeStatus{Conditions: []corev1.NodeCondition{{LastHeartbeatTime: metav1.NewTime(time.Now())}}}}, corev1.Node{ObjectMeta: metav1.ObjectMeta{ResourceVersion: "145"}, Status: corev1.NodeStatus{Conditions: []corev1.NodeCondition{{LastHeartbeatTime: metav1.NewTime(<-time.After(5))}}}}, false}, + {"if only a field changes", corev1.Node{Spec: corev1.NodeSpec{PodCIDR: "123"}}, corev1.Node{Spec: corev1.NodeSpec{PodCIDR: "145"}}, true}, + {"if a field and the Resourceversion change", corev1.Node{Spec: corev1.NodeSpec{PodCIDR: "123"}, ObjectMeta: metav1.ObjectMeta{ResourceVersion: "123"}}, corev1.Node{Spec: corev1.NodeSpec{PodCIDR: "145"}, ObjectMeta: metav1.ObjectMeta{ResourceVersion: "145"}}, true}, + {"if a field and the hertbeat change", corev1.Node{Spec: corev1.NodeSpec{PodCIDR: "123"}, Status: corev1.NodeStatus{Conditions: []corev1.NodeCondition{{LastHeartbeatTime: metav1.NewTime(time.Now())}}}}, corev1.Node{Spec: corev1.NodeSpec{PodCIDR: "145"}, Status: corev1.NodeStatus{Conditions: []corev1.NodeCondition{{LastHeartbeatTime: metav1.NewTime(<-time.After(5))}}}}, true}, + {"if a field, Resourceversion and the hertbeat change", corev1.Node{ObjectMeta: metav1.ObjectMeta{ResourceVersion: "123"}, Spec: corev1.NodeSpec{PodCIDR: "123"}, Status: corev1.NodeStatus{Conditions: []corev1.NodeCondition{{LastHeartbeatTime: metav1.NewTime(time.Now())}}}}, corev1.Node{ObjectMeta: metav1.ObjectMeta{ResourceVersion: "145"}, Spec: corev1.NodeSpec{PodCIDR: "145"}, Status: corev1.NodeStatus{Conditions: []corev1.NodeCondition{{LastHeartbeatTime: metav1.NewTime(<-time.After(5))}}}}, true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + //#nosec G601 -- since nothing is modifying the tests slic + assert.Equal(t, tt.reconcile, shouldReconcileChangedNode(&tt.oldobj, &tt.newobj)) + }) + } +}