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.

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

Allow the e2e_node runner to receive a KubeletConfiguration rather than requiring flags #105575

Merged
merged 7 commits into from
Nov 3, 2021
6 changes: 5 additions & 1 deletion hack/make-rules/test-e2e-node.sh
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ extra_envs=${EXTRA_ENVS:-}
runtime_config=${RUNTIME_CONFIG:-}
ssh_user=${SSH_USER:-"${USER}"}
ssh_key=${SSH_KEY:-}
kubelet_config_file=${KUBELET_CONFIG_FILE:-"test/e2e_node/jenkins/default-kubelet-config.yaml"}
Copy link
Member

Choose a reason for hiding this comment

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

Is there a better place to put this than a so-called jenkins directory?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not yet - I want to restructure that - but that's where our image configs live too (so felt like the natural place to park this for now).

Copy link
Member

Choose a reason for hiding this comment

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

I forget how much of cluster/ and kube-up.sh this relies on... I could see a case being made that it should move closer to e.g. config-test.sh. But I'm also fine with following historical precedent, I know the jenkins dir is basically where (some) node e2e CI config lives (kubernetes/test-infra/jobs/e2e_node being the other historical oddity)


# Parse the flags to pass to ginkgo
ginkgoflags=""
Expand Down Expand Up @@ -164,6 +165,8 @@ if [ "${remote}" = true ] ; then
echo "Ginkgo Flags: ${ginkgoflags}"
echo "Instance Metadata: ${metadata}"
echo "Image Config File: ${image_config_file}"
echo "Kubelet Config File: ${kubelet_config_file}"

# Invoke the runner
go run test/e2e_node/runner/remote/run_remote.go --logtostderr --vmodule=*=4 --ssh-env="gce" \
--zone="${zone}" --project="${project}" --gubernator="${gubernator}" \
Expand All @@ -174,7 +177,7 @@ if [ "${remote}" = true ] ; then
--image-config-file="${image_config_file}" --system-spec-name="${system_spec_name}" \
--runtime-config="${runtime_config}" --preemptible-instances="${preemptible_instances}" \
--ssh-user="${ssh_user}" --ssh-key="${ssh_key}" --image-config-dir="${image_config_dir}" \
--extra-envs="${extra_envs}" --test-suite="${test_suite}" \
--extra-envs="${extra_envs}" --kubelet-config-file="${kubelet_config_file}" --test-suite="${test_suite}" \
"${timeout_arg}" \
2>&1 | tee -i "${artifacts}/build-log.txt"
exit $?
Expand Down Expand Up @@ -210,6 +213,7 @@ else
--ginkgo-flags="${ginkgoflags}" --test-flags="--container-runtime=${runtime} \
--alsologtostderr --v 4 --report-dir=${artifacts} --node-name $(hostname) \
${test_args}" --runtime-config="${runtime_config}" \
--kubelet-config-file="${kubelet_config_file}" \
--build-dependencies=true 2>&1 | tee -i "${artifacts}/build-log.txt"
exit $?
fi
7 changes: 3 additions & 4 deletions test/e2e_node/e2e_node_suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,12 +168,13 @@ func TestE2eNode(t *testing.T) {
}
return
}
// If run-services-mode is not specified, run test.

