Skip to content

Commit 0a3f87e

Browse files
crosbymichaelcrosbymichael
authored
Merge pull request #2584 from jterry75/windows_r2_exec_fix
Fix incorrect ID usage in Windows runtime v2
2 parents 6b00aaa + e88ec1f commit 0a3f87e

File tree

1 file changed

+34
-34
lines changed

1 file changed

+34
-34
lines changed

‎runtime/v2/runhcs/service.go

+34-34
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)