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 <andaaron@cisco.com>

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")
+		dir := t.TempDir()
-		if err != nil {
-			panic(err)
-		}
 
 		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("du", "-ab", dir).Output()
-			out, err := exec.Command("bash", "-c", cmd).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)
+			t.Logf("Listing Storage root FS:\n%s\n", out)
-			os.RemoveAll(dir)
 		}()
 
-		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 "+
+		skopeoArgs := []string{
-			"--insecure-policy docker://public.ecr.aws/zomato/alpine:3.11.3 dir:%s/alpine", dir)
+			"copy", "--format=oci", "--insecure-policy", "--dest-tls-verify=false",
-		out, err := exec.Command("bash", "-c", cmd).Output()
+			"docker://public.ecr.aws/zomato/alpine:3.11.3", fmt.Sprintf("oci:%s:alpine", dir),
-		if err != nil {
+		}
-			fmt.Printf("\nCopy skopeo docker image:\n%s\n", out)
+		out, err := exec.Command("skopeo", skopeoArgs...).Output()
-			fmt.Printf("\nerror on skopeo copy:\n%s\n", err)
+		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 "+
+		sourceImg := fmt.Sprintf("oci:%s:alpine", rootDir)
-			"dir:%s/alpine  docker://localhost:%s/client%d:%d", rootDir, zotPort, id, i)
+		destImg := fmt.Sprintf("docker://localhost:%s/client%d:%d", zotPort, id, i)
-		err := exec.Command("bash", "-c", cmd).Run()
+
+		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)
+	}
 }