From 75d732b3f21bd250bf540d3486913c6498fb701a Mon Sep 17 00:00:00 2001 From: themilchenko Date: Mon, 26 Aug 2024 17:42:25 +0300 Subject: [PATCH] replicaset: add command 'roles remove' @TarantoolBot document Title: `tt replicaset roles remove` removes roles in the tarantool replicaset with cluster config (3.0) or cartridge orchestrator. This patch introduces new command for the replicaset module. ``` $ tt rs roles remove [--cartridge|--config|--custom] [-f] [--timeout secs] [flags] ``` It is possible to provide `cartridge`, `config` or `custom` flag to explicitly state which orchestrator to use. ROLE_NAME is a role to remove from local cluster config in case of `cluster config` orchestrator and directly from all instances of replicaset in case of `cartridge` orchestrator. Command supports `cartridge` and `cluster config` orchestrators only for the entire application. INSTANCE_NAME works only for `cluster config` to provide instance name to remove role from. There are flags supported by this command: - `--global (-G)` for a global scope to add a role (only for `cluster config` orchestrator); - `--instance (-i) string` for an application name target to specify an instance to add a role; - `--replicaset (-r) string` for an application name target to specify a replicaset to add a role; - `--group (-g) string` for an application name target to specify a group to specify a group to add a role (only for `cluster config`); - `--force (-f)` skips instances not found locally in `cluster config` orchestrator. Closes #916 --- CHANGELOG.md | 2 + cli/cluster/cmd/replicaset.go | 4 +- cli/cmd/cluster.go | 4 +- cli/cmd/replicaset.go | 90 +++- cli/replicaset/cartridge.go | 54 ++- cli/replicaset/cartridge_test.go | 35 +- cli/replicaset/cconfig.go | 27 +- cli/replicaset/cconfig_test.go | 35 +- cli/replicaset/cmd/common.go | 2 +- cli/replicaset/cmd/roles.go | 35 +- cli/replicaset/configsource.go | 6 +- cli/replicaset/configsource_test.go | 18 +- cli/replicaset/custom.go | 13 +- cli/replicaset/custom_test.go | 63 ++- cli/replicaset/roles.go | 75 +++- cli/replicaset/roles_test.go | 12 +- cli/running/running.go | 2 +- test/cartridge_helper.py | 16 +- .../replicaset/test_replicaset_bootstrap.py | 7 + .../replicaset/test_replicaset_roles_add.py | 9 +- .../test_replicaset_roles_remove.py | 394 ++++++++++++++++++ 21 files changed, 784 insertions(+), 119 deletions(-) create mode 100644 test/integration/replicaset/test_replicaset_roles_remove.py diff --git a/CHANGELOG.md b/CHANGELOG.md index f71794e31..34d4a8a9f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. config (3.0) or cartridge orchestrator. - `tt replicaset roles add`: command to add roles in the tarantool replicaset with cluster config (3.0) or cartridge orchestrator. +- `tt replicaset roles remove`: command to remove roles in the tarantool replicaset with + cluster config (3.0) or cartridge orchestrator. ### Fixed diff --git a/cli/cluster/cmd/replicaset.go b/cli/cluster/cmd/replicaset.go index 204a239bd..f544d73d3 100644 --- a/cli/cluster/cmd/replicaset.go +++ b/cli/cluster/cmd/replicaset.go @@ -301,7 +301,7 @@ type RolesChangeCtx struct { } // ChangeRole adds/removes a role by patching the cluster config. -func ChangeRole(uri *url.URL, ctx RolesChangeCtx, changeRoleFunc replicaset.ChangeRoleFunc) error { +func ChangeRole(uri *url.URL, ctx RolesChangeCtx, action replicaset.RolesChangerAction) error { opts, err := ParseUriOpts(uri) if err != nil { return fmt.Errorf("invalid URL %q: %w", uri, err) @@ -327,7 +327,7 @@ func ChangeRole(uri *url.URL, ctx RolesChangeCtx, changeRoleFunc replicaset.Chan IsGlobal: ctx.IsGlobal, RoleName: ctx.RoleName, Force: ctx.Force, - }, changeRoleFunc) + }, action) if err == nil { log.Info("Done.") } diff --git a/cli/cmd/cluster.go b/cli/cmd/cluster.go index cfdf93266..128984256 100644 --- a/cli/cmd/cluster.go +++ b/cli/cmd/cluster.go @@ -575,7 +575,7 @@ func internalClusterReplicasetRolesAddModule(cmdCtx *cmdcontext.CmdCtx, args []s } rolesChangeCtx.RoleName = args[1] - return clustercmd.ChangeRole(uri, rolesChangeCtx, replicaset.AddRole) + return clustercmd.ChangeRole(uri, rolesChangeCtx, replicaset.RolesAdder{}) } // internalClusterReplicasetRolesRemoveModule is a "cluster replicaset roles remove" command. @@ -597,7 +597,7 @@ func internalClusterReplicasetRolesRemoveModule(cmdCtx *cmdcontext.CmdCtx, args } rolesChangeCtx.RoleName = args[1] - return clustercmd.ChangeRole(uri, rolesChangeCtx, replicaset.RemoveRole) + return clustercmd.ChangeRole(uri, rolesChangeCtx, replicaset.RolesRemover{}) } // internalClusterFailoverSwitchModule is as "cluster failover switch" command diff --git a/cli/cmd/replicaset.go b/cli/cmd/replicaset.go index 42cd989ad..049145446 100644 --- a/cli/cmd/replicaset.go +++ b/cli/cmd/replicaset.go @@ -252,6 +252,7 @@ func newRolesCmd() *cobra.Command { } cmd.AddCommand(newRolesAddCmd()) + cmd.AddCommand(newRolesRemoveCmd()) return cmd } @@ -293,6 +294,44 @@ func newRolesAddCmd() *cobra.Command { return cmd } +// newRolesRemoveCmd creates a "replicaset roles remove" command. +func newRolesRemoveCmd() *cobra.Command { + cmd := &cobra.Command{ + Use: "remove [--cartridge|--config|--custom] [-f] [--timeout secs]" + + " [flags]", + Short: "Removes a role for Cartridge and Tarantool 3 orchestrator", + Long: "Removes a role for Cartridge and Tarantool 3 orchestrator", + Run: func(cmd *cobra.Command, args []string) { + cmdCtx.CommandName = cmd.Name() + err := modules.RunCmd(&cmdCtx, cmd.CommandPath(), &modulesInfo, + internalReplicasetRolesRemoveModule, args) + util.HandleCmdErr(cmd, err) + }, + Args: cobra.ExactArgs(2), + } + + cmd.Flags().StringVarP(&replicasetReplicasetName, "replicaset", "r", "", + "name of a target replicaset") + cmd.Flags().StringVarP(&replicasetGroupName, "group", "g", "", + "name of a target group (vhsard-group in the Cartridge case)") + cmd.Flags().StringVarP(&replicasetInstanceName, "instance", "i", "", + "name of a target instance") + cmd.Flags().BoolVarP(&replicasetIsGlobal, "global", "G", false, + "global config context") + + addOrchestratorFlags(cmd) + addTarantoolConnectFlags(cmd) + cmd.Flags().BoolVarP(&replicasetForce, "force", "f", false, + "to force a promotion:\n"+ + " * config: skip instances not found locally\n"+ + " * cartridge: force inconsistency") + cmd.Flags().IntVarP(&replicasetTimeout, "timeout", "", + replicasetcmd.DefaultTimeout, "adding timeout") + integrity.RegisterWithIntegrityFlag(cmd.Flags(), &replicasetIntegrityPrivateKey) + + return cmd +} + // NewReplicasetCmd creates a replicaset command. func NewReplicasetCmd() *cobra.Command { cmd := &cobra.Command{ @@ -402,8 +441,8 @@ func replicasetFillCtx(cmdCtx *cmdcontext.CmdCtx, ctx *replicasetCtx, args []str } } } - // In case of adding a role when user may not provide an instance. - if cmdCtx.CommandName == "add" && ctx.InstName == "" { + // In case of adding/removing role when user may not provide an instance. + if (cmdCtx.CommandName == "add" || cmdCtx.CommandName == "remove") && ctx.InstName == "" { if len(ctx.RunningCtx.Instances) == 0 { return fmt.Errorf("there are no running instances") } @@ -674,7 +713,7 @@ func internalReplicasetRolesAddModule(cmdCtx *cmdcontext.CmdCtx, args []string) return err } - return replicasetcmd.RolesAdd(replicasetcmd.RolesAddCtx{ + return replicasetcmd.RolesChange(replicasetcmd.RolesChangeCtx{ InstName: ctx.InstName, GroupName: replicasetGroupName, ReplicasetName: replicasetReplicasetName, @@ -688,5 +727,48 @@ func internalReplicasetRolesAddModule(cmdCtx *cmdcontext.CmdCtx, args []string) Orchestrator: ctx.Orchestrator, Force: replicasetForce, Timeout: replicasetTimeout, - }) + }, replicaset.RolesAdder{}) +} + +// internalReplicasetRolesRemoveModule is a "roles remove" command for the replicaset module. +func internalReplicasetRolesRemoveModule(cmdCtx *cmdcontext.CmdCtx, args []string) error { + var ctx replicasetCtx + if err := replicasetFillCtx(cmdCtx, &ctx, args, false); err != nil { + return err + } + defer ctx.Conn.Close() + if ctx.IsApplication && replicasetInstanceName == "" && ctx.InstName == "" && + !replicasetIsGlobal && replicasetGroupName == "" && replicasetReplicasetName == "" { + return fmt.Errorf("there is no destination provided where to remove role") + } + if ctx.InstName != "" && replicasetInstanceName != "" && + replicasetInstanceName != ctx.InstName { + return fmt.Errorf("there are different instance names passed after" + + " app name and in flag arg") + } + if replicasetInstanceName != "" { + ctx.InstName = replicasetInstanceName + } + + collectors, publishers, err := createDataCollectorsAndDataPublishers( + cmdCtx.Integrity, replicasetIntegrityPrivateKey) + if err != nil { + return err + } + + return replicasetcmd.RolesChange(replicasetcmd.RolesChangeCtx{ + InstName: ctx.InstName, + GroupName: replicasetGroupName, + ReplicasetName: replicasetReplicasetName, + IsGlobal: replicasetIsGlobal, + RoleName: args[1], + Collectors: collectors, + Publishers: publishers, + IsApplication: ctx.IsApplication, + Conn: ctx.Conn, + RunningCtx: ctx.RunningCtx, + Orchestrator: ctx.Orchestrator, + Force: replicasetForce, + Timeout: replicasetTimeout, + }, replicaset.RolesRemover{}) } diff --git a/cli/replicaset/cartridge.go b/cli/replicaset/cartridge.go index f2e6c5d9c..bf67a6d2a 100644 --- a/cli/replicaset/cartridge.go +++ b/cli/replicaset/cartridge.go @@ -206,9 +206,10 @@ func (c *CartridgeInstance) BootstrapVShard(ctx VShardBootstrapCtx) error { return nil } -// RolesAdd adds role for a single instance by the Cartridge orchestrator. -func (c *CartridgeInstance) RolesAdd(ctx RolesChangeCtx) error { - return newErrRolesAddByInstanceNotSupported(OrchestratorCartridge) +// RolesChange adds/removes role for a single instance by the Cartridge orchestrator. +func (c *CartridgeInstance) RolesChange(ctx RolesChangeCtx, + action RolesChangerAction) error { + return newErrRolesChangeByInstanceNotSupported(OrchestratorCartridge, action) } // CartridgeApplication is an application with the Cartridge orchestrator. @@ -315,7 +316,7 @@ func (c *CartridgeApplication) Demote(ctx DemoteCtx) error { type cartridgeReplicasetConfig struct { Alias string `yaml:"alias,omitempty"` Instances []string `yaml:"instances"` - Roles []string `yaml:"roles"` + Roles []string `yaml:"roles,omitempty"` Weight *float64 `yaml:"weight,omitempty"` AllRW *bool `yaml:"all_rw,omitempty"` VShardGroup *string `yaml:"vshard_group,omitempty"` @@ -668,8 +669,9 @@ func (c *CartridgeApplication) BootstrapVShard(ctx VShardBootstrapCtx) error { return nil } -// RolesAdd adds role for an application by the Cartridge orchestrator. -func (c *CartridgeApplication) RolesAdd(ctx RolesChangeCtx) error { +// RolesChange adds/removes role for an application by the Cartridge orchestrator. +func (c *CartridgeApplication) RolesChange(ctx RolesChangeCtx, + action RolesChangerAction) error { if len(c.runningCtx.Instances) == 0 { return fmt.Errorf("failed to add role: there are no running instances") } @@ -685,18 +687,11 @@ func (c *CartridgeApplication) RolesAdd(ctx RolesChangeCtx) error { return i.Alias == inst.InstName }) }) - if slices.Contains(targetReplicaset.Roles, ctx.RoleName) { - return fmt.Errorf("role %q already exists in replicaset %q", - ctx.RoleName, ctx.ReplicasetName) - } - targetReplicaset.Roles = append(targetReplicaset.Roles, ctx.RoleName) - cartridgeEditOpt := cartridgeEditReplicasetsOpts{ - UUID: &targetReplicaset.UUID, - Roles: targetReplicaset.Roles, - } - if ctx.GroupName != "" { - cartridgeEditOpt.VshardGroup = &ctx.GroupName + cartridgeEditOpt, err := getRolesChangedOpts(targetReplicaset, action, + ctx.RoleName, ctx.GroupName) + if err != nil { + return err } eval := func(instance running.InstanceCtx, evaler connector.Evaler) (bool, error) { @@ -736,7 +731,28 @@ func (c *CartridgeApplication) RolesAdd(ctx RolesChangeCtx) error { return nil } -// getReplicasetByAlias searches for a replicaset by its alias in discovered slice +// getRolesChangedOpts adds/removes role for replicaset roles list and returns +// options for updating a replciaset. +func getRolesChangedOpts(targetReplicaset Replicaset, action RolesChangerAction, + roleName, groupName string) (cartridgeEditReplicasetsOpts, error) { + var err error + targetReplicaset.Roles, err = action.Change(targetReplicaset.Roles, roleName) + if err != nil { + return cartridgeEditReplicasetsOpts{}, fmt.Errorf("failed to change role: %w", err) + } + + cartridgeEditOpt := cartridgeEditReplicasetsOpts{ + UUID: &targetReplicaset.UUID, + Roles: targetReplicaset.Roles, + } + if groupName != "" && action.Action() == AddAction { + cartridgeEditOpt.VshardGroup = &groupName + } + + return cartridgeEditOpt, nil +} + +// getReplicasetByAlias searches for replicaset by its alias in discovered slice // of replicasets. func getReplicasetByAlias(replicasets []Replicaset, alias string) (Replicaset, error) { for _, r := range replicasets { @@ -857,7 +873,7 @@ type cartridgeJoinServersOpts struct { type cartridgeEditReplicasetsOpts struct { UUID *string `msgpack:"uuid,omitempty"` Alias *string `msgpack:"alias,omitempty"` - Roles []string `msgpack:"roles,omitempty"` + Roles []string `msgpack:"roles"` AllRW *bool `msgpack:"all_rw,omitempty"` Weight *float64 `msgpack:"weight,omitempty"` VshardGroup *string `msgpack:"vshard_group,omitempty"` diff --git a/cli/replicaset/cartridge_test.go b/cli/replicaset/cartridge_test.go index 61fc71f7e..75bb569d5 100644 --- a/cli/replicaset/cartridge_test.go +++ b/cli/replicaset/cartridge_test.go @@ -19,14 +19,14 @@ var _ replicaset.Demoter = &replicaset.CartridgeInstance{} var _ replicaset.Expeller = &replicaset.CartridgeInstance{} var _ replicaset.VShardBootstrapper = &replicaset.CartridgeInstance{} var _ replicaset.Bootstrapper = &replicaset.CartridgeInstance{} -var _ replicaset.RolesAdder = &replicaset.CartridgeInstance{} +var _ replicaset.RolesChanger = &replicaset.CartridgeInstance{} var _ replicaset.Discoverer = &replicaset.CartridgeApplication{} var _ replicaset.Promoter = &replicaset.CartridgeApplication{} var _ replicaset.Demoter = &replicaset.CartridgeApplication{} var _ replicaset.Expeller = &replicaset.CartridgeApplication{} var _ replicaset.Bootstrapper = &replicaset.CartridgeApplication{} -var _ replicaset.RolesAdder = &replicaset.CartridgeApplication{} +var _ replicaset.RolesChanger = &replicaset.CartridgeApplication{} func TestCartridgeApplication_Demote(t *testing.T) { app := replicaset.NewCartridgeApplication(running.RunningCtx{}) @@ -978,9 +978,32 @@ func TestCartridgeInstance_Expel(t *testing.T) { `expel is not supported for a single instance by "cartridge" orchestrator`) } -func TestCartridgeInstance_RolesAdd(t *testing.T) { +func TestCartridgeInstance_RolesChange(t *testing.T) { + cases := []struct { + name string + changeAction replicaset.RolesChangerAction + errMsg string + }{ + { + name: "roles add", + changeAction: replicaset.RolesAdder{}, + errMsg: "roles add is not supported for a single instance by" + + ` "cartridge" orchestrator`, + }, + { + name: "roles remove", + changeAction: replicaset.RolesRemover{}, + errMsg: "roles remove is not supported for a single instance by" + + ` "cartridge" orchestrator`, + }, + } + inst := replicaset.NewCartridgeInstance(nil) - err := inst.RolesAdd(replicaset.RolesChangeCtx{}) - assert.EqualError(t, err, - `roles add is not supported for a single instance by "cartridge" orchestrator`) + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + err := inst.RolesChange(replicaset.RolesChangeCtx{}, tc.changeAction) + assert.EqualError(t, err, tc.errMsg) + }) + } } diff --git a/cli/replicaset/cconfig.go b/cli/replicaset/cconfig.go index 99079a0d7..c94cf895e 100644 --- a/cli/replicaset/cconfig.go +++ b/cli/replicaset/cconfig.go @@ -4,7 +4,6 @@ import ( _ "embed" "errors" "fmt" - "slices" "strings" "github.com/apex/log" @@ -136,10 +135,11 @@ func (c *CConfigInstance) BootstrapVShard(ctx VShardBootstrapCtx) error { return nil } -// RolesAdd is not supported for a single instance by the centralized config +// RolesChange is not supported for a single instance by the centralized config // orchestrator. -func (c *CConfigInstance) RolesAdd(ctx RolesChangeCtx) error { - return newErrRolesAddByInstanceNotSupported(OrchestratorCentralizedConfig) +func (c *CConfigInstance) RolesChange(ctx RolesChangeCtx, + changeRoleAction RolesChangerAction) error { + return newErrRolesChangeByInstanceNotSupported(OrchestratorCentralizedConfig, changeRoleAction) } // CConfigApplication is an application with the centralized config @@ -472,8 +472,9 @@ func (c *CConfigApplication) Bootstrap(BootstrapCtx) error { return newErrBootstrapByAppNotSupported(OrchestratorCentralizedConfig) } -// RolesAdd adds role for an application by the centralized config orchestrator. -func (c *CConfigApplication) RolesAdd(ctx RolesChangeCtx) error { +// RolesChange adds/removes role for an application by the centralized config orchestrator. +func (c *CConfigApplication) RolesChange(ctx RolesChangeCtx, + changeRoleAction RolesChangerAction) error { replicasets, err := c.Discovery(UseCache) if err != nil { return fmt.Errorf("failed to get replicasets: %w", err) @@ -518,7 +519,7 @@ func (c *CConfigApplication) RolesAdd(ctx RolesChangeCtx) error { log.Warn(msg) } - isConfigPublished, err := c.rolesAdd(ctx) + isConfigPublished, err := c.rolesChange(ctx, changeRoleAction) if isConfigPublished { err = errors.Join(err, reloadCConfig(instances)) } @@ -751,7 +752,8 @@ func (c *CConfigApplication) demoteElection(instanceCtx running.InstanceCtx, return } -func (c *CConfigApplication) rolesAdd(ctx RolesChangeCtx) (bool, error) { +func (c *CConfigApplication) rolesChange(ctx RolesChangeCtx, + action RolesChangerAction) (bool, error) { if len(c.runningCtx.Instances) == 0 { return false, fmt.Errorf("there are no running instances") } @@ -781,12 +783,11 @@ func (c *CConfigApplication) rolesAdd(ctx RolesChangeCtx) (bool, error) { return false, err } } - if len(existingRoles) > 0 && slices.Index(existingRoles, ctx.RoleName) != -1 { - return false, fmt.Errorf("role %q already exists in %s", - ctx.RoleName, strings.Join(path.path, "/")) + + existingRoles, err = action.Change(existingRoles, ctx.RoleName) + if err != nil { + return false, fmt.Errorf("failed to change roles: %w", err) } - // If the role does not exist in requested path, append it. - existingRoles = append(existingRoles, ctx.RoleName) pRoleTarget = append(pRoleTarget, patchRoleTarget{ path: path.path, diff --git a/cli/replicaset/cconfig_test.go b/cli/replicaset/cconfig_test.go index ed2780dd8..b4cd01316 100644 --- a/cli/replicaset/cconfig_test.go +++ b/cli/replicaset/cconfig_test.go @@ -18,7 +18,7 @@ var _ replicaset.Demoter = &replicaset.CConfigInstance{} var _ replicaset.Expeller = &replicaset.CConfigInstance{} var _ replicaset.VShardBootstrapper = &replicaset.CConfigInstance{} var _ replicaset.Bootstrapper = &replicaset.CConfigInstance{} -var _ replicaset.RolesAdder = &replicaset.CConfigInstance{} +var _ replicaset.RolesChanger = &replicaset.CConfigInstance{} var _ replicaset.Discoverer = &replicaset.CConfigApplication{} var _ replicaset.Promoter = &replicaset.CConfigApplication{} @@ -26,7 +26,7 @@ var _ replicaset.Demoter = &replicaset.CConfigApplication{} var _ replicaset.Expeller = &replicaset.CConfigApplication{} var _ replicaset.VShardBootstrapper = &replicaset.CConfigApplication{} var _ replicaset.Bootstrapper = &replicaset.CConfigApplication{} -var _ replicaset.RolesAdder = &replicaset.CConfigApplication{} +var _ replicaset.RolesChanger = &replicaset.CConfigApplication{} func TestCconfigApplication_Bootstrap(t *testing.T) { app := replicaset.NewCConfigApplication(running.RunningCtx{}, nil, nil) @@ -469,9 +469,32 @@ func TestCConfigInstance_Expel(t *testing.T) { `expel is not supported for a single instance by "centralized config" orchestrator`) } -func TestCConfigInstance_RolesAdd(t *testing.T) { +func TestCConfigInstance_RolesChange(t *testing.T) { + cases := []struct { + name string + changeAction replicaset.RolesChangerAction + errMsg string + }{ + { + name: "roles add", + changeAction: replicaset.RolesAdder{}, + errMsg: "roles add is not supported for a single instance by" + + ` "centralized config" orchestrator`, + }, + { + name: "roles remove", + changeAction: replicaset.RolesRemover{}, + errMsg: "roles remove is not supported for a single instance by" + + ` "centralized config" orchestrator`, + }, + } + instance := replicaset.NewCConfigInstance(nil) - err := instance.RolesAdd(replicaset.RolesChangeCtx{}) - assert.EqualError(t, err, - `roles add is not supported for a single instance by "centralized config" orchestrator`) + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + err := instance.RolesChange(replicaset.RolesChangeCtx{}, tc.changeAction) + assert.EqualError(t, err, tc.errMsg) + }) + } } diff --git a/cli/replicaset/cmd/common.go b/cli/replicaset/cmd/common.go index a1846de88..5cd0bac9d 100644 --- a/cli/replicaset/cmd/common.go +++ b/cli/replicaset/cmd/common.go @@ -22,7 +22,7 @@ type replicasetOrchestrator interface { replicaset.Expeller replicaset.VShardBootstrapper replicaset.Bootstrapper - replicaset.RolesAdder + replicaset.RolesChanger } // makeApplicationOrchestrator creates an orchestrator for the application. diff --git a/cli/replicaset/cmd/roles.go b/cli/replicaset/cmd/roles.go index e30ccae4a..20c0b0b76 100644 --- a/cli/replicaset/cmd/roles.go +++ b/cli/replicaset/cmd/roles.go @@ -10,9 +10,9 @@ import ( libcluster "github.com/tarantool/tt/lib/cluster" ) -// RolesAddCtx describes the context to add a role to +// RolesChangeCtx describes the context to add/remove role for // provided config scope. -type RolesAddCtx struct { +type RolesChangeCtx struct { // InstName is an instance name in which add or remove role. InstName string // GroupName is a replicaset name in which add or remove role. @@ -42,8 +42,8 @@ type RolesAddCtx struct { Timeout int } -// RolesAdd adds role with provided path target to config. -func RolesAdd(ctx RolesAddCtx) error { +// RolesChange adds/removes role with provided path target to config. +func RolesChange(ctx RolesChangeCtx, changeRoleAction replicaset.RolesChangerAction) error { orchestratorType, err := getInstanceOrchestrator(ctx.Orchestrator, ctx.Conn) if err != nil { return err @@ -79,29 +79,40 @@ func RolesAdd(ctx RolesAddCtx) error { statusReplicasets(replicasets) fmt.Println() + action := []string{"Add", "to"} + if changeRoleAction.Action() == replicaset.RemoveAction { + action = []string{"Remove", "from"} + } + if ctx.IsGlobal { if orchestratorType == replicaset.OrchestratorCartridge { return fmt.Errorf("cannot pass --global (-G) flag due to cluster with cartridge") } else { - log.Infof("Add role %s to global scope", ctx.RoleName) + log.Infof("%s role %s %s global scope") } } - if ctx.GroupName != "" && orchestratorType != replicaset.OrchestratorCartridge { - log.Infof("Add role %s to group: %s", ctx.RoleName, ctx.GroupName) + if ctx.GroupName != "" { + if orchestratorType == replicaset.OrchestratorCartridge && + changeRoleAction.Action() == replicaset.RemoveAction { + return fmt.Errorf("cannot provide vshard-group by removing role") + } + log.Infof("%s role %s %s group: %s", action[0], ctx.RoleName, action[1], ctx.GroupName) } if ctx.InstName != "" { if orchestratorType == replicaset.OrchestratorCartridge { return fmt.Errorf("cannot pass the instance or --instance (-i) flag due to cluster" + - " with cartridge orchestrator can't add role into instance scope") + " with cartridge orchestrator can't add/remove role for instance scope") } else { - log.Infof("Add role %s to instance: %s", ctx.RoleName, ctx.InstName) + log.Infof("%s role %s %s instance: %s", action[0], ctx.RoleName, + action[1], ctx.InstName) } } if ctx.ReplicasetName != "" { - log.Infof("Add role %s to replicaset: %s", ctx.RoleName, ctx.ReplicasetName) + log.Infof("%s role %s %s replicaset: %s", action[0], ctx.RoleName, + action[1], ctx.ReplicasetName) } - err = orchestrator.RolesAdd(replicaset.RolesChangeCtx{ + err = orchestrator.RolesChange(replicaset.RolesChangeCtx{ InstName: ctx.InstName, GroupName: ctx.GroupName, ReplicasetName: ctx.ReplicasetName, @@ -109,7 +120,7 @@ func RolesAdd(ctx RolesAddCtx) error { RoleName: ctx.RoleName, Force: ctx.Force, Timeout: ctx.Timeout, - }) + }, changeRoleAction) if err == nil { log.Info("Done.") } diff --git a/cli/replicaset/configsource.go b/cli/replicaset/configsource.go index 715f57b72..4f4856690 100644 --- a/cli/replicaset/configsource.go +++ b/cli/replicaset/configsource.go @@ -213,9 +213,9 @@ func (c *CConfigSource) Expel(ctx ExpelCtx) error { ) } -// ChangeRole patches a config to add role to a config. -func (c *CConfigSource) ChangeRole(ctx RolesChangeCtx, changeRoleFunc ChangeRoleFunc) error { - return c.patchConfigWithRoles(ctx, getCConfigRolesPath, changeRoleFunc, patchCConfigEditRole) +// ChangeRole patches a config with addition/removing role. +func (c *CConfigSource) ChangeRole(ctx RolesChangeCtx, action RolesChangerAction) error { + return c.patchConfigWithRoles(ctx, getCConfigRolesPath, action.Change, patchCConfigEditRole) } // getCConfigRolesPath returns a path and it's minimum interesting depth diff --git a/cli/replicaset/configsource_test.go b/cli/replicaset/configsource_test.go index f008c49c6..2a73b0679 100644 --- a/cli/replicaset/configsource_test.go +++ b/cli/replicaset/configsource_test.go @@ -120,12 +120,12 @@ func TestCConfigSource_collect_config_error(t *testing.T) { }, { func(source *replicaset.CConfigSource) error { - return source.ChangeRole(replicaset.RolesChangeCtx{}, replicaset.AddRole) + return source.ChangeRole(replicaset.RolesChangeCtx{}, replicaset.RolesAdder{}) }, }, { func(source *replicaset.CConfigSource) error { - return source.ChangeRole(replicaset.RolesChangeCtx{}, replicaset.RemoveRole) + return source.ChangeRole(replicaset.RolesChangeCtx{}, replicaset.RolesRemover{}) }, }, } @@ -281,7 +281,7 @@ func TestCConfigSource_passes_force(t *testing.T) { { func(source *replicaset.CConfigSource) error { return source.ChangeRole(replicaset.RolesChangeCtx{InstName: instName, Force: true}, - replicaset.AddRole) + replicaset.RolesAdder{}) }, }, } @@ -330,7 +330,7 @@ func TestCConfigSource_publish_error(t *testing.T) { { func(source *replicaset.CConfigSource) error { return source.ChangeRole(replicaset.RolesChangeCtx{InstName: instName}, - replicaset.AddRole) + replicaset.RolesAdder{}) }, }, } @@ -379,7 +379,7 @@ func TestCConfigSource_keypick_error(t *testing.T) { { func(source *replicaset.CConfigSource) error { return source.ChangeRole(replicaset.RolesChangeCtx{InstName: instName}, - replicaset.AddRole) + replicaset.RolesAdder{}) }, }, } @@ -421,12 +421,12 @@ func TestCConfigSource_Promote_invalid_config(t *testing.T) { }, { func(source *replicaset.CConfigSource) error { - return source.ChangeRole(replicaset.RolesChangeCtx{}, replicaset.AddRole) + return source.ChangeRole(replicaset.RolesChangeCtx{}, replicaset.RolesAdder{}) }, }, { func(source *replicaset.CConfigSource) error { - return source.ChangeRole(replicaset.RolesChangeCtx{}, replicaset.RemoveRole) + return source.ChangeRole(replicaset.RolesChangeCtx{}, replicaset.RolesRemover{}) }, }, } @@ -936,7 +936,7 @@ roles: publisher := newOnceMockDataPublisher(nil) source := replicaset.NewCConfigSource(collector, publisher, picker) - err := source.ChangeRole(tc.rolesChangeCtx, replicaset.AddRole) + err := source.ChangeRole(tc.rolesChangeCtx, replicaset.RolesAdder{}) if tc.errMsg != "" { require.EqualError(t, err, tc.errMsg) } else { @@ -1122,7 +1122,7 @@ roles: [] publisher := newOnceMockDataPublisher(nil) source := replicaset.NewCConfigSource(collector, publisher, picker) - err := source.ChangeRole(tc.rolesChangeCtx, replicaset.RemoveRole) + err := source.ChangeRole(tc.rolesChangeCtx, replicaset.RolesRemover{}) if tc.errMsg != "" { require.EqualError(t, err, tc.errMsg) } else { diff --git a/cli/replicaset/custom.go b/cli/replicaset/custom.go index e22c7f76e..5192f9bda 100644 --- a/cli/replicaset/custom.go +++ b/cli/replicaset/custom.go @@ -93,9 +93,9 @@ func (c *CustomInstance) Bootstrap(BootstrapCtx) error { return newErrBootstrapByInstanceNotSupported(OrchestratorCustom) } -// RolesAdd is not supported for a single instance by the Custom orchestrator. -func (c *CustomInstance) RolesAdd(RolesChangeCtx) error { - return newErrRolesAddByInstanceNotSupported(OrchestratorCustom) +// RolesChange is not supported for a single instance by the Custom orchestrator. +func (c *CustomInstance) RolesChange(_ RolesChangeCtx, action RolesChangerAction) error { + return newErrRolesChangeByInstanceNotSupported(OrchestratorCustom, action) } // CustomApplication is an application with a custom orchestrator. @@ -167,9 +167,10 @@ func (c *CustomApplication) Bootstrap(BootstrapCtx) error { return newErrBootstrapByAppNotSupported(OrchestratorCustom) } -// RolesAdd is not supported for an application by the Custom orchestrator. -func (c *CustomApplication) RolesAdd(RolesChangeCtx) error { - return newErrRolesAddByAppNotSupported(OrchestratorCustom) +// RolesChange is not supported for an application by the Custom orchestrator. +func (c *CustomApplication) RolesChange(_ RolesChangeCtx, + action RolesChangerAction) error { + return newErrRolesChangeByAppNotSupported(OrchestratorCustom, action) } // getCustomInstanceTopology returns a topology for an instance. diff --git a/cli/replicaset/custom_test.go b/cli/replicaset/custom_test.go index 7e962d109..7607f6cd0 100644 --- a/cli/replicaset/custom_test.go +++ b/cli/replicaset/custom_test.go @@ -17,7 +17,7 @@ var _ replicaset.Demoter = &replicaset.CustomInstance{} var _ replicaset.Expeller = &replicaset.CustomInstance{} var _ replicaset.VShardBootstrapper = &replicaset.CustomInstance{} var _ replicaset.Bootstrapper = &replicaset.CustomInstance{} -var _ replicaset.RolesAdder = &replicaset.CustomInstance{} +var _ replicaset.RolesChanger = &replicaset.CustomInstance{} var _ replicaset.Discoverer = &replicaset.CustomApplication{} var _ replicaset.Promoter = &replicaset.CustomApplication{} @@ -25,7 +25,7 @@ var _ replicaset.Demoter = &replicaset.CustomApplication{} var _ replicaset.Expeller = &replicaset.CustomApplication{} var _ replicaset.VShardBootstrapper = &replicaset.CustomApplication{} var _ replicaset.Bootstrapper = &replicaset.CustomApplication{} -var _ replicaset.RolesAdder = &replicaset.CustomApplication{} +var _ replicaset.RolesChanger = &replicaset.CustomApplication{} func TestCustomApplication_Promote(t *testing.T) { app := replicaset.NewCustomApplication(running.RunningCtx{}) @@ -63,10 +63,32 @@ func TestCustomApplication_Bootstrap(t *testing.T) { } func TestCustomApplication_RolesAdd(t *testing.T) { + cases := []struct { + name string + changeAction replicaset.RolesChangerAction + errMsg string + }{ + { + name: "roles add", + changeAction: replicaset.RolesAdder{}, + errMsg: `roles add is not supported for an application by "custom" orchestrator`, + }, + { + name: "roles remove", + changeAction: replicaset.RolesRemover{}, + errMsg: `roles remove is not supported for an application by "custom"` + + " orchestrator", + }, + } + instance := replicaset.NewCustomApplication(running.RunningCtx{}) - err := instance.RolesAdd(replicaset.RolesChangeCtx{}) - assert.EqualError(t, err, - `roles add is not supported for an application by "custom" orchestrator`) + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + err := instance.RolesChange(replicaset.RolesChangeCtx{}, tc.changeAction) + assert.EqualError(t, err, tc.errMsg) + }) + } } func TestCustomInstance_Discovery(t *testing.T) { @@ -452,9 +474,32 @@ func TestCustomInstance_Bootstrap(t *testing.T) { `bootstrap is not supported for a single instance by "custom" orchestrator`) } -func TestCustomInstance_RolesAdd(t *testing.T) { +func TestCustomInstance_RolesChange(t *testing.T) { + cases := []struct { + name string + changeAction replicaset.RolesChangerAction + errMsg string + }{ + { + name: "roles add", + changeAction: replicaset.RolesAdder{}, + errMsg: `roles add is not supported for a single instance by "custom"` + + " orchestrator", + }, + { + name: "roles remove", + changeAction: replicaset.RolesRemover{}, + errMsg: `roles remove is not supported for a single instance by "custom"` + + " orchestrator", + }, + } + instance := replicaset.NewCustomInstance(nil) - err := instance.RolesAdd(replicaset.RolesChangeCtx{}) - assert.EqualError(t, err, - `roles add is not supported for a single instance by "custom" orchestrator`) + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + err := instance.RolesChange(replicaset.RolesChangeCtx{}, tc.changeAction) + assert.EqualError(t, err, tc.errMsg) + }) + } } diff --git a/cli/replicaset/roles.go b/cli/replicaset/roles.go index 8674ee3a9..9674f1412 100644 --- a/cli/replicaset/roles.go +++ b/cli/replicaset/roles.go @@ -5,19 +5,45 @@ import ( "slices" ) -// ChangeRoleFunc is a function type for addition or removing a role. -type ChangeRoleFunc func([]string, string) ([]string, error) +// RoleAction is a type that describes an action that will +// happen with Roles. +// It can be "add" action which corresponds 0 or "remove" which is 1. +type RoleAction uint -// AddRole is a function that implements addition of role. -func AddRole(roles []string, r string) ([]string, error) { +const ( + AddAction RoleAction = iota + RemoveAction +) + +// RolesChangerAction implements changing (add or remove) of roles. +type RolesChangerAction interface { + // ChangeRoleFunc is a function type for addition or removing a role. + Change([]string, string) ([]string, error) + // Action is a method that returns RoleAction. + Action() RoleAction +} + +// RolesAdder is a struct that implements addition of role. +type RolesAdder struct{} + +// Change implements addition of role. +func (RolesAdder) Change(roles []string, r string) ([]string, error) { if len(roles) > 0 && slices.Index(roles, r) != -1 { return []string{}, fmt.Errorf("role %q already exists", r) } return append(roles, r), nil } -// RemoveRole is a function that implements removing of role. -func RemoveRole(roles []string, r string) ([]string, error) { +// Action implements getter of add action. +func (RolesAdder) Action() RoleAction { + return AddAction +} + +// RolesRemover is a struct that implements removing of role. +type RolesRemover struct{} + +// Change implements removing of role. +func (RolesRemover) Change(roles []string, r string) ([]string, error) { idx := slices.Index(roles, r) if idx == -1 { return []string{}, fmt.Errorf("role %q not found", r) @@ -28,6 +54,11 @@ func RemoveRole(roles []string, r string) ([]string, error) { return append(roles[:idx], roles[idx+1:]...), nil } +// Action implements getter of remove action. +func (RolesRemover) Action() RoleAction { + return RemoveAction +} + // RolesChangeCtx describes a context for adding/removing roles. type RolesChangeCtx struct { // InstName is an instance name in which add/remove role. @@ -48,24 +79,32 @@ type RolesChangeCtx struct { Timeout int } -// RolesAdder is an interface for adding roles to a replicaset. -type RolesAdder interface { - // RolesAdd adds role to a replicasets by its name. - RolesAdd(ctx RolesChangeCtx) error +// RolesChanger is an interface for adding/removing roles for a replicaset. +type RolesChanger interface { + // RolesChange adds/removes role for a replicasets by its name. + RolesChange(ctx RolesChangeCtx, action RolesChangerAction) error } -// newErrRolesAddByInstanceNotSupported creates a new error that 'roles add' is not +// newErrRolesChangeByInstanceNotSupported creates a new error that 'roles add/remove' is not // supported by the orchestrator for a single instance. -func newErrRolesAddByInstanceNotSupported(orchestrator Orchestrator) error { - return fmt.Errorf("roles add is not supported for a single instance by %q orchestrator", - orchestrator) +func newErrRolesChangeByInstanceNotSupported(orchestrator Orchestrator, + changeRoleAction RolesChangerAction) error { + msg := "roles %s is not supported for a single instance by %q orchestrator" + if changeRoleAction.Action() == RemoveAction { + return fmt.Errorf(msg, "remove", orchestrator) + } + return fmt.Errorf(msg, "add", orchestrator) } -// newErrRolesAddByAppNotSupported creates a new error that 'roles add' by URI is not +// newErrRolesChangeByAppNotSupported creates a new error that 'roles add/remove' by URI is not // supported by the orchestrator for an application. -func newErrRolesAddByAppNotSupported(orchestrator Orchestrator) error { - return fmt.Errorf("roles add is not supported for an application by %q orchestrator", - orchestrator) +func newErrRolesChangeByAppNotSupported(orchestrator Orchestrator, + changeRoleAction RolesChangerAction) error { + msg := "roles %s is not supported for an application by %q orchestrator" + if changeRoleAction.Action() == RemoveAction { + return fmt.Errorf(msg, "remove", orchestrator) + } + return fmt.Errorf(msg, "add", orchestrator) } // parseRoles is a function to convert roles type 'any' diff --git a/cli/replicaset/roles_test.go b/cli/replicaset/roles_test.go index b822258e2..f141855fd 100644 --- a/cli/replicaset/roles_test.go +++ b/cli/replicaset/roles_test.go @@ -21,7 +21,11 @@ func TestRoles_AddRole(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - res, err := replicaset.AddRole(tc.roles, tc.roleToAdd) + adder := replicaset.RolesAdder{} + + require.Equal(t, adder.Action(), replicaset.AddAction) + + res, err := adder.Change(tc.roles, tc.roleToAdd) if tc.errMsg != "" { require.EqualError(t, err, tc.errMsg) } else { @@ -47,7 +51,11 @@ func TestRoles_RemoveRole(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - res, err := replicaset.RemoveRole(tc.roles, tc.roleToRemove) + remover := replicaset.RolesRemover{} + + require.Equal(t, remover.Action(), replicaset.RemoveAction) + + res, err := remover.Change(tc.roles, tc.roleToRemove) if tc.errMsg != "" { require.EqualError(t, err, tc.errMsg) } else { diff --git a/cli/running/running.go b/cli/running/running.go index 94bc44f47..dd453cefb 100644 --- a/cli/running/running.go +++ b/cli/running/running.go @@ -680,7 +680,7 @@ func FillCtx(cliOpts *config.CliOpts, cmdCtx *cmdcontext.CmdCtx, var err error if len(args) > 1 && cmdCtx.CommandName != "run" && cmdCtx.CommandName != "connect" && - cmdCtx.CommandName != "add" { + cmdCtx.CommandName != "add" && cmdCtx.CommandName != "remove" { return util.NewArgError("currently, you can specify only one instance at a time") } diff --git a/test/cartridge_helper.py b/test/cartridge_helper.py index 7d9b79ae8..ef79a64a9 100644 --- a/test/cartridge_helper.py +++ b/test/cartridge_helper.py @@ -16,11 +16,12 @@ instances = ["router", "s1-master", "s1-replica", "s2-master", "s2-replica-1", "s2-replica-2", - "stateboard"] + "stateboard", + "s3-master"] def get_instances_cfg(): - ports = find_ports(13) + ports = find_ports(15) cfg = { f"{cartridge_name}.router": { "advertise_uri": f"localhost:{ports[0]}", @@ -50,6 +51,10 @@ def get_instances_cfg(): "listen": f"localhost:{ports[12]}", "password": "passwd", }, + f"{cartridge_name}.s3-master": { + "advertise_uri": f"localhost:{ports[13]}", + "http_port": ports[14], + }, } return cfg @@ -74,6 +79,13 @@ def get_instances_cfg(): "all_rw": False, "vshard_group": "default" }, + "s-3": { + "instances": ["s3-master"], + "roles": ["app.roles.custom"], + "weight": 1, + "all_rw": False, + "vshard_group": "default" + } } diff --git a/test/integration/replicaset/test_replicaset_bootstrap.py b/test/integration/replicaset/test_replicaset_bootstrap.py index d0bead855..8590ac2ae 100644 --- a/test/integration/replicaset/test_replicaset_bootstrap.py +++ b/test/integration/replicaset/test_replicaset_bootstrap.py @@ -153,6 +153,13 @@ def test_replicaset_bootstrap_cartridge_app_second_bootstrap(tt_cmd, cartridge_a "all_rw": False, "vshard_group": "default" }, + "s-3": { + "instances": ["s3-master"], + "roles": ["app.roles.custom"], + "weight": 1, + "all_rw": False, + "vshard_group": "default" + } } with open(os.path.join(cartridge_app.workdir, cartridge_name, "replicasets.yml"), "w") as f: f.write(yaml.dump(replicasets_cfg)) diff --git a/test/integration/replicaset/test_replicaset_roles_add.py b/test/integration/replicaset/test_replicaset_roles_add.py index 6719bb6ba..11f53e641 100644 --- a/test/integration/replicaset/test_replicaset_roles_add.py +++ b/test/integration/replicaset/test_replicaset_roles_add.py @@ -125,8 +125,7 @@ def test_roles_add_missing_args(tt_cmd, tmpdir_with_cfg, args, err_msg): inst="instance-001", role_name="greeter", is_add_role=True, - err_msg="role \"greeter\" already exists in groups/group-001/replicasets/" + - "replicaset-001/instances/instance-001/roles" + err_msg="role \"greeter\" already exists" ), make_test_roles_add_param( False, @@ -239,7 +238,7 @@ def test_replicaset_cconfig_roles_add( role_name="failover-coordinator", inst_flg="s1-master", err_msg=("cannot pass the instance or --instance (-i) flag due to cluster" - " with cartridge orchestrator can't add role into instance scope") + " with cartridge orchestrator can't add/remove role for instance scope") ), make_test_roles_add_param( True, @@ -259,7 +258,7 @@ def test_replicaset_cconfig_roles_add( True, role_name="vshard-storage", rs="s-1", - err_msg="role \"vshard-storage\" already exists in replicaset \"s-1\"", + err_msg="failed to change role: role \"vshard-storage\" already exists", ), make_test_roles_add_param( True, @@ -329,6 +328,8 @@ def test_replicaset_cartridge_roles_add(tt_cmd, buf.readline() # Skip init status in the output. parse_status(buf) + if group: + assert f"Add role {role_name} to group: {group}" in buf.readline() assert f"Add role {role_name} to replicaset: {rs}" in buf.readline() assert f"Replicaset {rs} now has these roles enabled:" in buf.readline() assert role_name if group == "" else f"{role_name} ({group})" in buf.readline() diff --git a/test/integration/replicaset/test_replicaset_roles_remove.py b/test/integration/replicaset/test_replicaset_roles_remove.py new file mode 100644 index 000000000..07b88430e --- /dev/null +++ b/test/integration/replicaset/test_replicaset_roles_remove.py @@ -0,0 +1,394 @@ +import io +import os +import shutil + +import pytest +from cartridge_helper import (cartridge_name, cartridge_password, + cartridge_username) +from integration.replicaset.replicaset_helpers import ( + get_group_by_replicaset_name, get_group_replicaset_by_instance_name, + parse_status, parse_yml, start_application, stop_application) + +from utils import get_tarantool_version, read_kv, run_command_and_get_output + +tarantool_major_version, tarantool_minor_version = get_tarantool_version() + + +TEST_ROLES_REMOVE_PARAMS_CCONFIG = ("role_name, inst, inst_flg, group, rs, is_uri, is_global," + " err_msg, stop_instance, is_force, patch_cfg_content") +TEST_ROLES_REMOVE_PARAMS_CARTRIDGE = ("role_name, inst_flg, group, rs, is_uri, is_global," + " is_custom, is_empty, err_msg") + + +def make_test_roles_remove_param( + is_cartridge_orchestrator, + role_name, + inst=None, + inst_flg=None, + group=None, + rs=None, + is_global=False, + err_msg="", + stop_instance=None, + is_uri=False, + is_force=False, + is_custom=False, + is_empty=True, + patch_cfg_content=None +): + if is_cartridge_orchestrator: + return pytest.param(role_name, inst_flg, group, rs, is_uri, is_global, is_custom, is_empty, + err_msg) + return pytest.param(role_name, inst, inst_flg, group, rs, is_uri, is_global, err_msg, + stop_instance, is_force, patch_cfg_content) + + +@pytest.mark.parametrize("args, err_msg", [ + pytest.param(["some_role"], "Error: accepts 2 arg(s), received 1"), + pytest.param(["some_app", "some_role"], "can't collect instance information for some_app"), +]) +def test_roles_add_missing_args(tt_cmd, tmpdir_with_cfg, args, err_msg): + cmd = [tt_cmd, "rs", "roles", "remove"] + cmd.extend(args) + rc, out = run_command_and_get_output(cmd, cwd=tmpdir_with_cfg) + assert rc != 0 + assert err_msg in out + + +@pytest.mark.skipif(tarantool_major_version < 3, + reason="skip centralized config test for Tarantool < 3") +@pytest.mark.parametrize(TEST_ROLES_REMOVE_PARAMS_CCONFIG, [ + make_test_roles_remove_param( + False, + inst="instance-005", + role_name="greeter", + patch_cfg_content="""\ + roles: + - greeter +""" + ), + make_test_roles_remove_param( + False, + is_global=True, + role_name="greeter", + patch_cfg_content="""\ +roles: + - greeter +""" + ), + make_test_roles_remove_param( + False, + group="group-002", + role_name="greeter", + patch_cfg_content="""\ + roles: + - greeter +""" + ), + make_test_roles_remove_param( + False, + rs="replicaset-002", + role_name="greeter", + patch_cfg_content="""\ + roles: + - greeter +""" + ), + make_test_roles_remove_param( + False, + inst_flg="instance-005", + role_name="greeter", + patch_cfg_content="""\ + roles: + - greeter +""" + ), + make_test_roles_remove_param( + False, + is_global=True, + group="group-002", + rs="replicaset-002", + inst_flg="instance-005", + role_name="greeter", + patch_cfg_content="""\ + roles: + - greeter + roles: + - greeter + roles: + - greeter +roles: + - greeter +""" + ), + make_test_roles_remove_param( + False, + is_global=True, + inst="instance-001", + inst_flg="instance-002", + role_name="greeter", + err_msg="there are different instance names passed after app name and in flag arg", + ), + make_test_roles_remove_param( + False, + role_name="greeter", + err_msg="there is no destination provided where to remove role", + ), + make_test_roles_remove_param( + False, + inst="instance-005", + role_name="greeter", + stop_instance="instance-001", + is_force=True, + patch_cfg_content="""\ + roles: + - greeter +""" + ), + make_test_roles_remove_param( + False, + inst="instance-002", + role_name="greeter", + rs="replicaset-001", + stop_instance="instance-001", + err_msg="all instances in the target replicaset should be online," + + " could not connect to: instance-001", + ), + make_test_roles_remove_param( + False, + inst="instance-001", + role_name="greeter", + err_msg="role \"greeter\" not found" + ), + make_test_roles_remove_param( + False, + role_name="greeter", + is_uri=True, + err_msg="roles remove is not supported for a single instance by" + + " \"centralized config\" orchestrator", + ), +]) +def test_replicaset_cconfig_roles_remove( + role_name, + inst, + inst_flg, + group, + rs, + is_global, + err_msg, + stop_instance, + tt_cmd, + tmpdir_with_cfg, + is_uri, + is_force, + patch_cfg_content, +): + app_name = "test_ccluster_app" + app_path = os.path.join(tmpdir_with_cfg, app_name) + shutil.copytree(os.path.join(os.path.dirname(__file__), app_name), app_path) + + if patch_cfg_content: + with open(os.path.join(app_path, "config.yaml"), 'a') as f: + f.write(patch_cfg_content) + + kv = read_kv(app_path) + instances = parse_yml(kv["instances"]).keys() + + try: + start_application(tt_cmd, tmpdir_with_cfg, app_name, instances) + + if stop_instance: + stop_cmd = [tt_cmd, "stop", f"{app_name}:{stop_instance}"] + rc, _ = run_command_and_get_output(stop_cmd, cwd=tmpdir_with_cfg) + assert rc == 0 + + flags = [] + if is_force: + flags.extend(["-f"]) + if is_global: + flags.extend(["-G"]) + if group: + flags.extend(["-g", group]) + if rs: + flags.extend(["-r", rs]) + if inst_flg: + flags.extend(["-i", inst_flg]) + + uri = None + if is_uri: + uri = f"client:secret@{tmpdir_with_cfg}/{app_name}/{list(instances)[0]}.iproto" + + roles_add_cmd = [tt_cmd, "rs", "roles", "remove", + (f"{app_name}:{inst}" if inst else app_name + if not is_uri else uri), role_name] + if len(flags) != 0: + roles_add_cmd.extend(flags) + rc, out = run_command_and_get_output(roles_add_cmd, cwd=tmpdir_with_cfg) + if err_msg == "": + assert rc == 0 + kv = read_kv(app_path) + cluster_cfg = parse_yml(kv["config"]) + if is_global: + assert "Remove role from global scope" + assert role_name not in cluster_cfg["roles"] + if group: + assert f"Remove role from group: {group}" + assert role_name not in cluster_cfg["groups"][group]["roles"] + if rs: + assert f"Remove role from replicaset: {rs}" + gr = get_group_by_replicaset_name(cluster_cfg, rs) + assert role_name not in cluster_cfg["groups"][gr]["replicasets"][rs]["roles"] + if inst_flg or inst: + i = inst if inst else inst_flg + assert f"Remove role from instance: {i}" + g, r = get_group_replicaset_by_instance_name(cluster_cfg, i) + assert (role_name not in + cluster_cfg["groups"][g]["replicasets"][r]["instances"][i]["roles"]) + else: + assert rc == 1 + assert err_msg in out + finally: + stop_application(tt_cmd, + app_name, + tmpdir_with_cfg, instances, + force=True if stop_instance else False) + + +@pytest.mark.skipif(tarantool_major_version >= 3, + reason="skip cartridge tests for Tarantool 3.0") +@pytest.mark.parametrize(TEST_ROLES_REMOVE_PARAMS_CARTRIDGE, [ + make_test_roles_remove_param( + True, + rs="router", + role_name="app.roles.custom", + is_empty=False, + ), + make_test_roles_remove_param( + True, + rs="s-3", + role_name="app.roles.custom", + ), + make_test_roles_remove_param( + True, + rs="s-3", + role_name="app.roles.custom", + group="g", + err_msg="cannot provide vshard-group by removing role", + ), + make_test_roles_remove_param( + True, + rs="s-1", + role_name="failover-coordinator", + inst_flg="s1-master", + err_msg=("cannot pass the instance or --instance (-i) flag due to cluster" + " with cartridge orchestrator can't add/remove role for instance scope") + ), + make_test_roles_remove_param( + True, + rs="s-1", + group="default", + role_name="failover-coordinator", + is_global=True, + err_msg="cannot pass --global (-G) flag due to cluster with cartridge" + ), + make_test_roles_remove_param( + True, + inst_flg="s-1.master", + role_name="failover-coordinator", + err_msg="in cartridge replicaset name must be specified via --replicaset flag" + ), + make_test_roles_remove_param( + True, + role_name="my_role", + rs="s-1", + err_msg="failed to change role: role \"my_role\" not found", + ), + make_test_roles_remove_param( + True, + rs="unknown", + role_name="some_role", + err_msg="failed to find replicaset \"unknown\"" + ), + make_test_roles_remove_param( + True, + rs="r", + role_name="role", + is_custom=True, + err_msg="roles remove is not supported for an application by \"custom\" orchestrator", + ), + make_test_roles_remove_param( + True, + rs="r", + role_name="role", + is_custom=True, + err_msg="roles remove is not supported for an application by \"custom\" orchestrator", + ), + make_test_roles_remove_param( + True, + rs="r", + role_name="role", + is_uri=True, + err_msg="roles remove is not supported for a single instance by \"cartridge\" orchestrator", + ), +]) +def test_replicaset_cartridge_roles_remove(tt_cmd, + cartridge_app, + role_name, + rs, + inst_flg, + group, + is_uri, + is_global, + is_custom, + is_empty, + err_msg): + flags = [] + if is_global: + flags.extend(["-G"]) + if group: + flags.extend(["-g", group]) + if rs: + flags.extend(["-r", rs]) + if inst_flg: + flags.extend(["-i", inst_flg]) + + uri = None + if is_uri: + uri = cartridge_app.instances_cfg[f"{cartridge_name}.s1-master"]["advertise_uri"] + uri = f"{cartridge_username}:{cartridge_password}@{uri}" + + roles_add_cmd = [tt_cmd, "rs", "roles", "remove"] + if is_custom: + roles_add_cmd.append("--custom") + roles_add_cmd.extend([cartridge_name if not is_uri else uri, role_name]) + roles_add_cmd.extend(flags) + rc, out = run_command_and_get_output(roles_add_cmd, cwd=cartridge_app.workdir) + + if err_msg == "": + assert rc == 0 + + buf = io.StringIO(out) + assert "• Discovery application..." in buf.readline() + buf.readline() + # Skip init status in the output. + parse_status(buf) + assert f"Remove role {role_name} from replicaset: {rs}" in buf.readline() + if is_empty: + assert f"Now replicaset {rs} has no roles enabled" in buf.readline() + else: + assert f"Replicaset {rs} now has these roles enabled:" in buf.readline() + assert "failover-coordinator" in buf.readline() + assert "vshard-router" in buf.readline() + assert "Done." in buf.readline() + + status_cmd = [tt_cmd, "rs", "status", cartridge_name] + rc, out = run_command_and_get_output(status_cmd, cwd=cartridge_app.workdir) + assert rc == 0 + + # Check status if there are roles left. + if not is_empty: + actual_roles = parse_status(io.StringIO(out))["replicasets"][rs]["roles"] + assert role_name not in actual_roles + else: + assert rc == 1 + assert err_msg in out