// We're not running in a special mode so lets run tests.
gomega.RegisterFailHandler(ginkgo.Fail)
reporters := []ginkgo.Reporter{}
reportDir := framework.TestContext.ReportDir
if reportDir != "" {
// Create the directory if it doesn't already exists
// Create the directory if it doesn't already exist
if err := os.MkdirAll(reportDir, 0755); err != nil {
klog.Errorf("Failed creating report directory: %v", err)
} else {
Expand Down Expand Up @@ -294,8 +295,6 @@ func waitForNodeReady() {
}

// updateTestContext updates the test context with the node name.
// TODO(random-liu): Using dynamic kubelet configuration feature to
// update test context with node configuration.
func updateTestContext() error {
setExtraEnvs()
updateImageAllowList()
Expand Down
27 changes: 27 additions & 0 deletions test/e2e_node/jenkins/default-kubelet-config.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
apiVersion: kubelet.config.k8s.io/v1beta1
kind: KubeletConfiguration
cgroupDriver: cgroupfs
cgroupRoot: /
endocrimes marked this conversation as resolved.
Show resolved Hide resolved

# Assign a fixed CIDR to the node because we do not run a node controller
# This MUST be in sync with IPs in:
# - cluster/gce/config-test.sh and
# - test/e2e_node/conformance/run_test.sh
podCIDR: "10.100.0.0/24"
endocrimes marked this conversation as resolved.
Show resolved Hide resolved

# Aggregate volumes frequently to reduce test wait times
volumeStatsAggPeriod: 10s
# Check files frequently to reduce test wait times
fileCheckFrequency: 10s

evictionPressureTransitionPeriod: 30s
evictionHard:
memory.available: 250Mi
nodefs.available: 10%
nodefs.inodesFree: 5%
evictionMinimumReclaim:
nodefs.available: 5%
nodefs.inodesFree: 5%

serializeImagePulls: false

44 changes: 40 additions & 4 deletions test/e2e_node/remote/remote.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package remote
import (
"flag"
"fmt"
"io"
"io/ioutil"
"os"
"os/exec"
Expand All @@ -36,15 +37,51 @@ var resultsDir = flag.String("results-dir", "/tmp/", "Directory to scp test resu

const archiveName = "e2e_node_test.tar.gz"

func copyKubeletConfigIfExists(kubeletConfigFile, dstDir string) error {
srcStat, err := os.Stat(kubeletConfigFile)
if err != nil {
if os.IsNotExist(err) {
return nil
} else {
return err
}
}

if !srcStat.Mode().IsRegular() {
return fmt.Errorf("%s is not a regular file", kubeletConfigFile)
}

source, err := os.Open(kubeletConfigFile)
if err != nil {
return err
}
defer source.Close()

dst := filepath.Join(dstDir, "kubeletconfig.yaml")
destination, err := os.Create(dst)
if err != nil {
return err
}
defer destination.Close()

_, err = io.Copy(destination, source)
return err
}

// CreateTestArchive creates the archive package for the node e2e test.
func CreateTestArchive(suite TestSuite, systemSpecName string) (string, error) {
func CreateTestArchive(suite TestSuite, systemSpecName, kubeletConfigFile string) (string, error) {
klog.V(2).Infof("Building archive...")
tardir, err := ioutil.TempDir("", "node-e2e-archive")
if err != nil {
return "", fmt.Errorf("failed to create temporary directory %v", err)
}
defer os.RemoveAll(tardir)

err = copyKubeletConfigIfExists(kubeletConfigFile, tardir)
if err != nil {
return "", fmt.Errorf("failed to copy kubelet config: %v", err)
}

// Call the suite function to setup the test package.
err = suite.SetupTestPackage(tardir, systemSpecName)
if err != nil {
Expand All @@ -65,8 +102,7 @@ func CreateTestArchive(suite TestSuite, systemSpecName string) (string, error) {
}

// RunRemote returns the command output, whether the exit was ok, and any errors
// TODO(random-liu): junitFilePrefix is not prefix actually, the file name is junit-junitFilePrefix.xml. Change the variable name.
func RunRemote(suite TestSuite, archive string, host string, cleanup bool, imageDesc, junitFilePrefix, testArgs, ginkgoArgs, systemSpecName, extraEnvs, runtimeConfig string) (string, bool, error) {
func RunRemote(suite TestSuite, archive string, host string, cleanup bool, imageDesc, junitFileName, testArgs, ginkgoArgs, systemSpecName, extraEnvs, runtimeConfig string) (string, bool, error) {
// Create the temp staging directory
klog.V(2).Infof("Staging test binaries on %q", host)
workspace := newWorkspaceDir()
Expand Down Expand Up @@ -111,7 +147,7 @@ func RunRemote(suite TestSuite, archive string, host string, cleanup bool, image
}

klog.V(2).Infof("Running test on %q", host)
output, err := suite.RunTest(host, workspace, resultDir, imageDesc, junitFilePrefix, testArgs, ginkgoArgs, systemSpecName, extraEnvs, runtimeConfig, *testTimeout)
output, err := suite.RunTest(host, workspace, resultDir, imageDesc, junitFileName, testArgs, ginkgoArgs, systemSpecName, extraEnvs, runtimeConfig, *testTimeout)

aggErrs := []error{}
// Do not log the output here, let the caller deal with the test output.
Expand Down
4 changes: 4 additions & 0 deletions test/e2e_node/runner/local/run_local.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ var testFlags = flag.String("test-flags", "", "Space-separated list of arguments
var systemSpecName = flag.String("system-spec-name", "", fmt.Sprintf("The name of the system spec used for validating the image in the node conformance test. The specs are at %s. If unspecified, the default built-in spec (system.DefaultSpec) will be used.", system.SystemSpecPath))
var extraEnvs = flag.String("extra-envs", "", "The extra environment variables needed for node e2e tests. Format: a list of key=value pairs, e.g., env1=val1,env2=val2")
var runtimeConfig = flag.String("runtime-config", "", "The runtime configuration for the API server on the node e2e tests. Format: a list of key=value pairs, e.g., env1=val1,env2=val2")
var kubeletConfigFile = flag.String("kubelet-config-file", "", "The KubeletConfiguration file that should be applied to the kubelet")

func main() {
klog.InitFlags(nil)
Expand Down Expand Up @@ -67,6 +68,9 @@ func main() {
systemSpecFile := filepath.Join(rootDir, system.SystemSpecPath, *systemSpecName+".yaml")
args = append(args, fmt.Sprintf("--system-spec-name=%s --system-spec-file=%s --extra-envs=%s", *systemSpecName, systemSpecFile, *extraEnvs))
}
if *kubeletConfigFile != "" {
args = append(args, fmt.Sprintf("--kubelet-config-file=\"%s\"", *kubeletConfigFile))
}
if err := runCommand(ginkgo, args...); err != nil {
klog.Exitf("Test failed: %v", err)
}
Expand Down
21 changes: 11 additions & 10 deletions test/e2e_node/runner/remote/run_remote.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ var ginkgoFlags = flag.String("ginkgo-flags", "", "Passed to ginkgo to specify a
var systemSpecName = flag.String("system-spec-name", "", fmt.Sprintf("The name of the system spec used for validating the image in the node conformance test. The specs are at %s. If unspecified, the default built-in spec (system.DefaultSpec) will be used.", system.SystemSpecPath))
var extraEnvs = flag.String("extra-envs", "", "The extra environment variables needed for node e2e tests. Format: a list of key=value pairs, e.g., env1=val1,env2=val2")
var runtimeConfig = flag.String("runtime-config", "", "The runtime configuration for the API server on the node e2e tests.. Format: a list of key=value pairs, e.g., env1=val1,env2=val2")
var kubeletConfigFile = flag.String("kubelet-config-file", "", "The KubeletConfiguration file that should be applied to the kubelet")

// envs is the type used to collect all node envs. The key is the env name,
// and the value is the env value
Expand Down Expand Up @@ -218,7 +219,7 @@ func main() {
rand.Seed(time.Now().UnixNano())
if *buildOnly {
// Build the archive and exit
remote.CreateTestArchive(suite, *systemSpecName)
remote.CreateTestArchive(suite, *systemSpecName, *kubeletConfigFile)
return
}

Expand Down Expand Up @@ -340,16 +341,16 @@ func main() {
imageConfig := gceImages.images[shortName]
fmt.Printf("Initializing e2e tests using image %s/%s/%s.\n", shortName, imageConfig.project, imageConfig.image)
running++
go func(image *internalGCEImage, junitFilePrefix string) {
results <- testImage(image, junitFilePrefix)
go func(image *internalGCEImage, junitFileName string) {
results <- testImage(image, junitFileName)
}(&imageConfig, shortName)
}
if *hosts != "" {
for _, host := range strings.Split(*hosts, ",") {
fmt.Printf("Initializing e2e tests using host %s.\n", host)
running++
go func(host string, junitFilePrefix string) {
results <- testHost(host, *cleanup, "", junitFilePrefix, *ginkgoFlags)
go func(host string, junitFileName string) {
results <- testHost(host, *cleanup, "", junitFileName, *ginkgoFlags)
}(host, host)
}
}
Expand Down Expand Up @@ -404,7 +405,7 @@ func callGubernator(gubernator bool) {
}

func (a *Archive) getArchive() (string, error) {
a.Do(func() { a.path, a.err = remote.CreateTestArchive(suite, *systemSpecName) })
a.Do(func() { a.path, a.err = remote.CreateTestArchive(suite, *systemSpecName, *kubeletConfigFile) })
return a.path, a.err
}

Expand Down Expand Up @@ -436,7 +437,7 @@ func getImageMetadata(input string) *compute.Metadata {
}

// Run tests in archive against host
func testHost(host string, deleteFiles bool, imageDesc, junitFilePrefix, ginkgoFlagsStr string) *TestResult {
func testHost(host string, deleteFiles bool, imageDesc, junitFileName, ginkgoFlagsStr string) *TestResult {
instance, err := computeService.Instances.Get(*project, *zone, host).Do()
if err != nil {
return &TestResult{
Expand Down Expand Up @@ -466,7 +467,7 @@ func testHost(host string, deleteFiles bool, imageDesc, junitFilePrefix, ginkgoF
}
}

output, exitOk, err := remote.RunRemote(suite, path, host, deleteFiles, imageDesc, junitFilePrefix, *testArgs, ginkgoFlagsStr, *systemSpecName, *extraEnvs, *runtimeConfig)
output, exitOk, err := remote.RunRemote(suite, path, host, deleteFiles, imageDesc, junitFileName, *testArgs, ginkgoFlagsStr, *systemSpecName, *extraEnvs, *runtimeConfig)
return &TestResult{
output: output,
err: err,
Expand Down Expand Up @@ -526,7 +527,7 @@ func getGCEImage(imageRegex, imageFamily string, project string) (string, error)

// Provision a gce instance using image and run the tests in archive against the instance.
// Delete the instance afterward.
func testImage(imageConfig *internalGCEImage, junitFilePrefix string) *TestResult {
func testImage(imageConfig *internalGCEImage, junitFileName string) *TestResult {
ginkgoFlagsStr := *ginkgoFlags
// Check whether the test is for benchmark.
if len(imageConfig.tests) > 0 {
Expand All @@ -552,7 +553,7 @@ func testImage(imageConfig *internalGCEImage, junitFilePrefix string) *TestResul
// If we are going to delete the instance, don't bother with cleaning up the files
deleteFiles := !*deleteInstances && *cleanup

result := testHost(host, deleteFiles, imageConfig.imageDesc, junitFilePrefix, ginkgoFlagsStr)
result := testHost(host, deleteFiles, imageConfig.imageDesc, junitFileName, ginkgoFlagsStr)
// This is a temporary solution to collect serial node serial log. Only port 1 contains useful information.
// TODO(random-liu): Extract out and unify log collection logic with cluste e2e.
serialPortOutput, err := computeService.Instances.GetSerialPortOutput(*project, *zone, host).Port(1).Do()
Expand Down