mirror of
https://github.com/mattermost/mattermost.git
synced 2026-09-21 05:54:10 +08:00
MM-68622: start inter-cluster services before plugin activation (#36366)
* MM-68622: start inter-cluster services before plugin activation
Move startInterClusterServices from the end of Server.Start() to the
beginning, before Channels().Start() initializes plugins. This lets
plugins call shared channels APIs (ShareChannel, InviteRemoteToChannel,
UninviteRemoteFromChannel, UnshareChannel, UpdateSharedChannel,
CheckCanInviteToSharedChannel) during OnActivate instead of failing with
"Shared Channels Service is disabled".
Side-effect analysis:
* Plugin API gating: getSharedChannelsService in
channels/app/shared_channel.go:32 only requires the service to be
non-nil. The plugin-facing wrappers all pass ensureIsActive=false, so
Active() is bypassed. Once SetSharedChannelService runs, calls succeed
on both leader and follower nodes. This is the fix path.
* Multi-node leader timing: the enterprise cluster impl in
enterprise/cluster/cluster.go:70 initializes currentLeader="", so
IsLeader() returns false before StartInterNodeCommunication runs. The
immediate onClusterLeaderChange in scs.Start at
platform/services/sharedchannel/service.go:151 therefore takes the
pause path, which is a no-op since the service was never active. When
memberlist.Create fires NotifyJoin for the local node,
addPotentialLeader runs and InvokeClusterLeaderChangedListeners drives
the registered listener to resume() the sync loop on the elected
leader. End state matches the prior ordering.
* Single-node: IsLeader() returns true unconditionally per
channels/app/platform/cluster.go:33, so SharedChannelSyncHandler is
active during plugin OnActivate. Events emitted by plugins during
activation (posts to shared channels, DM creation) now flow through
sync where they were previously dropped. This is intended correctness,
not a regression.
* Transport and handlers: api4 remote-cluster routes are registered
before Server.Start, so HTTP handlers exist when rcs.Start runs early.
rcs and scs do not send cluster-broadcast messages during Start; they
only register topic listeners on the rcs transport, which is
independent of cluster gossip. registerClusterHandlers ordering is
unaffected.
* Config: scs reads only ConnectedWorkspacesSettings and the License at
construction, both stable from the initial config load. ReloadConfig
at server.go:912 has no bearing on inter-cluster service init.
Errors from startInterClusterServices remain logged and non-fatal,
matching prior behavior.
This commit is contained in:
@@ -4,11 +4,13 @@
|
||||
package app
|
||||
|
||||
import (
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"github.com/mattermost/mattermost/server/public/model"
|
||||
"github.com/mattermost/mattermost/server/v8/channels/testlib"
|
||||
)
|
||||
|
||||
func setupRemoteCluster(tb testing.TB) *TestHelper {
|
||||
@@ -18,6 +20,57 @@ func setupRemoteCluster(tb testing.TB) *TestHelper {
|
||||
})
|
||||
}
|
||||
|
||||
// TestSharedChannelServicesAvailableBeforePluginActivation guards the fix for
|
||||
// MM-68622. Plugins that call shared channels APIs from OnActivate previously
|
||||
// failed with "Shared Channels Service is disabled" because
|
||||
// startInterClusterServices ran after Channels().Start() initialized plugins.
|
||||
// Server.Start now starts the inter-cluster services first, so the services
|
||||
// must be available by the time plugin initialization begins.
|
||||
func TestSharedChannelServicesAvailableBeforePluginActivation(t *testing.T) {
|
||||
mainHelper.Parallel(t)
|
||||
th := setupRemoteCluster(t).InitBasic(t)
|
||||
|
||||
require.NotNil(t, th.Server.GetRemoteClusterService(),
|
||||
"remote cluster service must be initialized after Server.Start")
|
||||
require.NotNil(t, th.Server.GetSharedChannelSyncService(),
|
||||
"shared channel sync service must be initialized after Server.Start")
|
||||
|
||||
// Order check: scs.resume() logs "Shared Channel Service active" from
|
||||
// scs.Start(), and initPlugins logs "Starting up plugins" from
|
||||
// Channels().Start(). The first must precede the second, otherwise plugin
|
||||
// OnActivate would observe a nil service.
|
||||
require.NoError(t, th.TestLogger.Flush())
|
||||
entries := testlib.ParseLogEntries(t, strings.NewReader(th.LogBuffer.String()))
|
||||
|
||||
scsActiveIdx, pluginInitIdx := -1, -1
|
||||
for i, e := range entries {
|
||||
if scsActiveIdx == -1 && e.Msg == "Shared Channel Service active" {
|
||||
scsActiveIdx = i
|
||||
}
|
||||
if pluginInitIdx == -1 && e.Msg == "Starting up plugins" {
|
||||
pluginInitIdx = i
|
||||
}
|
||||
}
|
||||
require.NotEqual(t, -1, scsActiveIdx,
|
||||
"expected log message 'Shared Channel Service active' from scs.resume()")
|
||||
require.NotEqual(t, -1, pluginInitIdx,
|
||||
"expected log message 'Starting up plugins' from initPlugins")
|
||||
require.Less(t, scsActiveIdx, pluginInitIdx,
|
||||
"shared channel service must activate before plugin initialization (MM-68622)")
|
||||
|
||||
// Plugin entry path: this is the App-layer call a plugin would make from
|
||||
// OnActivate. Before MM-68622 it would return "Shared Channels Service is
|
||||
// disabled" because GetSharedChannelSyncService() was still nil.
|
||||
pluginID := "com.test.startup-" + model.NewId()
|
||||
_, err := th.App.RegisterPluginForSharedChannels(th.Context, model.RegisterPluginOpts{
|
||||
Displayname: "startup test plugin",
|
||||
PluginID: pluginID,
|
||||
CreatorID: th.BasicUser.Id,
|
||||
})
|
||||
require.NoError(t, err,
|
||||
"RegisterPluginForSharedChannels must succeed when shared channels is enabled")
|
||||
}
|
||||
|
||||
func TestAddRemoteCluster(t *testing.T) {
|
||||
mainHelper.Parallel(t)
|
||||
th := setupRemoteCluster(t).InitBasic(t)
|
||||
|
||||
@@ -856,6 +856,12 @@ func stripPort(hostport string) string {
|
||||
}
|
||||
|
||||
func (s *Server) Start() error {
|
||||
// Start inter-cluster services first so shared channels APIs are
|
||||
// available when plugins activate during channels startup.
|
||||
if err := s.startInterClusterServices(s.License()); err != nil {
|
||||
mlog.Error("Error starting inter-cluster services", mlog.Err(err))
|
||||
}
|
||||
|
||||
// Start channels.
|
||||
// This needs to happen before because channels is dependent on the HTTP server.
|
||||
if err := s.Channels().Start(); err != nil {
|
||||
@@ -1113,10 +1119,6 @@ func (s *Server) Start() error {
|
||||
}
|
||||
}
|
||||
|
||||
if err := s.startInterClusterServices(s.License()); err != nil {
|
||||
mlog.Error("Error starting inter-cluster services", mlog.Err(err))
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user