From 846791aa65217996b25c7d6ec9304e0e780dbde5 Mon Sep 17 00:00:00 2001 From: Doug Lauder Date: Mon, 4 May 2026 08:53:59 -0400 Subject: [PATCH] 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. --- server/channels/app/remote_cluster_test.go | 53 ++++++++++++++++++++++ server/channels/app/server.go | 10 ++-- 2 files changed, 59 insertions(+), 4 deletions(-) diff --git a/server/channels/app/remote_cluster_test.go b/server/channels/app/remote_cluster_test.go index 2ea1b56fe5c..f4b5ec604dd 100644 --- a/server/channels/app/remote_cluster_test.go +++ b/server/channels/app/remote_cluster_test.go @@ -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) diff --git a/server/channels/app/server.go b/server/channels/app/server.go index c81dd89bb37..547c5448251 100644 --- a/server/channels/app/server.go +++ b/server/channels/app/server.go @@ -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 }