-
Notifications
You must be signed in to change notification settings - Fork 40.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Only expose top N images in NodeStatus
#25328
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,6 +26,7 @@ import ( | |
"os" | ||
"reflect" | ||
"sort" | ||
"strconv" | ||
"strings" | ||
"testing" | ||
"time" | ||
|
@@ -67,6 +68,7 @@ import ( | |
"k8s.io/kubernetes/pkg/util/diff" | ||
"k8s.io/kubernetes/pkg/util/flowcontrol" | ||
"k8s.io/kubernetes/pkg/util/mount" | ||
"k8s.io/kubernetes/pkg/util/rand" | ||
utilruntime "k8s.io/kubernetes/pkg/util/runtime" | ||
"k8s.io/kubernetes/pkg/util/sets" | ||
"k8s.io/kubernetes/pkg/util/wait" | ||
|
@@ -80,10 +82,19 @@ func init() { | |
utilruntime.ReallyCrash = true | ||
} | ||
|
||
const testKubeletHostname = "127.0.0.1" | ||
const ( | ||
testKubeletHostname = "127.0.0.1" | ||
|
||
const testReservationCPU = "200m" | ||
const testReservationMemory = "100M" | ||
testReservationCPU = "200m" | ||
testReservationMemory = "100M" | ||
|
||
maxImageTagsForTest = 3 | ||
|
||
// TODO(harry) any global place for these two? | ||
// Reasonable size range of all container images. 90%ile of images on dockerhub drops into this range. | ||
minImgSize int64 = 23 * 1024 * 1024 | ||
maxImgSize int64 = 1000 * 1024 * 1024 | ||
) | ||
|
||
type fakeHTTP struct { | ||
url string | ||
|
@@ -105,11 +116,9 @@ type TestKubelet struct { | |
mounter mount.Interface | ||
} | ||
|
||
// newTestKubelet returns test kubelet with two images. | ||
func newTestKubelet(t *testing.T) *TestKubelet { | ||
fakeRuntime := &containertest.FakeRuntime{} | ||
fakeRuntime.RuntimeType = "test" | ||
fakeRuntime.VersionInfo = "1.5.0" | ||
fakeRuntime.ImageList = []kubecontainer.Image{ | ||
imageList := []kubecontainer.Image{ | ||
{ | ||
ID: "abc", | ||
RepoTags: []string{"gcr.io/google_containers:v1", "gcr.io/google_containers:v2"}, | ||
|
@@ -121,6 +130,53 @@ func newTestKubelet(t *testing.T) *TestKubelet { | |
Size: 456, | ||
}, | ||
} | ||
return newTestKubeletWithImageList(t, imageList) | ||
} | ||
|
||
// generateTestingImageList generate randomly generated image list and corresponding expectedImageList. | ||
func generateTestingImageList(count int) ([]kubecontainer.Image, []api.ContainerImage) { | ||
// imageList is randomly generated image list | ||
var imageList []kubecontainer.Image | ||
for ; count > 0; count-- { | ||
imageItem := kubecontainer.Image{ | ||
ID: string(util.NewUUID()), | ||
RepoTags: generateImageTags(), | ||
Size: rand.Int63nRange(minImgSize, maxImgSize+1), | ||
} | ||
imageList = append(imageList, imageItem) | ||
} | ||
|
||
// expectedImageList is generated by imageList according to size and maxImagesInNodeStatus | ||
// 1. sort the imageList by size | ||
sort.Sort(byImageSize(imageList)) | ||
// 2. convert sorted imageList to api.ContainerImage list | ||
var expectedImageList []api.ContainerImage | ||
for _, kubeImage := range imageList { | ||
apiImage := api.ContainerImage{ | ||
Names: kubeImage.RepoTags, | ||
SizeBytes: kubeImage.Size, | ||
} | ||
|
||
expectedImageList = append(expectedImageList, apiImage) | ||
} | ||
// 3. only returns the top maxImagesInNodeStatus images in expectedImageList | ||
return imageList, expectedImageList[0:maxImagesInNodeStatus] | ||
} | ||
|
||
func generateImageTags() []string { | ||
var tagList []string | ||
count := rand.IntnRange(1, maxImageTagsForTest+1) | ||
for ; count > 0; count-- { | ||
tagList = append(tagList, "gcr.io/google_containers:v"+strconv.Itoa(count)) | ||
} | ||
return tagList | ||
} | ||
|
||
func newTestKubeletWithImageList(t *testing.T, imageList []kubecontainer.Image) *TestKubelet { | ||
fakeRuntime := &containertest.FakeRuntime{} | ||
fakeRuntime.RuntimeType = "test" | ||
fakeRuntime.VersionInfo = "1.5.0" | ||
fakeRuntime.ImageList = imageList | ||
fakeRecorder := &record.FakeRecorder{} | ||
fakeKubeClient := &fake.Clientset{} | ||
kubelet := &Kubelet{} | ||
|
@@ -2349,7 +2405,9 @@ func updateDiskSpacePolicy(kubelet *Kubelet, mockCadvisor *cadvisortest.Mock, ro | |
} | ||
|
||
func TestUpdateNewNodeStatus(t *testing.T) { | ||
testKubelet := newTestKubelet(t) | ||
// generate one more than maxImagesInNodeStatus in inputImageList | ||
inputImageList, expectedImageList := generateTestingImageList(maxImagesInNodeStatus + 1) | ||
testKubelet := newTestKubeletWithImageList(t, inputImageList) | ||
kubelet := testKubelet.kubelet | ||
kubeClient := testKubelet.fakeKubeClient | ||
kubeClient.ReactionChain = fake.NewSimpleClientset(&api.NodeList{Items: []api.Node{ | ||
|
@@ -2434,16 +2492,7 @@ func TestUpdateNewNodeStatus(t *testing.T) { | |
{Type: api.NodeLegacyHostIP, Address: "127.0.0.1"}, | ||
{Type: api.NodeInternalIP, Address: "127.0.0.1"}, | ||
}, | ||
Images: []api.ContainerImage{ | ||
{ | ||
Names: []string{"gcr.io/google_containers:v1", "gcr.io/google_containers:v2"}, | ||
SizeBytes: 123, | ||
}, | ||
{ | ||
Names: []string{"gcr.io/google_containers:v3", "gcr.io/google_containers:v4"}, | ||
SizeBytes: 456, | ||
}, | ||
}, | ||
Images: expectedImageList, | ||
}, | ||
} | ||
|
||
|
@@ -2478,9 +2527,14 @@ func TestUpdateNewNodeStatus(t *testing.T) { | |
t.Errorf("unexpected node condition order. NodeReady should be last.") | ||
} | ||
|
||
if !api.Semantic.DeepEqual(expectedNode, updatedNode) { | ||
t.Errorf("unexpected objects: %s", diff.ObjectDiff(expectedNode, updatedNode)) | ||
if maxImagesInNodeStatus != len(updatedNode.Status.Images) { | ||
t.Errorf("unexpected image list length in node status, expected: %v, got: %v", maxImagesInNodeStatus, len(updatedNode.Status.Images)) | ||
} else { | ||
if !api.Semantic.DeepEqual(expectedNode, updatedNode) { | ||
t.Errorf("unexpected objects: %s", diff.ObjectDiff(expectedNode, updatedNode)) | ||
} | ||
} | ||
|
||
} | ||
|
||
func TestUpdateNewNodeOutOfDiskStatusWithTransitionFrequency(t *testing.T) { | ||
|
@@ -2685,15 +2739,16 @@ func TestUpdateExistingNodeStatus(t *testing.T) { | |
{Type: api.NodeLegacyHostIP, Address: "127.0.0.1"}, | ||
{Type: api.NodeInternalIP, Address: "127.0.0.1"}, | ||
}, | ||
// images will be sorted from max to min in node status. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would be great if we can test the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. +1. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tests added for this. |
||
Images: []api.ContainerImage{ | ||
{ | ||
Names: []string{"gcr.io/google_containers:v1", "gcr.io/google_containers:v2"}, | ||
SizeBytes: 123, | ||
}, | ||
{ | ||
Names: []string{"gcr.io/google_containers:v3", "gcr.io/google_containers:v4"}, | ||
SizeBytes: 456, | ||
}, | ||
{ | ||
Names: []string{"gcr.io/google_containers:v1", "gcr.io/google_containers:v2"}, | ||
SizeBytes: 123, | ||
}, | ||
}, | ||
}, | ||
} | ||
|
@@ -2969,14 +3024,14 @@ func TestUpdateNodeStatusWithRuntimeStateError(t *testing.T) { | |
{Type: api.NodeInternalIP, Address: "127.0.0.1"}, | ||
}, | ||
Images: []api.ContainerImage{ | ||
{ | ||
Names: []string{"gcr.io/google_containers:v1", "gcr.io/google_containers:v2"}, | ||
SizeBytes: 123, | ||
}, | ||
{ | ||
Names: []string{"gcr.io/google_containers:v3", "gcr.io/google_containers:v4"}, | ||
SizeBytes: 456, | ||
}, | ||
{ | ||
Names: []string{"gcr.io/google_containers:v1", "gcr.io/google_containers:v2"}, | ||
SizeBytes: 123, | ||
}, | ||
}, | ||
}, | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@resouer @vishh Are we going to expose this as a kubelet flag?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@yifan-gu Why do you think this should be configurable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc @ncdc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not in my plan