Handle config file removal and rename as an empty config file#250
Handle config file removal and rename as an empty config file#250ronenkat wants to merge 13 commits into
Conversation
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>
noyitz
left a comment
There was a problem hiding this comment.
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") |
There was a problem hiding this comment.
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.
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
NONEif no user-facing change):