Skip to content

fix(cube-master): panic if http server fails to start #127

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
yyyang91 wants to merge 3 commits into
base: master
Choose a base branch
from
+2 −2
Open

fix(cube-master): panic if http server fails to start #127

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9e7ebd3
fix(cube-master): improve port check logic and add startup/failure logs
yyyang91 Apr 30, 2026
fa84318
Revert "fix(cube-master): improve port check logic and add startup/fa…
yyyang91 May 7, 2026
7e241e0
fix(cube-master): panic if http server fails to start
yyyang91 May 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions CubeMaster/cmd/cubemaster/app/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import (
"github.com/tencentcloud/CubeSandbox/CubeMaster/pkg/service/sandbox"
"github.com/tencentcloud/CubeSandbox/CubeMaster/pkg/task"
"github.com/tencentcloud/CubeSandbox/CubeMaster/pkg/templatecenter"
"github.com/tencentcloud/CubeSandbox/cubelog"
CubeLog "github.com/tencentcloud/CubeSandbox/cubelog"
)

type App struct {
Expand Down Expand Up @@ -105,7 +105,7 @@ func (a *App) Run() {
case serverC <- serverTmp:
}

done := handleSignals(ctx, signals, serverC, cancel)
done := handleSignals(ctx, signals, serverC, serverTmp.ErrChan, cancel)

signal.Notify(signals, handledSignals...)

Expand Down
11 changes: 10 additions & 1 deletion CubeMaster/cmd/cubemaster/app/singal.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,14 @@ package app
import (
"context"
"fmt"
stdlog "log"
"os"
"path/filepath"
"runtime"
"time"

"github.com/tencentcloud/CubeSandbox/CubeMaster/pkg/server"
"github.com/tencentcloud/CubeSandbox/cubelog"
CubeLog "github.com/tencentcloud/CubeSandbox/cubelog"
"golang.org/x/sys/unix"
)

Expand All @@ -25,6 +26,7 @@ var handledSignals = []os.Signal{
}

func handleSignals(ctx context.Context, signals chan os.Signal, serverC chan *server.Server,
serverErr chan error,
cancelOutside func()) chan struct{} {
done := make(chan struct{}, 1)
go func() {
Expand Down Expand Up @@ -52,6 +54,13 @@ func handleSignals(ctx context.Context, signals chan os.Signal, serverC chan *se
close(done)
return
}
case err := <-serverErr:
stdlog.Printf("server error: %v", err)
CubeLog.WithContext(ctx).Errorf("server error: %v", err)
cancelOutside()
graceFullStop()
close(done)
return
}
}
}()
Expand Down
33 changes: 25 additions & 8 deletions CubeMaster/pkg/server/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ package server
import (
"context"
"fmt"
stdlog "log"
"net"
"net/http"
"os"
"sync"
Expand All @@ -23,18 +25,19 @@ import (
metahttp "github.com/tencentcloud/CubeSandbox/CubeMaster/pkg/service/httpservice/meta"
"github.com/tencentcloud/CubeSandbox/CubeMaster/pkg/service/httpservice/middleware"
"github.com/tencentcloud/CubeSandbox/CubeMaster/pkg/service/httpservice/notify"
"github.com/tencentcloud/CubeSandbox/cubelog"
CubeLog "github.com/tencentcloud/CubeSandbox/cubelog"
)

type Server struct {
InternalHttpServer *internalHttp
ErrChan chan error
}

func New(ctx context.Context, cfg *config.Config) (*Server, error) {
if cfg == nil || cfg.Common == nil {
return nil, errors.New("config is nil")
}
s := &Server{}
s := &Server{ErrChan: make(chan error, 1)}
var err error
s.InternalHttpServer, err = NewInternalHttp(ctx, cfg)
if err != nil {
Expand Down Expand Up @@ -111,20 +114,34 @@ func (s *internalHttp) registerHandlers() {
}

func (s *internalHttp) Start() error {
if err := s.ListenAndServe(); err != nil {
if err == http.ErrServerClosed {
return nil
}
return errors.WithStack(err)
addr := s.Addr
if addr == "" {
addr = ":http"
}

ln, err := net.Listen("tcp", addr)
Copy link
Copy Markdown
Collaborator

@chenhengqi chenhengqi May 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this change work with cubemastercli?

Copy link
Copy Markdown
Author

@yyyang91 yyyang91 May 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This logic is ported from the Go standard library (net/http/server.go, v1.24.8).

I migrated the original implementation here to gain more granular control over the startup sequence. Specifically, it allows us to inject the startup/failure logs and port check logic directly into the serving loop, which addresses the observability requirements for cube-master.

Copy link
Copy Markdown
Collaborator

@chenhengqi chenhengqi May 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You did not answer my question.

Copy link
Copy Markdown
Author

@yyyang91 yyyang91 May 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it works

Copy link
Copy Markdown
Collaborator

@chenhengqi chenhengqi May 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the real issue is that the requested port is in use, we can simply panic here.

Copy link
Copy Markdown
Author

@yyyang91 yyyang91 May 7, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that a panic only terminates the program, while a non-panic means the process is still running. To speed up troubleshooting, it's necessary to have logs indicating whether the service started successfully. Should we write success and error messages to `/var/log/cube-sandbox-one-click/cubemaster.log? This is because when the installation script encounters an error, it prompts us to check the files in the directory.

Copy link
Copy Markdown
Collaborator

@chenhengqi chenhengqi May 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Panic will log to stderr and should be captured, no?

Copy link
Copy Markdown
Author

@yyyang91 yyyang91 May 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Panic will log to stderr and can be captured. but it will print the stack. Is it ok to print the stack?

if err != nil {
return err
}
return nil

actualAddr := ln.Addr().String()
CubeLog.Infof("cube-master listening on %s", actualAddr)
stdlog.Printf("cube-master listening on %s", actualAddr)

err = s.Serve(ln)
if err == http.ErrServerClosed {
return nil
}

return errors.WithStack(err)
}

func (s *Server) Run() {
if s.InternalHttpServer != nil {
go func() {
if err := s.InternalHttpServer.Start(); err != nil {
CubeLog.Errorf("ListenAndServe:%v", err)
s.ErrChan <- err
}
}()
}
Expand Down