Skip to content

Commit e88ec1f

Browse files
jterry75jterry75
committed
Fix incorrect ID usage in Windows runtime v2
Sometimes the wrong ID was being used because its not correct to assume that ExecID is always set. The assumption was that for API's where it is not an exec ID == ExecID but thats not true. ExecID == "" if it is not an exec. This uses the correct ID in all cases. Signed-off-by: Justin Terry (VM) <[email protected]>
1 parent 6b00aaa commit e88ec1f

1 file changed

Lines changed: 34 additions & 34 deletions

File tree

‎runtime/v2/runhcs/service.go‎

Lines changed: 34 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -55,19 +55,12 @@ const (
5555
)
5656

5757
var (
58-
empty = &ptypes.Empty{}
59-
runhcsDebug = false
58+
empty = &ptypes.Empty{}
6059
)
6160

62-
func init() {
63-
if logrus.GetLevel() == logrus.DebugLevel {
64-
runhcsDebug = true
65-
}
66-
}
67-
6861
func newRunhcs(bundle string) *runhcs.Runhcs {
6962
rhs := &runhcs.Runhcs{
70-
Debug: runhcsDebug,
63+
Debug: logrus.GetLevel() == logrus.DebugLevel,
7164
LogFormat: runhcs.JSON,
7265
Owner: "containerd-runhcs-shim-v1",
7366
}
@@ -104,16 +97,20 @@ type service struct {
10497

10598
// getProcess attempts to get a process by id.
10699
// The caller MUST NOT have locked s.mu previous to calling this function.
107-
func (s *service) getProcess(id string) (*process, error) {
100+
func (s *service) getProcess(id, execID string) (*process, error) {
108101
s.mu.Lock()
109102
defer s.mu.Unlock()
110103

111-
return s.getProcessLocked(id)
104+
return s.getProcessLocked(id, execID)
112105
}
113106

114107
// getProcessLocked attempts to get a process by id.
115108
// The caller MUST protect s.mu previous to calling this function.
116-
func (s *service) getProcessLocked(id string) (*process, error) {
109+
func (s *service) getProcessLocked(id, execID string) (*process, error) {
110+
if execID != "" {
111+
id = execID
112+
}
113+
117114
var p *process
118115
var ok bool
119116
if p, ok = s.processes[id]; !ok {
@@ -200,11 +197,11 @@ func (s *service) State(ctx context.Context, r *taskAPI.StateRequest) (*taskAPI.
200197

201198
var p *process
202199
var err error
203-
if p, err = s.getProcessLocked(r.ID); err != nil {
200+
if p, err = s.getProcessLocked(r.ID, r.ExecID); err != nil {
204201
return nil, err
205202
}
206203

207-
cmd := exec.Command(runhcsBinary, runhcsDebugLegacy, "state", r.ID)
204+
cmd := exec.Command(runhcsBinary, runhcsDebugLegacy, "state", p.id)
208205
sout := getBuffer()
209206
defer putBuffer(sout)
210207

@@ -255,7 +252,7 @@ func (s *service) State(ctx context.Context, r *taskAPI.StateRequest) (*taskAPI.
255252
}
256253
pe := p.stat()
257254
return &taskAPI.StateResponse{
258-
ID: r.ID,
255+
ID: p.id,
259256
Bundle: cs.Bundle,
260257
Pid: uint32(cs.InitProcessPid),
261258
Status: status,
@@ -330,6 +327,8 @@ func writeMountsToConfig(bundle string, mounts []*containerd_types.Mount) error
330327

331328
// Create a new initial process and container with runhcs
332329
func (s *service) Create(ctx context.Context, r *taskAPI.CreateTaskRequest) (*taskAPI.CreateTaskResponse, error) {
330+
log.G(ctx).Infof("Create: %s", r.ID)
331+
333332
// Hold the lock for the entire duration to avoid duplicate process creation.
334333
s.mu.Lock()
335334
defer s.mu.Unlock()
@@ -421,15 +420,7 @@ func (s *service) Start(ctx context.Context, r *taskAPI.StartRequest) (*taskAPI.
421420
log.G(ctx).Infof("Start: %s: %s", r.ID, r.ExecID)
422421
var p *process
423422
var err error
424-
425-
var id string
426-
if r.ExecID != "" {
427-
id = r.ExecID
428-
} else {
429-
id = r.ID
430-
}
431-
432-
if p, err = s.getProcess(id); err != nil {
423+
if p, err = s.getProcess(r.ID, r.ExecID); err != nil {
433424
return nil, err
434425
}
435426
p.Lock()
@@ -453,6 +444,7 @@ func (s *service) Start(ctx context.Context, r *taskAPI.StartRequest) (*taskAPI.
453444
Detach: true,
454445
}
455446

447+
// ID here is the containerID to exec the process in.
456448
err = rhcs.Exec(ctx, r.ID, procConfig, eopts)
457449
if err != nil {
458450
return nil, errors.Wrapf(err, "failed to exec process: %s", r.ExecID)
@@ -507,9 +499,11 @@ func (s *service) Start(ctx context.Context, r *taskAPI.StartRequest) (*taskAPI.
507499

508500
// Delete the initial process and container
509501
func (s *service) Delete(ctx context.Context, r *taskAPI.DeleteRequest) (*taskAPI.DeleteResponse, error) {
502+
log.G(ctx).Infof("Delete: %s: %s", r.ID, r.ExecID)
503+
510504
var p *process
511505
var err error
512-
if p, err = s.getProcess(r.ExecID); err != nil {
506+
if p, err = s.getProcess(r.ID, r.ExecID); err != nil {
513507
return nil, err
514508
}
515509

@@ -532,7 +526,7 @@ func (s *service) Delete(ctx context.Context, r *taskAPI.DeleteRequest) (*taskAP
532526
exit := p.stat()
533527

534528
s.mu.Lock()
535-
delete(s.processes, r.ID)
529+
delete(s.processes, p.id)
536530
s.mu.Unlock()
537531
return &taskAPI.DeleteResponse{
538532
ExitedAt: exit.exitedAt,
@@ -549,7 +543,7 @@ func (s *service) Pids(ctx context.Context, r *taskAPI.PidsRequest) (*taskAPI.Pi
549543
// Pause the container
550544
func (s *service) Pause(ctx context.Context, r *taskAPI.PauseRequest) (*ptypes.Empty, error) {
551545
// TODO: Validate that 'id' is actually a valid parent container ID
552-
if _, err := s.getProcess(r.ID); err != nil {
546+
if _, err := s.getProcess(r.ID, ""); err != nil {
553547
return nil, err
554548
}
555549

@@ -565,7 +559,7 @@ func (s *service) Pause(ctx context.Context, r *taskAPI.PauseRequest) (*ptypes.E
565559
// Resume the container
566560
func (s *service) Resume(ctx context.Context, r *taskAPI.ResumeRequest) (*ptypes.Empty, error) {
567561
// TODO: Validate that 'id' is actually a valid parent container ID
568-
if _, err := s.getProcess(r.ID); err != nil {
562+
if _, err := s.getProcess(r.ID, ""); err != nil {
569563
return nil, err
570564
}
571565

@@ -585,9 +579,11 @@ func (s *service) Checkpoint(ctx context.Context, r *taskAPI.CheckpointTaskReque
585579

586580
// Kill a process with the provided signal
587581
func (s *service) Kill(ctx context.Context, r *taskAPI.KillRequest) (*ptypes.Empty, error) {
582+
log.G(ctx).Infof("Kill: %s: %s", r.ID, r.ExecID)
583+
588584
var p *process
589585
var err error
590-
if p, err = s.getProcess(r.ExecID); err != nil {
586+
if p, err = s.getProcess(r.ID, r.ExecID); err != nil {
591587
return nil, err
592588
}
593589

@@ -605,13 +601,15 @@ func (s *service) Kill(ctx context.Context, r *taskAPI.KillRequest) (*ptypes.Emp
605601

606602
// Exec an additional process inside the container
607603
func (s *service) Exec(ctx context.Context, r *taskAPI.ExecProcessRequest) (*ptypes.Empty, error) {
604+
log.G(ctx).Infof("Exec: %s: %s", r.ID, r.ExecID)
605+
608606
s.mu.Lock()
609607
defer s.mu.Unlock()
610608

611609
var parent *process
612610
var err error
613611
// Get the parent container
614-
if parent, err = s.getProcessLocked(r.ID); err != nil {
612+
if parent, err = s.getProcessLocked(r.ID, ""); err != nil {
615613
return nil, err
616614
}
617615

@@ -688,15 +686,15 @@ func (s *service) Exec(ctx context.Context, r *taskAPI.ExecProcessRequest) (*pty
688686
func (s *service) ResizePty(ctx context.Context, r *taskAPI.ResizePtyRequest) (*ptypes.Empty, error) {
689687
var p *process
690688
var err error
691-
if p, err = s.getProcess(r.ExecID); err != nil {
689+
if p, err = s.getProcess(r.ID, r.ExecID); err != nil {
692690
return nil, err
693691
}
694692

695693
cmd := exec.Command(
696694
runhcsBinary,
697695
runhcsDebugLegacy,
698696
"resize-tty",
699-
r.ID,
697+
p.cid,
700698
"-p",
701699
strconv.FormatUint(uint64(p.pid), 10),
702700
strconv.FormatUint(uint64(r.Width), 10),
@@ -714,7 +712,7 @@ func (s *service) ResizePty(ctx context.Context, r *taskAPI.ResizePtyRequest) (*
714712
func (s *service) CloseIO(ctx context.Context, r *taskAPI.CloseIORequest) (*ptypes.Empty, error) {
715713
var p *process
716714
var err error
717-
if p, err = s.getProcess(r.ExecID); err != nil {
715+
if p, err = s.getProcess(r.ID, r.ExecID); err != nil {
718716
return nil, err
719717
}
720718
p.closeIO()
@@ -730,7 +728,7 @@ func (s *service) Update(ctx context.Context, r *taskAPI.UpdateTaskRequest) (*pt
730728
func (s *service) Wait(ctx context.Context, r *taskAPI.WaitRequest) (*taskAPI.WaitResponse, error) {
731729
var p *process
732730
var err error
733-
if p, err = s.getProcess(r.ExecID); err != nil {
731+
if p, err = s.getProcess(r.ID, r.ExecID); err != nil {
734732
return nil, err
735733
}
736734
pe := p.wait()
@@ -756,6 +754,8 @@ func (s *service) Connect(ctx context.Context, r *taskAPI.ConnectRequest) (*task
756754

757755
// Shutdown stops this instance of the runhcs shim
758756
func (s *service) Shutdown(ctx context.Context, r *taskAPI.ShutdownRequest) (*ptypes.Empty, error) {
757+
log.G(ctx).Infof("Shutdown: %s", r.ID)
758+
759759
os.Exit(0)
760760
return empty, nil
761761
}

0 commit comments

Comments
 (0)