From 9b4c2fdc19c0604dfca3094603b5565a5c5ec2ba Mon Sep 17 00:00:00 2001 From: parth-gr Date: Thu, 18 Jul 2024 18:30:58 +0530 Subject: [PATCH] external: enable log rotation and log collection for external mode with 4.17 csi pods will be collecting logs by the log rtoation setting, so even if its external mode turn on the log collector setting Signed-off-by: parth-gr --- controllers/storagecluster/cephcluster.go | 28 ++++++------- .../storagecluster/cephcluster_test.go | 40 +++++++------------ .../storagecluster_controller_test.go | 5 +-- 3 files changed, 30 insertions(+), 43 deletions(-) diff --git a/controllers/storagecluster/cephcluster.go b/controllers/storagecluster/cephcluster.go index 81793cda09..03e931a0c9 100644 --- a/controllers/storagecluster/cephcluster.go +++ b/controllers/storagecluster/cephcluster.go @@ -192,15 +192,9 @@ func (obj *ocsCephCluster) ensureCreated(r *StorageClusterReconciler, sc *ocsv1. return reconcile.Result{}, err } } - cephCluster, err = newCephCluster(sc, r.images.Ceph, kmsConfigMap, r.Log) - if err != nil { - return reconcile.Result{}, err - } + cephCluster = newCephCluster(sc, r.images.Ceph, kmsConfigMap, r.Log) } else { - cephCluster, err = newCephCluster(sc, r.images.Ceph, nil, r.Log) - if err != nil { - return reconcile.Result{}, err - } + cephCluster = newCephCluster(sc, r.images.Ceph, nil, r.Log) } } @@ -411,15 +405,12 @@ func getCephClusterMonitoringLabels(sc ocsv1.StorageCluster) map[string]string { } // newCephCluster returns a CephCluster object. -func newCephCluster(sc *ocsv1.StorageCluster, cephImage string, kmsConfigMap *corev1.ConfigMap, reqLogger logr.Logger) (*rookCephv1.CephCluster, error) { +func newCephCluster(sc *ocsv1.StorageCluster, cephImage string, kmsConfigMap *corev1.ConfigMap, reqLogger logr.Logger) *rookCephv1.CephCluster { labels := map[string]string{ "app": sc.Name, } - maxLogSize, err := resource.ParseQuantity("500Mi") - if err != nil { - return &rookCephv1.CephCluster{}, fmt.Errorf("Failed to parse maxLogSize for log rotate. %v", err) - } + maxLogSize := resource.MustParse("500Mi") logCollector := rookCephv1.LogCollectorSpec{ Enabled: true, @@ -585,7 +576,7 @@ func newCephCluster(sc *ocsv1.StorageCluster, cephImage string, kmsConfigMap *co cephCluster.Spec.Security.KeyRotation.Enabled = isEnabled cephCluster.Spec.Security.KeyRotation.Schedule = rotationSchedule - return cephCluster, nil + return cephCluster } func isMultus(nwSpec *rookCephv1.NetworkSpec) bool { @@ -631,6 +622,14 @@ func newExternalCephCluster(sc *ocsv1.StorageCluster, monitoringIP, monitoringPo "app": sc.Name, } + maxLogSize := resource.MustParse("500Mi") + + logCollector := rookCephv1.LogCollectorSpec{ + Enabled: true, + Periodicity: "daily", + MaxLogSize: &maxLogSize, + } + var monitoringSpec = rookCephv1.MonitoringSpec{Enabled: false} if monitoringIP != "" { @@ -671,6 +670,7 @@ func newExternalCephCluster(sc *ocsv1.StorageCluster, monitoringIP, monitoringPo rookCephv1.KeyMonitoring: getCephClusterMonitoringLabels(*sc), rookCephv1.KeyCephExporter: getCephClusterMonitoringLabels(*sc), }, + LogCollector: logCollector, CSI: rookCephv1.CSIDriverSpec{ ReadAffinity: getReadAffinityOptions(sc), CephFS: rookCephv1.CSICephFSSpec{ diff --git a/controllers/storagecluster/cephcluster_test.go b/controllers/storagecluster/cephcluster_test.go index b79d0feccf..a8919da11f 100644 --- a/controllers/storagecluster/cephcluster_test.go +++ b/controllers/storagecluster/cephcluster_test.go @@ -89,8 +89,7 @@ func TestEnsureCephCluster(t *testing.T) { reconciler := createFakeStorageClusterReconciler(t, networkConfig) - expected, err := newCephCluster(mockStorageCluster.DeepCopy(), "", nil, log) - assert.NilError(t, err) + expected := newCephCluster(mockStorageCluster.DeepCopy(), "", nil, log) expected.Status.State = c.cephClusterState if !c.shouldCreate { @@ -123,7 +122,7 @@ func TestEnsureCephCluster(t *testing.T) { } var obj ocsCephCluster - _, err = obj.ensureCreated(&reconciler, sc) + _, err := obj.ensureCreated(&reconciler, sc) assert.NilError(t, err) actual := &rookCephv1.CephCluster{} @@ -204,8 +203,7 @@ func TestCephClusterMonTimeout(t *testing.T) { _, err := obj.ensureCreated(&reconciler, sc) assert.NilError(t, err) - cc, err := newCephCluster(sc, "", nil, log) - assert.NilError(t, err) + cc := newCephCluster(sc, "", nil, log) err = reconciler.Client.Get(context.TODO(), mockCephClusterNamespacedName, cc) assert.NilError(t, err) if c.platform == configv1.IBMCloudPlatformType { @@ -271,8 +269,7 @@ func TestNewCephClusterMonData(t *testing.T) { c.sc.Spec.MonDataDirHostPath = c.monDataPath c.sc.Status.Images.Ceph = &ocsv1.ComponentImageStatus{} - actual, err := newCephCluster(c.sc, "", nil, log) - assert.NilError(t, err) + actual := newCephCluster(c.sc, "", nil, log) assert.Equal(t, generateNameForCephCluster(c.sc), actual.Name) assert.Equal(t, c.sc.Namespace, actual.Namespace) assert.Equal(t, c.expectedMonDataPath, actual.Spec.DataDirHostPath) @@ -1249,8 +1246,7 @@ func TestGetCephClusterMonitoringLabels(t *testing.T) { func TestLogCollector(t *testing.T) { sc := &ocsv1.StorageCluster{} mockStorageCluster.DeepCopyInto(sc) - maxLogSize, err := resource.ParseQuantity("500Mi") - assert.NilError(t, err) + maxLogSize := resource.MustParse("500Mi") defaultLogCollector := rookCephv1.LogCollectorSpec{ Enabled: true, @@ -1260,22 +1256,18 @@ func TestLogCollector(t *testing.T) { sc.Spec.LogCollector = &defaultLogCollector - actual, err := newCephCluster(sc, "", nil, log) - assert.NilError(t, err) + actual := newCephCluster(sc, "", nil, log) assert.DeepEqual(t, actual.Spec.LogCollector, defaultLogCollector) // when disabled in storageCluster sc.Spec.LogCollector = &rookCephv1.LogCollectorSpec{} - actual, err = newCephCluster(sc, "", nil, log) - assert.NilError(t, err) + actual = newCephCluster(sc, "", nil, log) assert.DeepEqual(t, actual.Spec.LogCollector, defaultLogCollector) - maxLogSize, err = resource.ParseQuantity("6Gi") - assert.NilError(t, err) + maxLogSize = resource.MustParse("6Gi") sc.Spec.LogCollector.MaxLogSize = &maxLogSize - actual, err = newCephCluster(sc, "", nil, log) - assert.NilError(t, err) + actual = newCephCluster(sc, "", nil, log) assert.DeepEqual(t, actual.Spec.LogCollector.MaxLogSize, &maxLogSize) } @@ -1414,7 +1406,7 @@ func TestCephClusterNetworkConnectionsSpec(t *testing.T) { mockStorageCluster.DeepCopyInto(sc) sc.Spec.Network = testCase.scSpec.Network testCase.ccSpec.Network.Connections.RequireMsgr2 = true - cc, _ := newCephCluster(sc, "", nil, log) + cc := newCephCluster(sc, "", nil, log) assert.DeepEqual(t, cc.Spec.Network.Connections, testCase.ccSpec.Network.Connections) } } @@ -1487,8 +1479,7 @@ func TestCephClusterStoreType(t *testing.T) { sc := &ocsv1.StorageCluster{} t.Run("ensure no bluestore optimization", func(t *testing.T) { - actual, err := newCephCluster(sc, "", nil, log) - assert.NilError(t, err) + actual := newCephCluster(sc, "", nil, log) assert.Equal(t, "", actual.Spec.Storage.Store.Type) }) @@ -1497,15 +1488,13 @@ func TestCephClusterStoreType(t *testing.T) { DisasterRecoveryTargetAnnotation: "true", } sc.Annotations = annotations - actual, err := newCephCluster(sc, "", nil, log) - assert.NilError(t, err) + actual := newCephCluster(sc, "", nil, log) assert.Equal(t, "bluestore-rdr", actual.Spec.Storage.Store.Type) }) t.Run("ensure no bluestore optimization for external clusters", func(t *testing.T) { sc.Spec.ExternalStorage.Enable = true - actual, err := newCephCluster(sc, "", nil, log) - assert.NilError(t, err) + actual := newCephCluster(sc, "", nil, log) assert.Equal(t, "", actual.Spec.Storage.Store.Type) }) } @@ -1576,8 +1565,7 @@ func TestEnsureUpgradeReliabilityParams(t *testing.T) { sc.Spec.ManagedResources.CephCluster.WaitTimeoutForHealthyOSDInMinutes = 20 * time.Minute sc.Spec.ManagedResources.CephCluster.OsdMaintenanceTimeout = 45 * time.Minute - expected, err := newCephCluster(sc, "", nil, log) - assert.NilError(t, err) + expected := newCephCluster(sc, "", nil, log) assert.Equal(t, true, expected.Spec.ContinueUpgradeAfterChecksEvenIfNotHealthy) assert.Equal(t, true, expected.Spec.SkipUpgradeChecks) assert.Equal(t, true, expected.Spec.UpgradeOSDRequiresHealthyPGs) diff --git a/controllers/storagecluster/storagecluster_controller_test.go b/controllers/storagecluster/storagecluster_controller_test.go index f1ab717eee..46c710e26d 100644 --- a/controllers/storagecluster/storagecluster_controller_test.go +++ b/controllers/storagecluster/storagecluster_controller_test.go @@ -1326,9 +1326,8 @@ func TestStorageClusterOnMultus(t *testing.T) { func assertCephClusterNetwork(t assert.TestingT, reconciler StorageClusterReconciler, cr *api.StorageCluster, request reconcile.Request) { request.Name = "ocsinit-cephcluster" - cephCluster, err := newCephCluster(cr, "", nil, log) - assert.NoError(t, err) - err = reconciler.Client.Get(context.TODO(), request.NamespacedName, cephCluster) + cephCluster := newCephCluster(cr, "", nil, log) + err := reconciler.Client.Get(context.TODO(), request.NamespacedName, cephCluster) assert.NoError(t, err) if cr.Spec.Network == nil { assert.Equal(t, "", cephCluster.Spec.Network.Provider)