Skip to content

Commit

Permalink
GCS tests and features/bugfixes to support them (microsoft#1360)
Browse files Browse the repository at this point in the history
Exposed data and functionality for testing GCS:
* `internal\guest\runtime\hcsv2.Container.InitProcess()`
* `internal\guest\runtime\hcsv2.GetOrAddNetworkNamespace()`
* `internal\guest\runtime\hcsv2.RemoveNetworkNamespace()`
* `internal\guest\runtime\hcsv2.Host.SecurityPolicyEnforcer()`
* `internal\guest\runtime\hcsv2.Host.Transport()`

Fixed bug where `host.RemoveContainer` did not remove the network
namespace for standalone and pod containers.

Updated go-winio version to include bugfixes for closing hvsockets,
specifically to close a socket for writing (needed by internal\cmd
to signal that the stdin stream has finished).

Added doc.go files to guest packages to prevent linter/compiler errors
under windows.

The tests themselves are broken out here:
microsoft#1352
microsoft#1351

Signed-off-by: Hamza El-Saawy <hamzaelsaawy@microsoft.com>
  • Loading branch information
helsaawy authored May 3, 2022
1 parent 3b35520 commit d51e522
Show file tree
Hide file tree
Showing 12 changed files with 619 additions and 549 deletions.
2 changes: 2 additions & 0 deletions guest/linux/doc.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
// Package linux contains definitions required for making a linux ioctl.
package linux
1 change: 0 additions & 1 deletion guest/linux/ioctl.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
//go:build linux
// +build linux

// Package linux contains definitions required for making a linux ioctl.
package linux

import (
Expand Down
1 change: 1 addition & 0 deletions guest/policy/doc.go
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
package policy
11 changes: 10 additions & 1 deletion guest/runtime/hcsv2/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,14 +123,19 @@ func (c *Container) ExecProcess(ctx context.Context, process *oci.Process, conSe
return pid, nil
}

// InitProcess returns the container's init process
func (c *Container) InitProcess() Process {
return c.initProcess
}

// GetProcess returns the Process with the matching 'pid'. If the 'pid' does
// not exit returns error.
func (c *Container) GetProcess(pid uint32) (Process, error) {
//todo: thread a context to this function call
logrus.WithFields(logrus.Fields{
logfields.ContainerID: c.id,
logfields.ProcessID: pid,
}).Info("opengcs::Container::GetProcesss")
}).Info("opengcs::Container::GetProcess")
if c.initProcess.pid == pid {
return c.initProcess, nil
}
Expand Down Expand Up @@ -245,3 +250,7 @@ func (c *Container) getStatus() containerStatus {
func (c *Container) setStatus(st containerStatus) {
atomic.StoreUint32((*uint32)(&c.status), uint32(st))
}

func (c *Container) ID() string {
return c.id
}
10 changes: 5 additions & 5 deletions guest/runtime/hcsv2/network.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,9 @@ func getNetworkNamespace(id string) (*namespace, error) {
return ns, nil
}

// getOrAddNetworkNamespace returns the namespace found by `id` or creates a new
// GetOrAddNetworkNamespace returns the namespace found by `id` or creates a new
// one and assigns `id.
func getOrAddNetworkNamespace(id string) *namespace {
func GetOrAddNetworkNamespace(id string) *namespace {
id = strings.ToLower(id)

namespaceSync.Lock()
Expand All @@ -69,8 +69,8 @@ func getOrAddNetworkNamespace(id string) *namespace {
return ns
}

// removeNetworkNamespace removes the in-memory `namespace` found by `id`.
func removeNetworkNamespace(ctx context.Context, id string) (err error) {
// RemoveNetworkNamespace removes the in-memory `namespace` found by `id`.
func RemoveNetworkNamespace(ctx context.Context, id string) (err error) {
_, span := trace.StartSpan(ctx, "hcsv2::removeNetworkNamespace")
defer span.End()
defer func() { oc.SetSpanStatus(span, err) }()
Expand Down Expand Up @@ -123,7 +123,7 @@ func (n *namespace) AssignContainerPid(ctx context.Context, pid int) (err error)
defer n.m.Unlock()

if n.pid != 0 {
return errors.Errorf("previously assigned container pid: %d", n.pid)
return errors.Errorf("previously assigned container pid %d to network namespace %q", n.pid, n.id)
}

n.pid = pid
Expand Down
26 changes: 13 additions & 13 deletions guest/runtime/hcsv2/network_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (

func Test_getNetworkNamespace_NotExist(t *testing.T) {
defer func() {
err := removeNetworkNamespace(context.Background(), t.Name())
err := RemoveNetworkNamespace(context.Background(), t.Name())
if err != nil {
t.Errorf("failed to remove ns with error: %v", err)
}
Expand All @@ -29,13 +29,13 @@ func Test_getNetworkNamespace_NotExist(t *testing.T) {

func Test_getNetworkNamespace_PreviousExist(t *testing.T) {
defer func() {
err := removeNetworkNamespace(context.Background(), t.Name())
err := RemoveNetworkNamespace(context.Background(), t.Name())
if err != nil {
t.Errorf("failed to remove ns with error: %v", err)
}
}()

ns1 := getOrAddNetworkNamespace(t.Name())
ns1 := GetOrAddNetworkNamespace(t.Name())
if ns1 == nil {
t.Fatal("namespace ns1 should not be nil")
}
Expand All @@ -50,43 +50,43 @@ func Test_getNetworkNamespace_PreviousExist(t *testing.T) {

func Test_getOrAddNetworkNamespace_NotExist(t *testing.T) {
defer func() {
err := removeNetworkNamespace(context.Background(), t.Name())
err := RemoveNetworkNamespace(context.Background(), t.Name())
if err != nil {
t.Errorf("failed to remove ns with error: %v", err)
}
}()

ns := getOrAddNetworkNamespace(t.Name())
ns := GetOrAddNetworkNamespace(t.Name())
if ns == nil {
t.Fatalf("namespace should not be nil")
}
}

func Test_getOrAddNetworkNamespace_PreviousExist(t *testing.T) {
defer func() {
err := removeNetworkNamespace(context.Background(), t.Name())
err := RemoveNetworkNamespace(context.Background(), t.Name())
if err != nil {
t.Errorf("failed to remove ns with error: %v", err)
}
}()

ns1 := getOrAddNetworkNamespace(t.Name())
ns2 := getOrAddNetworkNamespace(t.Name())
ns1 := GetOrAddNetworkNamespace(t.Name())
ns2 := GetOrAddNetworkNamespace(t.Name())
if ns1 != ns2 {
t.Fatalf("ns1 %+v != ns2 %+v", ns1, ns2)
}
}

func Test_removeNetworkNamespace_NotExist(t *testing.T) {
err := removeNetworkNamespace(context.Background(), t.Name())
err := RemoveNetworkNamespace(context.Background(), t.Name())
if err != nil {
t.Fatalf("failed to remove non-existing ns with error: %v", err)
}
}

func Test_removeNetworkNamespace_HasAdapters(t *testing.T) {
defer func() {
err := removeNetworkNamespace(context.Background(), t.Name())
err := RemoveNetworkNamespace(context.Background(), t.Name())
if err != nil {
t.Errorf("failed to remove ns with error: %v", err)
}
Expand All @@ -96,7 +96,7 @@ func Test_removeNetworkNamespace_HasAdapters(t *testing.T) {
networkInstanceIDToName = nsOld
}()

ns := getOrAddNetworkNamespace(t.Name())
ns := GetOrAddNetworkNamespace(t.Name())

networkInstanceIDToName = func(ctx context.Context, id string, _ bool) (string, error) {
return "/dev/sdz", nil
Expand All @@ -105,15 +105,15 @@ func Test_removeNetworkNamespace_HasAdapters(t *testing.T) {
if err != nil {
t.Fatalf("failed to add adapter: %v", err)
}
err = removeNetworkNamespace(context.Background(), t.Name())
err = RemoveNetworkNamespace(context.Background(), t.Name())
if err == nil {
t.Fatal("should have failed to delete namespace with adapters")
}
err = ns.RemoveAdapter(context.Background(), "test")
if err != nil {
t.Fatalf("failed to remove adapter: %v", err)
}
err = removeNetworkNamespace(context.Background(), t.Name())
err = RemoveNetworkNamespace(context.Background(), t.Name())
if err != nil {
t.Fatalf("should not have failed to delete empty namepace got: %v", err)
}
Expand Down
4 changes: 4 additions & 0 deletions guest/runtime/hcsv2/process.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ type containerProcess struct {
writersCalled bool
}

var _ Process = &containerProcess{}

// newProcess returns a containerProcess struct that has been initialized with
// an outstanding wait for process exit, and post exit an outstanding wait for
// process cleanup to release all resources once at least 1 waiter has
Expand Down Expand Up @@ -262,6 +264,8 @@ type externalProcess struct {
remove func(pid int)
}

var _ Process = &externalProcess{}

func (ep *externalProcess) Kill(ctx context.Context, signal syscall.Signal) error {
if err := syscall.Kill(int(ep.cmd.Process.Pid), signal); err != nil {
if err == syscall.ESRCH {
Expand Down
2 changes: 1 addition & 1 deletion guest/runtime/hcsv2/standalone_container.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ func setupStandaloneContainerSpec(ctx context.Context, id string, spec *oci.Spec

// Write resolv.conf
if !specInternal.MountPresent("/etc/resolv.conf", spec.Mounts) {
ns := getOrAddNetworkNamespace(getNetworkNamespaceID(spec))
ns := GetOrAddNetworkNamespace(getNetworkNamespaceID(spec))
var searches, servers []string
for _, n := range ns.Adapters() {
if len(n.DNSSuffix) > 0 {
Expand Down
23 changes: 21 additions & 2 deletions guest/runtime/hcsv2/uvm.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,10 +115,29 @@ func (h *Host) SetSecurityPolicy(base64Policy string) error {
return nil
}

func (h *Host) SecurityPolicyEnforcer() securitypolicy.SecurityPolicyEnforcer {
return h.securityPolicyEnforcer
}

func (h *Host) Transport() transport.Transport {
return h.vsock
}

func (h *Host) RemoveContainer(id string) {
h.containersMutex.Lock()
defer h.containersMutex.Unlock()

c, ok := h.containers[id]
if !ok {
return
}

// delete the network namespace for standalone and sandbox containers
criType, isCRI := c.spec.Annotations[annotations.KubernetesContainerType]
if !isCRI || criType == "sandbox" {
RemoveNetworkNamespace(context.Background(), id)
}

delete(h.containers, id)
}

Expand Down Expand Up @@ -580,15 +599,15 @@ func modifyCombinedLayers(ctx context.Context, rt guestrequest.RequestType, cl *
func modifyNetwork(ctx context.Context, rt guestrequest.RequestType, na *guestresource.LCOWNetworkAdapter) (err error) {
switch rt {
case guestrequest.RequestTypeAdd:
ns := getOrAddNetworkNamespace(na.NamespaceID)
ns := GetOrAddNetworkNamespace(na.NamespaceID)
if err := ns.AddAdapter(ctx, na); err != nil {
return err
}
// This code doesnt know if the namespace was already added to the
// container or not so it must always call `Sync`.
return ns.Sync(ctx)
case guestrequest.RequestTypeRemove:
ns := getOrAddNetworkNamespace(na.ID)
ns := GetOrAddNetworkNamespace(na.ID)
if err := ns.RemoveAdapter(ctx, na.ID); err != nil {
return err
}
Expand Down
Loading

0 comments on commit d51e522

Please sign in to comment.