From fc08e87d4493e1289417086220984a2b52fe9441 Mon Sep 17 00:00:00 2001 From: Blaine Gardner Date: Wed, 20 Nov 2024 10:54:44 -0700 Subject: [PATCH] Revert "object: create cosi user for each object store" This reverts commit a941b3c33f452fe7e4c1af34b5e61bc28ffb6cbe. Stop creating the 'cosi' user in the CephObjectStore reconcile. This step often fails for some amount of time during initial object store creation, causing frequent user concern. It has also been the source of some reported failures that would otherwise be non-breaking for certain users. Signed-off-by: Blaine Gardner --- .../Object-Storage-RGW/cosi.md | 24 +++++- deploy/examples/cosi/cephcosidriver.yaml | 14 ++++ pkg/operator/ceph/object/controller.go | 73 +------------------ pkg/operator/ceph/object/controller_test.go | 58 +-------------- pkg/operator/ceph/object/objectstore.go | 2 - pkg/operator/ceph/object/user.go | 60 --------------- pkg/operator/ceph/object/user/controller.go | 55 +++++++++++++- tests/integration/ceph_cosi_test.go | 4 +- 8 files changed, 93 insertions(+), 197 deletions(-) diff --git a/Documentation/Storage-Configuration/Object-Storage-RGW/cosi.md b/Documentation/Storage-Configuration/Object-Storage-RGW/cosi.md index a9541f6848e5..f3635d6a5c9f 100644 --- a/Documentation/Storage-Configuration/Object-Storage-RGW/cosi.md +++ b/Documentation/Storage-Configuration/Object-Storage-RGW/cosi.md @@ -33,6 +33,20 @@ metadata: namespace: rook-ceph spec: deploymentStrategy: "Auto" +--- +# The Ceph-COSI driver needs a privileged user for each CephObjectStore +# in order to provision buckets and users +apiVersion: ceph.rook.io/v1 +kind: CephObjectStoreUser +metadata: + name: cosi + namespace: rook-ceph # rook operator namespace +spec: + displayName: "cosi user" + store: my-store # name of the CephObjectStore + capabilities: + bucket: "*" + user: "*" ``` ```console @@ -40,13 +54,11 @@ cd deploy/examples/cosi kubectl create -f cephcosidriver.yaml ``` -The driver is created in the same namespace as Rook operator. - ## Admin Operations ### Create a BucketClass and BucketAccessClass -The BucketClass and BucketAccessClass are CRDs defined by COSI. The BucketClass defines the bucket class for the bucket. The BucketAccessClass defines the access class for the bucket. Rook will automatically create a secret named with `rook-ceph-object-user--cosi` which contains credentials used by the COSI driver. This secret is referred by the BucketClass and BucketAccessClass as defined below: +The BucketClass and BucketAccessClass are CRDs defined by COSI. The BucketClass defines the storage class for the bucket. The BucketAccessClass defines the access class for the bucket. The BucketClass and BucketAccessClass are defined as below: ```yaml kind: BucketClass @@ -58,7 +70,9 @@ deletionPolicy: Delete parameters: objectStoreUserSecretName: rook-ceph-object-user-my-store-cosi objectStoreUserSecretNamespace: rook-ceph ---- +``` + +```yaml kind: BucketAccessClass apiVersion: objectstorage.k8s.io/v1alpha1 metadata: @@ -74,6 +88,8 @@ parameters: kubectl create -f bucketclass.yaml -f bucketaccessclass.yaml ``` +The `objectStoreUserSecretName` and `objectStoreUserSecretNamespace` are the name and namespace of the CephObjectStoreUser created in the previous step. + ## User Operations ### Create a Bucket diff --git a/deploy/examples/cosi/cephcosidriver.yaml b/deploy/examples/cosi/cephcosidriver.yaml index a2157c164b92..d9594357ffef 100644 --- a/deploy/examples/cosi/cephcosidriver.yaml +++ b/deploy/examples/cosi/cephcosidriver.yaml @@ -5,3 +5,17 @@ metadata: namespace: rook-ceph spec: deploymentStrategy: "Auto" +--- +# The Ceph-COSI driver needs a privileged user for each CephObjectStore +# in order to provision buckets and users +apiVersion: ceph.rook.io/v1 +kind: CephObjectStoreUser +metadata: + name: cosi + namespace: rook-ceph # namespace:cluster +spec: + store: my-store # name of the CephObjectStore + displayName: "user for cosi" + capabilities: + user: "*" + bucket: "*" diff --git a/pkg/operator/ceph/object/controller.go b/pkg/operator/ceph/object/controller.go index 82e8b128c820..e57f9d8896c2 100644 --- a/pkg/operator/ceph/object/controller.go +++ b/pkg/operator/ceph/object/controller.go @@ -24,7 +24,6 @@ import ( "syscall" "time" - "github.com/ceph/go-ceph/rgw/admin" "github.com/coreos/pkg/capnslog" bktclient "github.com/kube-object-storage/lib-bucket-provisioner/pkg/client/clientset/versioned" "github.com/pkg/errors" @@ -82,9 +81,6 @@ var currentAndDesiredCephVersion = opcontroller.CurrentAndDesiredCephVersion // allow this to be overridden for unit tests var cephObjectStoreDependents = CephObjectStoreDependents -// newMultisiteAdminOpsCtxFunc help us mocking the admin ops API client in unit test -var newMultisiteAdminOpsCtxFunc = NewMultisiteAdminOpsContext - // ReconcileCephObjectStore reconciles a cephObjectStore object type ReconcileCephObjectStore struct { client client.Client @@ -243,15 +239,10 @@ func (r *ReconcileCephObjectStore) reconcile(request reconcile.Request) (reconci if err != nil { return reconcile.Result{}, *cephObjectStore, errors.Wrapf(err, "failed to get object context") } - opsCtx, err := newMultisiteAdminOpsCtxFunc(objCtx, &cephObjectStore.Spec) + opsCtx, err := NewMultisiteAdminOpsContext(objCtx, &cephObjectStore.Spec) if err != nil { return reconcile.Result{}, *cephObjectStore, errors.Wrapf(err, "failed to get admin ops API context") } - err = r.deleteCOSIUser(opsCtx) - if err != nil { - // Allow the object store removal to proceed even if the user deletion fails - logger.Warningf("failed to delete COSI user. %v", err) - } deps, err := cephObjectStoreDependents(r.context, r.clusterInfo, cephObjectStore, objCtx, opsCtx) if err != nil { return reconcile.Result{}, *cephObjectStore, err @@ -480,11 +471,9 @@ func (r *ReconcileCephObjectStore) reconcileCreateObjectStore(cephObjectStore *c if err != nil { return reconcile.Result{}, errors.Wrapf(err, "failed to create object store %q", cephObjectStore.Name) } - } - // Create COSI user and secret - return r.reconcileCOSIUser(cephObjectStore) + return reconcile.Result{}, nil } func (r *ReconcileCephObjectStore) retrieveMultisiteZone(store *cephv1.CephObjectStore, zoneGroupName string, realmName string) (reconcile.Result, error) { @@ -545,61 +534,3 @@ func (r *ReconcileCephObjectStore) getMultisiteResourceNames(cephObjectStore *ce return realm.Name, zonegroup.Name, zone.Name, zone, reconcile.Result{}, nil } - -func (r *ReconcileCephObjectStore) reconcileCOSIUser(cephObjectStore *cephv1.CephObjectStore) (reconcile.Result, error) { - // Create COSI user and secret - userConfig := generateCOSIUserConfig() - var user admin.User - - // Create COSI user - objCtx, err := NewMultisiteContext(r.context, r.clusterInfo, cephObjectStore) - if err != nil { - return reconcile.Result{}, errors.Wrapf(err, "failed to get object context") - } - - adminOpsCtx, err := newMultisiteAdminOpsCtxFunc(objCtx, &cephObjectStore.Spec) - if err != nil { - return reconcile.Result{}, errors.Wrapf(err, "failed to get admin ops API context") - } - - user, err = adminOpsCtx.AdminOpsClient.GetUser(r.opManagerContext, *userConfig) - if err != nil { - if errors.Is(err, admin.ErrNoSuchUser) { - logger.Infof("creating COSI user %q", userConfig.ID) - user, err = adminOpsCtx.AdminOpsClient.CreateUser(r.opManagerContext, *userConfig) - if err != nil { - return reconcile.Result{}, errors.Wrapf(err, "failed to create COSI user %q", userConfig.ID) - } - } else { - return reconcile.Result{}, errors.Wrapf(err, "failed to get COSI user %q", userConfig.ID) - } - } - - // Create COSI user secret - return ReconcileCephUserSecret(r.opManagerContext, r.client, r.scheme, cephObjectStore, &user, objCtx.Endpoint, cephObjectStore.Namespace, cephObjectStore.Name, cephObjectStore.Spec.Gateway.SSLCertificateRef) -} - -func generateCOSIUserConfig() *admin.User { - userConfig := admin.User{ - ID: cosiUserName, - DisplayName: cosiUserName, - } - - userConfig.UserCaps = cosiUserCaps - - return &userConfig -} - -func (r *ReconcileCephObjectStore) deleteCOSIUser(adminOpsCtx *AdminOpsContext) error { - userConfig := generateCOSIUserConfig() - err := adminOpsCtx.AdminOpsClient.RemoveUser(r.opManagerContext, *userConfig) - if err != nil { - if errors.Is(err, admin.ErrNoSuchUser) { - logger.Debugf("COSI user %q not found", userConfig.ID) - return nil - } else { - return errors.Wrapf(err, "failed to delete COSI user %q", userConfig.ID) - } - } - return nil -} diff --git a/pkg/operator/ceph/object/controller_test.go b/pkg/operator/ceph/object/controller_test.go index c6e1d2cda31e..50bc736326b7 100644 --- a/pkg/operator/ceph/object/controller_test.go +++ b/pkg/operator/ceph/object/controller_test.go @@ -18,10 +18,7 @@ limitations under the License. package object import ( - "bytes" "context" - "io" - "net/http" "os" "reflect" "testing" @@ -29,7 +26,6 @@ import ( k8serrors "k8s.io/apimachinery/pkg/api/errors" - "github.com/ceph/go-ceph/rgw/admin" "github.com/coreos/pkg/capnslog" "github.com/pkg/errors" cephv1 "github.com/rook/rook/pkg/apis/ceph.rook.io/v1" @@ -325,35 +321,6 @@ var ( store = "my-store" ) -var mockMultisiteAdminOpsCtxFunc = func(objContext *Context, spec *cephv1.ObjectStoreSpec) (*AdminOpsContext, error) { - mockClient := &MockClient{ - MockDo: func(req *http.Request) (*http.Response, error) { - if req.Method == http.MethodGet || req.Method == http.MethodPut { - return &http.Response{ - StatusCode: http.StatusOK, - Body: io.NopCloser(bytes.NewReader([]byte(userCreateJSON))), - }, nil - } - if req.Method == http.MethodDelete { - return &http.Response{ - StatusCode: http.StatusOK, - Body: io.NopCloser(bytes.NewReader([]byte(""))), - }, nil - } - return nil, errors.Errorf("unexpected method %q", req.Method) - }, - } - context := NewContext(objContext.Context, objContext.clusterInfo, store) - adminClient, _ := admin.New("rook-ceph-rgw-my-store.mycluster.svc", "53S6B9S809NUP19IJ2K3", "1bXPegzsGClvoGAiJdHQD1uOW2sQBLAZM9j9VtXR", mockClient) - - return &AdminOpsContext{ - Context: *context, - AdminOpsUserAccessKey: "EOE7FYCNOBZJ5VFV909G", - AdminOpsUserSecretKey: "qmIqpWm8HxCzmynCrD6U6vKWi4hnDBndOnmxXNsV", // notsecret - AdminOpsClient: adminClient, - }, nil -} - func TestCephObjectStoreController(t *testing.T) { ctx := context.TODO() // Set DEBUG logging @@ -370,13 +337,6 @@ func TestCephObjectStoreController(t *testing.T) { return nil } - // overwrite adminops context func - oldNewMultisiteAdminOpsCtxFunc := newMultisiteAdminOpsCtxFunc - newMultisiteAdminOpsCtxFunc = mockMultisiteAdminOpsCtxFunc - defer func() { - newMultisiteAdminOpsCtxFunc = oldNewMultisiteAdminOpsCtxFunc - }() - setupNewEnvironment := func(additionalObjects ...runtime.Object) *ReconcileCephObjectStore { // reset var we use to check if we have called to commit config changes calledCommitConfigChanges = false @@ -418,11 +378,9 @@ func TestCephObjectStoreController(t *testing.T) { s := scheme.Scheme s.AddKnownTypes(cephv1.SchemeGroupVersion, &cephv1.CephObjectStore{}) s.AddKnownTypes(cephv1.SchemeGroupVersion, &cephv1.CephCluster{}) - s.AddKnownTypes(v1.SchemeGroupVersion, &v1.Secret{}) // Create a fake client to mock API calls. cl := fake.NewClientBuilder().WithScheme(s).WithRuntimeObjects(objects...).Build() - // Create a ReconcileCephObjectStore object with the scheme and fake client. r := &ReconcileCephObjectStore{ client: cl, @@ -738,12 +696,7 @@ func TestCephObjectStoreControllerMultisite(t *testing.T) { commitConfigChangesOrig := commitConfigChanges defer func() { commitConfigChanges = commitConfigChangesOrig }() - // overwrite adminops context func - oldNewMultisiteAdminOpsCtxFunc := newMultisiteAdminOpsCtxFunc - newMultisiteAdminOpsCtxFunc = mockMultisiteAdminOpsCtxFunc - defer func() { - newMultisiteAdminOpsCtxFunc = oldNewMultisiteAdminOpsCtxFunc - }() + // make sure joining multisite calls to commit config changes calledCommitConfigChanges := false commitConfigChanges = func(c *Context) error { @@ -766,7 +719,7 @@ func TestCephObjectStoreControllerMultisite(t *testing.T) { // Register operator types with the runtime scheme. s := scheme.Scheme s.AddKnownTypes(cephv1.SchemeGroupVersion, &cephv1.CephObjectZone{}, &cephv1.CephObjectZoneList{}, &cephv1.CephCluster{}, &cephv1.CephClusterList{}, &cephv1.CephObjectStore{}, &cephv1.CephObjectStoreList{}) - s.AddKnownTypes(v1.SchemeGroupVersion, &v1.Secret{}) + // Create a fake client to mock API calls. cl := fake.NewClientBuilder().WithScheme(s).WithRuntimeObjects(object...).Build() @@ -971,13 +924,6 @@ func TestCephObjectExternalStoreController(t *testing.T) { rgwAdminOpsUserSecret, } - // overwrite adminops context func - oldNewMultisiteAdminOpsCtxFunc := newMultisiteAdminOpsCtxFunc - newMultisiteAdminOpsCtxFunc = mockMultisiteAdminOpsCtxFunc - defer func() { - newMultisiteAdminOpsCtxFunc = oldNewMultisiteAdminOpsCtxFunc - }() - r := getReconciler(objects) t.Run("create an external object store", func(t *testing.T) { diff --git a/pkg/operator/ceph/object/objectstore.go b/pkg/operator/ceph/object/objectstore.go index e2e79304afd5..054b7b9ec100 100644 --- a/pkg/operator/ceph/object/objectstore.go +++ b/pkg/operator/ceph/object/objectstore.go @@ -53,8 +53,6 @@ const ( SecretKeyName = "secret-key" svcDNSSuffix = "svc" rgwRadosPoolPgNum = "8" - cosiUserName = "cosi" - cosiUserCaps = "buckets=*;users=*" rgwApplication = "rgw" ) diff --git a/pkg/operator/ceph/object/user.go b/pkg/operator/ceph/object/user.go index 7cb2b670b682..d435c29951f5 100644 --- a/pkg/operator/ceph/object/user.go +++ b/pkg/operator/ceph/object/user.go @@ -17,23 +17,13 @@ limitations under the License. package object import ( - "context" "encoding/json" - "fmt" "strings" "syscall" "github.com/ceph/go-ceph/rgw/admin" "github.com/pkg/errors" - opcontroller "github.com/rook/rook/pkg/operator/ceph/controller" - "github.com/rook/rook/pkg/operator/k8sutil" "github.com/rook/rook/pkg/util/exec" - corev1 "k8s.io/api/core/v1" - metav1 "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/controller/controllerutil" - "sigs.k8s.io/controller-runtime/pkg/reconcile" ) const ( @@ -236,53 +226,3 @@ func DeleteUser(c *Context, id string, opts ...string) (string, error) { return result, errors.Wrapf(err, "failed to delete s3 user uid=%q", id) } - -func GenerateCephUserSecretName(store, username string) string { - return fmt.Sprintf("rook-ceph-object-user-%s-%s", store, username) -} - -func generateCephUserSecret(userConfig *admin.User, endpoint, namespace, storeName, tlsSecretName string) *corev1.Secret { - secretName := GenerateCephUserSecretName(storeName, userConfig.ID) - // Store the keys in a secret - secrets := map[string]string{ - "AccessKey": userConfig.Keys[0].AccessKey, - "SecretKey": userConfig.Keys[0].SecretKey, - "Endpoint": endpoint, - } - if tlsSecretName != "" { - secrets["SSLCertSecretName"] = tlsSecretName - } - secret := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: secretName, - Namespace: namespace, - Labels: map[string]string{ - "app": AppName, - "user": userConfig.ID, - "rook_cluster": namespace, - "rook_object_store": storeName, - }, - }, - StringData: secrets, - Type: k8sutil.RookType, - } - return secret -} - -func ReconcileCephUserSecret(ctx context.Context, k8sclient client.Client, scheme *runtime.Scheme, ownerRef metav1.Object, userConfig *admin.User, endpoint, namespace, storeName, tlsSecretName string) (reconcile.Result, error) { - // Generate Kubernetes Secret - secret := generateCephUserSecret(userConfig, endpoint, namespace, storeName, tlsSecretName) - - // Set owner ref to the object store user object - err := controllerutil.SetControllerReference(ownerRef, secret, scheme) - if err != nil { - return reconcile.Result{}, errors.Wrapf(err, "failed to set owner reference of ceph object user secret %q", secret.Name) - } - - // Create Kubernetes Secret - err = opcontroller.CreateOrUpdateObject(ctx, k8sclient, secret) - if err != nil { - return reconcile.Result{}, errors.Wrapf(err, "failed to create or update ceph object user %q secret", secret.Name) - } - return reconcile.Result{}, nil -} diff --git a/pkg/operator/ceph/object/user/controller.go b/pkg/operator/ceph/object/user/controller.go index 2ff73e3d85b2..f4f910d051ac 100644 --- a/pkg/operator/ceph/object/user/controller.go +++ b/pkg/operator/ceph/object/user/controller.go @@ -29,6 +29,7 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/tools/record" "sigs.k8s.io/controller-runtime/pkg/controller" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/manager" "sigs.k8s.io/controller-runtime/pkg/reconcile" @@ -265,7 +266,7 @@ func (r *ReconcileObjectStoreUser) reconcile(request reconcile.Request) (reconci } tlsSecretName := store.Spec.Gateway.SSLCertificateRef - reconcileResponse, err = object.ReconcileCephUserSecret(r.opManagerContext, r.client, r.scheme, cephObjectStoreUser, r.userConfig, r.advertiseEndpoint, cephObjectStoreUser.Namespace, cephObjectStoreUser.Spec.Store, tlsSecretName) + reconcileResponse, err = r.reconcileCephUserSecret(cephObjectStoreUser, tlsSecretName) if err != nil { r.updateStatus(k8sutil.ObservedGenerationNotAvailable, request.NamespacedName, k8sutil.ReconcileFailedStatus) return reconcileResponse, *cephObjectStoreUser, err @@ -509,12 +510,62 @@ func generateUserConfig(user *cephv1.CephObjectStoreUser) admin.User { return userConfig } +func generateCephUserSecretName(u *cephv1.CephObjectStoreUser) string { + return fmt.Sprintf("rook-ceph-object-user-%s-%s", u.Spec.Store, u.Name) +} + func generateStatusInfo(u *cephv1.CephObjectStoreUser) map[string]string { m := make(map[string]string) - m["secretName"] = object.GenerateCephUserSecretName(u.Spec.Store, u.Name) + m["secretName"] = generateCephUserSecretName(u) return m } +func (r *ReconcileObjectStoreUser) generateCephUserSecret(u *cephv1.CephObjectStoreUser, tlsSecretName string) *corev1.Secret { + // Store the keys in a secret + secrets := map[string]string{ + "AccessKey": r.userConfig.Keys[0].AccessKey, + "SecretKey": r.userConfig.Keys[0].SecretKey, + "Endpoint": r.objContext.Endpoint, + } + if tlsSecretName != "" { + secrets["SSLCertSecretName"] = tlsSecretName + } + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: generateCephUserSecretName(u), + Namespace: u.Namespace, + Labels: map[string]string{ + "app": appName, + "user": u.Name, + "rook_cluster": u.Namespace, + "rook_object_store": u.Spec.Store, + }, + }, + StringData: secrets, + Type: k8sutil.RookType, + } + return secret +} + +func (r *ReconcileObjectStoreUser) reconcileCephUserSecret(cephObjectStoreUser *cephv1.CephObjectStoreUser, tlsSecretName string) (reconcile.Result, error) { + // Generate Kubernetes Secret + secret := r.generateCephUserSecret(cephObjectStoreUser, tlsSecretName) + + // Set owner ref to the object store user object + err := controllerutil.SetControllerReference(cephObjectStoreUser, secret, r.scheme) + if err != nil { + return reconcile.Result{}, errors.Wrapf(err, "failed to set owner reference of ceph object user secret %q", secret.Name) + } + + // Create Kubernetes Secret + err = opcontroller.CreateOrUpdateObject(r.opManagerContext, r.client, secret) + if err != nil { + return reconcile.Result{}, errors.Wrapf(err, "failed to create or update ceph object user %q secret", secret.Name) + } + + return reconcile.Result{}, nil +} + func (r *ReconcileObjectStoreUser) objectStoreInitialized(cephObjectStoreUser *cephv1.CephObjectStoreUser) error { cephObjectStore, err := r.getObjectStore(cephObjectStoreUser.Spec.Store) if err != nil { diff --git a/tests/integration/ceph_cosi_test.go b/tests/integration/ceph_cosi_test.go index a4cfa8050941..3cb4fc73f214 100644 --- a/tests/integration/ceph_cosi_test.go +++ b/tests/integration/ceph_cosi_test.go @@ -6,7 +6,6 @@ import ( "time" "github.com/rook/rook/pkg/daemon/ceph/client" - "github.com/rook/rook/pkg/operator/ceph/object" rgw "github.com/rook/rook/pkg/operator/ceph/object" "github.com/rook/rook/tests/framework/clients" "github.com/rook/rook/tests/framework/installer" @@ -56,7 +55,8 @@ func testCOSIDriver(s *suite.Suite, helper *clients.TestClient, k8sh *utils.K8sH assert.NoError(t, k8sh.WaitForLabeledDeploymentsToBeReady("app=ceph-cosi-driver", operatorNamespace)) }) - objectStoreUserSecretName := object.GenerateCephUserSecretName(objectStoreCOSI, cosiUser) + createCephObjectUser(s, helper, k8sh, namespace, objectStoreCOSI, cosiUser, true) + objectStoreUserSecretName := "rook-ceph-object-user" + "-" + objectStoreCOSI + "-" + cosiUser t.Run("Creating BucketClass", func(t *testing.T) { err := helper.COSIClient.CreateBucketClass(bucketClassName, objectStoreUserSecretName, deletionPolicy) assert.NoError(t, err, "failed to create BucketClass")