Skip to content

cleanup: remove stale dead code (>1 mo old)#2707

Open
LukeAVanDrie wants to merge 1 commit intokubernetes-sigs:mainfrom
LukeAVanDrie:cleanup/dead-code-sweep
Open

cleanup: remove stale dead code (>1 mo old)#2707
LukeAVanDrie wants to merge 1 commit intokubernetes-sigs:mainfrom
LukeAVanDrie:cleanup/dead-code-sweep

Conversation

@LukeAVanDrie
Copy link
Copy Markdown
Contributor

@LukeAVanDrie LukeAVanDrie commented Mar 27, 2026

What type of PR is this?

/kind cleanup

What this PR does / why we need it:

This PR removes stale, unreachable code to minimize the codebase delta before a repository migration.

To ensure I didn't remove in-flight or newly added work (e.g., interfaces added but not yet implemented), I used a time-bounded audit to focus on dead code older than 1 month, using the following methodology.

  1. Discover Unreachable Code: I used the deadcode tool with the -test flag to locate uncalled functions across the workspace module:
    deadcode -test ./...
  2. Time-Filter (git blame): I ran found cadavers through a custom bash parser (audit_dead_code.sh) to evaluate the exact git blame timestamp of those specific lines.
  3. Audit Rule: Functions older than 1 month were marked as STALE for removal; functions newer were marked as RECENT and left untouched.

Here is the single pipeline used to run the time-bounded evaluator:

~/go/bin/deadcode -test ./... | ./audit_dead_code.sh /dev/stdin "2026-02-27"

Audit Script Used (audit_dead_code.sh):

Click to expand
#!/bin/bash
inputFile=$1
thresholdDate=$2 # e.g. "2026-02-27"

if [ -z "$inputFile" ] || [ -z "$thresholdDate" ]; then
  echo "Usage: $0 <file> <threshold_date (YYYY-MM-DD)>"
  exit 1
fi

# Convert threshold date to seconds since epoch
threshold_epoch=$(date -d "$thresholdDate" +%s 2>/dev/null)
if [ $? -ne 0 ]; then
  echo "Invalid date format: $thresholdDate"
  exit 1
fi

while IFS= read -r line; do
  [ -z "$line" ] && continue
  
  # Ignore headers if any
  if [[ "$line" == "unreachable func"* ]]; then
     continue
  fi

  # Sample line: cmd/epp/runner/runner.go:134:18: unreachable func: Runner.WithExecutableName
  path=$(echo "$line" | cut -d: -f1)
  line_num=$(echo "$line" | cut -d: -f2)
  func_info=$(echo "$line" | cut -d: -f4-)
  
  if [ ! -f "$path" ]; then
    echo "SKIPPED: File missing: $path"
    continue
  fi

  # run git blame with porcelain to get committer time
  blame_out=$(git blame -L "${line_num},${line_num}" --porcelain "$path" 2>/dev/null)
  
  if [ -z "$blame_out" ]; then
    echo "SKIPPED: $path:$line_num (blame failed)"
    continue
  fi
  
  time_line=$(echo "$blame_out" | grep '^committer-time ' | head -n 1)
  commit_epoch=$(echo "$time_line" | awk '{print $2}')
  
  if [ -z "$commit_epoch" ]; then
    echo "SKIPPED: $path:$line_num (no time found)"
    continue
  fi
  
  date_str=$(date -d "@$commit_epoch" +%Y-%m-%d 2>/dev/null)

  if [ "$commit_epoch" -lt "$threshold_epoch" ]; then
    echo "STALE: $date_str - $path:$line_num: $func_info"
  else
    echo "RECENT: $date_str - $path:$line_num: $func_info"
  fi
done < "$inputFile"

Audit Script Results:

Click to expand
STALE: 2025-11-18 - cmd/bbr/runner/runner.go:79:  unreachable func: Runner.WithExecutableName
RECENT: 2026-03-02 - cmd/bbr/runner/runner.go:84:  unreachable func: Runner.WithCustomCollectors
STALE: 2025-11-05 - cmd/epp/runner/runner.go:134:  unreachable func: Runner.WithExecutableName
STALE: 2025-06-12 - cmd/epp/runner/runner.go:139:  unreachable func: Runner.WithRequestControlConfig
STALE: 2025-06-12 - cmd/epp/runner/runner.go:144:  unreachable func: Runner.WithSchedulerConfig
STALE: 2025-10-29 - cmd/epp/runner/runner.go:149:  unreachable func: Runner.WithCustomCollectors
STALE: 2025-02-13 - internal/runnable/leader_election.go:19:  unreachable func: RequireLeaderElection
RECENT: 2026-03-18 - pkg/bbr/framework/cycle_state.go:73:  unreachable func: ReadCycleStateKey
STALE: 2025-08-05 - pkg/common/kubemeta.go:41:  unreachable func: Compare
STALE: 2025-08-05 - pkg/common/kubemeta.go:55:  unreachable func: Less
STALE: 2026-01-28 - pkg/epp/framework/interface/flowcontrol/mocks/mocks.go:46:  unreachable func: WithInferencePoolName
STALE: 2026-01-28 - pkg/epp/framework/interface/flowcontrol/mocks/mocks.go:53:  unreachable func: WithModelName
STALE: 2026-01-28 - pkg/epp/framework/interface/flowcontrol/mocks/mocks.go:60:  unreachable func: WithTargetModelName
STALE: 2025-07-10 - pkg/epp/framework/interface/plugin/handle.go:112:  unreachable func: PluginByType
STALE: 2026-01-22 - pkg/epp/framework/interface/scheduling/cycle_state.go:69:  unreachable func: ReadCycleStateKey
STALE: 2025-10-23 - pkg/epp/framework/plugins/datalayer/extractor/metrics/extractor.go:67:  unreachable func: Produces
RECENT: 2026-03-05 - pkg/epp/framework/plugins/datalayer/source/mocks/data_source_mock.go:40:  unreachable func: NewDataSource
STALE: 2025-11-19 - pkg/epp/metrics/metrics.go:888:  unreachable func: SetTTFTSLOThreshold
STALE: 2025-11-19 - pkg/epp/metrics/metrics.go:894:  unreachable func: SetTPOTSLOThreshold
STALE: 2025-03-19 - pkg/epp/server/controller_manager.go:122:  unreachable func: NewManagerWithOptions
STALE: 2025-11-20 - pkg/epp/util/pool/pool.go:64:  unreachable func: EndpointPoolToInferencePool
STALE: 2025-03-17 - pkg/epp/util/testing/wrappers.go:91:  unreachable func: PodWrapper.LabelsFromPoolSelector
STALE: 2025-11-20 - pkg/epp/util/testing/wrappers.go:237:  unreachable func: InferencePoolWrapper.ToGKNN
STALE: 2025-09-16 - test/utils/utils.go:474:  unreachable func: DeleteObjects

