Merge pull request #2540 from ctripcloud/addWarnLogForMissingContainerInfo

fix missing container info when getting sub-containers stats

manager/container.go

@@ -159,7 +159,7 @@ func (cd *containerData) notifyOnDemand() {
 
 func (cd *containerData) GetInfo(shouldUpdateSubcontainers bool) (*containerInfo, error) {
 	// Get spec and subcontainers.
-	if cd.clock.Since(cd.infoLastUpdatedTime) > 5*time.Second {
+	if cd.clock.Since(cd.infoLastUpdatedTime) > 5*time.Second || shouldUpdateSubcontainers {
 		err := cd.updateSpec()
 		if err != nil {
 			return nil, err

manager/manager.go

@@ -650,6 +650,7 @@ func (m *manager) containerDataSliceToContainerInfoSlice(containers []*container
 		cinfo, err := m.containerDataToContainerInfo(containers[i], query)
 		if err != nil {
 			// Skip containers with errors, we try to degrade gracefully.
+			klog.V(4).Infof("convert container data to container info failed with error %s", err.Error())
 			continue
 		}
 		output = append(output, cinfo)

manager/manager_test.go

@@ -85,6 +85,69 @@ func createManagerAndAddContainers(
 	return mif
 }
 
+func createManagerAndAddSubContainers(
+	memoryCache *memory.InMemoryCache,
+	sysfs *fakesysfs.FakeSysFs,
+	containers []string,
+	f func(*containertest.MockContainerHandler),
+	t *testing.T,
+) *manager {
+	container.ClearContainerHandlerFactories()
+	mif := &manager{
+		containers:   make(map[namespacedContainerName]*containerData),
+		quitChannels: make([]chan error, 0, 2),
+		memoryCache:  memoryCache,
+	}
+
+	subcontainers1 := []info.ContainerReference{
+		{Name: "/kubepods/besteffort"},
+		{Name: "/kubepods/burstable"},
+	}
+	subcontainers2 := []info.ContainerReference(nil)
+	subcontainers3 := []info.ContainerReference{
+		{Name: "/kubepods/burstable/pod01042b28-179d-446a-954a-7266557e12cd"},
+		{Name: "/kubepods/burstable/pod01042b28-179d-446a-954a-7266557e12ce"},
+	}
+	subcontainers4 := []info.ContainerReference{
+		{Name: "/kubepods/burstable/pod01042b28-179d-446a-954a-7266557e12cd/22f44d2a517778590e2d8bcafafe501f79e8a509e5b6de70b7700c4d37722bce"},
+		{Name: "/kubepods/burstable/pod01042b28-179d-446a-954a-7266557e12cd/ae9465f98d275998e148b6fc12f5f92e5d4a64fca0d255f6dc3a13cc6f93a10f"},
+	}
+
+	subcontainers5 := []info.ContainerReference(nil)
+	subcontainers6 := []info.ContainerReference(nil)
+
+	subcontainerList := [][]info.ContainerReference{subcontainers1, subcontainers2, subcontainers3, subcontainers4, subcontainers5, subcontainers6}
+
+	for idx, name := range containers {
+		mockHandler := containertest.NewMockContainerHandler(name)
+		spec := itest.GenerateRandomContainerSpec(4)
+		mockHandler.On("GetSpec").Return(
+			spec,
+			nil,
+		).Once()
+		mockHandler.On("ListContainers", container.ListSelf).Return(
+			subcontainerList[idx],
+			nil,
+		)
+		cont, err := newContainerData(name, memoryCache, mockHandler, false, &collector.GenericCollectorManager{}, 60*time.Second, true, clock.NewFakeClock(time.Now()))
+		if err != nil {
+			t.Fatal(err)
+		}
+		mif.containers[namespacedContainerName{
+			Name: name,
+		}] = cont
+		// Add Docker containers under their namespace.
+		if strings.HasPrefix(name, "/docker") {
+			mif.containers[namespacedContainerName{
+				Namespace: docker.DockerNamespace,
+				Name:      strings.TrimPrefix(name, "/docker/"),
+			}] = cont
+		}
+		f(mockHandler)
+	}
+	return mif
+}
+
 // Expect a manager with the specified containers and query. Returns the manager, map of ContainerInfo objects,
 // and map of MockContainerHandler objects.}
 func expectManagerWithContainers(containers []string, query *info.ContainerInfoRequest, t *testing.T) (*manager, map[string]*info.ContainerInfo, map[string]*containertest.MockContainerHandler) {
@@ -135,6 +198,51 @@ func expectManagerWithContainers(containers []string, query *info.ContainerInfoR
 	return m, infosMap, handlerMap
 }
 
+// Expect a manager with the specified containers and query. Returns the manager, map of ContainerInfo objects,
+// and map of MockContainerHandler objects.}
+func expectManagerWithSubContainers(containers []string, query *info.ContainerInfoRequest, t *testing.T) (*manager, map[string]*info.ContainerInfo, map[string]*containertest.MockContainerHandler) {
+	infosMap := make(map[string]*info.ContainerInfo, len(containers))
+	handlerMap := make(map[string]*containertest.MockContainerHandler, len(containers))
+
+	for _, container := range containers {
+		infosMap[container] = itest.GenerateRandomContainerInfo(container, 4, query, 1*time.Second)
+	}
+
+	memoryCache := memory.New(time.Duration(query.NumStats)*time.Second, nil)
+	sysfs := &fakesysfs.FakeSysFs{}
+	m := createManagerAndAddSubContainers(
+		memoryCache,
+		sysfs,
+		containers,
+		func(h *containertest.MockContainerHandler) {
+			cinfo := infosMap[h.Name]
+			ref, err := h.ContainerReference()
+			if err != nil {
+				t.Error(err)
+			}
+
+			cInfo := info.ContainerInfo{
+				ContainerReference: ref,
+			}
+			for _, stat := range cinfo.Stats {
+				err = memoryCache.AddStats(&cInfo, stat)
+				if err != nil {
+					t.Error(err)
+				}
+			}
+			spec := cinfo.Spec
+			h.On("GetSpec").Return(
+				spec,
+				nil,
+			).Once()
+			handlerMap[h.Name] = h
+		},
+		t,
+	)
+
+	return m, infosMap, handlerMap
+}
+
 // Expect a manager with the specified containers and query. Returns the manager, map of ContainerInfo objects,
 // and map of MockContainerHandler objects.}
 func expectManagerWithContainersV2(containers []string, query *info.ContainerInfoRequest, t *testing.T) (*manager, map[string]*info.ContainerInfo, map[string]*containertest.MockContainerHandler) {
@@ -337,6 +445,60 @@ func TestSubcontainersInfo(t *testing.T) {
 	}
 }
 
+func TestSubcontainersInfoError(t *testing.T) {
+	containers := []string{
+		"/kubepods",
+		"/kubepods/besteffort",
+		"/kubepods/burstable",
+		"/kubepods/burstable/pod01042b28-179d-446a-954a-7266557e12cd",
+		"/kubepods/burstable/pod01042b28-179d-446a-954a-7266557e12cd/22f44d2a517778590e2d8bcafafe501f79e8a509e5b6de70b7700c4d37722bce",
+		"/kubepods/burstable/pod01042b28-179d-446a-954a-7266557e12cd/ae9465f98d275998e148b6fc12f5f92e5d4a64fca0d255f6dc3a13cc6f93a10f",
+	}
+	query := &info.ContainerInfoRequest{
+		NumStats: 1,
+	}
+
+	m, _, _ := expectManagerWithSubContainers(containers, query, t)
+	result, err := m.SubcontainersInfo("/kubepods", query)
+	if err != nil {
+		t.Fatalf("expected to succeed: %s", err)
+	}
+
+	if len(result) != len(containers) {
+		t.Errorf("expected to received containers: %v, but received: %v", containers, result)
+	}
+
+	totalBurstable := 0
+	burstableCount := 0
+	totalBesteffort := 0
+	besteffortCount := 0
+
+	for _, res := range result {
+		found := false
+		if res.Name == "/kubepods/burstable" {
+			totalBurstable = len(res.Subcontainers)
+		} else if res.Name == "/kubepods/besteffort" {
+			totalBesteffort = len(res.Subcontainers)
+		} else if strings.HasPrefix(res.Name, "/kubepods/burstable") && len(res.Name) == len("/kubepods/burstable/pod01042b28-179d-446a-954a-7266557e12cd") {
+			burstableCount++
+		} else if strings.HasPrefix(res.Name, "/kubepods/besteffort") && len(res.Name) == len("/kubepods/besteffort/pod01042b28-179d-446a-954a-7266557e12cd") {
+			besteffortCount++
+		}
+		for _, name := range containers {
+			if res.Name == name {
+				found = true
+				break
+			}
+		}
+		if !found {
+			t.Errorf("unexpected container %q in result, expected one of %v", res.Name, containers)
+		}
+	}
+
+	assert.NotEqual(t, totalBurstable, burstableCount)
+	assert.Equal(t, totalBesteffort, besteffortCount)
+}
+
 func TestDockerContainersInfo(t *testing.T) {
 	containers := []string{
 		"/docker/c1a",