Skip to content
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.

Sign up for GitHub

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

Jump to bottom

e2e_node: fixes after dynamic configuration removal #106210

Merged
merged 1 commit into from
Nov 8, 2021
Merged

e2e_node: fixes after dynamic configuration removal #106210

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 5 additions & 3 deletions test/e2e_node/cpu_manager_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ func configureCPUManagerInKubelet(oldCfg *kubeletconfig.KubeletConfiguration, ku
}
}

return oldCfg
return newCfg
}

func runGuPodTest(f *framework.Framework, cpuCount int) {
Expand Down Expand Up @@ -532,8 +532,10 @@ func runCPUManagerTests(f *framework.Framework) {

ginkgo.BeforeEach(func() {
var err error
oldCfg, err = getCurrentKubeletConfig()
framework.ExpectNoError(err)
if oldCfg == nil {
oldCfg, err = getCurrentKubeletConfig()
framework.ExpectNoError(err)
}
})

ginkgo.It("should assign CPUs as expected based on the Pod spec", func() {
Expand Down
2 changes: 1 addition & 1 deletion test/e2e_node/e2e_node_suite_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ func registerNodeFlags(flags *flag.FlagSet) {
// It is hard and unnecessary to deal with the complexity inside the test suite.
flags.BoolVar(&framework.TestContext.NodeConformance, "conformance", false, "If true, the test suite will not start kubelet, and fetch system log (kernel, docker, kubelet log etc.) to the report directory.")
flags.BoolVar(&framework.TestContext.PrepullImages, "prepull-images", true, "If true, prepull images so image pull failures do not cause test failures.")
flags.BoolVar(&framework.TestContext.RestartKubelet, "restart-kubelet", true, "If true, restart Kubelet unit when the process is killed.")
flags.BoolVar(&framework.TestContext.RestartKubelet, "restart-kubelet", false, "If true, restart Kubelet unit when the process is killed.")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why this change?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that we do not have DynamicKubelet configuration, we do not need to run a restart loop.

How did it work before:

  1. Test updated kubelet via DynamicKubeletConfiguration
  2. DynamicKubeletConfiguration controller applied updated configuration and stopped the kubelet.
  3. The restart loop monitors the kubelet health link and once it does not respond restart the kubelet.

How does it work now:

  1. The test stops the kubelet.
  2. The test update kubelet static configuration.
  3. The test restarts kubelet.

So now it totally tests responsibility to make sure the kubelet runs after the test. It makes restarts more predictable because it is not external to the test component that can restart the kubelet in the middle of the update configuration.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm thinking if there were other uses like when kubelet paniced or something. I don't remember any tests explicitly stopping kubelet. But I saw supressed panics before (#101719).

I tend to ok this change since panic is bad enough condition to make test grid red.

flags.StringVar(&framework.TestContext.ImageDescription, "image-description", "", "The description of the image which the test will be running on.")
flags.StringVar(&framework.TestContext.SystemSpecName, "system-spec-name", "", "The name of the system spec (e.g., gke) that's used in the node e2e test. The system specs are in test/e2e_node/system/specs/. This is used by the test framework to determine which tests to run for validating the system requirements.")
flags.Var(cliflag.NewMapStringString(&framework.TestContext.ExtraEnvs), "extra-envs", "The extra environment variables needed for node e2e tests. Format: a list of key=value pairs, e.g., env1=val1,env2=val2")
Expand Down
22 changes: 9 additions & 13 deletions test/e2e_node/memory_manager_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ const (
resourceMemory = "memory"
staticPolicy = "Static"
nonePolicy = "None"
hugepages2MiCount = 8
)

// Helper for makeMemoryManagerPod().
Expand Down Expand Up @@ -318,23 +319,22 @@ var _ = SIGDescribe("Memory Manager [Disruptive] [Serial] [Feature:MemoryManager
if len(allNUMANodes) == 0 {
allNUMANodes = getAllNUMANodes()
}
})

// dynamically update the kubelet configuration
ginkgo.JustBeforeEach(func() {
hugepagesCount := 8

// allocate hugepages
if *is2MiHugepagesSupported {
ginkgo.By("Configuring hugepages")
gomega.Eventually(func() error {
return configureHugePages(hugepagesSize2M, hugepagesCount)
return configureHugePages(hugepagesSize2M, hugepages2MiCount)
}, 30*time.Second, framework.Poll).Should(gomega.BeNil())
}
})

restartKubelet(true)

// dynamically update the kubelet configuration
ginkgo.JustBeforeEach(func() {
// allocate hugepages
if *is2MiHugepagesSupported {
ginkgo.By("Waiting for hugepages resource to become available on the local node")
waitingForHugepages(hugepagesCount)
waitingForHugepages(hugepages2MiCount)

for i := 0; i < len(ctnParams); i++ {
ctnParams[i].hugepages2Mi = "8Mi"
Expand All @@ -358,10 +358,6 @@ var _ = SIGDescribe("Memory Manager [Disruptive] [Serial] [Feature:MemoryManager
gomega.Eventually(func() error {
return configureHugePages(hugepagesSize2M, 0)
}, 90*time.Second, 15*time.Second).ShouldNot(gomega.HaveOccurred(), "failed to release hugepages")

restartKubelet(true)

waitingForHugepages(0)
}
})

Expand Down
8 changes: 5 additions & 3 deletions test/e2e_node/topology_manager_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -923,7 +923,7 @@ func runTopologyManagerTests(f *framework.Framework) {

configMap := getSRIOVDevicePluginConfigMap(framework.TestContext.SriovdpConfigMapFile)

oldCfg, err := getCurrentKubeletConfig()
oldCfg, err = getCurrentKubeletConfig()
framework.ExpectNoError(err)

policy := topologymanager.PolicySingleNumaNode
Expand All @@ -936,8 +936,10 @@ func runTopologyManagerTests(f *framework.Framework) {
})

ginkgo.AfterEach(func() {
// restore kubelet config
updateKubeletConfig(f, oldCfg, true)
if oldCfg != nil {
// restore kubelet config
updateKubeletConfig(f, oldCfg, true)
}
})
}

Expand Down
3 changes: 2 additions & 1 deletion test/e2e_node/util.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,8 @@ func tempSetCurrentKubeletConfig(f *framework.Framework, updateFunction func(ini
var oldCfg *kubeletconfig.KubeletConfiguration

ginkgo.BeforeEach(func() {
oldCfg, err := getCurrentKubeletConfig()
var err error
oldCfg, err = getCurrentKubeletConfig()
framework.ExpectNoError(err)

newCfg := oldCfg.DeepCopy()
Expand Down