Skip to content

[ML] Fix serverless security trained models stats assertion on MKI - #291031

Closed
steliosmavro wants to merge 1 commit into
elastic:mainfrom
steliosmavro:stelios/fix-ml-trained-models-stats-flake
Closed

steliosmavro wants to merge 1 commit into
elastic:mainfrom
steliosmavro:stelios/fix-ml-trained-models-stats-flake

Conversation

@steliosmavro

Copy link
Copy Markdown
Contributor

Summary

The serverless security Trained models list FTR suite has been failing on the post merge MKI pipeline since #275476 landed:

Error: expected 'Total trained models: 2' to sort of equal 'Total trained models: 3'

That PR changed the assertion from assertStats(2) to assertStats(3). The extra model comes from the new Security Labs auto install hook that runs when the Security app mounts. In Kibana PR CI there is no EIS, so the default inference ID falls back to .elser-2-elasticsearch and ES deploys a local ELSER model that shows up in the trained models list. On a real serverless project EIS is available, so the same flow picks Jina v5 which is hosted remotely. No local model appears and the count stays at 2.

The hardcoded number therefore only matched one of the two environments. This change asserts against the count that ES itself reports so the test holds in both.

Why this comparison is safe: the ML list route does not filter by space, and its only other filter (filterForEnabledFeatureModels) is a no op for security projects because ad, dfa and nlp are all enabled there. So the UI total and _ml/trained_models always agree.

Test plan

  • Kibana CI passes the x-pack/solutions/security/test/serverless/functional/configs/config.ts FTR config
  • Next scheduled run of the appex-qa serverless kibana-ftr-tests pipeline on main is green for this suite

…rity test

The hardcoded total was bumped to 3 in elastic#275476 because the new Security Labs
auto-install deploys a local ELSER model in CI. On real serverless with EIS
available the same flow picks Jina, so no extra local model appears and the
count stays at 2. Assert against what ES reports instead so the test holds in
both environments.
@steliosmavro steliosmavro added :ml release_note:skip Skip the PR/issue when compiling release notes backport:skip This PR does not require backporting labels Sep 15, 2026
@steliosmavro steliosmavro self-assigned this Sep 15, 2026
@kibanamachine

Copy link
Copy Markdown
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

cc @steliosmavro

@steliosmavro
steliosmavro marked this pull request as ready for review September 15, 2026 08:47
@steliosmavro
steliosmavro requested a review from a team as a code owner September 15, 2026 08:47
@infra-vault-gh-plugin-prod

Copy link
Copy Markdown

Pinging @elastic/ml-ui (:ml)

@steliosmavro
steliosmavro enabled auto-merge (squash) September 15, 2026 09:20
await ml.trainedModels.assertStats(3);
// The set of built-in models varies per environment (e.g. EIS vs local ELSER), so
// compare against what ES reports rather than a hardcoded count.
const { trained_model_configs: models } = await ml.api.getTrainedModelsES();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think this change makes sense. Now we're just checking whether the list accurately reflects the API, but that's such a trivial check it's hardly worth doing.

Moreover, the fundamental problem is that we're now spinning up an ML node when we shouldn't. That's the thing we should fix.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good point, can we skip the test until a fix lands to unblock the ci?

@steliosmavro

Copy link
Copy Markdown
Contributor Author

Closing this in favor of the real fix from Sander in elastic/kibana#291195 which blocks local ELSER deploys for EIS inference IDs on Serverless.

auto-merge was automatically disabled September 15, 2026 15:52

Pull request was closed

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

Labels

backport:skip This PR does not require backporting :ml release_note:skip Skip the PR/issue when compiling release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants