Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
42d7a9f
Add read-ahead configuration and verification support for integration…
kislaykishore Jun 14, 2026
65ad250
Address code review comments: replace syscall with unix package and u…
kislaykishore Jun 14, 2026
53250e2
Fix errcheck lint issues: explicitly ignore os.RemoveAll error returns
kislaykishore Jun 14, 2026
d796168
Address code review comments: prioritize setup.MntDir() and skip veri…
kislaykishore Jun 14, 2026
970f287
feat(npi): add npi-conformance target to Makefile supporting read-ahe…
kislaykishore Jun 20, 2026
7dd45a1
Refactor NPI conformance to run via a dedicated CD script
kislaykishore Jun 20, 2026
e08125d
fix(npi): add --skip-emulator to bypass emulator tests in conformance…
kislaykishore Jun 20, 2026
64c8b89
Address PR review: prioritize resolving mount directory from command …
kislaykishore Jun 20, 2026
c608202
Address PR review: separate Linux-specific mounting code and tests to…
kislaykishore Jun 20, 2026
803fb49
style: format mounting.go using goimports and gofmt
kislaykishore Jun 20, 2026
85888f4
Address PR review: resolve latest gemini-code-assist comments (enable…
kislaykishore Jun 20, 2026
775f3f3
fix(integration_tests): reset global mntDir state on unmount to preve…
kislaykishore Jun 20, 2026
ffad885
Address PR review: prevent clearing pre-mounted directory path in Unm…
kislaykishore Jun 20, 2026
24824d1
revert: remove global SetMntDir reset on unmount since flag prioritiz…
kislaykishore Jun 20, 2026
19d1683
Address PR review: remove redundant getMountDir heuristic in favor of…
kislaykishore Jun 20, 2026
8f8d43b
Address PR review: declare READ_AHEAD_KB at the top of the Makefile f…
kislaykishore Jun 20, 2026
9216b01
Address PR review: add defensive empty mount directory checks in Linu…
kislaykishore Jun 20, 2026
5112c58
fix(integration_tests): introduce RootMntDir to cleanly separate FUSE…
kislaykishore Jun 20, 2026
7e2617d
feat: run NPI conformance suite once using default kernel read-ahead …
kislaykishore Jun 20, 2026
266c835
feat: add optional --read-ahead-kb flag back to npi_conformance.sh fo…
kislaykishore Jun 20, 2026
594e6f9
docs: update --read-ahead-kb help message in npi_conformance.sh usage
kislaykishore Jun 20, 2026
5f9c31e
feat: add FUSE filesystem check in Linux mounting utility for host sa…
kislaykishore Jun 20, 2026
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
7 changes: 7 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -89,3 +89,10 @@ e2e-test:
REGION=$$(echo $$ZONE | sed 's/-[a-z]$$//'); \
echo $$REGION; \
tools/integration_tests/improved_run_e2e_tests.sh --bucket-location $$REGION $(if $(PROJECT),--project-id $(PROJECT))

npi-conformance: fmt
@echo "Running NPI Conformance Suite via tools/cd_scripts/npi_conformance.sh..."
Comment on lines +93 to +94

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

Instead of using $(eval ...) inside the recipe, define the default value for READ_AHEAD_KB at the top of the Makefile alongside other variable definitions (e.g., CSI_VERSION ?= main). This is more idiomatic and avoids mixing Make evaluation with shell execution.

npi-conformance: fmt
	@echo "Running NPI Conformance Suite via tools/cd_scripts/npi_conformance.sh..."

ZONE=$$(curl -s -H "Metadata-Flavor: Google" metadata.google.internal/computeMetadata/v1/instance/zone 2>/dev/null | awk -F'/' '{print $$NF}'); \
REGION=$${BUCKET_LOCATION:-$$(echo $$ZONE | sed 's/-[a-z]$$//')}; \
tools/cd_scripts/npi_conformance.sh --local-run $$(if [ -n "$$REGION" ]; then echo "--bucket-location $$REGION"; fi) $(if $(PROJECT),--project-id $(PROJECT))

219 changes: 219 additions & 0 deletions tools/cd_scripts/npi_conformance.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,219 @@
#!/bin/bash
# Copyright 2026 Google LLC
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

# Exit on error, treat unset variables as errors, and propagate pipeline errors.
# Note: We handle command failures manually for the main test runs to ensure both run.
set -euo pipefail

# Logging Helpers
log_info() {
echo "[INFO] $(date +"%Y-%m-%d %H:%M:%S"): $1"
}

log_error() {
echo "[ERROR] $(date +"%Y-%m-%d %H:%M:%S"): $1"
}

# Defaults
LOCAL_RUN=false
RELEASE_PACKAGE_BUCKET=""
RELEASE_VERSION=""
RUN_TESTS_WITH_ZONAL_BUCKET=false
READ_AHEAD_KB=""

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

The help message for --read-ahead-kb states that the default value is 128, but the variable READ_AHEAD_KB is initialized to an empty string "". This results in the read-ahead configuration and verification being bypassed by default, skipping the TestVerifyReadAheadKB test.

To align the implementation with the documented default and ensure the verification test runs by default in the NPI conformance suite, initialize READ_AHEAD_KB to "128".

Suggested change
READ_AHEAD_KB=""
READ_AHEAD_KB="128"

PROJECT_ID=""
BUCKET_LOCATION=""

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.

high

The --read-ahead-kb parameter is documented in the usage description but is not initialized. Since set -u is enabled, referencing this variable later without initialization will cause the script to crash. Initialize READ_AHEAD_KB to -1 by default.

Suggested change
BUCKET_LOCATION=""
BUCKET_LOCATION=""
READ_AHEAD_KB=-1


usage() {
echo "Usage: $0 [options]"
echo "Options:"
echo " --local-run Pass this flag to run this script for local runs. If this flag is passed then gcsfuse is built"
echo " locally instead of getting installed by pre-built package from release bucket."
echo " --release-package-bucket <bkt> Name of the GCS bucket from which release packages will be fetched."
echo " Release package bucket is required if not running using --local-run"
echo " --release-version <3.0.0> Release version determines from which directory the pre-built package is used from release package bucket."
echo " Release version is required if not running using --local-run"
echo " --zonal Should run tests for zonal bucket (Default: false)"
echo " --read-ahead-kb <kb> The read-ahead size in KB to set on the FUSE mount point. If not specified, the kernel default is used. (Optional)"
echo " --project-id <project-id> Google Cloud Project ID. (Optional)"
echo " --bucket-location <location> Google Cloud Storage bucket location (e.g. 'us-central1'). (Optional)"
echo " --output-dir <output-dir> Directory in which all of log files generated by this script will be stored. (Default: /tmp)"
echo " --help Display this help and exit."
exit "$1"
}

# Define options for getopt
LONG=local-run,zonal,release-package-bucket:,release-version:,read-ahead-kb:,project-id:,bucket-location:,output-dir:,help

# Parse the options using getopt
if ! PARSED=$(getopt --options "" --longoptions "$LONG" --name "$0" -- "$@"); then
usage 1
fi

# Output directory where all artifacts generated by this script will be stored.
OUTPUT_DIR=""
# Read the parsed options back into the positional parameters.
eval set -- "$PARSED"

# Loop through the options and assign values to our variables
while (( $# >= 1 )); do
case "$1" in
--release-version)
RELEASE_VERSION="$2"
shift 2
RE="^[0-9]+\.[0-9]+\.[0-9]+$"
if [[ ! $RELEASE_VERSION =~ $RE ]]; then
log_error "--release-version value '$RELEASE_VERSION' is incorrectly formatted."
usage 1
fi
;;
--release-package-bucket)
RELEASE_PACKAGE_BUCKET="$2"
shift 2
;;
--zonal)
RUN_TESTS_WITH_ZONAL_BUCKET=true
shift
;;

