From 10d9b1514bc33bc595b8f1773356e6012580c1cd Mon Sep 17 00:00:00 2001 From: Andrei Aaron Date: Sun, 24 Jul 2022 05:37:58 +0000 Subject: [PATCH] Fixes/Improvements to pkg/cli/stress_test.go - Decrease RLIMIT_NOFILE and the number of goroutines used to reach this limit (from 512 to 100) - Reset RLIMIT_NOFILE to the initial value before the test finishes - Remove panic - Use temporary dir managed by test framework - Swith to using test logging in pkg/cli/stress_test.go - Execute commands without `bash -c` in pkg/cli/stress_test.go First item is needed as the GH runner seems to stop the test if stressed too much. The lower number is still good enough to reproduce the test conditions Signed-off-by: Andrei Aaron --- pkg/cli/stress_test.go | 77 +++++++++++++++++++++++------------------- 1 file changed, 42 insertions(+), 35 deletions(-) diff --git a/pkg/cli/stress_test.go b/pkg/cli/stress_test.go index 56202c15..7d2a24bd 100644 --- a/pkg/cli/stress_test.go +++ b/pkg/cli/stress_test.go @@ -22,7 +22,7 @@ import ( ) const ( - MaxFileDescriptors = 512 + MaxFileDescriptors = 100 WorkerRunningTime = 60 * time.Second ) @@ -36,7 +36,7 @@ func TestSressTooManyOpenFiles(t *testing.T) { // all the log files to figure out what happened in this test (zot log file, scrub output, storage rootFS tree) SetDefaultFailureMode(FailureContinues) - err := setMaxOpenFilesLimit(MaxFileDescriptors) + initialLimit, err := setMaxOpenFilesLimit(MaxFileDescriptors) So(err, ShouldBeNil) port := test.GetFreePort() @@ -52,32 +52,27 @@ func TestSressTooManyOpenFiles(t *testing.T) { defer func() { data, err := os.ReadFile(logFile.Name()) if err != nil { - fmt.Printf("error when reading zot log file:\n%s\n", err) + t.Logf("error when reading zot log file:\n%s\n", err) } - fmt.Printf("\n\n Zot log file content:\n%s\n", string(data)) + t.Logf("\n\n Zot log file content:\n%s\n", string(data)) os.Remove(logFile.Name()) }() - fmt.Println("Log file is: ", logFile.Name()) + t.Log("Log file is: ", logFile.Name()) conf.Log.Output = logFile.Name() ctlr := api.NewController(conf) - dir, err := ioutil.TempDir("", "oci-repo-test") - if err != nil { - panic(err) - } + dir := t.TempDir() defer func() { // list the content of the directory (useful in case of test fail) - cmd := fmt.Sprintf("du -ab %s", dir) - out, err := exec.Command("bash", "-c", cmd).Output() + out, err := exec.Command("du", "-ab", dir).Output() if err != nil { - fmt.Printf("error when listing storage files:\n%s\n", err) + t.Logf("error when listing storage files:\n%s\n", err) } - fmt.Printf("Listing Storage root FS:\n%s\n", out) - os.RemoveAll(dir) + t.Logf("Listing Storage root FS:\n%s\n", out) }() - fmt.Println("Storage root dir is: ", dir) + t.Log("Storage root dir is: ", dir) ctlr.Config.Storage.RootDirectory = dir go startServer(ctlr) @@ -106,15 +101,16 @@ func TestSressTooManyOpenFiles(t *testing.T) { err = cfgfile.Close() So(err, ShouldBeNil) - cmd := fmt.Sprintf("skopeo copy --format=oci --dest-tls-verify=false "+ - "--insecure-policy docker://public.ecr.aws/zomato/alpine:3.11.3 dir:%s/alpine", dir) - out, err := exec.Command("bash", "-c", cmd).Output() - if err != nil { - fmt.Printf("\nCopy skopeo docker image:\n%s\n", out) - fmt.Printf("\nerror on skopeo copy:\n%s\n", err) + skopeoArgs := []string{ + "copy", "--format=oci", "--insecure-policy", "--dest-tls-verify=false", + "docker://public.ecr.aws/zomato/alpine:3.11.3", fmt.Sprintf("oci:%s:alpine", dir), + } + out, err := exec.Command("skopeo", skopeoArgs...).Output() + if err != nil { + t.Logf("\nerror on skopeo copy:\n%s\n", err) } - So(err, ShouldBeNil) + t.Logf("\nCopy test image locally:\n%s\n", out) var wg sync.WaitGroup for i := 1; i <= MaxFileDescriptors; i++ { @@ -129,6 +125,9 @@ func TestSressTooManyOpenFiles(t *testing.T) { } wg.Wait() + _, err = setMaxOpenFilesLimit(initialLimit) + So(err, ShouldBeNil) + data, err := os.ReadFile(logFile.Name()) So(err, ShouldBeNil) So(string(data), ShouldContainSubstring, "too many open files") @@ -142,12 +141,12 @@ func TestSressTooManyOpenFiles(t *testing.T) { defer func() { data, err := os.ReadFile(scrubFile.Name()) if err != nil { - fmt.Printf("error when reading zot scrub file:\n%s\n", err) + t.Logf("error when reading zot scrub file:\n%s\n", err) } - fmt.Printf("\n\n Zot scrub file content:\n%s\n", string(data)) + t.Logf("\n\n Zot scrub file content:\n%s\n", string(data)) os.Remove(scrubFile.Name()) }() - fmt.Println("Scrub file is: ", scrubFile.Name()) + t.Log("Scrub file is: ", scrubFile.Name()) os.Args = []string{"cli_test", "scrub", cfgfile.Name()} cobraCmd := cli.NewServerRootCmd() @@ -165,9 +164,14 @@ func worker(id int, zotPort, rootDir string) { start := time.Now() for i := 0; ; i++ { - cmd := fmt.Sprintf("skopeo copy --format=oci --insecure-policy --dest-tls-verify=false "+ - "dir:%s/alpine docker://localhost:%s/client%d:%d", rootDir, zotPort, id, i) - err := exec.Command("bash", "-c", cmd).Run() + sourceImg := fmt.Sprintf("oci:%s:alpine", rootDir) + destImg := fmt.Sprintf("docker://localhost:%s/client%d:%d", zotPort, id, i) + + skopeoArgs := []string{ + "copy", "--format=oci", "--insecure-policy", "--dest-tls-verify=false", + sourceImg, destImg, + } + err := exec.Command("skopeo", skopeoArgs...).Run() if err != nil { // nolint: wsl continue // we expect clients to receive errors due to FD limit reached on server } @@ -182,42 +186,45 @@ func worker(id int, zotPort, rootDir string) { } } -func setMaxOpenFilesLimit(limit uint64) error { +func setMaxOpenFilesLimit(limit uint64) (uint64, error) { var rLimit syscall.Rlimit err := syscall.Getrlimit(syscall.RLIMIT_NOFILE, &rLimit) if err != nil { - return err + return 0, err } fmt.Println("Current max. open files ", rLimit.Cur) + initialLimit := rLimit.Cur rLimit.Cur = limit fmt.Println("Changing max. open files to ", limit) err = syscall.Setrlimit(syscall.RLIMIT_NOFILE, &rLimit) if err != nil { - return err + return initialLimit, err } err = syscall.Getrlimit(syscall.RLIMIT_NOFILE, &rLimit) if err != nil { - return err + return initialLimit, err } fmt.Println("Max. open files is set to", rLimit.Cur) - return nil + return initialLimit, nil } func startServer(c *api.Controller) { // this blocks ctx := context.Background() if err := c.Run(ctx); err != nil { - return + fmt.Printf("\nerror on starting zot server: %s\n", err) } } func stopServer(c *api.Controller) { ctx := context.Background() - _ = c.Server.Shutdown(ctx) + if err := c.Server.Shutdown(ctx); err != nil { + fmt.Printf("\nerror on stopping zot server: %s\n", err) + } }