**Removals Made:

  • BBR & EPP Runners (cmd/bbr, cmd/epp):
    • Removed unused setting setters (WithExecutableName, WithRequestControlConfig, etc.)
  • EPP Metrics (pkg/epp/metrics/metrics.go):
    • Removed dynamically unreachable metrics closures (SetTTFTSLOThreshold, SetTPOTSLOThreshold)
  • Common Utilities (pkg/common/kubemeta.go):
    • Removed unused Compare and Less
  • Testing Context Wrappers & Spares (test/utils/utils.go, pool.go, wrappers):
    • Removed DeleteObjects, EndpointPoolToInferencePool, ToGKNN, NewManagerWithOptions, etc.

Which issue(s) this PR fixes:

N/A

Does this PR introduce a user-facing change?:

NONE

@netlify
Copy link
Copy Markdown

netlify bot commented Mar 27, 2026

Deploy Preview for gateway-api-inference-extension ready!

Name Link
🔨 Latest commit 63e1533
🔍 Latest deploy log https://app.netlify.com/projects/gateway-api-inference-extension/deploys/69c704805f10b30008049f64
😎 Deploy Preview https://deploy-preview-2707--gateway-api-inference-extension.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@k8s-ci-robot k8s-ci-robot added the kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. label Mar 27, 2026
@k8s-ci-robot
Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: LukeAVanDrie
Once this PR has been reviewed and has the lgtm label, please assign nirrozenbaum for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 27, 2026
@LukeAVanDrie
Copy link
Copy Markdown
Contributor Author

Some of these funcs may be useful down the road, I'm not strongly advocating for removing all of them. We can evaluate these on a case-by-case basis.

Removes unreachable functions identified by a time-bounded dead code
audit to minimize the codebase delta before migration.

Methods from runners, metrics, and testing wrappers were removed after
verifying they were not modified in the last 30 days.
@LukeAVanDrie LukeAVanDrie force-pushed the cleanup/dead-code-sweep branch from 8144e92 to 63e1533 Compare March 27, 2026 22:28
Copy link
Copy Markdown
Contributor

@ahg-g ahg-g left a comment

Choose a reason for hiding this comment

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

Thanks, a couple of functions should be kept for now

}

// PluginByType retrieves the specified plugin by name and verifies its type
func PluginByType[P Plugin](handlePlugins HandlePlugins, name string) (P, error) {
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.

used in llm-d


// ReadCycleStateKey retrieves data with the given key from CycleState and asserts it to type T.
// Returns an error if the key is not found or the type assertion fails.
func ReadCycleStateKey[T plugin.StateData](c *CycleState, key plugin.StateKey) (T, error) {
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.

used in llm-d

Comment on lines -77 to -81
func (r *Runner) WithExecutableName(exeName string) *Runner {
r.bbrExecutableName = exeName
return r
}

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.

this is used as well.
pls revert

Comment on lines -129 to -133
func (r *Runner) WithExecutableName(exeName string) *Runner {
r.eppExecutableName = exeName
return r
}

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.

this is used in llm-d.pls revert

Comment on lines -144 to -147
func (r *Runner) WithCustomCollectors(collectors ...prometheus.Collector) *Runner {
r.customCollectors = collectors
return r
}
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.

ditto


// Compare returns the comparison of a and b where less than, equal, and greater than return -1, 0,
// and 1 respectively.
func Compare(a, b GKNN) int {
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.

isn't this used in tests for old GKNN?

@LukeAVanDrie
Copy link
Copy Markdown
Contributor Author

@nirrozenbaum Looks like this methodology doesn't work at the moment given x-repo dependencies. Do you want me to revert on a case-by-case basis or simply close this and repeat if/when EPP is moved into llmd-inference-scheduler?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants