diff --git a/Documentation/CRDs/specification.md b/Documentation/CRDs/specification.md index e0b015109..df8166674 100644 --- a/Documentation/CRDs/specification.md +++ b/Documentation/CRDs/specification.md @@ -10965,7 +10965,9 @@ bool -

Sets given placement as default. Only one placement in the list can be marked as default.

+(Optional) +

Sets given placement as default. Only one placement in the list can be marked as default. +Default is false.

diff --git a/deploy/charts/rook-ceph/templates/resources.yaml b/deploy/charts/rook-ceph/templates/resources.yaml index 77ff73089..f4d0dc5d8 100644 --- a/deploy/charts/rook-ceph/templates/resources.yaml +++ b/deploy/charts/rook-ceph/templates/resources.yaml @@ -12384,7 +12384,9 @@ spec: minLength: 1 type: string default: - description: Sets given placement as default. Only one placement in the list can be marked as default. + description: |- + Sets given placement as default. Only one placement in the list can be marked as default. + Default is false. type: boolean metadataPoolName: description: The metadata pool used to store ObjectStore bucket index. @@ -12423,7 +12425,6 @@ spec: type: array required: - dataPoolName - - default - metadataPoolName - name type: object @@ -13310,7 +13311,9 @@ spec: minLength: 1 type: string default: - description: Sets given placement as default. Only one placement in the list can be marked as default. + description: |- + Sets given placement as default. Only one placement in the list can be marked as default. + Default is false. type: boolean metadataPoolName: description: The metadata pool used to store ObjectStore bucket index. @@ -13349,7 +13352,6 @@ spec: type: array required: - dataPoolName - - default - metadataPoolName - name type: object diff --git a/deploy/examples/crds.yaml b/deploy/examples/crds.yaml index b10029784..db2c73513 100644 --- a/deploy/examples/crds.yaml +++ b/deploy/examples/crds.yaml @@ -12375,7 +12375,9 @@ spec: minLength: 1 type: string default: - description: Sets given placement as default. Only one placement in the list can be marked as default. + description: |- + Sets given placement as default. Only one placement in the list can be marked as default. + Default is false. type: boolean metadataPoolName: description: The metadata pool used to store ObjectStore bucket index. @@ -12414,7 +12416,6 @@ spec: type: array required: - dataPoolName - - default - metadataPoolName - name type: object @@ -13298,7 +13299,9 @@ spec: minLength: 1 type: string default: - description: Sets given placement as default. Only one placement in the list can be marked as default. + description: |- + Sets given placement as default. Only one placement in the list can be marked as default. + Default is false. type: boolean metadataPoolName: description: The metadata pool used to store ObjectStore bucket index. @@ -13337,7 +13340,6 @@ spec: type: array required: - dataPoolName - - default - metadataPoolName - name type: object diff --git a/pkg/apis/ceph.rook.io/v1/types.go b/pkg/apis/ceph.rook.io/v1/types.go index 633b9153c..26af881c7 100755 --- a/pkg/apis/ceph.rook.io/v1/types.go +++ b/pkg/apis/ceph.rook.io/v1/types.go @@ -1564,6 +1564,8 @@ type PoolPlacementSpec struct { Name string `json:"name"` // Sets given placement as default. Only one placement in the list can be marked as default. + // Default is false. + // +optional Default bool `json:"default"` // The metadata pool used to store ObjectStore bucket index. diff --git a/pkg/operator/ceph/object/objectstore_test.go b/pkg/operator/ceph/object/objectstore_test.go index ee5aa4c83..f384d6c70 100644 --- a/pkg/operator/ceph/object/objectstore_test.go +++ b/pkg/operator/ceph/object/objectstore_test.go @@ -152,7 +152,7 @@ const ( "notif_pool": "%[1]s:store-a.log.notif" }` - objectZonegroupJsonTempl = `{ + objectZonegroupJson = `{ "id": "610c9e3d-19e7-40b0-9f88-03319c4bc65a", "name": "store-a", "api_name": "test", @@ -174,14 +174,14 @@ const ( ], "placement_targets": [ { - "name": "%[1]s", + "name": "default-placement", "tags": [], "storage_classes": [ "STANDARD" ] } ], - "default_placement": "%[1]s", + "default_placement": "default-placement", "realm_id": "29e28253-be54-4581-90dd-206020d2fcdd", "sync_policy": { "groups": [] @@ -229,7 +229,6 @@ func TestReconcileRealm(t *testing.T) { func TestConfigureStoreWithSharedPools(t *testing.T) { sharedMetaPoolAlreadySet, sharedDataPoolAlreadySet := "", "" - defaultPlacement := defaultPlacementCephConfigName zoneGetCalled := false zoneSetCalled := false zoneGroupGetCalled := false @@ -274,7 +273,7 @@ func TestConfigureStoreWithSharedPools(t *testing.T) { } else if args[0] == "zonegroup" { if args[1] == "get" { zoneGroupGetCalled = true - return fmt.Sprintf(objectZonegroupJsonTempl, defaultPlacement), nil + return objectZonegroupJson, nil } else if args[1] == "set" { zoneGroupSetCalled = true for _, arg := range args { @@ -288,7 +287,7 @@ func TestConfigureStoreWithSharedPools(t *testing.T) { } return string(inBytes), nil } - return fmt.Sprintf(objectZonegroupJsonTempl, defaultPlacement), nil + return objectZonegroupJson, nil } } return "", errors.Errorf("unexpected ceph command %q", args) @@ -331,7 +330,7 @@ func TestConfigureStoreWithSharedPools(t *testing.T) { assert.True(t, zoneGroupGetCalled) assert.False(t, zoneGroupSetCalled) // zone group is set only if extra pool placements specified }) - t.Run("configure with default placement", func(t *testing.T) { + t.Run("configure with new default placement", func(t *testing.T) { sharedPools := cephv1.ObjectSharedPoolsSpec{ PoolPlacements: []cephv1.PoolPlacementSpec{ { @@ -342,22 +341,22 @@ func TestConfigureStoreWithSharedPools(t *testing.T) { }, }, } - defaultPlacement = "default" err := ConfigureSharedPoolsForZone(context, sharedPools) assert.NoError(t, err) assert.True(t, zoneGetCalled) assert.True(t, zoneSetCalled) assert.False(t, placementModifyCalled) // mock returns applied namespases, no workaround needed assert.True(t, zoneGroupGetCalled) - assert.False(t, zoneGroupSetCalled) // zone group is set only if extra pool placements specified + assert.True(t, zoneGroupSetCalled) }) t.Run("data pool already set", func(t *testing.T) { + // reset + zoneGroupSetCalled = false // Simulate that the data pool has already been set and the zone update can be skipped sharedPools := cephv1.ObjectSharedPoolsSpec{ MetadataPoolName: "test-meta", DataPoolName: "test-data", } - defaultPlacement = defaultPlacementCephConfigName sharedMetaPoolAlreadySet, sharedDataPoolAlreadySet = "test-meta", "test-data" zoneGetCalled = false zoneSetCalled = false diff --git a/pkg/operator/ceph/object/shared_pools.go b/pkg/operator/ceph/object/shared_pools.go index b447cdf46..95bafb14c 100644 --- a/pkg/operator/ceph/object/shared_pools.go +++ b/pkg/operator/ceph/object/shared_pools.go @@ -89,7 +89,6 @@ func adjustZonePlacementPools(zone map[string]interface{}, spec cephv1.ObjectSha fromSpec := toZonePlacementPools(spec, name) - defaultPlacementName := getDefaultPlacementName(spec) inConfig := map[string]struct{}{} idxToRemove := map[int]struct{}{} for i, p := range placements { @@ -102,10 +101,19 @@ func adjustZonePlacementPools(zone map[string]interface{}, spec cephv1.ObjectSha return nil, fmt.Errorf("unable to get pool placement name for zone %s: %w", name, err) } // check if placement should be removed - if _, inSpec := fromSpec[placementID]; !inSpec && placementID != defaultPlacementName { - // remove placement if it is not in spec, but don't remove default placement - idxToRemove[i] = struct{}{} - continue + if _, inSpec := fromSpec[placementID]; !inSpec { + if placementID == defaultPlacementCephConfigName { + // 'default-placement' should always be kept as a workaround for https://tracker.ceph.com/issues/68775. + // if user specified other placement as default, then copy pool names to 'default-placement' from it: + if userDefault, inSpec := fromSpec[getDefaultPlacementName(spec)]; inSpec { + // duplicate user defined default placement under 'default-placement' name in spec to update pools on the next step + fromSpec[defaultPlacementCephConfigName] = userDefault + } + } else { + // remove placement if it is not in spec + idxToRemove[i] = struct{}{} + continue + } } // update placement with values from spec: if pSpec, inSpec := fromSpec[placementID]; inSpec { @@ -155,6 +163,29 @@ func adjustZonePlacementPools(zone map[string]interface{}, spec cephv1.ObjectSha placements = append(placements, pObj) } + // sort placements array. + // Reason: 'radosgw-admin zone set --infile' sorts placement_pools by key before storing it in ceph + // and returns JSON with sorted placement_pools array. So we sort input array for easy comparison with applied JSON. + sort.Slice(placements, func(i, j int) bool { + pI, ok := placements[i].(map[string]interface{}) + if !ok { + return false + } + nameI, err := getObjProperty[string](pI, "key") + if err != nil { + return false + } + pJ, ok := placements[j].(map[string]interface{}) + if !ok { + return false + } + nameJ, err := getObjProperty[string](pJ, "key") + if err != nil { + return false + } + return nameI < nameJ + }) + _, err = updateObjProperty(zone, placements, "placement_pools") if err != nil { return nil, fmt.Errorf("unable to set pool placements for zone %q: %w", name, err) @@ -182,16 +213,9 @@ func getDefaultMetadataPool(spec cephv1.ObjectSharedPoolsSpec) string { // toZonePlacementPools converts pool placement CRD definition to zone config json format structures func toZonePlacementPools(spec cephv1.ObjectSharedPoolsSpec, ns string) map[string]ZonePlacementPool { - hasDefault := false res := make(map[string]ZonePlacementPool, len(spec.PoolPlacements)+1) - for _, pp := range spec.PoolPlacements { - if pp.Default { - hasDefault = true - } - res[pp.Name] = toZonePlacementPool(pp, ns) - } - if !hasDefault && spec.DataPoolName != "" && spec.MetadataPoolName != "" { - // set shared pools as default if no default placement was provided + // map sharedPools if presented: + if spec.DataPoolName != "" && spec.MetadataPoolName != "" { res[defaultPlacementCephConfigName] = ZonePlacementPool{ Key: defaultPlacementCephConfigName, Val: ZonePlacementPoolVal{ @@ -211,6 +235,10 @@ func toZonePlacementPools(spec cephv1.ObjectSharedPoolsSpec, ns string) map[stri }, } } + // map pool placements: + for _, pp := range spec.PoolPlacements { + res[pp.Name] = toZonePlacementPool(pp, ns) + } return res } @@ -493,7 +521,7 @@ func updateZoneGroupJSON(objContext *Context, group map[string]interface{}) (map args := []string{"zonegroup", "set", zoneArg, "--infile=" + configFilename, realmArg, zoneGroupArg} updatedBytes, err := RunAdminCommandNoMultisite(objContext, false, args...) if err != nil { - return nil, errors.Wrap(err, "failed to set zone config") + return nil, errors.Wrap(err, fmt.Sprintf("failed to set zonegroup config %s", updatedBytes)) } updated := map[string]interface{}{} err = json.Unmarshal([]byte(updatedBytes), &updated) diff --git a/pkg/operator/ceph/object/shared_pools_test.go b/pkg/operator/ceph/object/shared_pools_test.go index f40306141..81f26b799 100644 --- a/pkg/operator/ceph/object/shared_pools_test.go +++ b/pkg/operator/ceph/object/shared_pools_test.go @@ -630,7 +630,7 @@ func Test_toZonePlacementPools(t *testing.T) { }, }, { - name: "default placement overrides shared pools", + name: "shared pools not removed if default placement set", args: args{ spec: cephv1.ObjectSharedPoolsSpec{ MetadataPoolName: "meta", @@ -655,6 +655,19 @@ func Test_toZonePlacementPools(t *testing.T) { ns: "rgw-instance", }, want: map[string]ZonePlacementPool{ + defaultPlacementCephConfigName: { + Key: defaultPlacementCephConfigName, + Val: ZonePlacementPoolVal{ + DataExtraPool: "meta:rgw-instance.buckets.non-ec", + IndexPool: "meta:rgw-instance.buckets.index", + StorageClasses: map[string]ZonePlacementStorageClass{ + "STANDARD": { + DataPool: "data:rgw-instance.buckets.data", + }, + }, + InlineData: true, + }, + }, "some_name": { Key: "some_name", Val: ZonePlacementPoolVal{ @@ -1444,6 +1457,156 @@ func Test_adjustZonePlacementPools(t *testing.T) { } } ] +}`, + wantChanged: true, + wantErr: false, + }, + { + name: "sharedPools placement not removed if default set", + args: args{ + beforeJSON: `{ + "id": "f539c2c0-e1ed-4c42-9294-41742352eeae", + "name": "test", + "placement_pools": [ + { + "key": "default-placement", + "val": { + "index_pool": "test.rgw.buckets.index", + "storage_classes": { + "STANDARD": { + "data_pool": "test.rgw.buckets.data" + } + }, + "data_extra_pool": "test.rgw.buckets.non-ec", + "index_type": 5, + "inline_data": true + } + } + ] +}`, + spec: cephv1.ObjectSharedPoolsSpec{ + MetadataPoolName: "meta-pool", + DataPoolName: "data-pool", + PreserveRadosNamespaceDataOnDelete: false, + PoolPlacements: []cephv1.PoolPlacementSpec{ + { + Name: "slow", + Default: true, + MetadataPoolName: "slow-meta", + DataPoolName: "slow-data", + DataNonECPoolName: "slow-non-ec", + StorageClasses: []cephv1.PlacementStorageClassSpec{}, + }, + }, + }, + }, + wantJSON: `{ + "id": "f539c2c0-e1ed-4c42-9294-41742352eeae", + "name": "test", + "placement_pools": [ + { + "key": "default-placement", + "val": { + "index_pool": "meta-pool:test.buckets.index", + "storage_classes": { + "STANDARD": { + "data_pool": "data-pool:test.buckets.data" + } + }, + "data_extra_pool": "meta-pool:test.buckets.non-ec", + "index_type": 5, + "inline_data": true + } + }, + { + "key": "slow", + "val": { + "index_pool": "slow-meta:test.slow.index", + "storage_classes": { + "STANDARD": { + "data_pool": "slow-data:test.slow.data" + } + }, + "data_extra_pool": "slow-non-ec:test.slow.data.non-ec", + "inline_data": true + } + } + ] +}`, + wantChanged: true, + wantErr: false, + }, + { + name: "if sharedPools not set and default placement is set, then default placements values are copied to 'default-placement'", + args: args{ + beforeJSON: `{ + "id": "f539c2c0-e1ed-4c42-9294-41742352eeae", + "name": "test", + "placement_pools": [ + { + "key": "default-placement", + "val": { + "index_pool": "test.rgw.buckets.index", + "storage_classes": { + "STANDARD": { + "data_pool": "test.rgw.buckets.data" + } + }, + "data_extra_pool": "test.rgw.buckets.non-ec", + "index_type": 5, + "inline_data": true + } + } + ] +}`, + spec: cephv1.ObjectSharedPoolsSpec{ + MetadataPoolName: "", + DataPoolName: "", + PreserveRadosNamespaceDataOnDelete: false, + PoolPlacements: []cephv1.PoolPlacementSpec{ + { + Name: "slow", + Default: true, + MetadataPoolName: "slow-meta", + DataPoolName: "slow-data", + DataNonECPoolName: "slow-non-ec", + StorageClasses: []cephv1.PlacementStorageClassSpec{}, + }, + }, + }, + }, + wantJSON: `{ + "id": "f539c2c0-e1ed-4c42-9294-41742352eeae", + "name": "test", + "placement_pools": [ + { + "key": "default-placement", + "val": { + "index_pool": "slow-meta:test.slow.index", + "storage_classes": { + "STANDARD": { + "data_pool": "slow-data:test.slow.data" + } + }, + "data_extra_pool": "slow-non-ec:test.slow.data.non-ec", + "index_type": 5, + "inline_data": true + } + }, + { + "key": "slow", + "val": { + "index_pool": "slow-meta:test.slow.index", + "storage_classes": { + "STANDARD": { + "data_pool": "slow-data:test.slow.data" + } + }, + "data_extra_pool": "slow-non-ec:test.slow.data.non-ec", + "inline_data": true + } + } + ] }`, wantChanged: true, wantErr: false,