HIVE-3127: Add benchmark harness for resource/remoteclient optimization

Makefile

@@ -384,7 +384,14 @@ install-tools:
 	go install $(GO_MOD_FLAGS) github.com/golang/mock/mockgen
 	go install $(GO_MOD_FLAGS) golang.org/x/lint/golint
 	go install $(GO_MOD_FLAGS) github.com/golangci/golangci-lint/cmd/golangci-lint
+	go install $(GO_MOD_FLAGS) sigs.k8s.io/controller-runtime/tools/setup-envtest
 
 .PHONY: coverage
 coverage:
 	hack/codecov.sh
+
+.PHONY: setup-envtest
+# Downloads envtest binaries and prints the required export KUBEBUILDER_ASSETS=... command.
+# Run the printed export command (or add it to your shell rc) before running benchmarks.
+setup-envtest:
+	hack/setup-envtest.sh

go.mod

@@ -74,6 +74,7 @@ require (
 	k8s.io/utils v0.0.0-20251002143259-bc988d571ff4
 	sigs.k8s.io/cluster-api-provider-azure v1.21.1-0.20250929163617-2c4eaa611a39
 	sigs.k8s.io/controller-runtime v0.22.3
+	sigs.k8s.io/controller-runtime/tools/setup-envtest v0.0.0-20240927101401-4381fa0aeee4
 	sigs.k8s.io/controller-tools v0.19.0
 	sigs.k8s.io/yaml v1.6.0
 )

hack/benchmark-comparison.sh

