Skip to content

fix(catalog): watch YAML data files for live reload in MCP and agent catalogs - #2997

Merged
google-oss-prow[bot] merged 3 commits into
kubeflow:mainfrom
pboyd:fix/catalog-yaml-file-watching
Jul 28, 2026
Merged

fix(catalog): watch YAML data files for live reload in MCP and agent catalogs#2997
google-oss-prow[bot] merged 3 commits into
kubeflow:mainfrom
pboyd:fix/catalog-yaml-file-watching

Conversation

@pboyd

@pboyd pboyd commented Jul 20, 2026

Copy link
Copy Markdown
Member

Description

Previously, only sources.yaml was watched for file changes. YAML data files referenced via yamlCatalogPath required a pod restart or a sources.yaml touch to pick up edits.

MCP catalog: make yamlMCPProvider.Servers() long-lived. After emitting the initial batch it sends a zero-value sentinel record, registers a file watcher on each data file via basecatalog.GetMonitor().Path(), and re-emits + re-sends the sentinel on every detected change. The loader's PerformLeaderOperations now returns immediately after launching per-source goroutines; loadServersFromProvider uses sync.OnceFunc to release the pre-registered TrackWrite slot on the first sentinel so that WaitForInflightWrites unblocks correctly. Subsequent batches (file reloads) perform orphan cleanup and status updates the same way as the initial batch.

Agent catalog: add watchAndLoadFromYAML that performs the initial load, releases its TrackWrite slot, then loops on GetMonitor().Path() events to reload on change. PerformLeaderOperations is restructured to launch goroutines and return immediately, matching the MCP and model loader patterns.

Tests updated to use t.Context() so background goroutines are cancelled when each test ends, and to break on sentinel records rather than waiting for channel close.

How Has This Been Tested?

Verified locally on a dev cluster.

Merge criteria:

  • All the commits have been signed-off (To pass the DCO check)
  • The commits have meaningful messages
  • Automated tests are provided as part of the PR for major new functionalities; testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
  • The developer has manually tested the changes and verified that the changes work.
  • Code changes follow the kubeflow contribution guidelines.

@google-oss-prow
google-oss-prow Bot requested review from chambridge and fege July 20, 2026 19:25
@pboyd
pboyd force-pushed the fix/catalog-yaml-file-watching branch from 2567140 to 7e3f389 Compare July 21, 2026 14:51
@pboyd
pboyd force-pushed the fix/catalog-yaml-file-watching branch 3 times, most recently from b4fa893 to edadfa7 Compare July 22, 2026 17:05
ch, err := basecatalog.GetMonitor().Path(ctx, path)
if err != nil {
glog.Errorf("unable to watch MCP catalog file %s: %v", path, err)
watchFailed = true

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.

one wrong path, skips all the live updates even in correct path, what do you think about skipping only the bad ones?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is fixed in the last commit.

// cross-contamination when cleaning up shared CatalogSource records.
// This is called by the unified loader when becoming leader.
func (l *ModelLoader) PerformLeaderOperations(ctx context.Context, allKnownSourceIDs mapset.Set[string]) error {
return l.performLeaderWrites(ctx, allKnownSourceIDs)

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.

shouldn't we add also here .state.WaitForInflightWrites(30 * time.Second) like we have done in the other plugins?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Also fixed in the last commit.

pboyd added 3 commits July 28, 2026 09:37
…catalogs

Previously, only sources.yaml was watched for file changes. YAML data
files referenced via yamlCatalogPath required a pod restart or a
sources.yaml touch to pick up edits.

MCP catalog: make yamlMCPProvider.Servers() long-lived. After emitting
the initial batch it sends a zero-value sentinel record, registers a
file watcher on each data file via basecatalog.GetMonitor().Path(), and
re-emits + re-sends the sentinel on every detected change. The loader's
PerformLeaderOperations now returns immediately after launching per-source
goroutines; loadServersFromProvider uses sync.OnceFunc to release the
pre-registered TrackWrite slot on the first sentinel so that
WaitForInflightWrites unblocks correctly. Subsequent batches (file
reloads) perform orphan cleanup and status updates the same way as the
initial batch.

Agent catalog: add watchAndLoadFromYAML that performs the initial load,
releases its TrackWrite slot, then loops on GetMonitor().Path() events
to reload on change. PerformLeaderOperations is restructured to launch
goroutines and return immediately, matching the MCP and model loader
patterns.

Tests updated to use t.Context() so background goroutines are cancelled
when each test ends, and to break on sentinel records rather than
waiting for channel close.

Assisted-by: Claude Sonnet 4.6
Signed-off-by: Paul Boyd <paul@pboyd.io>
…e leak

- Set up file watcher before initial read to avoid missing changes that
  arrive during the read
- Emit error + sentinel on YAML read failure so the consumer updates
  source status to "error" instead of silently continuing
- Skip orphan cleanup when all models fail to load, preserving stale
  data until the error is fixed
- Reset failedModels between batch cycles so partial-failure state does
  not leak into subsequent batches

Assisted-by: Claude Sonnet 4.6
Signed-off-by: Paul Boyd <paul@pboyd.io>
- MCP provider: skip individual bad file watcher paths instead of
  failing all live updates. Changed break to continue on error and
  replaced watchFailed boolean with watchCount integer.

- Model loader: add missing WaitForInflightWrites drain step to
  PerformLeaderOperations, matching MCP and agent loader patterns.
  This prevents a race where old goroutine writes could re-insert
  data during orphan cleanup.

Assisted-by: Claude Sonnet 4.5
Signed-off-by: Paul Boyd <paul@pboyd.io>
@pboyd
pboyd force-pushed the fix/catalog-yaml-file-watching branch from edadfa7 to bac0260 Compare July 28, 2026 13:46
@Al-Pragliola

Copy link
Copy Markdown
Contributor

/lgtm
/approve

@google-oss-prow

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Al-Pragliola

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

The pull request process is described 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

@google-oss-prow
google-oss-prow Bot merged commit 628aff5 into kubeflow:main Jul 28, 2026
34 of 36 checks passed
@pboyd
pboyd deleted the fix/catalog-yaml-file-watching branch July 28, 2026 18:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants