Refactor flags/filters applied to set volume options

'--advanced' in the CLI and REST API during volume create doesn't intuitively
and clearly indicate that it's for the volume options. A new user may read this
as creation of an "advanced volume" which isn't true.

The flags have now been prepended with 'allow' in the fields of request structs.
The API has been renamed as 'allow-<type>-options'.

Also, created 'VolOptionFlags' struct for flags of options and embeded it in
all request structs.

Signed-off-by: Prashanth Pai <ppai@redhat.com>

e2e/glustershd_test.go

@@ -57,7 +57,7 @@ func testSelfHeal(t *testing.T, tc *testCluster) {
 	var optionReq api.VolOptionReq
 
 	optionReq.Options = map[string]string{"replicate.self-heal-daemon": "on"}
-	optionReq.Advanced = true
+	optionReq.AllowAdvanced = true
 
 	r.Nil(client.VolumeSet(vol1.Name, optionReq))
 	r.True(isProcessRunning(pidpath), "glustershd is not running")
@@ -69,7 +69,7 @@ func testSelfHeal(t *testing.T, tc *testCluster) {
 	r.Nil(client.VolumeStop(vol1.Name), "Volume stop failed")
 
 	optionReq.Options = map[string]string{"replicate.self-heal-daemon": "off"}
-	optionReq.Advanced = true
+	optionReq.AllowAdvanced = true
 
 	r.Nil(client.VolumeSet(vol1.Name, optionReq))
 	r.False(isProcessRunning(pidpath), "glustershd is still running")
@@ -126,11 +126,11 @@ func testGranularEntryHeal(t *testing.T, tc *testCluster) {
 
 	var optionReq api.VolOptionReq
 	optionReq.Options = map[string]string{"replicate.granular-entry-heal": "enable"}
-	optionReq.Advanced = true
+	optionReq.AllowAdvanced = true
 	r.Nil(client.VolumeSet(volname, optionReq))
 
 	optionReq.Options = map[string]string{"replicate.self-heal-daemon": "off"}
-	optionReq.Advanced = true
+	optionReq.AllowAdvanced = true
 	r.Nil(client.VolumeSet(volname, optionReq))
 	r.False(isProcessRunning(pidpath), "glustershd is still running")
 
@@ -170,7 +170,7 @@ func testGranularEntryHeal(t *testing.T, tc *testCluster) {
 	}
 
 	optionReq.Options = map[string]string{"replicate.granular-entry-heal": "disable"}
-	optionReq.Advanced = true
+	optionReq.AllowAdvanced = true
 	r.Nil(client.VolumeSet(volname, optionReq))
 
 	// Stop Volume
@@ -178,7 +178,7 @@ func testGranularEntryHeal(t *testing.T, tc *testCluster) {
 	r.Nil(client.VolumeStart(volname, false), "volume start failed")
 
 	optionReq.Options = map[string]string{"replicate.granular-entry-heal": "enable"}
-	optionReq.Advanced = true
+	optionReq.AllowAdvanced = true
 	r.NotNil(client.VolumeSet(volname, optionReq))
 
 	err = syscall.Unmount(mntPath, 0)

e2e/quota_enable.go

@@ -79,7 +79,7 @@ func testQuotaEnable(t *testing.T, tc *testCluster) {
 
 	quotaKey := "quota.enable"
 	var optionReqOff api.VolOptionReq
-	optionReqOff.Advanced = true
+	optionReqOff.AllowAdvanced = true
 
 	optionReqOff.Options = map[string]string{quotaKey: "off"}
 
@@ -91,7 +91,7 @@ func testQuotaEnable(t *testing.T, tc *testCluster) {
 	r.False(isProcessRunning(pidpath))
 
 	var optionReqOn api.VolOptionReq
-	optionReqOn.Advanced = true
+	optionReqOn.AllowAdvanced = true
 
 	// Enable quota
 	quotaKey = "quota.enable"

e2e/volume_ops_test.go

@@ -532,12 +532,10 @@ func TestVolumeOptions(t *testing.T) {
 			},
 		},
 		Force: true,
-		// XXX: Setting advanced, as all options are advanced by default
-		// TODO: Remove this later if the default changes
-		VolOptionReq: api.VolOptionReq{
-			Advanced: true,
-		},
 	}
+	// XXX: Setting advanced, as all options are advanced by default
+	// TODO: Remove this later if the default changes
+	createReq.AllowAdvanced = true
 
 	validOpKeys := []string{"gfproxy.afr.eager-lock", "afr.eager-lock"}
 	invalidOpKeys := []string{"..eager-lock", "a.b.afr.eager-lock", "afr.non-existent", "eager-lock"}
@@ -587,7 +585,7 @@ func TestVolumeOptions(t *testing.T) {
 	var optionReq api.VolOptionReq
 	// XXX: Setting advanced, as all options are advanced by default
 	// TODO: Remove this later if the default changes
-	optionReq.Advanced = true
+	optionReq.AllowAdvanced = true
 
 	settableKey := "afr.use-compound-fops"
 	optionReq.Options = map[string]string{settableKey: "on"}
@@ -647,10 +645,11 @@ func TestVolumeOptions(t *testing.T) {
 				{Name: "opt3", OnValue: "off"}},
 			Description: "Test profile 2",
 		},
-		// XXX: Setting advanced, as all options are advanced by default
-		// TODO: Remove this later if the default changes
-		Advanced: true,
 	}
+	// XXX: Setting advanced, as all options are advanced by default
+	// TODO: Remove this later if the default changes
+	optionGroupReq.AllowAdvanced = true
+
 	err = client.OptionGroupCreate(optionGroupReq)
 	r.NotNil(err)
 
@@ -662,10 +661,11 @@ func TestVolumeOptions(t *testing.T) {
 			},
 			Description: "Test profile 2",
 		},
-		// XXX: Setting advanced, as all options are advanced by default
-		// TODO: Remove this later if the default changes
-		Advanced: true,
 	}
+	// XXX: Setting advanced, as all options are advanced by default
+	// TODO: Remove this later if the default changes
+	optionGroupReq.AllowAdvanced = true
+
 	err = client.OptionGroupCreate(optionGroupReq)
 	r.Nil(err)
 
@@ -820,7 +820,7 @@ func testShdOnVolumeStartAndStop(t *testing.T, tc *testCluster) {
 
 	var optionReq api.VolOptionReq
 	optionReq.Options = map[string]string{"replicate.self-heal-daemon": "on"}
-	optionReq.Advanced = true
+	optionReq.AllowAdvanced = true
 
 	r.Nil(client.VolumeSet(vol1.Name, optionReq))
 

glustercli/cmd/volume-create.go

@@ -62,9 +62,12 @@ func init() {
 	volumeCreateCmd.Flags().BoolVar(&flagCreateForce, "force", false, "Force")
 	volumeCreateCmd.Flags().StringSliceVar(&flagCreateVolumeOptions, "options", nil,
 		"Volume options in the format option:value,option:value")
-	volumeCreateCmd.Flags().BoolVar(&flagCreateAdvOpts, "advanced", false, "Allow advanced options")
-	volumeCreateCmd.Flags().BoolVar(&flagCreateExpOpts, "experimental", false, "Allow experimental options")
-	volumeCreateCmd.Flags().BoolVar(&flagCreateDepOpts, "deprecated", false, "Allow deprecated options")
+
+	// set volume options during volume create
+	volumeCreateCmd.Flags().BoolVar(&flagCreateAdvOpts, "allow-advanced-options", false, "Allow setting advanced volume options")
+	volumeCreateCmd.Flags().BoolVar(&flagCreateExpOpts, "allow-experimental-options", false, "Allow setting experimental volume options")
+	volumeCreateCmd.Flags().BoolVar(&flagCreateDepOpts, "allow-deprecated-options", false, "Allow setting deprecated volume options")
+
 	volumeCreateCmd.Flags().BoolVar(&flagReuseBricks, "reuse-bricks", false, "Reuse bricks")
 	volumeCreateCmd.Flags().BoolVar(&flagAllowRootDir, "allow-root-dir", false, "Allow root directory")
 	volumeCreateCmd.Flags().BoolVar(&flagAllowMountAsBrick, "allow-mount-as-brick", false, "Allow mount as bricks")
@@ -259,10 +262,12 @@ func volumeCreateCmdRun(cmd *cobra.Command, args []string) {
 		Subvols: subvols,
 		Force:   flagCreateForce,
 		VolOptionReq: api.VolOptionReq{
-			Options:      options,
-			Advanced:     flagCreateAdvOpts,
-			Experimental: flagCreateExpOpts,
-			Deprecated:   flagCreateDepOpts,
+			Options: options,
+			VolOptionFlags: api.VolOptionFlags{
+				AllowAdvanced:     flagCreateAdvOpts,
+				AllowExperimental: flagCreateExpOpts,
+				AllowDeprecated:   flagCreateDepOpts,
+			},
 		},
 
 		Flags: flags,
@@ -304,6 +309,6 @@ func addThinArbiter(req *api.VolCreateReq, thinArbiter string) error {
 	req.Options = map[string]string{
 		"replicate.thin-arbiter": thinArbiter,
 	}
-	req.Advanced = true
+	req.AllowAdvanced = true
 	return nil
 }

glustercli/cmd/volume-set.go

@@ -78,10 +78,13 @@ func volumeOptionJSONHandler(cmd *cobra.Command, volname string, options []strin
 	}
 
 	err := client.VolumeSet(volname, api.VolOptionReq{
-		Options:      vopt,
-		Advanced:     flagSetAdv,
-		Experimental: flagSetExp,
-		Deprecated:   flagSetDep,
+		Options: vopt,
+		VolOptionFlags: api.VolOptionFlags{
+			AllowAdvanced:     flagSetAdv,
+			AllowExperimental: flagSetExp,
+			AllowDeprecated:   flagSetDep,
+		},
 	})
+
 	return err
 }

glusterd2/commands/volumes/common.go

@@ -29,7 +29,7 @@ const volumeIDXattrKey = "trusted.glusterfs.volume-id"
 
 // validateOptions validates if the options and their values are valid and can
 // be set on a volume.
-func validateOptions(opts map[string]string, adv, exp, dep bool) error {
+func validateOptions(opts map[string]string, flags api.VolOptionFlags) error {
 
 	for k, v := range opts {
 		o, err := xlator.FindOption(k)
@@ -41,13 +41,13 @@ func validateOptions(opts map[string]string, adv, exp, dep bool) error {
 		case !o.IsSettable():
 			return fmt.Errorf("Option %s cannot be set", k)
 
-		case o.IsAdvanced() && !adv:
+		case o.IsAdvanced() && !flags.AllowAdvanced:
 			return fmt.Errorf("Option %s is an advanced option. To set it pass the advanced flag", k)
 
-		case o.IsExperimental() && !exp:
+		case o.IsExperimental() && !flags.AllowExperimental:
 			return fmt.Errorf("Option %s is an experimental option. To set it pass the experimental flag", k)
 
-		case o.IsDeprecated() && !dep:
+		case o.IsDeprecated() && !flags.AllowDeprecated:
 			// TODO: Return deprecation version and alternative option if available
 			return fmt.Errorf("Option %s will be deprecated in future releases. To set it pass the deprecated flag", k)
 		}

glusterd2/commands/volumes/optiongroup-create.go

@@ -16,7 +16,7 @@ func validateOptionSet(req api.OptionGroupReq) error {
 	for _, o := range req.Options {
 		o1[o.Name] = o.OnValue
 	}
-	return validateOptions(o1, req.Advanced, req.Experimental, req.Deprecated)
+	return validateOptions(o1, req.VolOptionFlags)
 }
 
 func optionGroupCreateHandler(w http.ResponseWriter, r *http.Request) {

glusterd2/commands/volumes/volume-create.go

@@ -143,7 +143,7 @@ func volumeCreateHandler(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	if err := validateOptions(req.Options, req.Advanced, req.Experimental, req.Deprecated); err != nil {
+	if err := validateOptions(req.Options, req.VolOptionFlags); err != nil {
 		restutils.SendHTTPError(ctx, w, http.StatusBadRequest, err)
 		return
 	}

glusterd2/commands/volumes/volume-option.go

@@ -34,7 +34,7 @@ func optionSetValidate(c transaction.TxnCtx) error {
 	// TODO: Validate op versions of the options. Either here or inside
 	// validateOptions.
 
-	if err := validateOptions(options, req.Advanced, req.Experimental, req.Deprecated); err != nil {
+	if err := validateOptions(options, req.VolOptionFlags); err != nil {
 		return fmt.Errorf("validation failed for volume option: %s", err.Error())
 	}
 

pkg/api/volume_req.go

@@ -62,12 +62,18 @@ type VolCreateReq struct {
 	VolOptionReq
 }
 
+// VolOptionFlags is set of flags that allow/disallow setting certain kinds
+// of volume options.
+type VolOptionFlags struct {
+	AllowAdvanced     bool `json:"allow-advanced-options,omitempty"`
+	AllowExperimental bool `json:"allow-experimental-options,omitempty"`
+	AllowDeprecated   bool `json:"allow-deprecated-options,omitempty"`
+}
+
 // VolOptionReq represents an incoming request to set volume options
 type VolOptionReq struct {
-	Options      map[string]string `json:"options"`
-	Advanced     bool              `json:"advanced,omitempty"`
-	Experimental bool              `json:"experimental,omitempty"`
-	Deprecated   bool              `json:"deprecated,omitempty"`
+	Options map[string]string `json:"options"`
+	VolOptionFlags
 }
 
 // VolOptionResetReq represents a request to reset volume options
@@ -107,9 +113,7 @@ type OptionGroup struct {
 // OptionGroupReq represents a request to create a new option group
 type OptionGroupReq struct {
 	OptionGroup
-	Advanced     bool `json:"advanced,omitempty"`
-	Experimental bool `json:"experimental,omitempty"`
-	Deprecated   bool `json:"deprecated,omitempty"`
+	VolOptionFlags
 }
 
 // ClientStatedump uniquely identifies a client (only gfapi) connected to