@@ -0,0 +1,127 @@
+#!/bin/bash
+# Compare benchmark performance before and after working changes.
+#
+# Uses git worktrees for safe comparison without touching your working tree.
+# Compares the current working tree against a base commit (default: HEAD~1).
+#
+# Usage:
+#   ./hack/benchmark-comparison.sh [base-commit]
+#
+# Examples:
+#   # Compare current working tree vs parent commit
+#   ./hack/benchmark-comparison.sh
+#
+#   # Compare current working tree vs main branch
+#   ./hack/benchmark-comparison.sh main
+#
+#   # Compare with custom benchmark settings
+#   BENCHTIME=10x COUNT=8 ./hack/benchmark-comparison.sh
+#
+#   # Benchmark specific packages
+#   BENCH_PKGS="./pkg/resource/" ./hack/benchmark-comparison.sh
+
+set -eo pipefail
+
+# Configurable benchmark parameters
+BENCHTIME=${BENCHTIME:-5x}
+COUNT=${COUNT:-6}
+BENCH_PATTERN=${BENCH_PATTERN:-'^Benchmark'}
+BENCH_PKGS=${BENCH_PKGS:-./test/benchmark/}
+BASE_COMMIT=${1:-HEAD~1}
+
+# Ensure we're in a git repository
+if ! git rev-parse --git-dir > /dev/null 2>&1; then
+    echo "Error: Not in a git repository"
+    exit 1
+fi
+
+# Ensure KUBEBUILDER_ASSETS is set
+if [ -z "$KUBEBUILDER_ASSETS" ]; then
+    echo "Error: KUBEBUILDER_ASSETS environment variable not set"
+    echo ""
+    echo "Run: make setup-envtest"
+    echo "Then: export KUBEBUILDER_ASSETS=\"\$(hack/setup-envtest.sh | tail -1 | cut -d'=' -f2 | tr -d '\"')\""
+    exit 1
+fi
+
+# Verify base commit exists
+if ! git rev-parse "$BASE_COMMIT" > /dev/null 2>&1; then
+    echo "Error: Base commit '$BASE_COMMIT' not found"
+    exit 1
+fi
+
+TEMP_DIR=$(mktemp -d)
+WORKTREE_DIR=$(mktemp -d -t hive-benchmark-worktree.XXXXXX)
+
+cleanup() {
+    if [ -d "$WORKTREE_DIR" ]; then
+        echo ""
+        echo "Cleaning up worktree..."
+        git worktree remove "$WORKTREE_DIR" --force 2>/dev/null || true
+        rm -rf "$WORKTREE_DIR" 2>/dev/null || true
+    fi
+}
+trap cleanup EXIT
+
+echo "==========================================================="
+echo "Benchmark Comparison"
+echo "==========================================================="
+echo "Base commit:     $BASE_COMMIT ($(git rev-parse --short "$BASE_COMMIT"))"
+echo "Working tree:    $(git rev-parse --short HEAD)$(git diff --quiet && git diff --cached --quiet || echo ' (dirty)')"
+echo "Benchmark time:  ${BENCHTIME}"
+echo "Run count:       ${COUNT}"
+echo "Pattern:         ${BENCH_PATTERN}"
+echo "Packages:        ${BENCH_PKGS}"
+echo "Results dir:     ${TEMP_DIR}"
+echo "==========================================================="
+echo ""
+
+# Run benchmarks in the current working tree
+echo "-> Running benchmarks in current working tree..."
+go test -bench="${BENCH_PATTERN}" -benchmem -benchtime="${BENCHTIME}" -count="${COUNT}" ${BENCH_PKGS} 2>&1 | tee "${TEMP_DIR}/new.txt"
+
+# Create worktree at base commit
+echo ""
+echo "-> Creating worktree at ${BASE_COMMIT}..."
+git worktree add --detach "$WORKTREE_DIR" "$BASE_COMMIT" --quiet
+
+# Run benchmarks at base commit (in worktree)
+echo "-> Running benchmarks at base commit..."
+(cd "$WORKTREE_DIR" && \
+    export KUBEBUILDER_ASSETS="$KUBEBUILDER_ASSETS" && \
+    go test -bench="${BENCH_PATTERN}" -benchmem -benchtime="${BENCHTIME}" -count="${COUNT}" ${BENCH_PKGS}) 2>&1 | tee "${TEMP_DIR}/old.txt"
+
+# Cleanup worktree early
+echo ""
+git worktree remove "$WORKTREE_DIR" --force
+rm -rf "$WORKTREE_DIR"
+
+# Compare results
+echo ""
+echo "==========================================================="
+echo "BENCHMARK COMPARISON"
+echo "==========================================================="
+echo ""
+
+if ! command -v benchstat &> /dev/null; then
+    echo "Warning: benchstat not installed. Install with:"
+    echo "  go install golang.org/x/perf/cmd/benchstat@latest"
+    echo ""
+    echo "Raw results saved to:"
+    echo "  Old: ${TEMP_DIR}/old.txt"
+    echo "  New: ${TEMP_DIR}/new.txt"
+else
+    benchstat "${TEMP_DIR}/old.txt" "${TEMP_DIR}/new.txt"
+fi
+
+echo ""
+echo "==========================================================="
+echo "Results saved to: ${TEMP_DIR}"
+echo "  Base ($BASE_COMMIT):  ${TEMP_DIR}/old.txt"
+echo "  Working tree:        ${TEMP_DIR}/new.txt"
+echo ""
+if command -v benchstat &> /dev/null; then
+    echo "To view again:"
+    echo "  benchstat ${TEMP_DIR}/old.txt ${TEMP_DIR}/new.txt"
+fi
+echo "==========================================================="

hack/setup-envtest.sh

@@ -0,0 +1,21 @@
+#!/usr/bin/env bash
+
+set -o errexit
+set -o nounset
+set -o pipefail
+
+if ! command -v setup-envtest &> /dev/null; then
+    echo "run 'make install-tools' to install setup-envtest"
+    exit 1
+fi
+
+K8S_VERSION=${K8S_VERSION:-$(go list -m -f "{{ .Version }}" k8s.io/api | awk -F'[v.]' '{printf "1.%d", $3}')}
+
+echo "using k8s version ${K8S_VERSION}"
+
+echo "Fetching binaries (on first run this may take some time)"
+ENVTEST_PATH=$(setup-envtest use "$K8S_VERSION" -p path 2>&1)
+
+echo
+echo "Run this now or add it to your shell rc:"
+echo "  export KUBEBUILDER_ASSETS=\"$ENVTEST_PATH\""

