Skip to content

fix: add nil checks for req.Filters to prevent panic on empty request… #109

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
CN-Antonio wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: add nil checks for req.Filters to prevent panic on empty request… #109

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 5 additions & 0 deletions server/internal/biz/node.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ type NodeRepo interface {
GetNode(context.Context, string) (*Node, error)
ListAllDevices(context.Context) ([]*DeviceInfo, error)
FindDeviceByAliasId(string) (*DeviceInfo, error)
Ready() bool
}

type NodeUsecase struct {
Expand All @@ -77,3 +78,7 @@ func (uc *NodeUsecase) ListAllDevices(ctx context.Context) ([]*DeviceInfo, error
func (uc *NodeUsecase) FindDeviceByAliasId(aliasId string) (*DeviceInfo, error) {
return uc.repo.FindDeviceByAliasId(aliasId)
}

func (uc *NodeUsecase) Ready() bool {
return uc.repo.Ready()
}
5 changes: 5 additions & 0 deletions server/internal/biz/pod.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ type PodInfo struct {
type PodRepo interface {
ListAll(context.Context) ([]*Container, error)
FindOne(context.Context, string, string) (*Container, error)
Ready() bool
}

type PodUseCase struct {
Expand Down Expand Up @@ -72,6 +73,10 @@ func (uc *PodUseCase) ListAll(ctx context.Context) ([]*Container, error) {
return uc.repo.ListAll(ctx)
}

func (uc *PodUseCase) Ready() bool {
return uc.repo.Ready()
}

func ContainersStatisticsInfo(containers []*Container, deviceId string) (int32, int32, int32) {
var vGPU int32 = 0
var core int32 = 0
Expand Down
32 changes: 31 additions & 1 deletion server/internal/data/node.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ type nodeRepo struct {
log *log.Helper
mutex sync.RWMutex
providers []provider.Provider
ready bool

@coderabbitai coderabbitai Bot Jul 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

nodeRepo.Ready() should stay false when cache sync fails. syncedOK is only used for logging in init(), while Ready() still flips on when updateLocalNodes populates data. That can expose /readyz as ready even after a failed informer sync; gate readiness on sync success as well.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@server/internal/data/node.go` at line 33, The readiness state in nodeRepo is
being set from local cache population even when informer sync fails, so /readyz
can become true too early. Update the nodeRepo readiness logic around init(),
Ready(), and updateLocalNodes so readiness is only enabled when the initial sync
succeeds, not just when data is populated. Use the existing syncedOK handling in
nodeRepo to gate Ready() and keep ready false if cache sync fails.

}

// NewNodeRepo .
Expand Down Expand Up @@ -100,6 +101,16 @@ func (r *nodeRepo) updateLocalNodes() {
}
r.mutex.Lock()
r.nodes = n
if !r.ready {
r.ready = true
r.log.Infof("node data initialized: %d nodes, %d devices", len(n), func() int {
count := 0
for _, node := range n {
count += len(node.Devices)
}
return count
}())
}
r.mutex.Unlock()
}
}
Expand All @@ -116,7 +127,26 @@ func (r *nodeRepo) init() {
})
stopCh := make(chan struct{})
informerFactory.Start(stopCh)
informerFactory.WaitForCacheSync(stopCh)
synced := informerFactory.WaitForCacheSync(stopCh)
syncedOK := true
for _, ok := range synced {
if !ok {
syncedOK = false
break
}
}
if syncedOK {
r.log.Info("node informer synced successfully")
} else {
r.log.Warn("node informer failed to sync, data may be incomplete")
}
}

// Ready returns true if the node informer has successfully synced and populated data.
func (r *nodeRepo) Ready() bool {
r.mutex.RLock()
defer r.mutex.RUnlock()
return r.ready
}

func (r *nodeRepo) onAddNode(obj interface{}) {
Expand Down
25 changes: 24 additions & 1 deletion server/internal/data/pod.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ type podRepo struct {
pods map[k8stypes.UID]*biz.PodInfo
mutex sync.RWMutex
log *log.Helper
ready bool
}

func NewPodRepo(data *Data, logger log.Logger) biz.PodRepo {
Expand All @@ -47,7 +48,29 @@ func (r *podRepo) init() {
})
stopCh := make(chan struct{})
informerFactory.Start(stopCh)
informerFactory.WaitForCacheSync(stopCh)
synced := informerFactory.WaitForCacheSync(stopCh)
syncedOK := true
for _, ok := range synced {
if !ok {
syncedOK = false
break
}
}
if syncedOK {
r.mutex.Lock()
r.ready = true
r.mutex.Unlock()
r.log.Info("pod informer synced successfully")
} else {
r.log.Warn("pod informer failed to sync, data may be incomplete")
}
}

// Ready returns true if the pod informer has successfully synced.
func (r *podRepo) Ready() bool {
r.mutex.RLock()
defer r.mutex.RUnlock()
return r.ready
}

func (r *podRepo) onAddPod(obj interface{}) {
Expand Down
11 changes: 7 additions & 4 deletions server/internal/server/http.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,13 @@ func NewHTTPServer(c *conf.Bootstrap,
srv.HandlePrefix("/q/", openapiv2.NewHandler())
srv.Handle("/metrics", promhttp.Handler())
srv.HandleFunc("/readyz", func(w http.ResponseWriter, r *http.Request) {
// Reaching this handler means the server is listening and the informer
// caches have already synced (the backend only starts serving after that).
w.WriteHeader(nethttp.StatusOK)
_, _ = w.Write([]byte("ok"))
if node.Ready() {
w.WriteHeader(nethttp.StatusOK)
_, _ = w.Write([]byte("ok"))
} else {
w.WriteHeader(nethttp.StatusServiceUnavailable)
_, _ = w.Write([]byte("not ready"))
}
})
return srv
}
19 changes: 17 additions & 2 deletions server/internal/service/card.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,13 @@ func NewCardService(node *biz.NodeUsecase, pod *biz.PodUseCase, ms *MonitorServi
}

func (s *CardService) GetAllGPUs(ctx context.Context, req *pb.GetAllGpusReq) (*pb.GPUsReply, error) {
filters := req.Filters
var filters *pb.GetAllGpusReq_Filters
if req != nil {
filters = req.Filters
}
if filters == nil {
filters = &pb.GetAllGpusReq_Filters{}
}
deviceInfos, err := s.node.ListAllDevices(ctx)
if err != nil {
return nil, err
Expand Down Expand Up @@ -115,7 +121,13 @@ func (s *CardService) GetAllGPUTypes(ctx context.Context, req *pb.GetAllGpusReq)
var res = &pb.GPUsReply{List: []*pb.GPUReply{}}
seenTypes := make(map[string]struct{})

filters := req.Filters
var filters *pb.GetAllGpusReq_Filters
if req != nil {
filters = req.Filters
}
if filters == nil {
filters = &pb.GetAllGpusReq_Filters{}
}
provider := strings.Trim(filters.Provider, " ")
for _, device := range deviceInfos {
if provider != "" && provider != device.Provider {
Expand All @@ -134,6 +146,9 @@ func (s *CardService) GetAllGPUTypes(ctx context.Context, req *pb.GetAllGpusReq)
}

func (s *CardService) GetGPU(ctx context.Context, req *pb.GetGpuReq) (*pb.GPUReply, error) {
if req == nil {
return &pb.GPUReply{}, nil
}
devices, err := s.node.ListAllDevices(ctx)
if err != nil {
return nil, err
Expand Down
8 changes: 7 additions & 1 deletion server/internal/service/container.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,13 @@ func uniqueNonEmpty(values []string) []string {
}

func (s *ContainerService) GetAllContainers(ctx context.Context, req *pb.GetAllContainersReq) (*pb.ContainersReply, error) {
filters := req.Filters
var filters *pb.GetAllContainersReq_Filters
if req != nil {
filters = req.Filters
}
if filters == nil {
filters = &pb.GetAllContainersReq_Filters{}
}
containers, err := s.pod.ListAllContainers(ctx)
if err != nil {
return nil, err
Expand Down
24 changes: 22 additions & 2 deletions server/internal/service/node.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,16 +26,33 @@ func NewNodeService(uc *biz.NodeUsecase, pod *biz.PodUseCase, summary *biz.Summa
return &NodeService{uc: uc, pod: pod, summary: summary, ms: ms}
}

// Ready returns true if both node and pod informers have synced.
func (s *NodeService) Ready() bool {
return s.uc.Ready() && s.pod.Ready()
}

func (s *NodeService) GetSummary(ctx context.Context, req *pb.GetSummaryReq) (*pb.DeviceSummaryReply, error) {
filters := req.Filters
var filters *pb.GetSummaryReq_Filters
if req != nil {
filters = req.Filters
}
var res = &pb.DeviceSummaryReply{}
if filters == nil {
filters = &pb.GetSummaryReq_Filters{}
}
t, err := s.summary.GetGPUSummary(ctx, filters.DeviceId, filters.NodeUid, filters.Type)
copier.Copy(&res, &t)
return res, err
}

func (s *NodeService) GetAllNodes(ctx context.Context, req *pb.GetAllNodesReq) (*pb.NodesReply, error) {
filters := req.Filters
var filters *pb.GetAllNodesReq_Filters
if req != nil {
filters = req.Filters
}
if filters == nil {
filters = &pb.GetAllNodesReq_Filters{}
}
nodes, err := s.uc.ListAllNodes(ctx)
if err != nil {
return nil, err
Expand Down Expand Up @@ -88,6 +105,9 @@ func (s *NodeService) GetAllNodes(ctx context.Context, req *pb.GetAllNodesReq) (
}

func (s *NodeService) GetNode(ctx context.Context, req *pb.GetNodeReq) (*pb.NodeReply, error) {
if req == nil || req.Uid == "" {
return &pb.NodeReply{}, nil
}
node, err := s.uc.GetNode(ctx, req.Uid)
if err != nil {
return nil, err
Expand Down