Skip to content

Commit 7e95a5e

Browse files
committed
fix(snapshot-agent): return error from BuildDirectMemoryConfig on unparsable PID strings
When strconv.ParseInt fails for an entry in pidStrings, return an error rather than silently skipping the entry. Update BuildDirectMemoryConfig signature to return (*pb.BackendConfig, error) and propagate any config-building error in server Snapshot and Restore handlers. TAG=agy CONV=9fda09b9-fe6d-48bc-b524-2d232e0cdd4b Signed-off-by: edwinhr716 <edandres249@gmail.com>
1 parent 18323ee commit 7e95a5e

3 files changed

Lines changed: 25 additions & 7 deletions

File tree

pkg/snapshot-agent/backends/direct_memory.go

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -148,18 +148,20 @@ func ExtractDirectMemoryPIDStrings(config *pb.BackendConfig) []string {
148148
}
149149

150150
// BuildDirectMemoryConfig wraps PID strings into a DirectMemory BackendConfig.
151-
func BuildDirectMemoryConfig(pidStrings []string) *pb.BackendConfig {
151+
func BuildDirectMemoryConfig(pidStrings []string) (*pb.BackendConfig, error) {
152152
pids := make([]int32, 0, len(pidStrings))
153153
for _, s := range pidStrings {
154-
if pid, err := strconv.ParseInt(s, 10, 32); err == nil {
155-
pids = append(pids, int32(pid))
154+
pid, err := strconv.ParseInt(s, 10, 32)
155+
if err != nil {
156+
return nil, fmt.Errorf("invalid PID string %q: %w", s, err)
156157
}
158+
pids = append(pids, int32(pid))
157159
}
158160
return &pb.BackendConfig{
159161
Backend: &pb.BackendConfig_DirectMemory{
160162
DirectMemory: &pb.DirectMemoryBackendConfig{
161163
ExplicitTarget: &pb.ProcessTarget{Pids: pids},
162164
},
163165
},
164-
}
166+
}, nil
165167
}

pkg/snapshot-agent/backends/direct_memory_test.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,10 @@ func TestDirectMemoryHealthCheck(t *testing.T) {
195195

196196
func TestDirectMemoryConfigHelpers(t *testing.T) {
197197
pids := []string{"100", "200"}
198-
cfg := backends.BuildDirectMemoryConfig(pids)
198+
cfg, err := backends.BuildDirectMemoryConfig(pids)
199+
if err != nil {
200+
t.Fatalf("BuildDirectMemoryConfig() unexpected error: %v", err)
201+
}
199202
extracted := backends.ExtractDirectMemoryPIDStrings(cfg)
200203
if !reflect.DeepEqual(extracted, pids) {
201204
t.Errorf("ExtractDirectMemoryPIDStrings() = %v, want %v", extracted, pids)
@@ -204,4 +207,9 @@ func TestDirectMemoryConfigHelpers(t *testing.T) {
204207
if len(backends.ExtractDirectMemoryPIDStrings(nil)) != 0 {
205208
t.Errorf("Expected nil when extracting from nil config")
206209
}
210+
211+
_, err = backends.BuildDirectMemoryConfig([]string{"100", "not-a-pid"})
212+
if err == nil {
213+
t.Errorf("BuildDirectMemoryConfig() expected error for invalid PID string, got nil")
214+
}
207215
}

pkg/snapshot-agent/server/server.go

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -162,11 +162,15 @@ func (s *Server) buildSnapshotFn(
162162
return pidErr
163163
}
164164
var reqConfig *pb.BackendConfig
165+
var cfgErr error
165166
if backendType == backends.BackendDirectMemory {
166-
reqConfig = backends.BuildDirectMemoryConfig(allPIDStrings)
167+
reqConfig, cfgErr = backends.BuildDirectMemoryConfig(allPIDStrings)
167168
} else {
168169
reqConfig = backends.BuildCudaConfig(allPIDStrings)
169170
}
171+
if cfgErr != nil {
172+
return fmt.Errorf("failed to build backend config for job %s: %w", jobID, cfgErr)
173+
}
170174
req := backends.Request{JobID: jobID, Config: reqConfig}
171175
if err := backend.Snapshot(bgCtx, req); err != nil {
172176
return fmt.Errorf("failed to snapshot job %s: %w", jobID, err)
@@ -241,11 +245,15 @@ func (s *Server) buildRestoreFn(
241245
}
242246
slog.InfoContext(bgCtx, "Restoring PIDs", "pids", pidStrings, "backend", backendType)
243247
var reqConfig *pb.BackendConfig
248+
var cfgErr error
244249
if backendType == backends.BackendDirectMemory {
245-
reqConfig = backends.BuildDirectMemoryConfig(pidStrings)
250+
reqConfig, cfgErr = backends.BuildDirectMemoryConfig(pidStrings)
246251
} else {
247252
reqConfig = backends.BuildCudaConfig(pidStrings)
248253
}
254+
if cfgErr != nil {
255+
return fmt.Errorf("failed to build backend config for job %s: %w", jobID, cfgErr)
256+
}
249257
return backend.Restore(bgCtx, backends.Request{JobID: jobID, Config: reqConfig})
250258
}, nil
251259
case backends.BackendAppEndpoint, backends.BackendAppChannel:

0 commit comments

Comments
 (0)