--read-ahead-kb)
READ_AHEAD_KB="$2"
shift 2
;;
--project-id)
PROJECT_ID="$2"
shift 2
;;
--bucket-location)
BUCKET_LOCATION="$2"
shift 2
;;
--output-dir)
OUTPUT_DIR="$2"
shift 2
;;
--local-run)
LOCAL_RUN=true
shift
;;
--help)
usage 0
;;
--)

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.

high

Add a case in the getopt parsing loop to handle the --read-ahead-kb option.

Suggested change
--)
--read-ahead-kb)
READ_AHEAD_KB="$2"
shift 2
;;
--)

shift
break
;;
*)
log_error "Unrecognized arguments [$*]."
usage 1
;;
esac
done

# Argument validation
if [[ "$LOCAL_RUN" == "false" ]]; then
if [[ -z "$RELEASE_VERSION" ]]; then
log_error "--release-version required if not running with --local-run"
usage 1
fi
if [[ -z "$RELEASE_PACKAGE_BUCKET" ]]; then
log_error "--release-package-bucket required if not running with --local-run"
usage 1
fi
fi

# Build args for the e2e script
ARGS=()

# Local Run Validation and gcsfuse package installation.
if ${LOCAL_RUN}; then
log_info "Running script in local mode, gcsfuse binary will be built from current repository."
else
log_info "Running script with release version package from release bucket for version $RELEASE_VERSION will be installed."
# Identify the OS and Architecture
if [ -f /etc/os-release ]; then
DISTRO_DATA=$( (source /etc/os-release; echo "${ID:-} ${ID_LIKE:-}") )
if [[ "$DISTRO_DATA" == *"debian"* ]] || [[ "$DISTRO_DATA" == *"ubuntu"* ]]; then
ARCH=$(dpkg --print-architecture)
ARGS+=(
"--install-package-from-path=gs://${RELEASE_PACKAGE_BUCKET}/v${RELEASE_VERSION}/gcsfuse_${RELEASE_VERSION}_${ARCH}.deb"
)
elif [[ "$DISTRO_DATA" == *"rhel"* ]] || [[ "$DISTRO_DATA" == *"centos"* ]]; then
ARCH=$(uname -m)
ARGS+=(
"--install-package-from-path=gs://${RELEASE_PACKAGE_BUCKET}/v${RELEASE_VERSION}/gcsfuse-${RELEASE_VERSION}-1.${ARCH}.rpm"
)
else
log_error "This script only supports Debian/Ubuntu/rhel/centos based distributions."
exit 1
fi
else
log_error "/etc/os-release not found. Unable to determine distribution"
exit 1
fi
fi

