Add unit tests to existing code

pkg/kubelet/stats/cri_stats_provider.go

@@ -67,6 +67,7 @@ type criStatsProvider struct {
 	imageService internalapi.ImageManagerService
 	// hostStatsProvider is used to get the status of the host filesystem consumed by pods.
 	hostStatsProvider HostStatsProvider
+	hcsshimInterface  interface{}
 
 	// cpuUsageCache caches the cpu usage for containers.
 	cpuUsageCache                  map[string]*cpuUsageRecord

pkg/kubelet/stats/cri_stats_provider_windows.go

@@ -31,9 +31,30 @@ import (
 	statsapi "k8s.io/kubelet/pkg/apis/stats/v1alpha1"
 )
 
+type hcsShimInterface interface {
+	GetContainers(q hcsshim.ComputeSystemQuery) ([]hcsshim.ContainerProperties, error)
+	GetHNSEndpointByID(endpointID string) (*hcsshim.HNSEndpoint, error)
+	OpenContainer(id string) (hcsshim.Container, error)
+}
+
+type windowshim struct{}
+
+func (s windowshim) GetContainers(q hcsshim.ComputeSystemQuery) ([]hcsshim.ContainerProperties, error) {
+	return hcsshim.GetContainers(q)
+}
+
+func (s windowshim) GetHNSEndpointByID(endpointID string) (*hcsshim.HNSEndpoint, error) {
+	return hcsshim.GetHNSEndpointByID(endpointID)
+}
+
+func (s windowshim) OpenContainer(id string) (hcsshim.Container, error) {
+	return hcsshim.OpenContainer(id)
+}
+
 // listContainerNetworkStats returns the network stats of all the running containers.
 func (p *criStatsProvider) listContainerNetworkStats() (map[string]*statsapi.NetworkStats, error) {
-	containers, err := hcsshim.GetContainers(hcsshim.ComputeSystemQuery{
+	shim := newHcsShim(p)
+	containers, err := shim.GetContainers(hcsshim.ComputeSystemQuery{
 		Types: []string{"Container"},
 	})
 	if err != nil {
@@ -42,24 +63,34 @@ func (p *criStatsProvider) listContainerNetworkStats() (map[string]*statsapi.Net
 
 	stats := make(map[string]*statsapi.NetworkStats)
 	for _, c := range containers {
-		cstats, err := fetchContainerStats(c)
+		cstats, err := fetchContainerStats(shim, c)
 		if err != nil {
 			klog.V(4).InfoS("Failed to fetch statistics for container, continue to get stats for other containers", "containerID", c.ID, "err", err)
 			continue
 		}
 		if len(cstats.Network) > 0 {
-			stats[c.ID] = hcsStatsToNetworkStats(cstats.Timestamp, cstats.Network)
+			stats[c.ID] = hcsStatsToNetworkStats(shim, cstats.Timestamp, cstats.Network)
 		}
 	}
 
 	return stats, nil
 }
 
-func fetchContainerStats(c hcsshim.ContainerProperties) (stats hcsshim.Statistics, err error) {
+func newHcsShim(p *criStatsProvider) hcsShimInterface {
+	var shim hcsShimInterface
+	if p.hcsshimInterface == nil {
+		shim = windowshim{}
+	} else {
+		shim = p.hcsshimInterface.(hcsShimInterface)
+	}
+	return shim
+}
+
+func fetchContainerStats(hcsshimInterface hcsShimInterface, c hcsshim.ContainerProperties) (stats hcsshim.Statistics, err error) {
 	var (
 		container hcsshim.Container
 	)
-	container, err = hcsshim.OpenContainer(c.ID)
+	container, err = hcsshimInterface.OpenContainer(c.ID)
 	if err != nil {
 		return
 	}
@@ -77,7 +108,7 @@ func fetchContainerStats(c hcsshim.ContainerProperties) (stats hcsshim.Statistic
 }
 
 // hcsStatsToNetworkStats converts hcsshim.Statistics.Network to statsapi.NetworkStats
-func hcsStatsToNetworkStats(timestamp time.Time, hcsStats []hcsshim.NetworkStats) *statsapi.NetworkStats {
+func hcsStatsToNetworkStats(hcsshimInterface hcsShimInterface, timestamp time.Time, hcsStats []hcsshim.NetworkStats) *statsapi.NetworkStats {
 	result := &statsapi.NetworkStats{
 		Time:       metav1.NewTime(timestamp),
 		Interfaces: make([]statsapi.InterfaceStats, 0),
@@ -85,7 +116,7 @@ func hcsStatsToNetworkStats(timestamp time.Time, hcsStats []hcsshim.NetworkStats
 
 	adapters := sets.NewString()
 	for _, stat := range hcsStats {
-		iStat, err := hcsStatsToInterfaceStats(stat)
+		iStat, err := hcsStatsToInterfaceStats(hcsshimInterface, stat)
 		if err != nil {
 			klog.InfoS("Failed to get HNS endpoint, continue to get stats for other endpoints", "endpointID", stat.EndpointId, "err", err)
 			continue
@@ -109,8 +140,8 @@ func hcsStatsToNetworkStats(timestamp time.Time, hcsStats []hcsshim.NetworkStats
 }
 
 // hcsStatsToInterfaceStats converts hcsshim.NetworkStats to statsapi.InterfaceStats.
-func hcsStatsToInterfaceStats(stat hcsshim.NetworkStats) (*statsapi.InterfaceStats, error) {
+func hcsStatsToInterfaceStats(hcsshimInterface hcsShimInterface, stat hcsshim.NetworkStats) (*statsapi.InterfaceStats, error) {
-	endpoint, err := hcsshim.GetHNSEndpointByID(stat.EndpointId)
+	endpoint, err := hcsshimInterface.GetHNSEndpointByID(stat.EndpointId)
 	if err != nil {
 		return nil, err
 	}

pkg/kubelet/stats/cri_stats_provider_windows_test.go

@@ -0,0 +1,453 @@
+package stats
+
+import (
+	"reflect"
+	"testing"
+	"time"
+
+	"github.com/Microsoft/hcsshim"
+
+	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	statsapi "k8s.io/kubelet/pkg/apis/stats/v1alpha1"
+)
+
+
+
+type fakeConatiner struct {
+	stat hcsshim.Statistics
+}
+
+func (f fakeConatiner) Start() error {
+	return nil
+}
+
+func (f fakeConatiner) Shutdown() error {
+	return nil
+}
+
+func (f fakeConatiner) Terminate() error {
+	return nil
+}
+
+func (f fakeConatiner) Wait() error {
+	return nil
+}
+
+func (f fakeConatiner) WaitTimeout(duration time.Duration) error {
+	return nil
+}
+
+func (f fakeConatiner) Pause() error {
+	return nil
+}
+
+func (f fakeConatiner) Resume() error {
+	return nil
+}
+
+func (f fakeConatiner) HasPendingUpdates() (bool, error) {
+	return false, nil
+}
+
+func (f fakeConatiner) Statistics() (hcsshim.Statistics, error) {
+	return f.stat, nil
+}
+
+func (f fakeConatiner) ProcessList() ([]hcsshim.ProcessListItem, error) {
+	return []hcsshim.ProcessListItem{}, nil
+}
+
+func (f fakeConatiner) MappedVirtualDisks() (map[int]hcsshim.MappedVirtualDiskController, error) {
+	return map[int]hcsshim.MappedVirtualDiskController{}, nil
+}
+
+func (f fakeConatiner) CreateProcess(c *hcsshim.ProcessConfig) (hcsshim.Process, error) {
+	return nil, nil
+}
+
+func (f fakeConatiner) OpenProcess(pid int) (hcsshim.Process, error) {
+	return nil, nil
+}
+
+func (f fakeConatiner) Close() error {
+	return nil
+}
+
+func (f fakeConatiner) Modify(config *hcsshim.ResourceModificationRequestResponse) error {
+	return nil
+}
+
+func (s fakehcsshim) GetContainers(q hcsshim.ComputeSystemQuery) ([]hcsshim.ContainerProperties, error) {
+	cp := []hcsshim.ContainerProperties{}
+	for _, c := range s.containers  {
+		cp = append(cp, c.container)
+	}
+
+	return cp, nil
+}
+
+func (s fakehcsshim) GetHNSEndpointByID(endpointID string) (*hcsshim.HNSEndpoint, error) {
+	e := hcsshim.HNSEndpoint{
+		Name: endpointID,
+	}
+	return &e, nil
+}
+
+func (s fakehcsshim) OpenContainer(id string) (hcsshim.Container, error) {
+	fc := fakeConatiner{}
+	for _, c := range s.containers {
+		if c.container.ID == id {
+			for _, s := range c.hcsStats{
+				fc.stat.Network = append(fc.stat.Network, s)
+			}
+		}
+	}
+
+	return fc, nil
+}
+
+type fakehcsshim struct {
+	containers       []containerStats
+}
+
+type containerStats struct {
+	container       hcsshim.ContainerProperties
+	hcsStats         []hcsshim.NetworkStats
+}
+
+
+func Test_criStatsProvider_listContainerNetworkStats(t *testing.T) {
+	tests := []struct {
+		name    string
+		fields  fakehcsshim
+		want    map[string]*statsapi.NetworkStats
+		wantErr bool
+	}{
+		{
+			name: "basic example",
+			fields: fakehcsshim{
+				containers: []containerStats{
+					{
+						container: hcsshim.ContainerProperties{
+						ID: "c1",
+						}, hcsStats: []hcsshim.NetworkStats{
+					{
+						BytesReceived: 1,
+						BytesSent:     10,
+						EndpointId:    "test",
+						InstanceId:    "test",
+							},
+						},
+					},
+					{
+						container: hcsshim.ContainerProperties{
+						ID: "c2",
+						}, hcsStats: []hcsshim.NetworkStats{
+							{
+						BytesReceived: 2,
+						BytesSent:     20,
+						EndpointId:    "test2",
+						InstanceId:    "test2",
+					},
+				},
+					},
+				},
+			},
+			want: map[string]*statsapi.NetworkStats{
+				"c1": &statsapi.NetworkStats{
+					Time: v1.Time{},
+					InterfaceStats: statsapi.InterfaceStats{
+						Name:    "test",
+						RxBytes: toP(1),
+						TxBytes: toP(10),
+					},
+					Interfaces: []statsapi.InterfaceStats{
+						{
+							Name:    "test",
+							RxBytes: toP(1),
+
+							TxBytes: toP(10),
+						},
+					},
+				},
+				"c2": &statsapi.NetworkStats{
+					Time: v1.Time{},
+					InterfaceStats: statsapi.InterfaceStats{
+						Name:    "test2",
+						RxBytes: toP(2),
+						TxBytes: toP(20),
+					},
+					Interfaces: []statsapi.InterfaceStats{
+						{
+							Name:    "test2",
+							RxBytes: toP(2),
+							TxBytes: toP(20),
+						},
+					},
+				},
+			},
+			wantErr: false,
+		},
+		{
+			name: "multiple containers same endpoint",
+			fields: fakehcsshim{
+				containers: []containerStats{
+					{
+						container: hcsshim.ContainerProperties{
+						ID: "c1",
+						}, hcsStats: []hcsshim.NetworkStats{
+					{
+						BytesReceived: 1,
+						BytesSent:     10,
+						EndpointId:    "test",
+						InstanceId:    "test",
+							},
+						},
+					},
+					{
+						container: hcsshim.ContainerProperties{
+						ID: "c2",
+						}, hcsStats: []hcsshim.NetworkStats{
+							{
+						BytesReceived: 2,
+						BytesSent:     20,
+						EndpointId:    "test2",
+						InstanceId:    "test2",
+							},
+						},
+					},
+					{
+						container: hcsshim.ContainerProperties{
+						ID: "c3",
+						}, hcsStats: []hcsshim.NetworkStats{
+							{
+						BytesReceived: 3,
+						BytesSent:     30,
+						EndpointId:    "test2",
+						InstanceId:    "test3",
+					},
+				},
+					},
+				},
+			},
+			want: map[string]*statsapi.NetworkStats{
+				"c1": &statsapi.NetworkStats{
+					Time: v1.Time{},
+					InterfaceStats: statsapi.InterfaceStats{
+						Name:    "test",
+						RxBytes: toP(1),
+						TxBytes: toP(10),
+					},
+					Interfaces: []statsapi.InterfaceStats{
+						{
+							Name:    "test",
+							RxBytes: toP(1),
+
+							TxBytes: toP(10),
+						},
+					},
+				},
+				"c2": &statsapi.NetworkStats{
+					Time: v1.Time{},
+					InterfaceStats: statsapi.InterfaceStats{
+						Name:    "test2",
+						RxBytes: toP(2),
+						TxBytes: toP(20),
+					},
+					Interfaces: []statsapi.InterfaceStats{
+						{
+							Name:    "test2",
+							RxBytes: toP(2),
+							TxBytes: toP(20),
+						},
+					},
+				},
+				"c3": &statsapi.NetworkStats{
+					Time: v1.Time{},
+					InterfaceStats: statsapi.InterfaceStats{
+						Name:    "test2",
+						RxBytes: toP(3),
+						TxBytes: toP(30),
+					},
+					Interfaces: []statsapi.InterfaceStats{
+						{
+							Name:    "test2",
+							RxBytes: toP(3),
+							TxBytes: toP(30),
+						},
+					},
+				},
+			},
+			wantErr: false,
+		},
+		{
+			name: "multiple stats instances of same interface only picks up first",
+			fields: fakehcsshim{
+				containers: []containerStats{
+					{
+						container: hcsshim.ContainerProperties{
+							ID: "c1",
+						}, hcsStats: []hcsshim.NetworkStats{
+					{
+						BytesReceived: 1,
+						BytesSent:     10,
+						EndpointId:    "test",
+						InstanceId:    "test",
+					},
+					{
+								BytesReceived: 3,
+								BytesSent:     30,
+								EndpointId:    "test",
+								InstanceId:    "test3",
+							},
+						},
+					},
+					{
+						container: hcsshim.ContainerProperties{
+							ID: "c2",
+						}, hcsStats: []hcsshim.NetworkStats{
+							{
+						BytesReceived: 2,
+						BytesSent:     20,
+						EndpointId:    "test2",
+						InstanceId:    "test2",
+					},
+					},
+				},
+				},
+			},
+			want: map[string]*statsapi.NetworkStats{
+				"c1": &statsapi.NetworkStats{
+					Time: v1.Time{},
+					InterfaceStats: statsapi.InterfaceStats{
+						Name:    "test",
+						RxBytes: toP(1),
+						TxBytes: toP(10),
+					},
+					Interfaces: []statsapi.InterfaceStats{
+						{
+							Name:    "test",
+							RxBytes: toP(1),
+
+							TxBytes: toP(10),
+						},
+					},
+				},
+				"c2": &statsapi.NetworkStats{
+					Time: v1.Time{},
+					InterfaceStats: statsapi.InterfaceStats{
+						Name:    "test2",
+						RxBytes: toP(2),
+						TxBytes: toP(20),
+					},
+					Interfaces: []statsapi.InterfaceStats{
+						{
+							Name:    "test2",
+							RxBytes: toP(2),
+							TxBytes: toP(20),
+						},
+					},
+				},
+			},
+			wantErr: false,
+		},
+		{
+			name: "multiple endpoints per container",
+			fields: fakehcsshim{
+				containers: []containerStats{
+					{
+						container: hcsshim.ContainerProperties{
+							ID: "c1",
+						}, hcsStats: []hcsshim.NetworkStats{
+					{
+						BytesReceived: 1,
+						BytesSent:     10,
+						EndpointId:    "test",
+						InstanceId:    "test",
+					},
+												{
+								BytesReceived: 3,
+								BytesSent:     30,
+								EndpointId:    "test3",
+								InstanceId:    "test3",
+							},
+						},
+					},
+					{
+						container: hcsshim.ContainerProperties{
+							ID: "c2",
+						}, hcsStats: []hcsshim.NetworkStats{
+					{
+						BytesReceived: 2,
+						BytesSent:     20,
+						EndpointId:    "test2",
+						InstanceId:    "test2",
+					},
+					},
+				},
+				},
+			},
+			want: map[string]*statsapi.NetworkStats{
+				"c1": &statsapi.NetworkStats{
+					Time: v1.Time{},
+					InterfaceStats: statsapi.InterfaceStats{
+						Name:    "test",
+						RxBytes: toP(1),
+						TxBytes: toP(10),
+					},
+					Interfaces: []statsapi.InterfaceStats{
+						{
+							Name:    "test",
+							RxBytes: toP(1),
+
+							TxBytes: toP(10),
+						},
+						{
+							Name:    "test3",
+							RxBytes: toP(3),
+
+							TxBytes: toP(30),
+						},
+					},
+				},
+				"c2": &statsapi.NetworkStats{
+					Time: v1.Time{},
+					InterfaceStats: statsapi.InterfaceStats{
+						Name:    "test2",
+						RxBytes: toP(2),
+						TxBytes: toP(20),
+					},
+					Interfaces: []statsapi.InterfaceStats{
+						{
+							Name:    "test2",
+							RxBytes: toP(2),
+							TxBytes: toP(20),
+						},
+					},
+				},
+			},
+			wantErr: false,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			p := &criStatsProvider{
+				hcsshimInterface: fakehcsshim{
+					containers:       tt.fields.containers,
+				},
+			}
+			got, err := p.listContainerNetworkStats()
+			if (err != nil) != tt.wantErr {
+				t.Errorf("listContainerNetworkStats() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+			if !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("listContainerNetworkStats() got = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
+
+func toP(i uint64) *uint64 {
+	return &i
+}