diff --git a/pkg/operator/ceph/object/zone/controller.go b/pkg/operator/ceph/object/zone/controller.go index e809caaac..ef51bd712 100644 --- a/pkg/operator/ceph/object/zone/controller.go +++ b/pkg/operator/ceph/object/zone/controller.go @@ -61,6 +61,12 @@ type domainRootType struct { var waitForRequeueIfObjectZoneGroupNotReady = reconcile.Result{Requeue: true, RequeueAfter: 10 * time.Second} +// allow this to be overridden for unit tests +var createObjectStorePoolsFunc = object.CreateObjectStorePools + +// allow this to be overridden for unit tests +var commitConfigChangesFunc = object.CommitConfigChanges + var logger = capnslog.NewPackageLogger("github.com/rook/rook", controllerName) var cephObjectZoneKind = reflect.TypeOf(cephv1.CephObjectZone{}).Name() @@ -224,28 +230,76 @@ func (r *ReconcileObjectZone) reconcile(request reconcile.Request) (reconcile.Re func (r *ReconcileObjectZone) createorUpdateCephZone(zone *cephv1.CephObjectZone, realmName string) (reconcile.Result, error) { logger.Infof("creating object zone %q in zonegroup %q in realm %q", zone.Name, zone.Spec.ZoneGroup, realmName) - realmArg := fmt.Sprintf("--rgw-realm=%s", realmName) - zoneGroupArg := fmt.Sprintf("--rgw-zonegroup=%s", zone.Spec.ZoneGroup) - zoneArg := fmt.Sprintf("--rgw-zone=%s", zone.Name) objContext := object.NewContext(r.context, r.clusterInfo, zone.Name) objContext.Realm = realmName objContext.ZoneGroup = zone.Spec.ZoneGroup objContext.Zone = zone.Name + err := r.createPoolsAndZone(objContext, zone) + if err != nil { + return reconcile.Result{}, err + } + + return reconcile.Result{}, nil +} + +func (r *ReconcileObjectZone) createPoolsAndZone(objContext *object.Context, zone *cephv1.CephObjectZone) error { + // create pools for zone + logger.Debugf("creating pools ceph zone %q", zone.Name) + err := object.ValidateObjectStorePoolsConfig(zone.Spec.MetadataPool, zone.Spec.DataPool, zone.Spec.SharedPools) + if err != nil { + return fmt.Errorf("invalid zone pools config: %w", err) + } + if object.IsNeedToCreateObjectStorePools(zone.Spec.SharedPools) { + err = createObjectStorePoolsFunc(objContext, r.clusterSpec, zone.Spec.MetadataPool, zone.Spec.DataPool) + if err != nil { + return fmt.Errorf("unable to create pools for zone: %w", err) + } + logger.Debugf("created pools ceph zone %q", zone.Name) + } + + err = r.createZoneIfNotExists(objContext, zone) + if err != nil { + return err + } + + // Configure the zone for RADOS namespaces + err = object.ConfigureSharedPoolsForZone(objContext, zone.Spec.SharedPools) + if err != nil { + return errors.Wrapf(err, "failed to configure rados namespaces for zone") + } + + // Commit rgw zone config changes + err = commitConfigChangesFunc(objContext) + if err != nil { + return errors.Wrapf(err, "failed to commit zone config changes") + } + + return nil +} + +func (r *ReconcileObjectZone) createZoneIfNotExists(objContext *object.Context, zone *cephv1.CephObjectZone) error { + realmArg := fmt.Sprintf("--rgw-realm=%s", objContext.Realm) + zoneGroupArg := fmt.Sprintf("--rgw-zonegroup=%s", zone.Spec.ZoneGroup) + zoneArg := fmt.Sprintf("--rgw-zone=%s", zone.Name) // get zone group to see if master zone exists yet output, err := object.RunAdminCommandNoMultisite(objContext, true, "zonegroup", "get", realmArg, zoneGroupArg) if err != nil { if code, ok := exec.ExitStatus(err); ok && code == int(syscall.ENOENT) { - return reconcile.Result{}, errors.Wrapf(err, "ceph zone group %q not found", zone.Spec.ZoneGroup) + return errors.Wrapf(err, "ceph zone group %q not found", zone.Spec.ZoneGroup) } else { - return reconcile.Result{}, errors.Wrapf(err, "radosgw-admin zonegroup get failed with code %d", code) + return errors.Wrapf(err, "radosgw-admin zonegroup get failed with code %d", code) } } - // check if master zone does not exist yet for period zoneGroupJson, err := object.DecodeZoneGroupConfig(output) if err != nil { - return reconcile.Result{}, errors.Wrap(err, "failed to parse `radosgw-admin zonegroup get` output") + return errors.Wrap(err, "failed to parse `radosgw-admin zonegroup get` output") + } + + zoneIsMaster := false + if zoneGroupJson.MasterZoneID == "" { + zoneIsMaster = true } // create/update zone @@ -254,7 +308,7 @@ func (r *ReconcileObjectZone) createorUpdateCephZone(zone *cephv1.CephObjectZone logger.Debugf("ceph zone %q already exists, new zone and pools will not be created but checking for update", zone.Name) zoneEndpointsModified, err := object.ShouldUpdateZoneEndpointList(zoneGroupJson.Zones, zone.Spec.CustomEndpoints, objContext.Zone) if err != nil { - return reconcile.Result{}, err + return err } if zoneEndpointsModified { zoneEndpoints := strings.Join(zone.Spec.CustomEndpoints, ",") @@ -262,50 +316,19 @@ func (r *ReconcileObjectZone) createorUpdateCephZone(zone *cephv1.CephObjectZone endpointArg := fmt.Sprintf("--endpoints=%s", zoneEndpoints) err = object.JoinMultisite(objContext, endpointArg, zoneEndpoints, zone.Namespace) if err != nil { - return reconcile.Result{}, err + return err } } - return reconcile.Result{}, nil + logger.Debugf("skip creating zone %q: already exists", zone.Name) + return nil } if code, ok := exec.ExitStatus(err); ok && code != int(syscall.ENOENT) { - return reconcile.Result{}, errors.Wrapf(err, "radosgw-admin zone get failed with code %d for reason %q", code, output) + return errors.Wrapf(err, "radosgw-admin zone get failed with code %d for reason %q", code, output) } logger.Debugf("ceph zone %q not found, running `radosgw-admin zone create`", zone.Name) - zoneIsMaster := false - if zoneGroupJson.MasterZoneID == "" { - zoneIsMaster = true - } - - err = r.createPoolsAndZone(objContext, zone, realmName, zoneIsMaster) - if err != nil { - return reconcile.Result{}, err - } - - return reconcile.Result{}, nil -} - -func (r *ReconcileObjectZone) createPoolsAndZone(objContext *object.Context, zone *cephv1.CephObjectZone, realmName string, zoneIsMaster bool) error { - // create pools for zone - logger.Debugf("creating pools ceph zone %q", zone.Name) - err := object.ValidateObjectStorePoolsConfig(zone.Spec.MetadataPool, zone.Spec.DataPool, zone.Spec.SharedPools) - if err != nil { - return fmt.Errorf("invalid zone pools config: %w", err) - } - if object.IsNeedToCreateObjectStorePools(zone.Spec.SharedPools) { - err = object.CreateObjectStorePools(objContext, r.clusterSpec, zone.Spec.MetadataPool, zone.Spec.DataPool) - if err != nil { - return fmt.Errorf("unable to create pools for zone: %w", err) - } - logger.Debugf("created pools ceph zone %q", zone.Name) - } - - realmArg := fmt.Sprintf("--rgw-realm=%s", realmName) - zoneGroupArg := fmt.Sprintf("--rgw-zonegroup=%s", zone.Spec.ZoneGroup) - zoneArg := fmt.Sprintf("--rgw-zone=%s", zone.Name) - - accessKeyArg, secretKeyArg, err := object.GetRealmKeyArgs(r.opManagerContext, r.context, realmName, zone.Namespace) + accessKeyArg, secretKeyArg, err := object.GetRealmKeyArgs(r.opManagerContext, r.context, objContext.Realm, zone.Namespace) if err != nil { return errors.Wrap(err, "failed to get keys for realm") } @@ -320,24 +343,11 @@ func (r *ReconcileObjectZone) createPoolsAndZone(objContext *object.Context, zon zoneEndpoints := strings.Join(zone.Spec.CustomEndpoints, ",") args = append(args, fmt.Sprintf("--endpoints=%s", zoneEndpoints)) } - output, err := object.RunAdminCommandNoMultisite(objContext, false, args...) + output, err = object.RunAdminCommandNoMultisite(objContext, false, args...) if err != nil { return errors.Wrapf(err, "failed to create ceph zone %q for reason %q", zone.Name, output) } logger.Debugf("created ceph zone %q", zone.Name) - - // Configure the zone for RADOS namespaces - err = object.ConfigureSharedPoolsForZone(objContext, zone.Spec.SharedPools) - if err != nil { - return errors.Wrapf(err, "failed to configure rados namespaces for zone") - } - - // Commit rgw zone config changes - err = object.CommitConfigChanges(objContext) - if err != nil { - return errors.Wrapf(err, "failed to commit zone config changes") - } - return nil } diff --git a/pkg/operator/ceph/object/zone/controller_test.go b/pkg/operator/ceph/object/zone/controller_test.go index e90dc4414..e28e53bc5 100644 --- a/pkg/operator/ceph/object/zone/controller_test.go +++ b/pkg/operator/ceph/object/zone/controller_test.go @@ -25,6 +25,7 @@ import ( "github.com/coreos/pkg/capnslog" cephv1 "github.com/rook/rook/pkg/apis/ceph.rook.io/v1" rookclient "github.com/rook/rook/pkg/client/clientset/versioned/fake" + "github.com/rook/rook/pkg/operator/ceph/object" "github.com/rook/rook/pkg/operator/test" "github.com/rook/rook/pkg/clusterd" @@ -131,6 +132,24 @@ func TestCephObjectZoneController(t *testing.T) { zonegroup := "zonegroup-a" namespace := "rook-ceph" + createPoolsCalled := false + createObjectStorePoolsFunc = func(context *object.Context, cluster *cephv1.ClusterSpec, metadataPool, dataPool cephv1.PoolSpec) error { + createPoolsCalled = true + return nil + } + defer func() { + createObjectStorePoolsFunc = object.CreateObjectStorePools + }() + + commitChangesCalled := false + commitConfigChangesFunc = func(c *object.Context) error { + commitChangesCalled = true + return nil + } + defer func() { + commitConfigChangesFunc = object.CommitConfigChanges + }() + // // TEST 1 SETUP // @@ -356,9 +375,13 @@ func TestCephObjectZoneController(t *testing.T) { }, } + assert.False(t, createPoolsCalled) + assert.False(t, commitChangesCalled) res, err = r.Reconcile(ctx, req) assert.NoError(t, err) assert.False(t, res.Requeue) err = r.client.Get(context.TODO(), req.NamespacedName, objectZone) assert.NoError(t, err) + assert.True(t, createPoolsCalled) + assert.True(t, commitChangesCalled) }