# Set parallelism to 4 as it is optimal for all of the release VM(s).
ARGS+=("--package-level-parallelism=4")

# Set --zonal arg if required
if ${RUN_TESTS_WITH_ZONAL_BUCKET}; then
log_info "Running zonal NPI conformance tests."
ARGS+=("--zonal")
else
log_info "Running regional NPI conformance tests."
fi

# Fallback to /tmp if OUTPUT_DIR is unset
OUTPUT_DIR="${OUTPUT_DIR:-/tmp}"
mkdir -p "$OUTPUT_DIR" || {
log_error "Failed to create or access output directory '$OUTPUT_DIR'";
exit 1
}
ARGS+=("--output-dir=$OUTPUT_DIR")

# Set --flake-attempts to 3 for NPI conformance tests.
ARGS+=("--flake-attempts=3")

# Pass project-id if provided
if [[ -n "$PROJECT_ID" ]]; then
ARGS+=("--project-id=$PROJECT_ID")
fi

# Pass bucket-location if provided
if [[ -n "$BUCKET_LOCATION" ]]; then
ARGS+=("--bucket-location=$BUCKET_LOCATION")
fi
Comment on lines +195 to +197

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.

high

Forward the --read-ahead-kb parameter to improved_run_e2e_tests.sh if it has been configured.

Suggested change
if [[ -n "$BUCKET_LOCATION" ]]; then
ARGS+=("--bucket-location=$BUCKET_LOCATION")
fi
if [[ -n "$BUCKET_LOCATION" ]]; then
ARGS+=("--bucket-location=$BUCKET_LOCATION")
fi
if [[ "$READ_AHEAD_KB" -ne -1 ]]; then
ARGS+=("--read-ahead-kb=$READ_AHEAD_KB")
fi


# Pass read-ahead-kb if explicitly provided, otherwise default to kernel default
if [[ -n "$READ_AHEAD_KB" ]]; then
ARGS+=("--read-ahead-kb=$READ_AHEAD_KB")
fi

# Configure NPI packages to run
ARGS+=(
"--skip-emulator"
"--run-package=concurrent_operations|kernel_list_cache|list_large_dir|dentry_cache|read_cache|local_file|operations"
)


log_info "=========================================================================="
log_info "🚀 Running NPI Conformance Suite..."
log_info "=========================================================================="
if ! bash ./tools/integration_tests/improved_run_e2e_tests.sh "${ARGS[@]}"; then
log_error "NPI Conformance Suite failed."
exit 1
else
log_info "NPI Conformance Suite completed successfully!"
fi
13 changes: 12 additions & 1 deletion tools/integration_tests/improved_run_e2e_tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -137,10 +137,11 @@ PACKAGE_LEVEL_PARALLELISM=10 # Controls how many test packages are run in parall
RUN_PACKAGE_REGEX=""
SKIP_EMULATOR=false
FLAKE_ATTEMPTS=1
READ_AHEAD_KB=-1

