[Merged by Bors] - Use random port for post service in supervised mode

CHANGELOG.md

@@ -15,6 +15,10 @@ See [RELEASE](./RELEASE.md) for workflow instructions.
 * [#5730](https://github.com/spacemeshos/go-spacemesh/pull/5730) Fixed a bug where the node behaves incorrectly when
   first started with supervised smeshing.
 
+* [#5731](https://github.com/spacemeshos/go-spacemesh/pull/5731) The default listen address for `PostService` is now
+  `127.0.0.1:0` instead of `127.0.0.1:9094`. This will ensure that a node binds the post service to a random free port
+  and prevents multiple instances of the post service from binding to the same port.
+
 ## Release v1.4.1
 
 ### Improvements

activation/e2e/nipost_test.go

@@ -76,10 +76,8 @@ func launchPostSupervisor(
 
 	builder := activation.NewMockAtxBuilder(gomock.NewController(tb))
 	builder.EXPECT().Register(gomock.Any())
-	ps, err := activation.NewPostSupervisor(log, cmdCfg, postCfg, provingOpts, mgr, builder)
-	require.NoError(tb, err)
-	require.NotNil(tb, ps)
-	require.NoError(tb, ps.Start(postOpts, sig))
+	ps := activation.NewPostSupervisor(log, postCfg, provingOpts, mgr, builder)
+	require.NoError(tb, ps.Start(cmdCfg, postOpts, sig))
 	return func() { assert.NoError(tb, ps.Stop(false)) }
 }
 

activation/post_supervisor.go

@@ -66,7 +66,6 @@ type PostSupervisorConfig struct {
 type PostSupervisor struct {
 	logger *zap.Logger
 
-	cmdCfg      PostSupervisorConfig
 	postCfg     PostConfig
 	provingOpts PostProvingOpts
 
@@ -83,25 +82,19 @@ type PostSupervisor struct {
 // NewPostSupervisor returns a new post service.
 func NewPostSupervisor(
 	logger *zap.Logger,
-	cmdCfg PostSupervisorConfig,
 	postCfg PostConfig,
 	provingOpts PostProvingOpts,
 	postSetupProvider postSetupProvider,
 	atxBuilder AtxBuilder,
-) (*PostSupervisor, error) {
-	if _, err := os.Stat(cmdCfg.PostServiceCmd); err != nil {
-		return nil, fmt.Errorf("post service binary not found: %s", cmdCfg.PostServiceCmd)
-	}
-
+) *PostSupervisor {
 	return &PostSupervisor{
 		logger:      logger,
-		cmdCfg:      cmdCfg,
 		postCfg:     postCfg,
 		provingOpts: provingOpts,
 
 		postSetupProvider: postSetupProvider,
 		atxBuilder:        atxBuilder,
-	}, nil
+	}
 }
 
 func (ps *PostSupervisor) Config() PostConfig {
@@ -137,13 +130,17 @@ func (ps *PostSupervisor) Status() *PostSetupStatus {
 	return ps.postSetupProvider.Status()
 }
 
-func (ps *PostSupervisor) Start(opts PostSetupOpts, sig *signing.EdSigner) error {
+func (ps *PostSupervisor) Start(cmdCfg PostSupervisorConfig, opts PostSetupOpts, sig *signing.EdSigner) error {
 	ps.mtx.Lock()
 	defer ps.mtx.Unlock()
 	if ps.stop != nil {
 		return fmt.Errorf("post service already started")
 	}
 
+	if _, err := os.Stat(cmdCfg.PostServiceCmd); err != nil {
+		return fmt.Errorf("stat post service binary %s: %w", cmdCfg.PostServiceCmd, err)
+	}
+
 	// TODO(mafa): verify that opts don't delete existing files
 
 	ps.eg = errgroup.Group{} // reset errgroup to allow restarts.
@@ -171,7 +168,7 @@ func (ps *PostSupervisor) Start(opts PostSetupOpts, sig *signing.EdSigner) error
 		}
 		ps.atxBuilder.Register(sig)
 
-		return ps.runCmd(ctx, ps.cmdCfg, ps.postCfg, opts, ps.provingOpts, sig.NodeID())
+		return ps.runCmd(ctx, cmdCfg, ps.postCfg, opts, ps.provingOpts, sig.NodeID())
 	})
 	return nil
 }

activation/post_supervisor_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"encoding/json"
 	"errors"
+	"io/fs"
 	"os"
 	"path/filepath"
 	"runtime"
@@ -68,24 +69,24 @@ func Test_PostSupervisor_ErrorOnMissingBinary(t *testing.T) {
 	cmdCfg := DefaultTestPostServiceConfig()
 	cmdCfg.PostServiceCmd = "missing"
 	postCfg := DefaultPostConfig()
+	postOpts := DefaultPostSetupOpts()
 	provingOpts := DefaultPostProvingOpts()
+	sig, err := signing.NewEdSigner()
+	require.NoError(t, err)
 
-	ps, err := NewPostSupervisor(log.Named("supervisor"), cmdCfg, postCfg, provingOpts, nil, nil)
-	require.ErrorContains(t, err, "post service binary not found")
-	require.Nil(t, ps)
+	ps := NewPostSupervisor(log.Named("supervisor"), postCfg, provingOpts, nil, nil)
+	err = ps.Start(cmdCfg, postOpts, sig)
+	require.ErrorIs(t, err, fs.ErrNotExist)
+	require.ErrorContains(t, err, "stat post service binary missing")
 }
 
 func Test_PostSupervisor_StopWithoutStart(t *testing.T) {
 	log := zaptest.NewLogger(t)
 
-	cmdCfg := DefaultTestPostServiceConfig()
 	postCfg := DefaultPostConfig()
 	provingOpts := DefaultPostProvingOpts()
 
-	ps, err := NewPostSupervisor(log.Named("supervisor"), cmdCfg, postCfg, provingOpts, nil, nil)
-	require.NoError(t, err)
-	require.NotNil(t, ps)
-
+	ps := NewPostSupervisor(log.Named("supervisor"), postCfg, provingOpts, nil, nil)
 	require.NoError(t, ps.Stop(false))
 }
 
@@ -104,11 +105,9 @@ func Test_PostSupervisor_Start_FailPrepare(t *testing.T) {
 	testErr := errors.New("test error")
 	mgr.EXPECT().PrepareInitializer(gomock.Any(), postOpts, sig.NodeID()).Return(testErr)
 	builder := NewMockAtxBuilder(ctrl)
-	ps, err := NewPostSupervisor(log.Named("supervisor"), cmdCfg, postCfg, provingOpts, mgr, builder)
-	require.NoError(t, err)
-	require.NotNil(t, ps)
+	ps := NewPostSupervisor(log.Named("supervisor"), postCfg, provingOpts, mgr, builder)
 
-	require.NoError(t, ps.Start(postOpts, sig))
+	require.NoError(t, ps.Start(cmdCfg, postOpts, sig))
 	require.ErrorIs(t, ps.Stop(false), testErr)
 }
 
@@ -141,11 +140,9 @@ func Test_PostSupervisor_Start_FailStartSession(t *testing.T) {
 	mgr.EXPECT().PrepareInitializer(gomock.Any(), postOpts, sig.NodeID()).Return(nil)
 	mgr.EXPECT().StartSession(gomock.Any(), sig.NodeID()).Return(errors.New("failed start session"))
 	builder := NewMockAtxBuilder(ctrl)
-	ps, err := NewPostSupervisor(log.Named("supervisor"), cmdCfg, postCfg, provingOpts, mgr, builder)
-	require.NoError(t, err)
-	require.NotNil(t, ps)
+	ps := NewPostSupervisor(log.Named("supervisor"), postCfg, provingOpts, mgr, builder)
 
-	require.NoError(t, ps.Start(postOpts, sig))
+	require.NoError(t, ps.Start(cmdCfg, postOpts, sig))
 	require.EqualError(t, ps.eg.Wait(), "failed start session")
 }
 
@@ -163,11 +160,9 @@ func Test_PostSupervisor_StartsServiceCmd(t *testing.T) {
 	mgr := newPostManager(t, postCfg, postOpts)
 	builder := NewMockAtxBuilder(ctrl)
 	builder.EXPECT().Register(sig)
-	ps, err := NewPostSupervisor(log.Named("supervisor"), cmdCfg, postCfg, provingOpts, mgr, builder)
-	require.NoError(t, err)
-	require.NotNil(t, ps)
+	ps := NewPostSupervisor(log.Named("supervisor"), postCfg, provingOpts, mgr, builder)
 
-	require.NoError(t, ps.Start(postOpts, sig))
+	require.NoError(t, ps.Start(cmdCfg, postOpts, sig))
 	t.Cleanup(func() { assert.NoError(t, ps.Stop(false)) })
 
 	require.Eventually(t, func() bool { return ps.pid.Load() != 0 }, 5*time.Second, 100*time.Millisecond)
@@ -202,19 +197,17 @@ func Test_PostSupervisor_Restart_Possible(t *testing.T) {
 	mgr := newPostManager(t, postCfg, postOpts)
 	builder := NewMockAtxBuilder(ctrl)
 	builder.EXPECT().Register(sig)
-	ps, err := NewPostSupervisor(log.Named("supervisor"), cmdCfg, postCfg, provingOpts, mgr, builder)
-	require.NoError(t, err)
-	require.NotNil(t, ps)
+	ps := NewPostSupervisor(log.Named("supervisor"), postCfg, provingOpts, mgr, builder)
 
-	require.NoError(t, ps.Start(postOpts, sig))
+	require.NoError(t, ps.Start(cmdCfg, postOpts, sig))
 	t.Cleanup(func() { assert.NoError(t, ps.Stop(false)) })
 	require.Eventually(t, func() bool { return ps.pid.Load() != 0 }, 5*time.Second, 100*time.Millisecond)
 
 	require.NoError(t, ps.Stop(false))
 	require.Eventually(t, func() bool { return ps.pid.Load() == 0 }, 5*time.Second, 100*time.Millisecond)
 
 	builder.EXPECT().Register(sig)
-	require.NoError(t, ps.Start(postOpts, sig))
+	require.NoError(t, ps.Start(cmdCfg, postOpts, sig))
 	require.Eventually(t, func() bool { return ps.pid.Load() != 0 }, 5*time.Second, 100*time.Millisecond)
 
 	require.NoError(t, ps.Stop(false))
@@ -235,11 +228,9 @@ func Test_PostSupervisor_LogFatalOnCrash(t *testing.T) {
 	mgr := newPostManager(t, postCfg, postOpts)
 	builder := NewMockAtxBuilder(ctrl)
 	builder.EXPECT().Register(sig)
-	ps, err := NewPostSupervisor(log.Named("supervisor"), cmdCfg, postCfg, provingOpts, mgr, builder)
-	require.NoError(t, err)
-	require.NotNil(t, ps)
+	ps := NewPostSupervisor(log.Named("supervisor"), postCfg, provingOpts, mgr, builder)
 
-	require.NoError(t, ps.Start(postOpts, sig))
+	require.NoError(t, ps.Start(cmdCfg, postOpts, sig))
 	t.Cleanup(func() { assert.NoError(t, ps.Stop(false)) })
 
 	require.Eventually(t, func() bool { return ps.pid.Load() != 0 }, 5*time.Second, 100*time.Millisecond)
@@ -270,11 +261,9 @@ func Test_PostSupervisor_LogFatalOnInvalidConfig(t *testing.T) {
 	mgr := newPostManager(t, postCfg, postOpts)
 	builder := NewMockAtxBuilder(ctrl)
 	builder.EXPECT().Register(sig)
-	ps, err := NewPostSupervisor(log.Named("supervisor"), cmdCfg, postCfg, provingOpts, mgr, builder)
-	require.NoError(t, err)
-	require.NotNil(t, ps)
+	ps := NewPostSupervisor(log.Named("supervisor"), postCfg, provingOpts, mgr, builder)
 
-	require.NoError(t, ps.Start(postOpts, sig))
+	require.NoError(t, ps.Start(cmdCfg, postOpts, sig))
 	t.Cleanup(func() { assert.NoError(t, ps.Stop(false)) })
 
 	require.Eventually(t, func() bool { return ps.pid.Load() != 0 }, 5*time.Second, 100*time.Millisecond)
@@ -312,11 +301,9 @@ func Test_PostSupervisor_StopOnError(t *testing.T) {
 	})
 	builder := NewMockAtxBuilder(ctrl)
 	builder.EXPECT().Register(sig)
-	ps, err := NewPostSupervisor(log.Named("supervisor"), cmdCfg, postCfg, provingOpts, mgr, builder)
-	require.NoError(t, err)
-	require.NotNil(t, ps)
+	ps := NewPostSupervisor(log.Named("supervisor"), postCfg, provingOpts, mgr, builder)
 
-	require.NoError(t, ps.Start(postOpts, sig))
+	require.NoError(t, ps.Start(cmdCfg, postOpts, sig))
 	t.Cleanup(func() { assert.NoError(t, ps.Stop(false)) })
 	require.Eventually(t, func() bool { return ps.pid.Load() != 0 }, 5*time.Second, 100*time.Millisecond)
 
@@ -328,15 +315,13 @@ func Test_PostSupervisor_StopOnError(t *testing.T) {
 func Test_PostSupervisor_Providers_includesCPU(t *testing.T) {
 	log := zaptest.NewLogger(t)
 
-	cmdCfg := DefaultTestPostServiceConfig()
 	postCfg := DefaultPostConfig()
 	provingOpts := DefaultPostProvingOpts()
 
 	ctrl := gomock.NewController(t)
 	mgr := NewMockpostSetupProvider(ctrl)
 	builder := NewMockAtxBuilder(ctrl)
-	ps, err := NewPostSupervisor(log.Named("supervisor"), cmdCfg, postCfg, provingOpts, mgr, builder)
-	require.NoError(t, err)
+	ps := NewPostSupervisor(log.Named("supervisor"), postCfg, provingOpts, mgr, builder)
 
 	providers, err := ps.Providers()
 	require.NoError(t, err)
@@ -352,15 +337,13 @@ func Test_PostSupervisor_Providers_includesCPU(t *testing.T) {
 func Test_PostSupervisor_Benchmark(t *testing.T) {
 	log := zaptest.NewLogger(t)
 
-	cmdCfg := DefaultTestPostServiceConfig()
 	postCfg := DefaultPostConfig()
 	provingOpts := DefaultPostProvingOpts()
 
 	ctrl := gomock.NewController(t)
 	mgr := NewMockpostSetupProvider(ctrl)
 	builder := NewMockAtxBuilder(ctrl)
-	ps, err := NewPostSupervisor(log.Named("supervisor"), cmdCfg, postCfg, provingOpts, mgr, builder)
-	require.NoError(t, err)
+	ps := NewPostSupervisor(log.Named("supervisor"), postCfg, provingOpts, mgr, builder)
 
 	providers, err := ps.Providers()
 	require.NoError(t, err)

api/grpcserver/config.go

@@ -54,7 +54,7 @@ func DefaultConfig() Config {
 		PrivateServices:       []Service{Admin, Smesher, Debug, ActivationStreamV2Alpha1, RewardStreamV2Alpha1},
 		PrivateListener:       "127.0.0.1:9093",
 		PostServices:          []Service{Post, PostInfo},
-		PostListener:          "127.0.0.1:9094",
+		PostListener:          "127.0.0.1:0",
 		TLSServices:           []Service{Post, PostInfo},
 		TLSListener:           "",
 		JSONListener:          "",

api/grpcserver/grpc.go

@@ -79,6 +79,8 @@ func NewWithServices(
 	ip := net.ParseIP(host)
 	if host != "localhost" && !ip.IsPrivate() && !ip.IsLoopback() {
 		logger.Warn("unsecured grpc server is listening on a public IP address", zap.String("address", listener))
+	} else {
+		logger.Info("grpc server is listening on a private IP address", zap.String("address", listener))
 	}
 
 	server := New(listener, logger, config, grpcOpts...)
@@ -170,6 +172,7 @@ func (s *Server) Start() error {
 	}
 	s.BoundAddress = lis.Addr().String()
 	reflection.Register(s.GrpcServer)
+	s.logger.Info("bound to address", zap.String("address", s.BoundAddress))
 	s.grp.Go(func() error {
 		if err := s.GrpcServer.Serve(lis); err != nil {
 			s.logger.Error("serving grpc server", zap.Error(err))

api/grpcserver/grpcserver_test.go

@@ -562,9 +562,10 @@ func setupSmesherService(t *testing.T, sig *signing.EdSigner) (*smesherServiceCo
 		smeshingProvider,
 		postSupervisor,
 		10*time.Millisecond,
-		sig,
 		activation.DefaultPostSetupOpts(),
+		sig,
 	)
+	svc.SetPostServiceConfig(activation.DefaultTestPostServiceConfig())
 	cfg, cleanup := launchServer(t, svc)
 	t.Cleanup(cleanup)
 
@@ -584,7 +585,11 @@ func setupSmesherService(t *testing.T, sig *signing.EdSigner) (*smesherServiceCo
 func TestSmesherService(t *testing.T) {
 	t.Run("IsSmeshing", func(t *testing.T) {
 		t.Parallel()
-		c, ctx := setupSmesherService(t, nil)
+
+		sig, err := signing.NewEdSigner()
+		require.NoError(t, err)
+
+		c, ctx := setupSmesherService(t, sig)
 		c.smeshingProvider.EXPECT().Smeshing().Return(false)
 		res, err := c.IsSmeshing(ctx, &emptypb.Empty{})
 		require.NoError(t, err)
@@ -593,8 +598,12 @@ func TestSmesherService(t *testing.T) {
 
 	t.Run("StartSmeshingMissingArgs", func(t *testing.T) {
 		t.Parallel()
-		c, ctx := setupSmesherService(t, nil)
-		_, err := c.StartSmeshing(ctx, &pb.StartSmeshingRequest{})
+
+		sig, err := signing.NewEdSigner()
+		require.NoError(t, err)
+
+		c, ctx := setupSmesherService(t, sig)
+		_, err = c.StartSmeshing(ctx, &pb.StartSmeshingRequest{})
 		require.Equal(t, codes.InvalidArgument, status.Code(err))
 	})
 
@@ -611,15 +620,16 @@ func TestSmesherService(t *testing.T) {
 
 		c, ctx := setupSmesherService(t, sig)
 		c.smeshingProvider.EXPECT().StartSmeshing(gomock.Any()).Return(nil)
-		c.postSupervisor.EXPECT().Start(gomock.All(
-			gomock.Cond(func(postOpts any) bool { return postOpts.(activation.PostSetupOpts).DataDir == opts.DataDir }),
-			gomock.Cond(
-				func(postOpts any) bool { return postOpts.(activation.PostSetupOpts).NumUnits == opts.NumUnits },
-			),
-			gomock.Cond(
-				func(postOpts any) bool { return postOpts.(activation.PostSetupOpts).MaxFileSize == opts.MaxFileSize },
-			),
-		), sig).Return(nil)
+		c.postSupervisor.EXPECT().Start(gomock.Any(),
+			gomock.All(
+				gomock.Cond(func(postOpts any) bool { return postOpts.(activation.PostSetupOpts).DataDir == opts.DataDir }),
+				gomock.Cond(
+					func(postOpts any) bool { return postOpts.(activation.PostSetupOpts).NumUnits == opts.NumUnits },
+				),
+				gomock.Cond(
+					func(postOpts any) bool { return postOpts.(activation.PostSetupOpts).MaxFileSize == opts.MaxFileSize },
+				),
+			), sig).Return(nil)
 		res, err := c.StartSmeshing(ctx, &pb.StartSmeshingRequest{
 			Opts:     opts,
 			Coinbase: coinbase,

api/grpcserver/interface.go

@@ -63,7 +63,7 @@ type postState interface {
 }
 
 type postSupervisor interface {
-	Start(opts activation.PostSetupOpts, sig *signing.EdSigner) error
+	Start(cmdCfg activation.PostSupervisorConfig, opts activation.PostSetupOpts, sig *signing.EdSigner) error
 	Stop(deleteFiles bool) error
 
 	Config() activation.PostConfig