Merge pull request microsoft#2220 from katiewasnothere/kabaldau/guest_caps

Refactor guest defined capabilities

Author: katiewasnothere

internal/gcs/guestcaps.go

@@ -0,0 +1,102 @@
+//go:build windows
+
+package gcs
+
+import (
+	"encoding/json"
+	"fmt"
+
+	"github.com/Microsoft/hcsshim/internal/guest/prot"
+	"github.com/Microsoft/hcsshim/internal/hcs/schema1"
+)
+
+// GuestDefinedCapabilities is an interface for different guest defined capabilities.
+// This allows us to define different capabilities by OS.
+//
+// When adding new fields to the implementations of type GuestDefinedCapabilities, a
+// new interface function should only be added if that capability describes a feature
+// available for both WCOW and LCOW. Otherwise, you can use the helper functions
+// GetWCOWCapabilities or GetLCOWCapabilities to check for capabilities specific to
+// one implementation.
+type GuestDefinedCapabilities interface {
+	IsSignalProcessSupported() bool
+	IsDeleteContainerStateSupported() bool
+	IsDumpStacksSupported() bool
+	IsNamespaceAddRequestSupported() bool
+}
+
+func GetWCOWCapabilities(gdc GuestDefinedCapabilities) *WCOWGuestDefinedCapabilities {
+	g, ok := gdc.(*WCOWGuestDefinedCapabilities)
+	if !ok {
+		return nil
+	}
+	return g
+}
+
+func GetLCOWCapabilities(gdc GuestDefinedCapabilities) *LCOWGuestDefinedCapabilities {
+	g, ok := gdc.(*LCOWGuestDefinedCapabilities)
+	if !ok {
+		return nil
+	}
+	return g
+}
+
+func unmarshalGuestCapabilities(os string, data json.RawMessage) (GuestDefinedCapabilities, error) {
+	if os == "windows" {
+		gdc := &WCOWGuestDefinedCapabilities{}
+		if err := json.Unmarshal(data, gdc); err != nil {
+			return nil, fmt.Errorf("unmarshal returned GuestDefinedCapabilities for windows: %w", err)
+		}
+		return gdc, nil
+	}
+	// linux
+	gdc := &LCOWGuestDefinedCapabilities{}
+	if err := json.Unmarshal(data, gdc); err != nil {
+		return nil, fmt.Errorf("unmarshal returned GuestDefinedCapabilities for lcow: %w", err)
+	}
+	return gdc, nil
+}
+
+var _ GuestDefinedCapabilities = &LCOWGuestDefinedCapabilities{}
+
+type LCOWGuestDefinedCapabilities struct {
+	prot.GcsGuestCapabilities
+}
+
+func (l *LCOWGuestDefinedCapabilities) IsNamespaceAddRequestSupported() bool {
+	return l.NamespaceAddRequestSupported
+}
+
+func (l *LCOWGuestDefinedCapabilities) IsSignalProcessSupported() bool {
+	return l.SignalProcessSupported
+}
+
+func (l *LCOWGuestDefinedCapabilities) IsDumpStacksSupported() bool {
+	return l.DumpStacksSupported
+}
+
+func (l *LCOWGuestDefinedCapabilities) IsDeleteContainerStateSupported() bool {
+	return l.DeleteContainerStateSupported
+}
+
+var _ GuestDefinedCapabilities = &WCOWGuestDefinedCapabilities{}
+
+type WCOWGuestDefinedCapabilities struct {
+	schema1.GuestDefinedCapabilities
+}
+
+func (w *WCOWGuestDefinedCapabilities) IsNamespaceAddRequestSupported() bool {
+	return w.NamespaceAddRequestSupported
+}
+
+func (w *WCOWGuestDefinedCapabilities) IsSignalProcessSupported() bool {
+	return w.SignalProcessSupported
+}
+
+func (w *WCOWGuestDefinedCapabilities) IsDumpStacksSupported() bool {
+	return w.DumpStacksSupported
+}
+
+func (w *WCOWGuestDefinedCapabilities) IsDeleteContainerStateSupported() bool {
+	return w.DeleteContainerStateSupported
+}

internal/gcs/guestconnection.go