# Define options for getopt
# A long option name followed by a colon indicates it requires an argument.
LONG=bucket-location:,project-id:,test-installed-package,install-package-from-path:,skip-non-essential-tests,no-build-binary-in-script,test-on-tpc-endpoint,presubmit,zonal,package-level-parallelism:,track-resource-usage,output-dir:,help,run-package:,skip-emulator,flake-attempts:
LONG=bucket-location:,project-id:,test-installed-package,install-package-from-path:,skip-non-essential-tests,no-build-binary-in-script,test-on-tpc-endpoint,presubmit,zonal,package-level-parallelism:,track-resource-usage,output-dir:,help,run-package:,skip-emulator,flake-attempts:,read-ahead-kb:

# Parse the options using getopt
# --options "" specifies that there are no short options.
Expand Down Expand Up @@ -216,6 +217,10 @@ while (( $# >= 1 )); do
FLAKE_ATTEMPTS="$2"
shift 2
;;
--read-ahead-kb)
READ_AHEAD_KB="$2"
shift 2
;;
--help)
usage 0
;;
Expand Down Expand Up @@ -273,6 +278,9 @@ validate_option_value "--bucket-location" "$BUCKET_LOCATION"
validate_option_value "--project-id" "$PROJECT_ID"
validate_option_value "--package-level-parallelism" "$PACKAGE_LEVEL_PARALLELISM"
validate_option_value "--flake-attempts" "$FLAKE_ATTEMPTS"
if [[ "$READ_AHEAD_KB" -ne -1 ]]; then
validate_option_value "--read-ahead-kb" "$READ_AHEAD_KB"
fi

# Validate test install package from path
if ${TEST_INSTALLED_PACKAGE} && [[ -n "$INSTALL_PACKAGE_FROM_PATH" ]]; then
Expand Down Expand Up @@ -757,6 +765,9 @@ test_package() {
if [[ -n "$BUILT_BY_SCRIPT_GCSFUSE_BUILD_DIR" ]]; then
go_test_cmd_parts+=("--gcsfuse_prebuilt_dir=${BUILT_BY_SCRIPT_GCSFUSE_BUILD_DIR}")
fi
if [[ "$READ_AHEAD_KB" -ne -1 ]]; then
go_test_cmd_parts+=("--read-ahead-kb=${READ_AHEAD_KB}")
fi

local go_test_cmd test_package_log_file start=$SECONDS exit_code=0
# Use printf %q to quote each argument safely for eval
Expand Down
34 changes: 34 additions & 0 deletions tools/integration_tests/operations/read_ahead_verify_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
// Copyright 2026 Google LLC
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package operations_test

import (
"testing"

"github.com/googlecloudplatform/gcsfuse/v3/tools/integration_tests/util/mounting"
"github.com/googlecloudplatform/gcsfuse/v3/tools/integration_tests/util/setup"
)

func TestVerifyReadAheadKB(t *testing.T) {
expectedKB := setup.ReadAheadKB()
if expectedKB <= 0 {
t.Skip("Skipping read-ahead verification as read-ahead-kb is not configured.")
}

err := mounting.VerifyReadAhead(setup.RootMntDir(), expectedKB)
if err != nil {
t.Errorf("read-ahead verification failed: %v", err)
}
Comment thread
kislaykishore marked this conversation as resolved.
}
8 changes: 8 additions & 0 deletions tools/integration_tests/util/mounting/mounting.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,5 +55,13 @@ func MountGcsfuse(binaryFile string, flags []string) error {
log.Println("Error: ", string(output))
return fmt.Errorf("cannot mount gcsfuse: %w\n", err)
}

readAheadKB := setup.ReadAheadKB()
if readAheadKB > 0 {
if err := ConfigureReadAhead(setup.RootMntDir(), readAheadKB); err != nil {
return fmt.Errorf("failed to configure read-ahead: %w", err)
}
}
Comment on lines +59 to +64

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

In integration tests, GCSFuse might be mounted to a custom directory passed via flags rather than the global setup.RootMntDir(). To ensure the read-ahead configuration is applied to the correct mount point, dynamically resolve the mount directory from the last argument of flags if it exists and is a directory, falling back to setup.RootMntDir().

	readAheadKB := setup.ReadAheadKB()
	if readAheadKB > 0 {
		mountDir := setup.RootMntDir()
		if len(flags) > 0 {
			lastArg := flags[len(flags)-1]
			if fi, err := os.Stat(lastArg); err == nil && fi.IsDir() {
				mountDir = lastArg
			}
		}
		if err := ConfigureReadAhead(mountDir, readAheadKB); err != nil {
			return fmt.Errorf("failed to configure read-ahead: %w", err)
		}
	}


return nil
}
Loading
Loading