Skip to content

Make SessionAffinity configurable via CRD field#3994

Merged
eleftherias merged 1 commit intomainfrom
feat/configurable-session-affinity
Mar 4, 2026
Merged

Make SessionAffinity configurable via CRD field#3994
eleftherias merged 1 commit intomainfrom
feat/configurable-session-affinity

Conversation

@JAORMX
Copy link
Copy Markdown
Collaborator

@JAORMX JAORMX commented Mar 4, 2026

Summary

  • Add sessionAffinity field (enum: ClientIP/None, default: ClientIP) to MCPServerSpec, MCPRemoteProxySpec, and VirtualMCPServerSpec
  • Wire the field through service creation and drift detection in all three controllers
  • Fix pre-existing bug in MCPServer controller where service update path did not copy Labels/Annotations (could cause infinite reconciliation)

Follow-up to #3986 which hardcoded SessionAffinity: ClientIP. Review feedback identified this should be configurable: stateless MCP servers don't need stickiness and external load balancers may conflict with client-side affinity.

EmbeddingServer is intentionally excluded — embedding servers are stateless inference endpoints, not MCP protocol services.

Test plan

  • task lint passes (0 issues)
  • task test passes
  • Generated CRDs contain new field with default and enum validation
  • Manual: deploy operator, create MCPServer without sessionAffinity → Service has ClientIP
  • Manual: create MCPServer with sessionAffinity: None → Service has None
  • Manual: change sessionAffinity on existing resource → Service updates via drift detection

🤖 Generated with Claude Code

@JAORMX @claude
Make SessionAffinity configurable via CRD field
bcc1af9 
PR #3986 hardcoded SessionAffinity: ClientIP on all operator-created
Services. Review feedback identified this should be configurable:
stateless MCP servers don't need stickiness and external load balancers
may conflict.

Add a sessionAffinity field (enum: ClientIP/None, default: ClientIP) to
MCPServerSpec, MCPRemoteProxySpec, and VirtualMCPServerSpec. Wire the
field through service creation and drift detection in all three
controllers.

Also fix a pre-existing bug in the MCPServer controller where the
service update path did not copy Labels/Annotations, which could cause
infinite reconciliation when those fields drifted.

EmbeddingServer is intentionally excluded — embedding servers are
stateless inference endpoints, not MCP protocol services.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions github-actions bot added the size/M Medium PR: 300-599 lines changed label Mar 4, 2026
@JAORMX JAORMX requested a review from eleftherias March 4, 2026 13:06
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 4, 2026 •
edited
Loading

Codecov Report

❌ Patch coverage is 93.75000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 68.49%. Comparing base (b7cf0f7) to head (bcc1af9).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
...d/thv-operator/controllers/mcpserver_controller.go 83.33% 2 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #3994      +/-   ##
==========================================
+ Coverage   68.45%   68.49%   +0.04%     
==========================================
  Files         437      437              
  Lines       44725    44751      +26     
==========================================
+ Hits        30616    30652      +36     
+ Misses      11724    11709      -15     
- Partials     2385     2390       +5

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@JAORMX
Copy link
Copy Markdown
Collaborator Author

JAORMX commented Mar 4, 2026

@claude be a home and use a skill to fix the helm chart linter issue.

@claude
Copy link
Copy Markdown
Contributor

claude bot commented Mar 4, 2026 •
edited
Loading

Claude finished @JAORMX's task in 3m 32s —— View job

Helm Chart Contribution Check Report

  • Check CI status to identify helm chart linter failures
  • Run check-contribution skill to identify required fixes
  • Verify all helm chart contribution practices
  • Report findings

Contribution Check Results

Check Status Notes
Commit signatures ⚠️ Not GPG-signed Commits are unsigned (no PGP block). Consider signing if required by org policy.
Helm template renders ✅ Pass helm template test . renders valid YAML without errors
ct lint ⚠️ Cannot run locally ct (chart-testing) not installed in this environment
Documentation (helm-docs) ✅ Pass helm-docs --chart-search-root=deploy/charts produces zero changes — READMEs are up to date
Chart version bump ✅ Pass ct.yaml has check-version-increment: false — no version bump required

Root Cause of CI Failure

The "Helm Charts / Lint and Test Helm Charts" CI failure was not a helm linter issue — it was a transient network error during the "Set up ko" step:

curl -fsL https://github.com/ko-build/ko/releases/download/v0.18.1/ko_0.18.1_Linux_x86_64.tar.gz
gzip: stdin: unexpected end of file
tar: Error is not recoverable: exiting now

The ko binary download failed mid-stream. This prevented the workflow from reaching the actual linting steps (task helm-docs → check uncommitted changes → ct lint).

Locally verified: Running helm-docs --chart-search-root=deploy/charts generates no uncommitted changes, confirming the documentation is already correct.

Recommendation

The CI failure is transient and unrelated to the PR changes. Simply re-run the "Helm Charts / Lint and Test Helm Charts" job in GitHub Actions — no code changes are needed.

@eleftherias eleftherias merged commit a447aea into main Mar 4, 2026
60 of 62 checks passed
@eleftherias eleftherias deleted the feat/configurable-session-affinity branch March 4, 2026 15:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@eleftherias eleftherias eleftherias approved these changes

@ChrisJBurns ChrisJBurns Awaiting requested review from ChrisJBurns ChrisJBurns is a code owner

@yrobla yrobla Awaiting requested review from yrobla yrobla is a code owner

@jerm-dro jerm-dro Awaiting requested review from jerm-dro jerm-dro is a code owner

@jhrozek jhrozek Awaiting requested review from jhrozek jhrozek is a code owner

Assignees

No one assigned

Labels

size/M Medium PR: 300-599 lines changed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@JAORMX @eleftherias