Repository: containerd/containerd
File: core/mount/mount_linux_test.go (Go)

Code before:

	tcases := []testCase{
		{
			name:              "normal",
			injectUmountFault: false,
		},
		{
			name:              "umount-fault",
			injectUmountFault: true,
		},
	}

	for _, tc := range tcases {
		t.Run(tc.name, func(t *testing.T) {
			fakeLowerDirsDir := t.TempDir()
			if !supportsIDMap(fakeLowerDirsDir) {
				t.Skip("IDmapped mounts not supported on filesystem selected by t.TempDir()")
			}

			lowerDirs := []string{filepath.Join(fakeLowerDirsDir, "lower1"), filepath.Join(fakeLowerDirsDir, "lower2")}
			for _, dir := range lowerDirs {
				require.NoError(t, os.Mkdir(dir, 0755))
				require.NoError(t, os.WriteFile(filepath.Join(dir, filepath.Base(dir)), []byte("foo"), 0644))
			}

			remountsLocation := t.TempDir()

			tmpLowerDirs, cleanup, err := doPrepareIDMappedOverlay(remountsLocation, lowerDirs, int(usernsFD.Fd()))
			require.NoError(t, err)
			require.Len(t, tmpLowerDirs, len(lowerDirs))

			lowerContents := make([][]byte, len(lowerDirs))

			for i, dir := range lowerDirs {
				correspondingRemount := tmpLowerDirs[i]
				filename := filepath.Base(dir)

				expectedFile, err := os.ReadFile(filepath.Join(dir, filename))
				require.NoError(t, err, "reading comparison test fixture file")
				lowerContents[i] = expectedFile

				actualFile, err := os.ReadFile(filepath.Join(correspondingRemount, filename))
				require.NoError(t, err, "reading file in temporary remount")

				assert.Equal(t, expectedFile, actualFile, "file content in temporary remount")
			}

			var busyDh *os.File
			if tc.injectUmountFault {
				busyDh, err = os.Open(tmpLowerDirs[0])
				require.NoError(t, err)

Review comment (bug):
could you please add more information why we need to change this?

The original idea is to verify that `remountsLocation` should be deleted after called cleanup, if there is no failpoint.
But in this change, you manually remove it. we couldn't figure it out who removes it.

Code after:
			lowerContents := make([][]byte, len(lowerDirs))

			for i, dir := range lowerDirs {
				correspondingRemount := tmpLowerDirs[i]
				filename := filepath.Base(dir)

				expectedFile, err := os.ReadFile(filepath.Join(dir, filename))
				require.NoError(t, err, "reading comparison test fixture file")
				lowerContents[i] = expectedFile

				actualFile, err := os.ReadFile(filepath.Join(correspondingRemount, filename))
				require.NoError(t, err, "reading file in temporary remount")

				assert.Equal(t, expectedFile, actualFile, "file content in temporary remount")
			}

			var busyDh *os.File
			if tc.injectUmountFault {
				busyDh, err = os.Open(tmpLowerDirs[0])
				require.NoError(t, err)
				defer busyDh.Close()
			}

			cleanup()

			err = os.Remove(remountsLocation)

			if tc.injectUmountFault {
				// We should have failed to remove the remounts location if the unmount failed.
				assert.Error(t, err, "expected remove to fail (dir not empty), expected remount child locations to still exist after unmount failure")
			} else {
				assert.NoError(t, err, "expected remove to work (dir empty), the child directory should be unmounted and removed")
			}

			// Original lowerdirs should be unaffected.
			for i, dir := range lowerDirs {
				filename := filepath.Base(dir)

				actualFile, err := os.ReadFile(filepath.Join(dir, filename))
				require.NoError(t, err, "reading file in original lowerdir")
				assert.Equal(t, lowerContents[i], actualFile, "file content in original lowerdir")
			}

			// If we blocked cleanup, allow it now so the test stays tidy.
			if tc.injectUmountFault {
				require.NoError(t, busyDh.Close())
				cleanup()
			}
		})
	}
}