review feedback: make monValueFromSecret override monitors if both are set

Signed-off-by: Huamin Chen <hchen@redhat.com>
This commit is contained in:
Huamin Chen 2019-01-21 09:21:03 -05:00
parent 3f196b5d73
commit 0151792684
4 changed files with 16 additions and 21 deletions

View File

@ -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

View File

@ -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)

View File

@ -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)

View File

@ -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
}