Skip to content
This repository has been archived by the owner on Jul 28, 2021. It is now read-only.

Adds console resize support. #84

Merged
merged 5 commits into from
Aug 8, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 3 additions & 3 deletions service/gcs/bridge/bridge.go
Original file line number Diff line number Diff line change
Expand Up @@ -355,8 +355,6 @@ func (b *bridge) waitOnProcess(message []byte, header *prot.MessageHeader) (*pro
return response, nil
}

// resizeConsole is currently a nop until the functionality is implemented.
// TODO: Tests still need to be written when it's no longer a nop.
func (b *bridge) resizeConsole(message []byte) (*prot.MessageResponseBase, error) {
response := newResponseBase()
var request prot.ContainerResizeConsole
Expand All @@ -365,7 +363,9 @@ func (b *bridge) resizeConsole(message []byte) (*prot.MessageResponseBase, error
}
response.ActivityID = request.ActivityID

// NOP
if err := b.coreint.ResizeConsole(int(request.ProcessID), request.Height, request.Width); err != nil {
return response, err
}

return response, nil
}
Expand Down
10 changes: 5 additions & 5 deletions service/gcs/bridge/bridge_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -587,7 +587,7 @@ var _ = Describe("Bridge", func() {
Describe("calling resizeConsole", func() {
var (
response prot.MessageResponseBase
//callArgs mockcore.ResizeConsoleCall
callArgs mockcore.ResizeConsoleCall
)
BeforeEach(func() {
messageType = prot.ComputeSystemResizeConsoleV1
Expand All @@ -597,7 +597,7 @@ var _ = Describe("Bridge", func() {
err := json.Unmarshal([]byte(responseString), &response)
Expect(err).NotTo(HaveOccurred())
responseBase = &response
//callArgs = coreint.LastResizeConsole
callArgs = coreint.LastResizeConsole
})
Context("the message is normal ASCII", func() {
BeforeEach(func() {
Expand All @@ -613,10 +613,10 @@ var _ = Describe("Bridge", func() {
})
AssertNoResponseErrors()
AssertActivityIDCorrect()
// TODO: Add tests on callArgs when resizing the console is
// implemented.
It("should receive the correct values", func() {
//e.g. Expect(callArgs.Pid).To(Equal(101))
Expect(callArgs.Pid).To(Equal(101))
Expect(callArgs.Height).To(Equal(uint16(30)))
Expect(callArgs.Width).To(Equal(uint16(72)))
})
})
})
Expand Down
1 change: 1 addition & 0 deletions service/gcs/core/core.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,5 @@ type Core interface {
ModifySettings(id string, request prot.ResourceModificationRequestResponse) error
RegisterContainerExitHook(id string, onExit func(oslayer.ProcessExitState)) error
RegisterProcessExitHook(pid int, onExit func(oslayer.ProcessExitState)) error
ResizeConsole(pid int, height, width uint16) error
}
98 changes: 48 additions & 50 deletions service/gcs/core/gcs/gcs.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@ import (
"github.com/Microsoft/opengcs/service/gcs/prot"
"github.com/Microsoft/opengcs/service/gcs/runtime"
"github.com/Microsoft/opengcs/service/gcs/stdio"
"github.com/sirupsen/logrus"
shellwords "github.com/mattn/go-shellwords"
oci "github.com/opencontainers/runtime-spec/specs-go"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
)

// gcsCore is an implementation of the Core interface, defining the
Expand All @@ -38,39 +38,31 @@ type gcsCore struct {
containerCache map[string]*containerCacheEntry

processCacheMutex sync.RWMutex
// processCache stores information about processes which persists between
// calls into the gcsCore. It is structured as a map from pid to cache
// entry.
// processCache stores information about processes which persists between calls
// into the gcsCore. It is structured as a map from pid to cache entry.
processCache map[int]*processCacheEntry

externalProcessCacheMutex sync.RWMutex
// externalProcessCache stores information about external processes which
// persists between calls into the gcsCore. It is structured as a map from
// pid to cache entry.
externalProcessCache map[int]*processCacheEntry
}

// NewGCSCore creates a new gcsCore struct initialized with the given Runtime.
func NewGCSCore(rtime runtime.Runtime, os oslayer.OS) *gcsCore {
return &gcsCore{
Rtime: rtime,
OS: os,
containerCache: make(map[string]*containerCacheEntry),
processCache: make(map[int]*processCacheEntry),
externalProcessCache: make(map[int]*processCacheEntry),
Rtime: rtime,
OS: os,
containerCache: make(map[string]*containerCacheEntry),
processCache: make(map[int]*processCacheEntry),
}
}

// containerCacheEntry stores cached information for a single container.
type containerCacheEntry struct {
ID string
ExitStatus oslayer.ProcessExitState
Processes []int
ExitHooks []func(oslayer.ProcessExitState)
MappedVirtualDisks map[uint8]prot.MappedVirtualDisk
MappedDirectories map[uint32]prot.MappedDirectory
NetworkAdapters []prot.NetworkAdapter
container runtime.Container
hasRunInitProcess bool
}

func newContainerCacheEntry(id string) *containerCacheEntry {
Expand All @@ -83,9 +75,6 @@ func newContainerCacheEntry(id string) *containerCacheEntry {
func (e *containerCacheEntry) AddExitHook(hook func(oslayer.ProcessExitState)) {
e.ExitHooks = append(e.ExitHooks, hook)
}
func (e *containerCacheEntry) AddProcess(pid int) {
e.Processes = append(e.Processes, pid)
}
func (e *containerCacheEntry) AddNetworkAdapter(adapter prot.NetworkAdapter) {
e.NetworkAdapters = append(e.NetworkAdapters, adapter)
}
Expand Down Expand Up @@ -120,12 +109,14 @@ func (e *containerCacheEntry) RemoveMappedDirectory(dir prot.MappedDirectory) {

// processCacheEntry stores cached information for a single process.
type processCacheEntry struct {
ExitStatus oslayer.ProcessExitState
ExitHooks []func(oslayer.ProcessExitState)
ExitStatus oslayer.ProcessExitState
ExitHooks []func(oslayer.ProcessExitState)
Tty *stdio.TtyRelay
ContainerID string // If "" a host process otherwise a container process.
}

func newProcessCacheEntry() *processCacheEntry {
return &processCacheEntry{}
func newProcessCacheEntry(containerID string) *processCacheEntry {
return &processCacheEntry{ContainerID: containerID}
}
func (e *processCacheEntry) AddExitHook(hook func(oslayer.ProcessExitState)) {
e.ExitHooks = append(e.ExitHooks, hook)
Expand Down Expand Up @@ -203,12 +194,11 @@ func (c *gcsCore) ExecProcess(id string, params prot.ProcessParameters, stdioSet
if containerEntry == nil {
return -1, errors.WithStack(gcserr.NewContainerDoesNotExistError(id))
}
processEntry := newProcessCacheEntry()
processEntry := newProcessCacheEntry(id)

var p runtime.Process

isInitProcess := len(containerEntry.Processes) == 0
if isInitProcess {
if !containerEntry.hasRunInitProcess {
containerEntry.hasRunInitProcess = true
if err := c.writeConfigFile(id, params.OCISpecification); err != nil {
return -1, err
}
Expand All @@ -220,6 +210,7 @@ func (c *gcsCore) ExecProcess(id string, params prot.ProcessParameters, stdioSet

containerEntry.container = container
p = container
processEntry.Tty = p.Tty()

// Configure network adapters in the namespace.
for _, adapter := range containerEntry.NetworkAdapters {
Expand Down Expand Up @@ -271,6 +262,7 @@ func (c *gcsCore) ExecProcess(id string, params prot.ProcessParameters, stdioSet
if err != nil {
return -1, err
}
processEntry.Tty = p.Tty()

go func() {
state, err := p.Wait()
Expand Down Expand Up @@ -306,7 +298,6 @@ func (c *gcsCore) ExecProcess(id string, params prot.ProcessParameters, stdioSet
// applies to external processes as well.
c.processCache[p.Pid()] = processEntry
c.processCacheMutex.Unlock()
containerEntry.AddProcess(p.Pid())
return p.Pid(), nil
}

Expand All @@ -332,16 +323,11 @@ func (c *gcsCore) SignalContainer(id string, signal oslayer.Signal) error {
// SignalProcess sends the signal specified in options to the given process.
func (c *gcsCore) SignalProcess(pid int, options prot.SignalProcessOptions) error {
c.processCacheMutex.Lock()
c.externalProcessCacheMutex.Lock()
if _, ok := c.processCache[pid]; !ok {
if _, ok := c.externalProcessCache[pid]; !ok {
c.processCacheMutex.Unlock()
c.externalProcessCacheMutex.Unlock()
return errors.WithStack(gcserr.NewProcessDoesNotExistError(pid))
}
c.processCacheMutex.Unlock()
return errors.WithStack(gcserr.NewProcessDoesNotExistError(pid))
}
c.processCacheMutex.Unlock()
c.externalProcessCacheMutex.Unlock()

// Interpret signal value 0 as SIGKILL.
// TODO: Remove this special casing when we are not worried about breaking
Expand Down Expand Up @@ -398,8 +384,8 @@ func (c *gcsCore) RunExternalProcess(params prot.ProcessParameters, stdioSet *st
if params.EmulateConsole {
// Allocate a console for the process.
var (
consolePath string
master *os.File
consolePath string
)
master, consolePath, err = stdio.NewConsole()
if err != nil {
Expand Down Expand Up @@ -440,7 +426,8 @@ func (c *gcsCore) RunExternalProcess(params prot.ProcessParameters, stdioSet *st
relay.Start()
}

processEntry := newProcessCacheEntry()
processEntry := newProcessCacheEntry("")
processEntry.Tty = relay
go func() {
if err := cmd.Wait(); err != nil {
// TODO: When cmd is a shell, and last command in the shell
Expand All @@ -458,18 +445,18 @@ func (c *gcsCore) RunExternalProcess(params prot.ProcessParameters, stdioSet *st

// Run exit hooks for the process.
state := cmd.ExitState()
c.externalProcessCacheMutex.Lock()
c.processCacheMutex.Lock()
processEntry.ExitStatus = state
for _, hook := range processEntry.ExitHooks {
hook(state)
}
c.externalProcessCacheMutex.Unlock()
c.processCacheMutex.Unlock()
}()

pid = cmd.Process().Pid()
c.externalProcessCacheMutex.Lock()
c.externalProcessCache[pid] = processEntry
c.externalProcessCacheMutex.Unlock()
c.processCacheMutex.Lock()
c.processCache[pid] = processEntry
c.processCacheMutex.Unlock()
return pid, nil
}

Expand Down Expand Up @@ -556,17 +543,11 @@ func (c *gcsCore) RegisterContainerExitHook(id string, exitHook func(oslayer.Pro
func (c *gcsCore) RegisterProcessExitHook(pid int, exitHook func(oslayer.ProcessExitState)) error {
c.processCacheMutex.Lock()
defer c.processCacheMutex.Unlock()
c.externalProcessCacheMutex.Lock()
defer c.externalProcessCacheMutex.Unlock()

var entry *processCacheEntry
var ok bool
entry, ok = c.processCache[pid]
if !ok {
entry, ok = c.externalProcessCache[pid]
if !ok {
return errors.WithStack(gcserr.NewProcessDoesNotExistError(pid))
}
if entry, ok = c.processCache[pid]; !ok {
return errors.WithStack(gcserr.NewProcessDoesNotExistError(pid))
}

exitStatus := entry.ExitStatus
Expand All @@ -580,6 +561,23 @@ func (c *gcsCore) RegisterProcessExitHook(pid int, exitHook func(oslayer.Process
return nil
}

func (c *gcsCore) ResizeConsole(pid int, height, width uint16) error {
c.processCacheMutex.Lock()
var p *processCacheEntry
var ok bool
if p, ok = c.processCache[pid]; !ok {
c.processCacheMutex.Unlock()
return errors.WithStack(gcserr.NewProcessDoesNotExistError(pid))
}
c.processCacheMutex.Unlock()

if p.Tty == nil {
return fmt.Errorf("pid: %d, is not a tty and cannot be resized", pid)
}

return p.Tty.ResizeConsole(height, width)
}

// setupMappedVirtualDisks is a helper function which calls into the functions
// in storage.go to set up a set of mapped virtual disks for a given container.
// It then adds them to the container's cache entry.
Expand Down
19 changes: 19 additions & 0 deletions service/gcs/core/mockcore/mockcore.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,13 @@ type RegisterProcessExitHookCall struct {
ExitHook func(oslayer.ProcessExitState)
}

// ResizeConsoleCall captures the arguments of ResizeConsole
type ResizeConsoleCall struct {
Pid int
Height uint16
Width uint16
}

// MockCore serves as an argument capture mechanism which implements the Core
// interface. Arguments passed to one of its methods are stored to be queried
// later.
Expand All @@ -78,6 +85,7 @@ type MockCore struct {
LastModifySettings ModifySettingsCall
LastRegisterContainerExitHook RegisterContainerExitHookCall
LastRegisterProcessExitHook RegisterProcessExitHookCall
LastResizeConsole ResizeConsoleCall
}

// CreateContainer captures its arguments and returns a nil error.
Expand Down Expand Up @@ -167,3 +175,14 @@ func (c *MockCore) RegisterProcessExitHook(pid int, exitHook func(oslayer.Proces
exitHook(mockos.NewProcessExitState(103))
return nil
}

// ResizeConsole captures its arguments and returns a nil error.
func (c *MockCore) ResizeConsole(pid int, height, width uint16) error {
c.LastResizeConsole = ResizeConsoleCall{
Pid: pid,
Height: height,
Width: width,
}

return nil
}
4 changes: 4 additions & 0 deletions service/gcs/runtime/mockruntime/mockruntime.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,10 @@ func (c *container) Pid() int {
return 101
}

func (c *container) Tty() *stdio.TtyRelay {
return nil
}

func (c *container) ExecProcess(process oci.Process, stdioSet *stdio.ConnectionSet) (p runtime.Process, err error) {
return c, nil
}
Expand Down
8 changes: 8 additions & 0 deletions service/gcs/runtime/runc/runc.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,10 @@ func (c *container) Pid() int {
return c.init.Pid()
}

func (c *container) Tty() *stdio.TtyRelay {
return c.init.relay
}

type process struct {
c *container
pid int
Expand All @@ -58,6 +62,10 @@ func (p *process) Pid() int {
return p.pid
}

func (p *process) Tty() *stdio.TtyRelay {
return p.relay
}

// NewRuntime instantiates a new runcRuntime struct.
func NewRuntime() (*runcRuntime, error) {
rtime := &runcRuntime{}
Expand Down
1 change: 1 addition & 0 deletions service/gcs/runtime/runtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ type Process interface {
Wait() (oslayer.ProcessExitState, error)
Pid() int
Delete() error
Tty() *stdio.TtyRelay
Copy link
Member

Choose a reason for hiding this comment

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

Tty [](start = 1, length = 3)

Maybe better to just expose the Resize method directly here? This seems to leak an implementation detail.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will open an issue to unify the runtime Process and the oslayer Process interfaces. The issue is that for container/external processes we store them quite differently and the cache cant support both. We really should be thinking about them the same with a single Process interface from the GCS perspective. Will this work for now?

}

// Container is an interface to manipulate container state.
Expand Down