pkg/dependencymagnet/doc.go

@@ -25,4 +25,7 @@ import (
 	// TODO: remove this patch with kube bump to 1.34, which will carry the fix (https://github.com/kubernetes/kubernetes/pull/132378)
 	// Work around for https://github.com/kubernetes/kubernetes/issues/132377
 	_ "k8s.io/code-generator/cmd/validation-gen"
+
+	// Used for envtest
+	_ "sigs.k8s.io/controller-runtime/tools/setup-envtest"
 )

pkg/remoteclient/benchmark_test.go

@@ -0,0 +1,14 @@
+package remoteclient_test
+
+import (
+	"os"
+	"testing"
+
+	"github.com/openshift/hive/test/benchutil"
+)
+
+func TestMain(m *testing.M) {
+	code := m.Run()
+	benchutil.StopEnvTest()
+	os.Exit(code)
+}

pkg/remoteclient/kubeconfig.go

@@ -13,18 +13,24 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
-func NewBuilderFromKubeconfig(c client.Client, secret *corev1.Secret, controllerName hivev1.ControllerName) Builder {
+func NewBuilderFromKubeconfig(c client.Client, secret *corev1.Secret, controllerName hivev1.ControllerName, opts ...BuilderOption) Builder {
+	var bo builderOptions
+	for _, o := range opts {
+		o(&bo)
+	}
 	return &kubeconfigBuilder{
 		c:            c,
 		secret:       secret,
 		fieldManager: "hive3-" + string(controllerName),
+		opts:         bo,
 	}
 }
 
 type kubeconfigBuilder struct {
 	c            client.Client
 	secret       *corev1.Secret
 	fieldManager string
+	opts         builderOptions
 }
 
 // Build is also responsible for verifying reachability of client
@@ -89,5 +95,12 @@ func (b *kubeconfigBuilder) UseSecondaryAPIURL() Builder {
 }
 
 func (b *kubeconfigBuilder) RESTConfig() (*rest.Config, error) {
-	return utils.RestConfigFromSecret(b.secret, false)
+	cfg, err := utils.RestConfigFromSecret(b.secret, false)
+	if err != nil {
+		return nil, err
+	}
+	if b.opts.transportWrapper != nil {
+		cfg.Wrap(b.opts.transportWrapper)
+	}
+	return cfg, nil
 }

pkg/remoteclient/remoteclient.go

@@ -54,7 +54,11 @@ type Builder interface {
 // The controllerName is needed for metrics.
 // If the ClusterDeployment carries the fake cluster annotation, a fake client will be returned populated with
 // runtime.Objects we need to query for in all our controllers.
-func NewBuilder(c client.Client, cd *hivev1.ClusterDeployment, controllerName hivev1.ControllerName) Builder {
+func NewBuilder(c client.Client, cd *hivev1.ClusterDeployment, controllerName hivev1.ControllerName, opts ...BuilderOption) Builder {
+	var bo builderOptions
+	for _, o := range opts {
+		o(&bo)
+	}
 	if utils.IsFakeCluster(cd) {
 		clusterVersion := ""
 		if cd.Status.InstallVersion != nil {
@@ -70,6 +74,7 @@ func NewBuilder(c client.Client, cd *hivev1.ClusterDeployment, controllerName hi
 		cd:             cd,
 		controllerName: controllerName,
 		urlToUse:       activeURL,
+		opts:           bo,
 	}
 }
 
@@ -185,11 +190,29 @@ func SetUnreachableCondition(cd *hivev1.ClusterDeployment, connectionError error
 	return
 }
 
+// BuilderOption configures optional behavior on a Builder.
+type BuilderOption func(*builderOptions)
+
+type builderOptions struct {
+	transportWrapper func(http.RoundTripper) http.RoundTripper
+}
+
+// WithTransportWrapper adds a transport wrapper that will be applied to the
+// REST config returned by RESTConfig(). The wrapper is applied after the
+// controller metrics transport, so it sits on the outermost layer and
+// observes all HTTP round trips.
+func WithTransportWrapper(wrapper func(http.RoundTripper) http.RoundTripper) BuilderOption {
+	return func(o *builderOptions) {
+		o.transportWrapper = wrapper
+	}
+}
+
 type builder struct {
 	c              client.Client
 	cd             *hivev1.ClusterDeployment
 	controllerName hivev1.ControllerName
 	urlToUse       int
+	opts           builderOptions
 }
 
 const (
@@ -267,6 +290,10 @@ func (b *builder) RESTConfig() (*rest.Config, error) {
 
 	utils.AddControllerMetricsTransportWrapper(cfg, b.controllerName, true)
 
+	if b.opts.transportWrapper != nil {
+		cfg.Wrap(b.opts.transportWrapper)
+	}
+
 	if override := b.cd.Spec.ControlPlaneConfig.APIURLOverride; override != "" {
 		if b.urlToUse == primaryURL ||
 			(b.urlToUse == activeURL && IsPrimaryURLActive(b.cd)) {

pkg/remoteclient/remoteclient_benchmark_test.go

@@ -0,0 +1,53 @@
+package remoteclient_test
+
+import (
+	"testing"
+
+	"github.com/openshift/hive/test/benchutil"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+)
+
+func benchBuilderOp(b *testing.B, fn func(*testing.B, *benchutil.BenchRemoteClient)) {
+	benchutil.ControllerHarness[*benchutil.BenchRemoteClient]{
+		Setup: benchutil.SetupRemoteClient,
+		Reconcile: func(b *testing.B, brc *benchutil.BenchRemoteClient, _ []client.Object, _ int) {
+			fn(b, brc)
+		},
+	}.Run(b)
+}
+
+// BenchmarkBuilderBuild measures Build: secret read + parse + discovery + client creation.
+func BenchmarkBuilderBuild(b *testing.B) {
+	benchBuilderOp(b, func(b *testing.B, brc *benchutil.BenchRemoteClient) {
+		if _, err := brc.NewBuilder().Build(); err != nil {
+			b.Fatalf("Build failed: %v", err)
+		}
+	})
+}
+
+// BenchmarkBuilderRESTConfig measures secret read + parse (no remote calls).
+func BenchmarkBuilderRESTConfig(b *testing.B) {
+	benchBuilderOp(b, func(b *testing.B, brc *benchutil.BenchRemoteClient) {
+		if _, err := brc.NewBuilder().RESTConfig(); err != nil {
+			b.Fatalf("RESTConfig failed: %v", err)
+		}
+	})
+}
+
+// BenchmarkBuilderBuildDynamic measures BuildDynamic (no discovery).
+func BenchmarkBuilderBuildDynamic(b *testing.B) {
+	benchBuilderOp(b, func(b *testing.B, brc *benchutil.BenchRemoteClient) {
+		if _, err := brc.NewBuilder().BuildDynamic(); err != nil {
+			b.Fatalf("BuildDynamic failed: %v", err)
+		}
+	})
+}
+
+// BenchmarkBuilderBuildKubeClient measures BuildKubeClient (no discovery).
+func BenchmarkBuilderBuildKubeClient(b *testing.B) {
+	benchBuilderOp(b, func(b *testing.B, brc *benchutil.BenchRemoteClient) {
+		if _, err := brc.NewBuilder().BuildKubeClient(); err != nil {
+			b.Fatalf("BuildKubeClient failed: %v", err)
+		}
+	})
+}

pkg/resource/benchmark_test.go

@@ -0,0 +1,14 @@
+package resource_test
+
+import (
+	"os"
+	"testing"
+
+	"github.com/openshift/hive/test/benchutil"
+)
+
+func TestMain(m *testing.M) {
+	code := m.Run()
+	benchutil.StopEnvTest()
+	os.Exit(code)
+}