Skip to content

Commit

Permalink
Merge pull request #2702 from parth-gr/log-collector
Browse files Browse the repository at this point in the history 
external: enable log rotation and log collection for external mode
  • Loading branch information
@openshift-merge-bot
openshift-merge-bot[bot] authored Jul 19, 2024
2 parents f998bb4 + 9b4c2fd commit f62ae00
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 43 deletions.
28 changes: 14 additions & 14 deletions controllers/storagecluster/cephcluster.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 != "" {
Expand Down Expand Up @@ -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{
Expand Down
40 changes: 14 additions & 26 deletions controllers/storagecluster/cephcluster_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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{}
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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,
Expand All @@ -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)
}

Expand Down Expand Up @@ -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)
}
}
Expand Down Expand Up @@ -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)
})

Expand All @@ -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)
})
}
Expand Down Expand Up @@ -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)
Expand Down
5 changes: 2 additions & 3 deletions controllers/storagecluster/storagecluster_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit f62ae00

Please sign in to comment.