Skip to content

Commit

Permalink
bugfix: specifying confusing instance names
Browse files Browse the repository at this point in the history
Added instance name support for cases:
* Application name as instance name
* Instance name with dots

Closes #763
  • Loading branch information
GRISHNOV committed Apr 20, 2023
1 parent 6b860f7 commit b4411f6
Show file tree
Hide file tree
Showing 9 changed files with 76 additions and 46 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.

- Set `compat.fiber_slice_default` to `new` by default in application template.

### Fixed

- Fixed support for instance names as application name and names with dots.

## [2.12.4] - 2022-12-31

### Changed
Expand Down
30 changes: 12 additions & 18 deletions cli/common/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -377,21 +377,11 @@ func InsertInStringSlice(s []string, i int, elem string) []string {
return res
}

func GetInstancesFromArgs(args []string, projectName string) ([]string, error) {
func GetInstancesFromArgs(args []string) ([]string, error) {
foundInstances := make(map[string]struct{})
var instances []string

for _, instanceName := range args {
if instanceName == projectName {
return nil, fmt.Errorf(appNameSpecifiedError)
}

parts := strings.SplitN(instanceName, ".", 2)

if len(parts) > 1 {
return nil, fmt.Errorf(instanceIDSpecified)
}

if instanceName == "stateboard" {
return nil, fmt.Errorf("Please, specify flag --stateboard for processing stateboard instance")
}
Expand All @@ -409,6 +399,17 @@ func GetInstancesFromArgs(args []string, projectName string) ([]string, error) {
return instances, nil
}

func CheckInstanceNameTypoWithProjectName(instanceName string, projectName string, errDesc string) error {
instanceNameParts := strings.SplitN(instanceName, ".", 2)
if instanceNameParts[0] == projectName {
return fmt.Errorf(errDesc+
"You may have specified an unnecessary prefix as the application name, "+
"perhaps you meant %s?", instanceNameParts[1])
}

return nil
}

func GetStringSlicesDifference(s1, s2 []string) []string {
uniqueStrings := arrayOperations.DifferenceString(s1, s2)
return arrayOperations.IntersectString(s1, uniqueStrings)
Expand Down Expand Up @@ -579,10 +580,3 @@ func ParseDependencies(rawDeps []string) (PackDependencies, error) {
func ContainsUpperSymbols(src string) bool {
return strings.ToLower(src) != src
}

const (
appNameSpecifiedError = "Application name is specified. " +
"Please, specify instance name(s)"
instanceIDSpecified = `[APP_NAME].INSTANCE_NAME is specified. ` +
"Please, specify instance name(s)"
)
49 changes: 25 additions & 24 deletions cli/common/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,40 +114,41 @@ func TestGetInstancesFromArgs(t *testing.T) {
var args []string
var instances []string

projectName := "myapp"

// wrong format
args = []string{"myapp.instance-1", "myapp.instance-2"}
_, err = GetInstancesFromArgs(args, projectName)
assert.EqualError(err, instanceIDSpecified)

args = []string{"instance-1", "myapp.instance-2"}
_, err = GetInstancesFromArgs(args, projectName)
assert.EqualError(err, instanceIDSpecified)

args = []string{"myapp"}
_, err = GetInstancesFromArgs(args, projectName)
assert.True(strings.Contains(err.Error(), appNameSpecifiedError))

// duplicate instance name
args = []string{"instance-1", "instance-1"}
_, err = GetInstancesFromArgs(args, projectName)
_, err = GetInstancesFromArgs(args)
assert.True(strings.Contains(err.Error(), "Duplicate instance name specified: instance-1"))

// instances are specified
args = []string{"instance-1", "instance-2"}
instances, err = GetInstancesFromArgs(args, projectName)
instances, err = GetInstancesFromArgs(args)
assert.Nil(err)
assert.Equal([]string{"instance-1", "instance-2"}, instances)
}

// specified both app name and instance name
args = []string{"instance-1", "myapp"}
instances, err = GetInstancesFromArgs(args, projectName)
assert.EqualError(err, appNameSpecifiedError)
func TestCheckInstanceNameTypoWithProjectName(t *testing.T) {
t.Parallel()

assert := assert.New(t)

args = []string{"myapp", "instance-1"}
instances, err = GetInstancesFromArgs(args, projectName)
assert.EqualError(err, appNameSpecifiedError)
var err error
var instanceName string
var projectName string
var errDesc string

instanceName = "mytestapp.mytestinstance"
projectName = "mytestapp"
errDesc = fmt.Sprintf("Instance %s is not running: ", instanceName)
err = CheckInstanceNameTypoWithProjectName(instanceName, projectName, errDesc)
assert.Equal("Instance mytestapp.mytestinstance is not running: "+
"You may have specified an unnecessary prefix as the application name, "+
"perhaps you meant mytestinstance?", err.Error())

instanceName = "mytestinstance"
projectName = "mytestapp"
errDesc = fmt.Sprintf("Instance %s is not running: ", instanceName)
err = CheckInstanceNameTypoWithProjectName(instanceName, projectName, errDesc)
assert.Equal(nil, err)
}

func TestCorrectDependencyParsing(t *testing.T) {
Expand Down
6 changes: 5 additions & 1 deletion cli/connect/connect.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ func Enter(ctx *context.Ctx, args []string) error {
return err
}

if ctx.Running.Instances, err = common.GetInstancesFromArgs(args, ctx.Project.Name); err != nil {
if ctx.Running.Instances, err = common.GetInstancesFromArgs(args); err != nil {
return err
}

Expand All @@ -33,6 +33,10 @@ func Enter(ctx *context.Ctx, args []string) error {

process := running.NewInstanceProcess(ctx, instanceName)
if !process.IsRunning() {
if err := common.CheckInstanceNameTypoWithProjectName(instanceName, ctx.Project.Name,
fmt.Sprintf("Instance %s is not running: ", instanceName)); err != nil {
return err
}
return fmt.Errorf("Instance %s is not running", instanceName)
}

Expand Down
5 changes: 5 additions & 0 deletions cli/replicasets/expel.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (

"github.com/apex/log"
"github.com/tarantool/cartridge-cli/cli/cluster"
"github.com/tarantool/cartridge-cli/cli/common"
"github.com/tarantool/cartridge-cli/cli/context"
"github.com/tarantool/cartridge-cli/cli/project"
)
Expand Down Expand Up @@ -64,6 +65,10 @@ func Expel(ctx *context.Ctx, args []string) error {

for instanceName, instanceFound := range instancesToExpelMap {
if !instanceFound {
if err := common.CheckInstanceNameTypoWithProjectName(instanceName, ctx.Project.Name,
fmt.Sprintf("Instance %s isn't found in cluster: ", instanceName)); err != nil {
return err
}
return fmt.Errorf("Instance %s isn't found in cluster", instanceName)
}
}
Expand Down
8 changes: 7 additions & 1 deletion cli/replicasets/failover_priority.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ func SetFailoverPriority(ctx *context.Ctx, args []string) error {
return err
}

if ctx.Replicasets.FailoverPriorityNames, err = common.GetInstancesFromArgs(args, ctx.Project.Name); err != nil {
if ctx.Replicasets.FailoverPriorityNames, err = common.GetInstancesFromArgs(args); err != nil {
return err
}

Expand All @@ -43,6 +43,12 @@ func SetFailoverPriority(ctx *context.Ctx, args []string) error {
ctx.Replicasets.FailoverPriorityNames, topologyReplicaset,
)
if err != nil {
for _, instanceName := range ctx.Replicasets.FailoverPriorityNames {
if err := common.CheckInstanceNameTypoWithProjectName(instanceName, ctx.Project.Name,
fmt.Sprintf("Failed to get edit_topology options for setting failover priority: %s: ", err)); err != nil {
return err
}
}
return fmt.Errorf("Failed to get edit_topology options for setting failover priority: %s", err)
}

Expand Down
8 changes: 7 additions & 1 deletion cli/replicasets/join.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ func Join(ctx *context.Ctx, args []string) error {
return err
}

if ctx.Replicasets.JoinInstancesNames, err = common.GetInstancesFromArgs(args, ctx.Project.Name); err != nil {
if ctx.Replicasets.JoinInstancesNames, err = common.GetInstancesFromArgs(args); err != nil {
return err
}

Expand Down Expand Up @@ -57,6 +57,12 @@ func Join(ctx *context.Ctx, args []string) error {
topologyReplicasets, instancesConf,
)
if err != nil {
for _, instanceName := range ctx.Replicasets.JoinInstancesNames {
if err := common.CheckInstanceNameTypoWithProjectName(instanceName, ctx.Project.Name,
fmt.Sprintf("Failed to get edit_topology options for joining instances: %s: ", instanceName)); err != nil {
return err
}
}
return fmt.Errorf("Failed to get edit_topology options for joining instances: %s", err)
}

Expand Down
10 changes: 10 additions & 0 deletions cli/running/process.go
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,16 @@ func NewStateboardProcess(ctx *context.Ctx) *Process {

func (process *Process) Log(follow bool, n int) error {
if _, err := os.Stat(process.logFile); err != nil {
if os.IsNotExist(err) {
logFilePathParts := strings.SplitN(process.logFile, "/", -1)
logFileName := logFilePathParts[len(logFilePathParts)-1]
logFilePathParts[len(logFilePathParts)-1] = strings.SplitN(logFileName, ".", 2)[1]
possibleLogFilePath := strings.Join(logFilePathParts, "/")
if _, err := os.Stat(possibleLogFilePath); err == nil {
return fmt.Errorf("Failed to use process log file: " +
"Try to delete unnecessary application name prefix from INSTANCE_NAME arg")
}
}
return fmt.Errorf("Failed to use process log file: %s", err)
}

Expand Down
2 changes: 1 addition & 1 deletion cli/running/running.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func FillCtx(ctx *context.Ctx, args []string) error {
ctx.Running.WithStateboard = true
}

if ctx.Running.Instances, err = common.GetInstancesFromArgs(args, ctx.Project.Name); err != nil {
if ctx.Running.Instances, err = common.GetInstancesFromArgs(args); err != nil {
return err
}

Expand Down

0 comments on commit b4411f6

Please sign in to comment.