@@ -16,7 +16,6 @@ import (
 	"github.com/Microsoft/go-winio"
 	"github.com/Microsoft/go-winio/pkg/guid"
 	"github.com/Microsoft/hcsshim/internal/cow"
-	"github.com/Microsoft/hcsshim/internal/hcs/schema1"
 	hcsschema "github.com/Microsoft/hcsshim/internal/hcs/schema2"
 	"github.com/Microsoft/hcsshim/internal/log"
 	"github.com/Microsoft/hcsshim/internal/logfields"
@@ -98,15 +97,15 @@ type GuestConnection struct {
 	mu         sync.Mutex
 	nextPort   uint32
 	notifyChs  map[string]chan struct{}
-	caps       schema1.GuestDefinedCapabilities
+	caps       GuestDefinedCapabilities
 	os         string
 }
 
 var _ cow.ProcessHost = &GuestConnection{}
 
 // Capabilities returns the guest's declared capabilities.
-func (gc *GuestConnection) Capabilities() *schema1.GuestDefinedCapabilities {
-	return &gc.caps
+func (gc *GuestConnection) Capabilities() GuestDefinedCapabilities {
+	return gc.caps
 }
 
 // Protocol returns the protocol version that is in use.
@@ -123,18 +122,24 @@ func (gc *GuestConnection) connect(ctx context.Context, isColdStart bool, initGu
 		MaximumVersion: protocolVersion,
 	}
 	var resp negotiateProtocolResponse
-	resp.Capabilities.GuestDefinedCapabilities = &gc.caps
 	err = gc.brdg.RPC(ctx, rpcNegotiateProtocol, &req, &resp, true)
 	if err != nil {
 		return err
 	}
 	if resp.Version != protocolVersion {
 		return fmt.Errorf("unexpected version %d returned", resp.Version)
 	}
+
 	gc.os = strings.ToLower(resp.Capabilities.RuntimeOsType)
 	if gc.os == "" {
 		gc.os = "windows"
 	}
+
+	gc.caps, err = unmarshalGuestCapabilities(gc.os, resp.Capabilities.GuestDefinedCapabilities)
+	if err != nil {
+		return fmt.Errorf("unmarshalGuestCapabilities: %w", err)
+	}
+
 	if isColdStart && resp.Capabilities.SendHostCreateMessage {
 		conf := &uvmConfig{
 			SystemType: "Container",

internal/gcs/protocol.go

@@ -323,7 +323,7 @@ type gcsCapabilities struct {
 	SendLifecycleNotifications bool
 	SupportedSchemaVersions    []hcsschema.Version
 	RuntimeOsType              string
-	GuestDefinedCapabilities   interface{}
+	GuestDefinedCapabilities   json.RawMessage
 }
 
 type containerCreateResponse struct {

internal/uvm/capabilities.go

@@ -2,25 +2,27 @@
 
 package uvm
 
-import "github.com/Microsoft/hcsshim/internal/hcs/schema1"
+import (
+	"github.com/Microsoft/hcsshim/internal/gcs"
+)
 
 // SignalProcessSupported returns `true` if the guest supports the capability to
 // signal a process.
 //
 // This support was added RS5+ guests.
 func (uvm *UtilityVM) SignalProcessSupported() bool {
-	return uvm.guestCaps.SignalProcessSupported
+	return uvm.guestCaps.IsSignalProcessSupported()
 }
 
 func (uvm *UtilityVM) DeleteContainerStateSupported() bool {
 	if uvm.gc == nil {
 		return false
 	}
-	return uvm.guestCaps.DeleteContainerStateSupported
+	return uvm.guestCaps.IsDeleteContainerStateSupported()
 }
 
 // Capabilities returns the protocol version and the guest defined capabilities.
 // This should only be used for testing.
-func (uvm *UtilityVM) Capabilities() (uint32, schema1.GuestDefinedCapabilities) {
+func (uvm *UtilityVM) Capabilities() (uint32, gcs.GuestDefinedCapabilities) {
 	return uvm.protocol, uvm.guestCaps
 }

internal/uvm/dumpstacks.go

@@ -7,7 +7,7 @@ import (
 )
 
 func (uvm *UtilityVM) DumpStacks(ctx context.Context) (string, error) {
-	if uvm.gc == nil || !uvm.guestCaps.DumpStacksSupported {
+	if uvm.gc == nil || !uvm.guestCaps.IsDumpStacksSupported() {
 		return "", nil
 	}
 

internal/uvm/network.go

@@ -529,7 +529,7 @@ func (uvm *UtilityVM) RemoveEndpointFromNS(ctx context.Context, id string, endpo
 
 // IsNetworkNamespaceSupported returns bool value specifying if network namespace is supported inside the guest
 func (uvm *UtilityVM) isNetworkNamespaceSupported() bool {
-	return uvm.guestCaps.NamespaceAddRequestSupported
+	return uvm.guestCaps.IsNamespaceAddRequestSupported()
 }
 
 func getNetworkModifyRequest(adapterID string, requestType guestrequest.RequestType, settings interface{}) interface{} {

internal/uvm/start.go

@@ -282,7 +282,7 @@ func (uvm *UtilityVM) Start(ctx context.Context) (err error) {
 		if err != nil {
 			return err
 		}
-		uvm.guestCaps = *uvm.gc.Capabilities()
+		uvm.guestCaps = uvm.gc.Capabilities()
 		uvm.protocol = uvm.gc.Protocol()
 
 		// initial setup required for external GCS connection
@@ -295,7 +295,7 @@ func (uvm *UtilityVM) Start(ctx context.Context) (err error) {
 		if err != nil {
 			return err
 		}
-		uvm.guestCaps = properties.GuestConnectionInfo.GuestDefinedCapabilities
+		uvm.guestCaps = &gcs.WCOWGuestDefinedCapabilities{GuestDefinedCapabilities: properties.GuestConnectionInfo.GuestDefinedCapabilities}
 		uvm.protocol = properties.GuestConnectionInfo.ProtocolVersion
 	}
 

internal/uvm/types.go

@@ -13,7 +13,6 @@ import (
 
 	"github.com/Microsoft/hcsshim/internal/gcs"
 	"github.com/Microsoft/hcsshim/internal/hcs"
-	"github.com/Microsoft/hcsshim/internal/hcs/schema1"
 	"github.com/Microsoft/hcsshim/internal/hns"
 	"github.com/Microsoft/hcsshim/internal/uvm/scsi"
 )
@@ -55,7 +54,7 @@ type UtilityVM struct {
 
 	// GCS bridge protocol and capabilities
 	protocol  uint32
-	guestCaps schema1.GuestDefinedCapabilities
+	guestCaps gcs.GuestDefinedCapabilities
 
 	// containerCounter is the current number of containers that have been created.
 	// This is never decremented in the life of the UVM.

test/functional/uvm_properties_test.go

@@ -9,44 +9,52 @@ import (
 
 	"github.com/Microsoft/hcsshim/osversion"
 
+	"github.com/Microsoft/hcsshim/internal/gcs"
 	"github.com/Microsoft/hcsshim/test/internal/util"
 	"github.com/Microsoft/hcsshim/test/pkg/require"
 	testuvm "github.com/Microsoft/hcsshim/test/pkg/uvm"
 )
 
 func TestPropertiesGuestConnection_LCOW(t *testing.T) {
-	t.Skip("not yet updated")
-
 	require.Build(t, osversion.RS5)
 	requireFeatures(t, featureLCOW, featureUVM)
 
 	ctx := util.Context(context.Background(), t)
-	uvm := testuvm.CreateAndStartLCOWFromOpts(ctx, t, defaultLCOWOptions(ctx, t))
+	uvm := testuvm.CreateAndStart(ctx, t, defaultLCOWOptions(ctx, t))
 	defer uvm.Close()
 
 	p, gc := uvm.Capabilities()
-	if gc.NamespaceAddRequestSupported ||
-		!gc.SignalProcessSupported ||
+	if !gc.IsNamespaceAddRequestSupported() ||
+		!gc.IsSignalProcessSupported() ||
 		p < 4 {
 		t.Fatalf("unexpected values: %d %+v", p, gc)
 	}
+
+	// check the type of the capabilities
+	gdc := gcs.GetLCOWCapabilities(gc)
+	if gdc == nil {
+		t.Fatal("capabilities are unexpected type")
+	}
 }
 
 func TestPropertiesGuestConnection_WCOW(t *testing.T) {
-	t.Skip("not yet updated")
-
 	require.Build(t, osversion.RS5)
 	requireFeatures(t, featureWCOW, featureUVM)
 
 	ctx := util.Context(context.Background(), t)
-	//nolint:staticcheck // SA1019: deprecated; will be replaced when test is updated
-	uvm, _, _ := testuvm.CreateWCOWUVM(ctx, t, t.Name(), "microsoft/nanoserver")
+	uvm := testuvm.CreateAndStart(ctx, t, defaultWCOWOptions(ctx, t))
 	defer uvm.Close()
 
 	p, gc := uvm.Capabilities()
-	if !gc.NamespaceAddRequestSupported ||
-		!gc.SignalProcessSupported ||
+	if !gc.IsNamespaceAddRequestSupported() ||
+		!gc.IsSignalProcessSupported() ||
 		p < 4 {
 		t.Fatalf("unexpected values: %d %+v", p, gc)
 	}
+
+	// check the type of the capabilities
+	gdc := gcs.GetWCOWCapabilities(gc)
+	if gdc == nil {
+		t.Fatal("capabilities are unexpected type")
+	}
 }