From 879b823fbfe95206b34d4f6502bb5853bff1c95f Mon Sep 17 00:00:00 2001
From: Tulsi Shah <46474643+Tulsishah@users.noreply.github.com>
Date: Thu, 18 Apr 2024 12:12:33 +0530
Subject: [PATCH] Fix flaky e2e tests for managed folder (#1848)

* adding check for permisisons

* updating file path and for loop condition

* adding log statement to check

* testing with sleep

* applying permisison out of setup

* deletion after creation

* lint fix

* lint fix

* lint fix

* review comment

* lint fix

* adding comment

* adding t.Cleanup

* small fix

* removing managedFolderPermission from struct

* review comments
---
 .../managed_folders/admin_permissions_test.go | 29 ++++++++++++-------
 .../util/operations/dir_operations.go         |  4 +--
 2 files changed, 20 insertions(+), 13 deletions(-)

diff --git a/tools/integration_tests/managed_folders/admin_permissions_test.go b/tools/integration_tests/managed_folders/admin_permissions_test.go
index ebd2e19fbb..8dce6c0e71 100644
--- a/tools/integration_tests/managed_folders/admin_permissions_test.go
+++ b/tools/integration_tests/managed_folders/admin_permissions_test.go
@@ -24,6 +24,7 @@ import (
 	"os"
 	"path"
 	"testing"
+	"time"
 
 	"github.com/googlecloudplatform/gcsfuse/v2/tools/integration_tests/util/creds_tests"
 	"github.com/googlecloudplatform/gcsfuse/v2/tools/integration_tests/util/operations"
@@ -50,23 +51,17 @@ var (
 // levels apply additively (union) throughout the resource hierarchy.
 // Hence here managed folder will have admin permission throughout all the tests.
 type managedFoldersAdminPermission struct {
-	managedFolderPermission string
 	bucketPermission        string
 }
 
 func (s *managedFoldersAdminPermission) Setup(t *testing.T) {
-	bucket, testDir = setup.GetBucketAndObjectBasedOnTypeOfMount(TestDirForManagedFolderTest)
 	createDirectoryStructureForNonEmptyManagedFolders(t)
-	if s.managedFolderPermission != "nil" {
-		providePermissionToManagedFolder(bucket, path.Join(testDir, ManagedFolder1), serviceAccount, s.managedFolderPermission, t)
-		providePermissionToManagedFolder(bucket, path.Join(testDir, ManagedFolder2), serviceAccount, s.managedFolderPermission, t)
-	}
 }
 
 func (s *managedFoldersAdminPermission) Teardown(t *testing.T) {
-	revokePermissionToManagedFolder(bucket, path.Join(testDir, ManagedFolder1), serviceAccount, s.managedFolderPermission, t)
-	revokePermissionToManagedFolder(bucket, path.Join(testDir, ManagedFolder2), serviceAccount, s.managedFolderPermission, t)
-	cleanup(bucket, testDir, serviceAccount, s.managedFolderPermission, t)
+	// The 'gsutil rm -rf' command doesn't work on managed folders.
+	// We'll clean up the test directory but leave managed folders.
+	setup.CleanupDirectoryOnGCS(path.Join(bucket, testDir))
 }
 
 func (s *managedFoldersAdminPermission) TestCreateObjectInManagedFolder(t *testing.T) {
@@ -207,13 +202,27 @@ func TestManagedFolders_FolderAdminPermission(t *testing.T) {
 
 	for i := 0; i < len(permissions); i++ {
 		log.Printf("Running tests with flags, bucket have %s permission and managed folder have %s permissions: %s", permissions[i][0], permissions[i][1], flags)
+		bucket, testDir = setup.GetBucketAndObjectBasedOnTypeOfMount(TestDirForManagedFolderTest)
 		ts.bucketPermission = permissions[i][0]
 		if ts.bucketPermission == ViewPermission {
 			creds_tests.RevokePermission(serviceAccount, AdminPermission, setup.TestBucket())
 			creds_tests.ApplyPermissionToServiceAccount(serviceAccount, ViewPermission)
 			defer creds_tests.RevokePermission(serviceAccount, ViewPermission, setup.TestBucket())
 		}
-		ts.managedFolderPermission = permissions[i][1]
+		managedFolderPermission := permissions[i][1]
+		if managedFolderPermission != "nil" {
+			providePermissionToManagedFolder(bucket, path.Join(testDir, ManagedFolder1), serviceAccount, managedFolderPermission, t)
+			providePermissionToManagedFolder(bucket, path.Join(testDir, ManagedFolder2), serviceAccount, managedFolderPermission, t)
+			// Waiting for 10 seconds as it usually takes 10 seconds for policy changes to propagate.
+			time.Sleep(10 * time.Second)
+		}
+
 		test_setup.RunTests(t, ts)
+		revokePermissionToManagedFolder(bucket, path.Join(testDir, ManagedFolder1), serviceAccount, managedFolderPermission, t)
+		revokePermissionToManagedFolder(bucket, path.Join(testDir, ManagedFolder2), serviceAccount, managedFolderPermission, t)
 	}
+	t.Cleanup(func() {
+		operations.DeleteManagedFoldersInBucket(path.Join(testDir, ManagedFolder1), setup.TestBucket())
+		operations.DeleteManagedFoldersInBucket(path.Join(testDir, ManagedFolder2), setup.TestBucket())
+	})
 }
diff --git a/tools/integration_tests/util/operations/dir_operations.go b/tools/integration_tests/util/operations/dir_operations.go
index 38fa6f92a7..ba2d26f02e 100644
--- a/tools/integration_tests/util/operations/dir_operations.go
+++ b/tools/integration_tests/util/operations/dir_operations.go
@@ -179,12 +179,10 @@ func DeleteManagedFoldersInBucket(managedFolderPath, bucket string) {
 }
 
 func CreateManagedFoldersInBucket(managedFolderPath, bucket string) {
-	// Delete if already exist.
-	DeleteManagedFoldersInBucket(managedFolderPath, bucket)
 	gcloudCreateManagedFolderCmd := fmt.Sprintf("alpha storage managed-folders create gs://%s/%s", bucket, managedFolderPath)
 
 	_, err := ExecuteGcloudCommandf(gcloudCreateManagedFolderCmd)
-	if err != nil {
+	if err != nil && !strings.Contains(err.Error(), "The specified managed folder already exists") {
 		log.Fatalf(fmt.Sprintf("Error while creating managed folder: %v", err))
 	}
 }