Skip to content

Handle config file removal and rename as an empty config file#250

Open
ronenkat wants to merge 13 commits into
llm-d:mainfrom
ronenkat:config-change-on-remove-rename
Open

Handle config file removal and rename as an empty config file#250
ronenkat wants to merge 13 commits into
llm-d:mainfrom
ronenkat:config-change-on-remove-rename

Conversation

@ronenkat

Copy link
Copy Markdown
Contributor

What type of PR is this?
/kind bug

What this PR does / why we need it:
When the config file was deleted or renamed the model and group configuration was not updated.

Which issue(s) this PR fixes:

Release note (write NONE if no user-facing change):

1. Deleting or renaming the configuration file cause a refresh of the models and groups. Essentially removing them.

ronenkat added 12 commits July 9, 2026 16:02
Signed-off-by: ronenkat <16743404+ronenkat@users.noreply.github.com>
Signed-off-by: ronenkat <16743404+ronenkat@users.noreply.github.com>
Signed-off-by: ronenkat <16743404+ronenkat@users.noreply.github.com>
Signed-off-by: ronenkat <16743404+ronenkat@users.noreply.github.com>
Signed-off-by: ronenkat <16743404+ronenkat@users.noreply.github.com>
Signed-off-by: ronenkat <16743404+ronenkat@users.noreply.github.com>
Signed-off-by: ronenkat <16743404+ronenkat@users.noreply.github.com>
Signed-off-by: ronenkat <16743404+ronenkat@users.noreply.github.com>
@github-actions github-actions Bot added kind/bug Categorizes issue or PR as related to a bug. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jul 17, 2026
Signed-off-by: ronenkat <16743404+ronenkat@users.noreply.github.com>

@noyitz noyitz left a comment

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.

clean change. the readModelsFile extraction is nice — separating file reading from sync logic lets the watcher and startup share the same flow. the converge-to-empty-config semantics make sense for configmap remounts.

left one comment about the startup path — a missing file at startup is now silently accepted, which could mask a misconfigured modelsPath.

return err
}
if data == nil {
logger.Info("configuration file is empty")

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.

at startup, if the file doesn't exist, this now silently proceeds with zero models instead of returning an error. in the watcher path this is the right behavior (file deleted -> clear models). but at startup, a missing file could mean a misconfigured modelsPath — the pod would start with no models and the operator wouldn't know until they check the datastore.

worth considering returning an error at startup (but not on the watcher path) when the file is missing. not a blocker.

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

Labels

kind/bug Categorizes issue or PR as related to a bug. 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.

2 participants