From db463edeefe2094fba0f7c1d3e5fabf7ce679794 Mon Sep 17 00:00:00 2001 From: Huamin Chen Date: Fri, 18 Jan 2019 10:27:48 -0500 Subject: [PATCH 1/3] allow ceph mon stored in secret so when mon changes, cephfs driver can get latest mons and override old ones Signed-off-by: Huamin Chen --- pkg/cephfs/controllerserver.go | 18 +++++++++++++++--- pkg/cephfs/credentials.go | 8 ++++++++ pkg/cephfs/nodeserver.go | 13 ++++++++++--- pkg/cephfs/volumeoptions.go | 10 ++++++++-- 4 files changed, 41 insertions(+), 8 deletions(-) diff --git a/pkg/cephfs/controllerserver.go b/pkg/cephfs/controllerserver.go index 6d1375fc0..765355775 100644 --- a/pkg/cephfs/controllerserver.go +++ b/pkg/cephfs/controllerserver.go @@ -51,7 +51,12 @@ func (cs *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol } volId := makeVolumeID(req.GetName()) - + secret := req.GetSecrets() + if len(volOptions.Monitors) == 0 { + if mon, err := getMonValFromSecret(secret); err == nil && len(mon) > 0 { + volOptions.Monitors = mon + } + } conf := cephConfigData{Monitors: volOptions.Monitors, VolumeID: volId} if err = conf.writeToFile(); err != nil { glog.Errorf("failed to write ceph config file to %s: %v", getCephConfPath(volId), err) @@ -62,7 +67,7 @@ func (cs *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol if volOptions.ProvisionVolume { // Admin credentials are required - cr, err := getAdminCredentials(req.GetSecrets()) + cr, err := getAdminCredentials(secret) if err != nil { return nil, status.Error(codes.InvalidArgument, err.Error()) } @@ -124,10 +129,17 @@ func (cs *controllerServer) DeleteVolume(ctx context.Context, req *csi.DeleteVol glog.Warningf("volume %s is provisioned statically, aborting delete", volId) return &csi.DeleteVolumeResponse{}, nil } + // mons may have changed since create volume, + // retrieve the latest mons and override old mons + secret := req.GetSecrets() + if mon, err := getMonValFromSecret(secret); err == nil && len(mon) > 0 { + glog.Infof("override old mons [%q] with [%q]", ce.VolOptions.Monitors, mon) + ce.VolOptions.Monitors = mon + } // Deleting a volume requires admin credentials - cr, err := getAdminCredentials(req.GetSecrets()) + cr, err := getAdminCredentials(secret) if err != nil { glog.Errorf("failed to retrieve admin credentials: %v", err) return nil, status.Error(codes.InvalidArgument, err.Error()) diff --git a/pkg/cephfs/credentials.go b/pkg/cephfs/credentials.go index b33349d81..343e9a12d 100644 --- a/pkg/cephfs/credentials.go +++ b/pkg/cephfs/credentials.go @@ -23,6 +23,7 @@ const ( credUserKey = "userKey" credAdminId = "adminID" credAdminKey = "adminKey" + credMonitors = "monitors" ) type credentials struct { @@ -54,3 +55,10 @@ func getUserCredentials(secrets map[string]string) (*credentials, error) { func getAdminCredentials(secrets map[string]string) (*credentials, error) { return getCredentials(credAdminId, credAdminKey, secrets) } + +func getMonValFromSecret(secrets map[string]string) (string, error) { + if mons, ok := secrets[credMonitors]; ok { + return mons, nil + } + return "", fmt.Errorf("missing %q", credMonitors) +} diff --git a/pkg/cephfs/nodeserver.go b/pkg/cephfs/nodeserver.go index c557569f1..601b45033 100644 --- a/pkg/cephfs/nodeserver.go +++ b/pkg/cephfs/nodeserver.go @@ -38,13 +38,13 @@ func getCredentialsForVolume(volOptions *volumeOptions, volId volumeID, req *csi userCr *credentials err error ) - + secret := req.GetSecrets() if volOptions.ProvisionVolume { // The volume is provisioned dynamically, get the credentials directly from Ceph // First, store admin credentials - those are needed for retrieving the user credentials - adminCr, err := getAdminCredentials(req.GetSecrets()) + adminCr, err := getAdminCredentials(secret) if err != nil { return nil, fmt.Errorf("failed to get admin credentials from node stage secrets: %v", err) } @@ -64,7 +64,7 @@ func getCredentialsForVolume(volOptions *volumeOptions, volId volumeID, req *csi } else { // The volume is pre-made, credentials are in node stage secrets - userCr, err = getUserCredentials(req.GetSecrets()) + userCr, err = getUserCredentials(secret) if err != nil { return nil, fmt.Errorf("failed to get user credentials from node stage secrets: %v", err) } @@ -103,6 +103,13 @@ func (ns *nodeServer) NodeStageVolume(ctx context.Context, req *csi.NodeStageVol return nil, status.Error(codes.Internal, err.Error()) } + // mons may have changed since create volume, + // retrieve the latest mons and override old mons + secret := req.GetSecrets() + if mon, err := getMonValFromSecret(secret); err == nil && len(mon) > 0 { + glog.Infof("override old mons [%q] with [%q]", volOptions.Monitors, mon) + volOptions.Monitors = mon + } cephConf := cephConfigData{Monitors: volOptions.Monitors, VolumeID: volId} if err = cephConf.writeToFile(); err != nil { glog.Errorf("failed to write ceph config file to %s for volume %s: %v", getCephConfPath(volId), volId, err) diff --git a/pkg/cephfs/volumeoptions.go b/pkg/cephfs/volumeoptions.go index d55bfe229..151624092 100644 --- a/pkg/cephfs/volumeoptions.go +++ b/pkg/cephfs/volumeoptions.go @@ -28,6 +28,8 @@ type volumeOptions struct { Mounter string `json:"mounter"` ProvisionVolume bool `json:"provisionVolume"` + + MonValueFromSecret string `json:"monValueFromSecret"` } func validateNonEmptyField(field, fieldName string) error { @@ -40,7 +42,9 @@ func validateNonEmptyField(field, fieldName string) error { func (o *volumeOptions) validate() error { if err := validateNonEmptyField(o.Monitors, "monitors"); err != nil { - return err + if err = validateNonEmptyField(o.MonValueFromSecret, "monValueFromSecret"); err != nil { + return err + } } if err := validateNonEmptyField(o.RootPath, "rootPath"); err != nil { @@ -97,7 +101,9 @@ func newVolumeOptions(volOptions map[string]string) (*volumeOptions, error) { ) if err = extractOption(&opts.Monitors, "monitors", volOptions); err != nil { - return nil, err + if err = extractOption(&opts.MonValueFromSecret, "monValueFromSecret", volOptions); err != nil { + return nil, err + } } if err = extractOption(&provisionVolumeBool, "provisionVolume", volOptions); err != nil { From 3f196b5d739f0ed556f53a3e58b48850acbb9816 Mon Sep 17 00:00:00 2001 From: Huamin Chen Date: Fri, 18 Jan 2019 10:38:32 -0500 Subject: [PATCH 2/3] update cephfs doc Signed-off-by: Huamin Chen --- docs/deploy-cephfs.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/deploy-cephfs.md b/docs/deploy-cephfs.md index 101312cad..f571f428c 100644 --- a/docs/deploy-cephfs.md +++ b/docs/deploy-cephfs.md @@ -38,6 +38,7 @@ Option | Default value | Description Parameter | Required | Description --------- | -------- | ----------- `monitors` | yes | Comma separated list of Ceph monitors (e.g. `192.168.100.1:6789,192.168.100.2:6789,192.168.100.3:6789`) +`monValueFromSecret` | one of `monitors` and `monValueFromSecret` must be set | a string pointing the key in the credential secret, whose value is the mon. This is used for the case when the monitors' IP or hostnames are changed, the secret can be updated to pick up the new monitors. `mounter` | no | Mount method to be used for this volume. Available options are `kernel` for Ceph kernel client and `fuse` for Ceph FUSE driver. Defaults to "default mounter", see command line arguments. `provisionVolume` | yes | Mode of operation. BOOL value. If `true`, a new CephFS volume will be provisioned. If `false`, an existing volume will be used. `pool` | for `provisionVolume=true` | Ceph pool into which the volume shall be created From 0151792684a04d2a755c2f073801e4147331c34f Mon Sep 17 00:00:00 2001 From: Huamin Chen Date: Mon, 21 Jan 2019 09:21:03 -0500 Subject: [PATCH 3/3] review feedback: make monValueFromSecret override monitors if both are set Signed-off-by: Huamin Chen --- docs/deploy-cephfs.md | 2 +- pkg/cephfs/controllerserver.go | 9 ++------- pkg/cephfs/nodeserver.go | 10 ++-------- pkg/cephfs/volumeoptions.go | 16 +++++++++++----- 4 files changed, 16 insertions(+), 21 deletions(-) diff --git a/docs/deploy-cephfs.md b/docs/deploy-cephfs.md index f571f428c..7c389dab9 100644 --- a/docs/deploy-cephfs.md +++ b/docs/deploy-cephfs.md @@ -38,7 +38,7 @@ Option | Default value | Description Parameter | Required | Description --------- | -------- | ----------- `monitors` | yes | Comma separated list of Ceph monitors (e.g. `192.168.100.1:6789,192.168.100.2:6789,192.168.100.3:6789`) -`monValueFromSecret` | one of `monitors` and `monValueFromSecret` must be set | a string pointing the key in the credential secret, whose value is the mon. This is used for the case when the monitors' IP or hostnames are changed, the secret can be updated to pick up the new monitors. +`monValueFromSecret` | one of `monitors` and `monValueFromSecret` must be set | a string pointing the key in the credential secret, whose value is the mon. This is used for the case when the monitors' IP or hostnames are changed, the secret can be updated to pick up the new monitors. If both `monitors` and `monValueFromSecret` are set and the monitors set in the secret exists, `monValueFromSecret` takes precedence. `mounter` | no | Mount method to be used for this volume. Available options are `kernel` for Ceph kernel client and `fuse` for Ceph FUSE driver. Defaults to "default mounter", see command line arguments. `provisionVolume` | yes | Mode of operation. BOOL value. If `true`, a new CephFS volume will be provisioned. If `false`, an existing volume will be used. `pool` | for `provisionVolume=true` | Ceph pool into which the volume shall be created diff --git a/pkg/cephfs/controllerserver.go b/pkg/cephfs/controllerserver.go index 765355775..7d775ada3 100644 --- a/pkg/cephfs/controllerserver.go +++ b/pkg/cephfs/controllerserver.go @@ -44,19 +44,14 @@ func (cs *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol return nil, err } // Configuration - volOptions, err := newVolumeOptions(req.GetParameters()) + secret := req.GetSecrets() + volOptions, err := newVolumeOptions(req.GetParameters(), secret) if err != nil { glog.Errorf("validation of volume options failed: %v", err) return nil, status.Error(codes.InvalidArgument, err.Error()) } volId := makeVolumeID(req.GetName()) - secret := req.GetSecrets() - if len(volOptions.Monitors) == 0 { - if mon, err := getMonValFromSecret(secret); err == nil && len(mon) > 0 { - volOptions.Monitors = mon - } - } conf := cephConfigData{Monitors: volOptions.Monitors, VolumeID: volId} if err = conf.writeToFile(); err != nil { glog.Errorf("failed to write ceph config file to %s: %v", getCephConfPath(volId), err) diff --git a/pkg/cephfs/nodeserver.go b/pkg/cephfs/nodeserver.go index 601b45033..ca20d4a5f 100644 --- a/pkg/cephfs/nodeserver.go +++ b/pkg/cephfs/nodeserver.go @@ -87,7 +87,8 @@ func (ns *nodeServer) NodeStageVolume(ctx context.Context, req *csi.NodeStageVol stagingTargetPath := req.GetStagingTargetPath() volId := volumeID(req.GetVolumeId()) - volOptions, err := newVolumeOptions(req.GetVolumeContext()) + secret := req.GetSecrets() + volOptions, err := newVolumeOptions(req.GetVolumeContext(), secret) if err != nil { glog.Errorf("error reading volume options for volume %s: %v", volId, err) return nil, status.Error(codes.InvalidArgument, err.Error()) @@ -103,13 +104,6 @@ func (ns *nodeServer) NodeStageVolume(ctx context.Context, req *csi.NodeStageVol return nil, status.Error(codes.Internal, err.Error()) } - // mons may have changed since create volume, - // retrieve the latest mons and override old mons - secret := req.GetSecrets() - if mon, err := getMonValFromSecret(secret); err == nil && len(mon) > 0 { - glog.Infof("override old mons [%q] with [%q]", volOptions.Monitors, mon) - volOptions.Monitors = mon - } cephConf := cephConfigData{Monitors: volOptions.Monitors, VolumeID: volId} if err = cephConf.writeToFile(); err != nil { glog.Errorf("failed to write ceph config file to %s for volume %s: %v", getCephConfPath(volId), volId, err) diff --git a/pkg/cephfs/volumeoptions.go b/pkg/cephfs/volumeoptions.go index 151624092..8b7028991 100644 --- a/pkg/cephfs/volumeoptions.go +++ b/pkg/cephfs/volumeoptions.go @@ -93,19 +93,25 @@ func validateMounter(m string) error { return nil } -func newVolumeOptions(volOptions map[string]string) (*volumeOptions, error) { +func newVolumeOptions(volOptions, secret map[string]string) (*volumeOptions, error) { var ( opts volumeOptions provisionVolumeBool string err error ) - if err = extractOption(&opts.Monitors, "monitors", volOptions); err != nil { - if err = extractOption(&opts.MonValueFromSecret, "monValueFromSecret", volOptions); err != nil { - return nil, err + // extract mon from secret first + if err = extractOption(&opts.MonValueFromSecret, "monValueFromSecret", volOptions); err == nil { + if mon, err := getMonValFromSecret(secret); err == nil && len(mon) > 0 { + opts.Monitors = mon + } + } + if len(opts.Monitors) == 0 { + // if not set in secret, get it from parameter + if err = extractOption(&opts.Monitors, "monitors", volOptions); err != nil { + return nil, fmt.Errorf("either monitors or monValueFromSecret should be set") } } - if err = extractOption(&provisionVolumeBool, "provisionVolume", volOptions); err != nil { return nil, err }