From 809a16458f7483a2b762cd546493780fea6220ea Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Wed, 7 Feb 2018 13:41:15 +0530 Subject: Abort on critical error during server startup (#8204) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Only a handful of critical errors are present in the codebase. They all occur during server startup (in `app.StartServer()`). Currently, when one of these critical error occurs, it is simpled mentionned in the logs – then the error is discarded, and the app attempts to continue the execution (and probably fails pretty quickly in a weird way). Rather than continuing operations in an unknow state, these errors should trigger a clean exit. This commit rewrites critical startup errors to be correctly propagated, logged, and then terminate the command execution. Additionnaly, it makes the server return a proper error code to the shell. --- cmd/platform/server.go | 9 +++++++-- cmd/platform/test.go | 12 ++++++++++-- 2 files changed, 17 insertions(+), 4 deletions(-) (limited to 'cmd') diff --git a/cmd/platform/server.go b/cmd/platform/server.go index e3742cef6..a8a6e8923 100644 --- a/cmd/platform/server.go +++ b/cmd/platform/server.go @@ -53,7 +53,7 @@ func runServer(configFileLocation string, disableConfigWatch bool) error { a, err := app.New(options...) if err != nil { - l4g.Error(err.Error()) + l4g.Critical(err.Error()) return err } defer a.Shutdown() @@ -87,7 +87,12 @@ func runServer(configFileLocation string, disableConfigWatch bool) error { } }) - a.StartServer() + serverErr := a.StartServer() + if serverErr != nil { + l4g.Critical(serverErr.Error()) + return serverErr + } + api4.Init(a, a.Srv.Router, false) api3 := api.Init(a, a.Srv.Router) wsapi.Init(a, a.Srv.WebSocketRouter) diff --git a/cmd/platform/test.go b/cmd/platform/test.go index 036df07de..9ab3fbb36 100644 --- a/cmd/platform/test.go +++ b/cmd/platform/test.go @@ -53,7 +53,11 @@ func webClientTestsCmdF(cmd *cobra.Command, args []string) error { defer a.Shutdown() utils.InitTranslations(a.Config().LocalizationSettings) - a.StartServer() + serverErr := a.StartServer() + if serverErr != nil { + return serverErr + } + api4.Init(a, a.Srv.Router, false) api.Init(a, a.Srv.Router) wsapi.Init(a, a.Srv.WebSocketRouter) @@ -71,7 +75,11 @@ func serverForWebClientTestsCmdF(cmd *cobra.Command, args []string) error { defer a.Shutdown() utils.InitTranslations(a.Config().LocalizationSettings) - a.StartServer() + serverErr := a.StartServer() + if serverErr != nil { + return serverErr + } + api4.Init(a, a.Srv.Router, false) api.Init(a, a.Srv.Router) wsapi.Init(a, a.Srv.WebSocketRouter) -- cgit v1.2.3-1-g7c22 From 7bd298ceaa24c0721e0acd65692cb2d1ca4983f3 Mon Sep 17 00:00:00 2001 From: Vordimous Date: Wed, 7 Feb 2018 09:17:18 -0500 Subject: PLT-7537: Move channel CLI command posts system message to channel. (#8161) * [PTL-7537] implement feature and test * [PTL-7537] Update feature to post the the room requiring a username flag to be used * [PTL-7537] update tests with username * update test to remove changes to the test helper struct * use the basic team and user --- cmd/platform/channel.go | 17 +++++++++++++---- cmd/platform/channel_test.go | 27 +++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 4 deletions(-) (limited to 'cmd') diff --git a/cmd/platform/channel.go b/cmd/platform/channel.go index 98bdcebb8..5d86ad9da 100644 --- a/cmd/platform/channel.go +++ b/cmd/platform/channel.go @@ -106,6 +106,8 @@ func init() { channelCreateCmd.Flags().String("purpose", "", "Channel purpose") channelCreateCmd.Flags().Bool("private", false, "Create a private channel.") + moveChannelsCmd.Flags().String("username", "", "Required. Username who is moving the channel.") + deleteChannelsCmd.Flags().Bool("confirm", false, "Confirm you really want to delete the channels.") modifyChannelCmd.Flags().Bool("private", false, "Convert the channel to a private channel") @@ -319,26 +321,33 @@ func moveChannelsCmdF(cmd *cobra.Command, args []string) error { return errors.New("Unable to find destination team '" + args[0] + "'") } + username, erru := cmd.Flags().GetString("username") + if erru != nil || username == "" { + return errors.New("Username is required") + } + user := getUserFromUserArg(a, username) + channels := getChannelsFromChannelArgs(a, args[1:]) for i, channel := range channels { if channel == nil { CommandPrintErrorln("Unable to find channel '" + args[i] + "'") continue } - if err := moveChannel(a, team, channel); err != nil { + originTeamID := channel.TeamId + if err := moveChannel(a, team, channel, user); err != nil { CommandPrintErrorln("Unable to move channel '" + channel.Name + "' error: " + err.Error()) } else { - CommandPrettyPrintln("Moved channel '" + channel.Name + "'") + CommandPrettyPrintln("Moved channel '" + channel.Name + "' to " + team.Name + "(" + team.Id + ") from " + originTeamID + ".") } } return nil } -func moveChannel(a *app.App, team *model.Team, channel *model.Channel) *model.AppError { +func moveChannel(a *app.App, team *model.Team, channel *model.Channel, user *model.User) *model.AppError { oldTeamId := channel.TeamId - if err := a.MoveChannel(team, channel); err != nil { + if err := a.MoveChannel(team, channel, user); err != nil { return err } diff --git a/cmd/platform/channel_test.go b/cmd/platform/channel_test.go index 1e6915679..cf8603cf3 100644 --- a/cmd/platform/channel_test.go +++ b/cmd/platform/channel_test.go @@ -44,6 +44,33 @@ func TestRemoveChannel(t *testing.T) { checkCommand(t, "channel", "remove", th.BasicTeam.Name+":"+channel.Name, th.BasicUser2.Email) } +func TestMoveChannel(t *testing.T) { + th := api.Setup().InitBasic() + defer th.TearDown() + + client := th.BasicClient + team1 := th.BasicTeam + team2 := th.CreateTeam(client) + user1 := th.BasicUser + th.LinkUserToTeam(user1, team2) + channel := th.BasicChannel + + th.LinkUserToTeam(user1, team1) + th.LinkUserToTeam(user1, team2) + + adminEmail := user1.Email + adminUsername := user1.Username + origin := team1.Name + ":" + channel.Name + dest := team2.Name + + checkCommand(t, "channel", "add", origin, adminEmail) + + // should fail with nill because errors are logged instead of returned when a channel does not exist + require.Nil(t, runCommand(t, "channel", "move", dest, team1.Name+":doesnotexist", "--username", adminUsername)) + + checkCommand(t, "channel", "move", dest, origin, "--username", adminUsername) +} + func TestListChannels(t *testing.T) { th := api.Setup().InitBasic() defer th.TearDown() -- cgit v1.2.3-1-g7c22 From a6309aaf48e216fe5f6779188071d4b621b643b6 Mon Sep 17 00:00:00 2001 From: Chris Date: Fri, 9 Feb 2018 10:04:48 -0600 Subject: Remove license globals entirely (#8229) * remove license globals entirely * fix infinite recursion * test fix --- cmd/platform/jobserver.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'cmd') diff --git a/cmd/platform/jobserver.go b/cmd/platform/jobserver.go index e664136c0..044ee6b6a 100644 --- a/cmd/platform/jobserver.go +++ b/cmd/platform/jobserver.go @@ -35,7 +35,7 @@ func jobserverCmdF(cmd *cobra.Command, args []string) { defer l4g.Close() defer a.Shutdown() - a.Jobs.LoadLicense() + a.LoadLicense() // Run jobs l4g.Info("Starting Mattermost job server") -- cgit v1.2.3-1-g7c22 From 07fd7aeeb8eb2b198b01b713a4ab57f6352faef2 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Mon, 12 Feb 2018 22:16:32 +0530 Subject: Add tests for the `platform server` command (#8231) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Cleanup app state on initialization error When returning an initialization error, the app state was not cleaned up. This is especially visible during tests, as `appCount` is not decremented, and makes the new app initialization fail. * Test the `platform server` command As the `platform server` command only exits when interrupted by a signal, it is not possible to test it as the other cobra commands. Instead we directly test the actual command function. The internal command handler is slighly refactored to take a channel in argument, and registers it as the signal handler. Nothing very different—except than controlling this channel from the outside allows the test to send the system signal itself, thus preventing the server to run forever. --- cmd/platform/server.go | 10 +++---- cmd/platform/server_test.go | 72 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 5 deletions(-) create mode 100644 cmd/platform/server_test.go (limited to 'cmd') diff --git a/cmd/platform/server.go b/cmd/platform/server.go index a8a6e8923..1b411cf20 100644 --- a/cmd/platform/server.go +++ b/cmd/platform/server.go @@ -42,10 +42,11 @@ func runServerCmd(cmd *cobra.Command, args []string) error { disableConfigWatch, _ := cmd.Flags().GetBool("disableconfigwatch") - return runServer(config, disableConfigWatch) + interruptChan := make(chan os.Signal, 1) + return runServer(config, disableConfigWatch, interruptChan) } -func runServer(configFileLocation string, disableConfigWatch bool) error { +func runServer(configFileLocation string, disableConfigWatch bool, interruptChan chan os.Signal) error { options := []app.Option{app.ConfigFile(configFileLocation)} if disableConfigWatch { options = append(options, app.DisableConfigWatch) @@ -165,9 +166,8 @@ func runServer(configFileLocation string, disableConfigWatch bool) error { // wait for kill signal before attempting to gracefully shutdown // the running service - c := make(chan os.Signal, 1) - signal.Notify(c, os.Interrupt, syscall.SIGINT, syscall.SIGTERM) - <-c + signal.Notify(interruptChan, os.Interrupt, syscall.SIGINT, syscall.SIGTERM) + <-interruptChan if a.Cluster != nil { a.Cluster.StopInterNodeCommunication() diff --git a/cmd/platform/server_test.go b/cmd/platform/server_test.go new file mode 100644 index 000000000..15f9a357a --- /dev/null +++ b/cmd/platform/server_test.go @@ -0,0 +1,72 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See License.txt for license information. + +package main + +import ( + "io/ioutil" + "os" + "syscall" + "testing" + + "github.com/mattermost/mattermost-server/jobs" + "github.com/mattermost/mattermost-server/utils" + "github.com/stretchr/testify/require" +) + +type ServerTestHelper struct { + configPath string + disableConfigWatch bool + interruptChan chan os.Signal + originalInterval int +} + +func SetupServerTest() *ServerTestHelper { + // Build a channel that will be used by the server to receive system signals… + interruptChan := make(chan os.Signal, 1) + // …and sent it immediately a SIGINT value. + // This will make the server loop stop as soon as it started successfully. + interruptChan <- syscall.SIGINT + + // Let jobs poll for termination every 0.2s (instead of every 15s by default) + // Otherwise we would have to wait the whole polling duration before the test + // terminates. + originalInterval := jobs.DEFAULT_WATCHER_POLLING_INTERVAL + jobs.DEFAULT_WATCHER_POLLING_INTERVAL = 200 + + th := &ServerTestHelper{ + configPath: utils.FindConfigFile("config.json"), + disableConfigWatch: true, + interruptChan: interruptChan, + originalInterval: originalInterval, + } + return th +} + +func (th *ServerTestHelper) TearDownServerTest() { + jobs.DEFAULT_WATCHER_POLLING_INTERVAL = th.originalInterval +} + +func TestRunServerSuccess(t *testing.T) { + th := SetupServerTest() + defer th.TearDownServerTest() + + err := runServer(th.configPath, th.disableConfigWatch, th.interruptChan) + require.NoError(t, err) +} + +func TestRunServerInvalidConfigFile(t *testing.T) { + th := SetupServerTest() + defer th.TearDownServerTest() + + // Start the server with an unreadable config file + unreadableConfigFile, err := ioutil.TempFile("", "mattermost-unreadable-config-file-") + if err != nil { + panic(err) + } + os.Chmod(unreadableConfigFile.Name(), 0200) + defer os.Remove(unreadableConfigFile.Name()) + + err = runServer(unreadableConfigFile.Name(), th.disableConfigWatch, th.interruptChan) + require.Error(t, err) +} -- cgit v1.2.3-1-g7c22