From 8e24b64791468c0fb00346f3f0b76d3f520c9499 Mon Sep 17 00:00:00 2001 From: Mikhail Elhimov Date: Thu, 10 Oct 2024 13:15:19 +0300 Subject: [PATCH] reduce requirements for stop/kill/clean/logrotate For multi-instance applications no need to have instance scripts. For tarantool3-based applications no need to have cluster config. Closes #880 --- cli/cmd/build.go | 2 +- cli/cmd/check.go | 2 +- cli/cmd/clean.go | 2 +- cli/cmd/cluster.go | 2 +- cli/cmd/connect.go | 2 +- cli/cmd/internal/completion.go | 2 +- cli/cmd/kill.go | 2 +- cli/cmd/log.go | 2 +- cli/cmd/logrotate.go | 2 +- cli/cmd/replicaset.go | 4 +- cli/cmd/start.go | 2 +- cli/cmd/status.go | 2 +- cli/cmd/stop.go | 2 +- cli/instances/list.go | 2 +- cli/pack/opts.go | 2 +- cli/replicaset/rebootstrap.go | 2 +- cli/running/running.go | 70 +++++++++++++++++----------------- cli/running/running_test.go | 24 ++++++------ 18 files changed, 65 insertions(+), 63 deletions(-) diff --git a/cli/cmd/build.go b/cli/cmd/build.go index 296705ec9..efcf195c3 100644 --- a/cli/cmd/build.go +++ b/cli/cmd/build.go @@ -30,7 +30,7 @@ func NewBuildCmd() *cobra.Command { args []string, toComplete string) ([]string, cobra.ShellCompDirective) { var runningCtx running.RunningCtx - if err := running.FillCtx(cliOpts, &cmdCtx, &runningCtx, []string{}); err != nil { + if err := running.FillCtx(cliOpts, &cmdCtx, &runningCtx, nil, false); err != nil { return nil, cobra.ShellCompDirectiveNoFileComp } return running.ExtractAppNames(runningCtx.Instances), diff --git a/cli/cmd/check.go b/cli/cmd/check.go index db44d54e2..b5aed35b5 100644 --- a/cli/cmd/check.go +++ b/cli/cmd/check.go @@ -32,7 +32,7 @@ func internalCheckModule(cmdCtx *cmdcontext.CmdCtx, args []string) error { } var runningCtx running.RunningCtx - if err := running.FillCtx(cliOpts, cmdCtx, &runningCtx, args); err != nil { + if err := running.FillCtx(cliOpts, cmdCtx, &runningCtx, args, true); err != nil { return err } diff --git a/cli/cmd/clean.go b/cli/cmd/clean.go index bd00796b9..db69c186c 100644 --- a/cli/cmd/clean.go +++ b/cli/cmd/clean.go @@ -104,7 +104,7 @@ func internalCleanModule(cmdCtx *cmdcontext.CmdCtx, args []string) error { } var runningCtx running.RunningCtx - if err := running.FillCtx(cliOpts, cmdCtx, &runningCtx, args); err != nil { + if err := running.FillCtx(cliOpts, cmdCtx, &runningCtx, args, false); err != nil { return err } diff --git a/cli/cmd/cluster.go b/cli/cmd/cluster.go index 310749af5..d317c655b 100644 --- a/cli/cmd/cluster.go +++ b/cli/cmd/cluster.go @@ -673,7 +673,7 @@ func parseAppStr(cmdCtx *cmdcontext.CmdCtx, appStr string) (string, string, stri // Fill context for the entire application. // publish app:inst can work even if the `inst` instance doesn't exist right now. var runningCtx running.RunningCtx - err := running.FillCtx(cliOpts, cmdCtx, &runningCtx, []string{appName}) + err := running.FillCtx(cliOpts, cmdCtx, &runningCtx, []string{appName}, false) if err != nil { return "", "", "", err } diff --git a/cli/cmd/connect.go b/cli/cmd/connect.go index 73c9ceb8d..9b247fc42 100644 --- a/cli/cmd/connect.go +++ b/cli/cmd/connect.go @@ -134,7 +134,7 @@ func resolveConnectOpts(cmdCtx *cmdcontext.CmdCtx, cliOpts *config.CliOpts, newArgs = args[1:] // FillCtx returns error if no instances found. var runningCtx running.RunningCtx - if fillErr := running.FillCtx(cliOpts, cmdCtx, &runningCtx, args); fillErr == nil { + if fillErr := running.FillCtx(cliOpts, cmdCtx, &runningCtx, args, false); fillErr == nil { if len(runningCtx.Instances) > 1 { err = fmt.Errorf("specify instance name") return diff --git a/cli/cmd/internal/completion.go b/cli/cmd/internal/completion.go index 65a2b2d7a..17bfd15f5 100644 --- a/cli/cmd/internal/completion.go +++ b/cli/cmd/internal/completion.go @@ -26,7 +26,7 @@ func ValidArgsFunction( directive = cobra.ShellCompDirectiveNoFileComp var runningCtx running.RunningCtx - if err := running.FillCtx(cliOpts, cmdCtx, &runningCtx, []string{}); err != nil { + if err := running.FillCtx(cliOpts, cmdCtx, &runningCtx, nil, false); err != nil { return } diff --git a/cli/cmd/kill.go b/cli/cmd/kill.go index 61deb22d1..fbee974c3 100644 --- a/cli/cmd/kill.go +++ b/cli/cmd/kill.go @@ -78,7 +78,7 @@ func internalKillModule(cmdCtx *cmdcontext.CmdCtx, args []string) error { if confirm { var runningCtx running.RunningCtx - if err = running.FillCtx(cliOpts, cmdCtx, &runningCtx, args); err != nil { + if err = running.FillCtx(cliOpts, cmdCtx, &runningCtx, args, false); err != nil { return err } diff --git a/cli/cmd/log.go b/cli/cmd/log.go index 9c85eeaa4..bbff37ccf 100644 --- a/cli/cmd/log.go +++ b/cli/cmd/log.go @@ -133,7 +133,7 @@ func internalLogModule(cmdCtx *cmdcontext.CmdCtx, args []string) error { var err error var runningCtx running.RunningCtx - if err = running.FillCtx(cliOpts, cmdCtx, &runningCtx, args); err != nil { + if err = running.FillCtx(cliOpts, cmdCtx, &runningCtx, args, false); err != nil { return err } diff --git a/cli/cmd/logrotate.go b/cli/cmd/logrotate.go index 87bb7e9b9..d6594da08 100644 --- a/cli/cmd/logrotate.go +++ b/cli/cmd/logrotate.go @@ -42,7 +42,7 @@ func internalLogrotateModule(cmdCtx *cmdcontext.CmdCtx, args []string) error { } var runningCtx running.RunningCtx - if err := running.FillCtx(cliOpts, cmdCtx, &runningCtx, args); err != nil { + if err := running.FillCtx(cliOpts, cmdCtx, &runningCtx, args, false); err != nil { return err } diff --git a/cli/cmd/replicaset.go b/cli/cmd/replicaset.go index 3820a309a..99b68ebd0 100644 --- a/cli/cmd/replicaset.go +++ b/cli/cmd/replicaset.go @@ -414,7 +414,7 @@ func replicasetFillCtx(cmdCtx *cmdcontext.CmdCtx, ctx *replicasetCtx, args []str } var connOpts connector.ConnectOpts - if err := running.FillCtx(cliOpts, cmdCtx, &ctx.RunningCtx, args); err == nil { + if err := running.FillCtx(cliOpts, cmdCtx, &ctx.RunningCtx, args, false); err == nil { ctx.IsApplication = true if len(ctx.RunningCtx.Instances) == 1 { if connectCtx.Username != "" || connectCtx.Password != "" { @@ -435,7 +435,7 @@ func replicasetFillCtx(cmdCtx *cmdcontext.CmdCtx, ctx *replicasetCtx, args []str } // Re-fill context for an application. ctx.InstName = instName - err := running.FillCtx(cliOpts, cmdCtx, &ctx.RunningCtx, []string{appName}) + err := running.FillCtx(cliOpts, cmdCtx, &ctx.RunningCtx, []string{appName}, false) if err != nil { // Should not happen. return err diff --git a/cli/cmd/start.go b/cli/cmd/start.go index 8880763db..9ef374529 100644 --- a/cli/cmd/start.go +++ b/cli/cmd/start.go @@ -127,7 +127,7 @@ func internalStartModule(cmdCtx *cmdcontext.CmdCtx, args []string) error { } var runningCtx running.RunningCtx - if err := running.FillCtx(cliOpts, cmdCtx, &runningCtx, args); err != nil { + if err := running.FillCtx(cliOpts, cmdCtx, &runningCtx, args, true); err != nil { return err } diff --git a/cli/cmd/status.go b/cli/cmd/status.go index 18c92fe80..afa332a06 100644 --- a/cli/cmd/status.go +++ b/cli/cmd/status.go @@ -62,7 +62,7 @@ func internalStatusModule(cmdCtx *cmdcontext.CmdCtx, args []string) error { } var runningCtx running.RunningCtx - if err := running.FillCtx(cliOpts, cmdCtx, &runningCtx, args); err != nil { + if err := running.FillCtx(cliOpts, cmdCtx, &runningCtx, args, false); err != nil { return err } diff --git a/cli/cmd/stop.go b/cli/cmd/stop.go index defaa5ac6..65a6a48b8 100644 --- a/cli/cmd/stop.go +++ b/cli/cmd/stop.go @@ -79,7 +79,7 @@ func internalStopModule(cmdCtx *cmdcontext.CmdCtx, args []string) error { var runningCtx running.RunningCtx var err error - if err = running.FillCtx(cliOpts, cmdCtx, &runningCtx, args); err != nil { + if err = running.FillCtx(cliOpts, cmdCtx, &runningCtx, args, false); err != nil { return err } diff --git a/cli/instances/list.go b/cli/instances/list.go index 3abbb4b7f..3e0fe1bec 100644 --- a/cli/instances/list.go +++ b/cli/instances/list.go @@ -31,7 +31,7 @@ func ListInstances(cmdCtx *cmdcontext.CmdCtx, cliOpts *config.CliOpts) error { fmt.Printf("instances enabled directory: %s\n", cliOpts.Env.InstancesEnabled) applications, err := running.CollectInstancesForApps(appList, cliOpts, cmdCtx.Cli.ConfigDir, - cmdCtx.Integrity) + cmdCtx.Integrity, false) if err != nil { return err } diff --git a/cli/pack/opts.go b/cli/pack/opts.go index b322b6ca7..bbfad2bf0 100644 --- a/cli/pack/opts.go +++ b/cli/pack/opts.go @@ -49,7 +49,7 @@ func initAppsInfo(cliOpts *config.CliOpts, cmdCtx *cmdcontext.CmdCtx, packCtx *P } packCtx.AppList = appList packCtx.AppsInfo, err = running.CollectInstancesForApps(packCtx.AppList, cliOpts, - cmdCtx.Cli.ConfigDir, cmdCtx.Integrity) + cmdCtx.Cli.ConfigDir, cmdCtx.Integrity, true) if err != nil { return fmt.Errorf("failed to collect applications info: %s", err) } diff --git a/cli/replicaset/rebootstrap.go b/cli/replicaset/rebootstrap.go index 84a32d176..a2e87b0ec 100644 --- a/cli/replicaset/rebootstrap.go +++ b/cli/replicaset/rebootstrap.go @@ -60,7 +60,7 @@ func cleanDataFiles(instCtx running.InstanceCtx) error { // and starting it again. func Rebootstrap(cmdCtx cmdcontext.CmdCtx, cliOpts config.CliOpts, rbCtx RebootstrapCtx) error { apps, err := running.CollectInstancesForApps([]string{rbCtx.AppName}, &cliOpts, - cmdCtx.Cli.ConfigDir, cmdCtx.Integrity) + cmdCtx.Cli.ConfigDir, cmdCtx.Integrity, true) if err != nil { return fmt.Errorf("cannot collect application instances info: %s", err) } diff --git a/cli/running/running.go b/cli/running/running.go index dd453cefb..ddbd455c5 100644 --- a/cli/running/running.go +++ b/cli/running/running.go @@ -152,7 +152,7 @@ func (provider *providerImpl) updateCtx() error { } var runningCtx RunningCtx - if err = FillCtx(cliOpts, provider.cmdCtx, &runningCtx, args); err != nil { + if err = FillCtx(cliOpts, provider.cmdCtx, &runningCtx, args, false); err != nil { return err } provider.instanceCtx = &runningCtx.Instances[0] @@ -274,20 +274,6 @@ func collectAppDirFiles(appDir string) (appDirCtx appDirCtx, err error) { return } - if appDirCtx.instCfgPath == "" { - if appDirCtx.clusterCfgPath != "" { - // Cluster config will work only if instances.yml exists nearby. - err = fmt.Errorf( - "cluster config %q is found, but instances config (instances.yml) is missing", - appDirCtx.clusterCfgPath) - } else { - if appDirCtx.defaultLuaPath == "" { - err = fmt.Errorf("require files are missing in application directory %q: "+ - "there must be instances config or the default instance script (%q)", - appDir, "init.lua") - } - } - } return } @@ -362,29 +348,41 @@ func loadInstanceConfig(configPath, instName string, // collectInstancesFromAppDir collects instances information from application directory. func collectInstancesFromAppDir(appDir string, selectedInstName string, - integrityCtx integrity.IntegrityCtx) ( + integrityCtx integrity.IntegrityCtx, instancesScriptsRequired bool) ( []InstanceCtx, error, ) { - log.Debugf("Collecting instances from application directory %q", appDir) - instances := []InstanceCtx{} + // log.Debugf("Collecting instances from application directory %q", appDir) if !util.IsDir(appDir) { - return instances, fmt.Errorf("%q doesn't exist or not a directory", appDir) + return nil, fmt.Errorf("%q doesn't exist or not a directory", appDir) } appDirFiles, err := collectAppDirFiles(appDir) + // log.Errorf("appDirFiles: %v", appDirFiles) + // log.Error("^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^") if err != nil { - return instances, err + return nil, err } if appDirFiles.instCfgPath == "" { - if appDirFiles.defaultLuaPath != "" { - return []InstanceCtx{{ - InstanceScript: appDirFiles.defaultLuaPath, - AppName: filepath.Base(appDir), - InstName: filepath.Base(appDir), - AppDir: appDir, - SingleApp: true}}, nil + if appDirFiles.clusterCfgPath != "" { + // Cluster config will work only if instances.yml exists nearby. + return nil, fmt.Errorf( + "cluster config %q is found, but instances config (instances.yml) is missing", + appDirFiles.clusterCfgPath) + } else { + if appDirFiles.defaultLuaPath != "" { + return []InstanceCtx{{ + InstanceScript: appDirFiles.defaultLuaPath, + AppName: filepath.Base(appDir), + InstName: filepath.Base(appDir), + AppDir: appDir, + SingleApp: true}}, nil + } else if instancesScriptsRequired { + return nil, fmt.Errorf("require files are missing in application directory %q: "+ + "there must be instances config or the default instance script (%q)", + appDir, "init.lua") + } } } @@ -399,6 +397,7 @@ func collectInstancesFromAppDir(appDir string, selectedInstName string, return nil, err } log.Debug("Processing application instances file") + instances := []InstanceCtx{} for inst := range instParams { instance := InstanceCtx{AppDir: appDir, ClusterConfigPath: appDirFiles.clusterCfgPath} instance.InstName = getInstanceName(inst, instance.ClusterConfigPath != "") @@ -420,7 +419,8 @@ func collectInstancesFromAppDir(appDir string, selectedInstName string, instance.SingleApp = false if instance.InstanceScript, err = findInstanceScriptInAppDir(appDir, instance.InstName, - appDirFiles.clusterCfgPath, appDirFiles.defaultLuaPath); err != nil { + appDirFiles.clusterCfgPath, appDirFiles.defaultLuaPath); err != nil && + instancesScriptsRequired { return instances, fmt.Errorf("cannot find instance script for %q in config %q: %w ", instance.InstName, appDirFiles.clusterCfgPath, err) } @@ -436,7 +436,7 @@ func collectInstancesFromAppDir(appDir string, selectedInstName string, // CollectInstances searches all instances available in application. func CollectInstances(appName string, applicationsDir string, - integrityCtx integrity.IntegrityCtx) ([]InstanceCtx, error) { + integrityCtx integrity.IntegrityCtx, instancesScriptsRequired bool) ([]InstanceCtx, error) { // The user can select a specific instance from the application. // Example: `tt status application:server`. selectedInstName := "" @@ -464,7 +464,8 @@ func CollectInstances(appName string, applicationsDir string, appDir = applicationsDir } - return collectInstancesFromAppDir(appDir, selectedInstName, integrityCtx) + return collectInstancesFromAppDir(appDir, selectedInstName, integrityCtx, + instancesScriptsRequired) } // cleanup removes runtime artifacts. @@ -626,7 +627,7 @@ func GetClusterConfigPath(cliOpts *config.CliOpts, // CollectInstancesForApps collects instances information per application. func CollectInstancesForApps(appList []string, cliOpts *config.CliOpts, - ttConfigDir string, integrityCtx integrity.IntegrityCtx) ( + ttConfigDir string, integrityCtx integrity.IntegrityCtx, instancesScriptsRequired bool) ( map[string][]InstanceCtx, error) { instEnabledPath := cliOpts.Env.InstancesEnabled if cliOpts.Env.InstancesEnabled == "." { @@ -635,7 +636,8 @@ func CollectInstancesForApps(appList []string, cliOpts *config.CliOpts, apps := make(map[string][]InstanceCtx) for _, appName := range appList { appName = strings.TrimSuffix(appName, ".lua") - collectedInstances, err := CollectInstances(appName, instEnabledPath, integrityCtx) + collectedInstances, err := CollectInstances(appName, instEnabledPath, integrityCtx, + instancesScriptsRequired) if err != nil { return apps, fmt.Errorf("can't collect instance information for %s: %w", appName, err) @@ -676,7 +678,7 @@ func createInstanceDataDirectories(instance InstanceCtx) error { // FillCtx fills the RunningCtx context. func FillCtx(cliOpts *config.CliOpts, cmdCtx *cmdcontext.CmdCtx, - runningCtx *RunningCtx, args []string) error { + runningCtx *RunningCtx, args []string, instancesScriptsRequired bool) error { var err error if len(args) > 1 && cmdCtx.CommandName != "run" && cmdCtx.CommandName != "connect" && @@ -703,7 +705,7 @@ func FillCtx(cliOpts *config.CliOpts, cmdCtx *cmdcontext.CmdCtx, } instances, err := CollectInstancesForApps(appList, cliOpts, - cmdCtx.Cli.ConfigDir, cmdCtx.Integrity) + cmdCtx.Cli.ConfigDir, cmdCtx.Integrity, instancesScriptsRequired) if err != nil { return err } diff --git a/cli/running/running_test.go b/cli/running/running_test.go index 991868435..aca2f4097 100644 --- a/cli/running/running_test.go +++ b/cli/running/running_test.go @@ -35,7 +35,7 @@ func Test_CollectInstances(t *testing.T) { instances, err := CollectInstances("script", instancesEnabledPath, integrity.IntegrityCtx{ Repository: &mockRepository{}, - }) + }, true) require.NoError(t, err) require.Equal(t, 1, len(instances)) require.Equal(t, InstanceCtx{ @@ -50,7 +50,7 @@ func Test_CollectInstances(t *testing.T) { instances, err = CollectInstances("single_inst", instancesEnabledPath, integrity.IntegrityCtx{ Repository: &mockRepository{}, - }) + }, true) require.NoError(t, err) require.Equal(t, 1, len(instances)) require.Equal(t, InstanceCtx{ @@ -67,7 +67,7 @@ func Test_CollectInstances(t *testing.T) { instances, err = CollectInstances(appName, instancesEnabledPath, integrity.IntegrityCtx{ Repository: &mockRepository{}, - }) + }, true) require.NoError(t, err) require.Equal(t, 3, len(instances)) assert.True(t, slices.Contains(instances, InstanceCtx{ @@ -103,7 +103,7 @@ func Test_CollectInstances(t *testing.T) { instances, err = CollectInstances("script", instancesEnabledPath, integrity.IntegrityCtx{ Repository: &mockRepository{}, - }) + }, true) assert.ErrorContains(t, err, "script\" doesn't exist or not a directory") assert.Equal(t, 0, len(instances)) @@ -113,7 +113,7 @@ func Test_CollectInstances(t *testing.T) { instances, err = CollectInstances("script", instancesEnabledPath, integrity.IntegrityCtx{ Repository: &mockRepository{}, - }) + }, true) assert.NoError(t, err) assert.Equal(t, 1, len(instances)) @@ -121,7 +121,7 @@ func Test_CollectInstances(t *testing.T) { instances, err = CollectInstances("script", instancesEnabledPath, integrity.IntegrityCtx{ Repository: &mockRepository{}, - }) + }, true) assert.ErrorContains(t, err, "script.lua: permission denied") assert.Equal(t, 1, len(instances)) require.NoError(t, os.Chmod(instancesEnabledPath, 0755)) @@ -131,7 +131,7 @@ func Test_collectAppDirFiles(t *testing.T) { tmpdir := t.TempDir() _, err := collectAppDirFiles(tmpdir) - require.Error(t, err) + require.NoError(t, err) expectedDefaultScript := filepath.Join(tmpdir, "init.lua") expectedInstancesConfig := filepath.Join(tmpdir, "instances.yml") @@ -140,7 +140,7 @@ func Test_collectAppDirFiles(t *testing.T) { // Cluster config exists, but no instances config. os.Create(expectedClusterConfig) appDirFiles, err := collectAppDirFiles(tmpdir) - require.Error(t, err) + require.NoError(t, err) require.Equal(t, expectedClusterConfig, appDirFiles.clusterCfgPath) require.Equal(t, "", appDirFiles.defaultLuaPath) require.Equal(t, "", appDirFiles.instCfgPath) @@ -148,7 +148,7 @@ func Test_collectAppDirFiles(t *testing.T) { // Cluster config and default instance script exist, but no instances config. os.Create(expectedDefaultScript) appDirFiles, err = collectAppDirFiles(tmpdir) - require.Error(t, err) + require.NoError(t, err) require.Equal(t, expectedClusterConfig, appDirFiles.clusterCfgPath) require.Equal(t, expectedDefaultScript, appDirFiles.defaultLuaPath) require.Equal(t, "", appDirFiles.instCfgPath) @@ -189,7 +189,7 @@ func Test_collectInstancesForApps(t *testing.T) { instances, err := CollectInstancesForApps(apps, cliOpts, "/etc/tarantool/", integrity.IntegrityCtx{ Repository: &mockRepository{}, - }) + }, true) require.NoError(t, err) require.Contains(t, instances, appName) @@ -316,7 +316,7 @@ func Test_collectInstancesForSingleInstApp(t *testing.T) { instances, err := CollectInstancesForApps(apps, cliOpts, "/etc/tarantool/", integrity.IntegrityCtx{ Repository: &mockRepository{}, - }) + }, true) require.NoError(t, err) require.Equal(t, 1, len(instances)) require.Contains(t, instances, appName) @@ -347,7 +347,7 @@ func Test_collectInstancesSingleInstanceTntCtlLayout(t *testing.T) { instances, err := CollectInstancesForApps(apps, cliOpts, cfgDir, integrity.IntegrityCtx{ Repository: &mockRepository{}, - }) + }, true) require.NoError(t, err) require.Len(t, instances, 1) require.Contains(t, instances, appName)