Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cmd/legacy_main.go
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,7 @@ func forwardedEnvVars() []string {
// should be ignored.
// Forward GCE_METADATA_HOST, GCE_METADATA_ROOT, GCE_METADATA_IP as these are used for mocked metadata services.
// Forward GRPC_GO_LOG_VERBOSITY_LEVEL and GRPC_GO_LOG_SEVERITY_LEVEL as these are used to enable grpc debug logs.
for _, envvar := range []string{"GOOGLE_APPLICATION_CREDENTIALS", "no_proxy", "GCE_METADATA_HOST", "GCE_METADATA_ROOT", "GCE_METADATA_IP", "GRPC_GO_LOG_VERBOSITY_LEVEL", "GRPC_GO_LOG_SEVERITY_LEVEL"} {
for _, envvar := range []string{"GOOGLE_APPLICATION_CREDENTIALS", "no_proxy", "GCE_METADATA_HOST", "GCE_METADATA_ROOT", "GCE_METADATA_IP", "GRPC_GO_LOG_VERBOSITY_LEVEL", "GRPC_GO_LOG_SEVERITY_LEVEL", "GOOGLE_CLOUD_DISABLE_DIRECT_PATH"} {
if envval, ok := os.LookupEnv(envvar); ok {
env = append(env, fmt.Sprintf("%s=%s", envvar, envval))
fmt.Fprintf(
Expand Down
130 changes: 99 additions & 31 deletions tools/integration_tests/grpc_validation/grpc_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package grpc_validation
import (
"fmt"
"os"
"strings"
"testing"
"time"

Expand Down Expand Up @@ -102,61 +103,128 @@ func TestGRPCValidationSuite(t *testing.T) {

func (g *gRPCValidation) TestGRPCDirectPathConnections() {
testCases := []struct {
name string
bucketName string
expectedSuccess bool
expectedLogSubstring string
name string
bucketName string
grpcPathStrategy string
expectedSuccess bool
expectedLogSubstrings []string
}{
{
name: "SingleRegion_Success",
bucketName: g.singleRegionBucketForGRPCSuccess,
expectedLogSubstring: fmt.Sprintf("Successfully connected over gRPC DirectPath for %s", g.singleRegionBucketForGRPCSuccess),
name: "SingleRegion_Success_FallbackStrategy",
bucketName: g.singleRegionBucketForGRPCSuccess,
grpcPathStrategy: "direct-path-with-fallback",
expectedSuccess: true,
expectedLogSubstrings: []string{"DirectPath verification succeeded, continuing with DirectPath."},
},
{
name: "SingleRegion_Failure",
bucketName: g.singleRegionBucketForGRPCFailure,
expectedLogSubstring: fmt.Sprintf("Direct path connectivity unavailable for %s, reason:", g.singleRegionBucketForGRPCFailure),
name: "SingleRegion_Failure_FallbackStrategy",
bucketName: g.singleRegionBucketForGRPCSuccess,
grpcPathStrategy: "direct-path-with-fallback",
expectedSuccess: true,
expectedLogSubstrings: []string{
"DirectPath verification failed",
"Grpc dp is not available and falling back to Http.",
},
},
{
name: "MultiRegion_Success",
bucketName: g.multiRegionBucketForGRPCSuccess,
expectedLogSubstring: fmt.Sprintf("Successfully connected over gRPC DirectPath for %s", g.multiRegionBucketForGRPCSuccess),
name: "SingleRegion_Success_DirectPathOnlyStrategy",
bucketName: g.singleRegionBucketForGRPCSuccess,
grpcPathStrategy: "direct-path-only",
expectedSuccess: true,
expectedLogSubstrings: []string{"DirectPath verification succeeded, continuing with DirectPath."},
},
{
name: "MultiRegion_Failure",
bucketName: g.multiRegionBucketForGRPCFailure,
expectedLogSubstring: fmt.Sprintf("Direct path connectivity unavailable for %s, reason: ", g.multiRegionBucketForGRPCFailure),
name: "SingleRegion_Failure_DirectPathOnlyStrategy",
bucketName: g.singleRegionBucketForGRPCSuccess,
grpcPathStrategy: "direct-path-only",
expectedSuccess: false,
expectedLogSubstrings: []string{
"DirectPath verification failed",
"Grpc dp is not available and not falling back to Http as gRPC path strategy is set to DirectPathOnly",
},
},
{
name: "MultiRegion_Success_FallbackStrategy",
bucketName: g.multiRegionBucketForGRPCSuccess,
grpcPathStrategy: "direct-path-with-fallback",
expectedSuccess: true,
expectedLogSubstrings: []string{"DirectPath verification succeeded, continuing with DirectPath."},
},
{
name: "MultiRegion_Failure_FallbackStrategy",
bucketName: g.multiRegionBucketForGRPCSuccess,
grpcPathStrategy: "direct-path-with-fallback",
expectedSuccess: true,
expectedLogSubstrings: []string{
"DirectPath verification failed",
"Grpc dp is not available and falling back to Http.",
},
},
{
name: "MultiRegion_Success_DirectPathOnlyStrategy",
bucketName: g.multiRegionBucketForGRPCSuccess,
grpcPathStrategy: "direct-path-only",
expectedSuccess: true,
expectedLogSubstrings: []string{"DirectPath verification succeeded, continuing with DirectPath."},
},
{
name: "MultiRegion_Failure_DirectPathOnlyStrategy",
bucketName: g.multiRegionBucketForGRPCSuccess,
grpcPathStrategy: "direct-path-only",
expectedSuccess: false,
expectedLogSubstrings: []string{
"DirectPath verification failed",
"Grpc dp is not available and not falling back to Http as gRPC path strategy is set to DirectPathOnly",
},
},
}

for _, tc := range testCases {
g.T().Run(tc.name, func(t *testing.T) {
if strings.Contains(tc.name, "Failure") {
t.Setenv("GOOGLE_CLOUD_DISABLE_DIRECT_PATH", "true")
} else {
t.Setenv("GOOGLE_CLOUD_DISABLE_DIRECT_PATH", "false")
}

mountPoint, err := os.MkdirTemp("", "grpc_validation_test")
assert.NoError(t, err)
logFile := fmt.Sprintf("/tmp/grpc_%s_%d.txt", tc.name, time.Now().UnixNano())
args := []string{"--client-protocol=grpc", "--log-severity=TRACE", fmt.Sprintf("--log-file=%s", logFile), tc.bucketName, mountPoint}
err = mounting.MountGcsfuse(setup.BinFile(), args)
if err != nil {
if tc.expectedSuccess {
t.Errorf("Unexpected mount failure: %v", err)
}
return
}

defer func() {
if err := util.Unmount(mountPoint); err != nil {
t.Logf("Warning: unmount failed: %v", err)
}
os.Remove(mountPoint)
_ = util.Unmount(mountPoint)
_ = os.Remove(mountPoint)
// Only remove the log file if the test succeeded
if t.Failed() {
t.Logf("Test failed, log file '%s' will not be deleted for inspection.", logFile)
} else {
os.Remove(logFile)
_ = os.Remove(logFile)
}
}()
success := operations.CheckLogFileForMessage(g.T(), tc.expectedLogSubstring, logFile)
require.Equal(t, true, success)

args := []string{
"--client-protocol=grpc",
"--grpc-path-strategy=" + tc.grpcPathStrategy,
"--log-severity=TRACE",
fmt.Sprintf("--log-file=%s", logFile),
tc.bucketName,
mountPoint,
}
err = mounting.MountGcsfuse(setup.BinFile(), args)
if err != nil {
if tc.expectedSuccess {
t.Errorf("Unexpected mount failure: %v", err)
}
} else {
if !tc.expectedSuccess {
t.Errorf("Expected mount failure but mount succeeded")
}
}
Comment on lines +214 to +222

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Using t.Errorf here allows the test to continue executing even when the mount result is unexpected. This leads to cascading failures when checking the log file for substrings that won't exist. Using t.Fatalf instead will stop the test immediately on the first unexpected result, making the test output cleaner and easier to debug.

			if tc.expectedSuccess && err != nil {
				t.Fatalf("Unexpected mount failure: %v", err)
			}
			if !tc.expectedSuccess && err == nil {
				t.Fatalf("Expected mount failure but mount succeeded")
			}


for _, logSubstring := range tc.expectedLogSubstrings {
success := operations.CheckLogFileForMessage(g.T(), logSubstring, logFile)
require.Equal(t, true, success, "Expected message %q not found in log file %s", logSubstring, logFile)
}
})
}
}
13 changes: 13 additions & 0 deletions tools/integration_tests/grpc_validation/setup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"testing"
"time"

"cloud.google.com/go/compute/metadata"
"cloud.google.com/go/storage"
client_util "github.com/googlecloudplatform/gcsfuse/v3/tools/integration_tests/util/client"
"github.com/googlecloudplatform/gcsfuse/v3/tools/integration_tests/util/setup"
Expand Down Expand Up @@ -148,6 +149,18 @@ func TestMain(m *testing.M) {

// Creating a common storage client for the test
ctx = context.Background()

// Skip tests if not running on GCE or if project is not whitelisted.
projectID, err := metadata.ProjectIDWithContext(ctx)
if err != nil {
log.Println("Skipping tests as we are not running on GCE.")
os.Exit(0)
}
if projectID != "gcs-fuse-test" {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Use the existing package-level variable gcpProject instead of hardcoding the string "gcs-fuse-test" again to maintain consistency and avoid duplication.

Suggested change
if projectID != "gcs-fuse-test" {
if projectID != gcpProject {

log.Printf("Skipping tests as project %q is not whitelisted for gRPC validation tests.", projectID)
os.Exit(0)
}

if client, err = client_util.CreateStorageClient(ctx); err != nil {
log.Fatalf("Creation of storage client failed with error : %v", err)
}
Expand Down
Loading