diff --git a/e2e/glustershd_test.go b/e2e/glustershd_test.go index b069c4f6..98c623cc 100644 --- a/e2e/glustershd_test.go +++ b/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) diff --git a/e2e/quota_enable.go b/e2e/quota_enable.go index e98717ac..65ab20be 100644 --- a/e2e/quota_enable.go +++ b/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" diff --git a/e2e/volume_ops_test.go b/e2e/volume_ops_test.go index effbf1d4..ae74f03f 100644 --- a/e2e/volume_ops_test.go +++ b/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)) diff --git a/glustercli/cmd/volume-create.go b/glustercli/cmd/volume-create.go index 1344b1d8..db79250f 100644 --- a/glustercli/cmd/volume-create.go +++ b/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 } diff --git a/glustercli/cmd/volume-set.go b/glustercli/cmd/volume-set.go index 34e0d304..fd63eae2 100644 --- a/glustercli/cmd/volume-set.go +++ b/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 } diff --git a/glusterd2/commands/volumes/common.go b/glusterd2/commands/volumes/common.go index 1375f61a..36cde6fc 100644 --- a/glusterd2/commands/volumes/common.go +++ b/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) } diff --git a/glusterd2/commands/volumes/optiongroup-create.go b/glusterd2/commands/volumes/optiongroup-create.go index d14fa0b2..9ef1ca87 100644 --- a/glusterd2/commands/volumes/optiongroup-create.go +++ b/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) { diff --git a/glusterd2/commands/volumes/volume-create.go b/glusterd2/commands/volumes/volume-create.go index 57f04dc2..8ddd4db5 100644 --- a/glusterd2/commands/volumes/volume-create.go +++ b/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 } diff --git a/glusterd2/commands/volumes/volume-option.go b/glusterd2/commands/volumes/volume-option.go index c0e6d3e3..38b70c9c 100644 --- a/glusterd2/commands/volumes/volume-option.go +++ b/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()) } diff --git a/pkg/api/volume_req.go b/pkg/api/volume_req.go index b94e3479..4b096836 100644 --- a/pkg/api/volume_req.go +++ b/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