diff --git a/core/application/startup.go b/core/application/startup.go index a5aec50908c7..12217e42687f 100644 --- a/core/application/startup.go +++ b/core/application/startup.go @@ -267,6 +267,10 @@ func New(opts ...config.AppOption) (*Application, error) { } // Initialize distributed mode services (NATS, object storage, node registry) + // revisionStore is built inside the distributed block below but used after + // the model configs are loaded, so it is declared out here. + var revisionStore modeladmin.RevisionStore + distSvc, err := initDistributed(options, application.authDB, application.ModelConfigLoader()) if err != nil { return nil, fmt.Errorf("distributed mode initialization failed: %w", err) @@ -373,16 +377,9 @@ func New(opts ...config.AppOption) (*Application, error) { cfgLoaderOpts := options.ToConfigLoaderOptions() modelRevisionLifecycle := modeladmin.NewDistributedModelRevisionLifecycle(distSvc.Registry, distSvc.ModelCleanup) gs.SetModelRevisionLifecycle(modelRevisionLifecycle) - // Bring the controller's stored revisions back in line with the - // configuration on disk. An inference request may only establish a - // revision, never replace one, so a model whose stored value had - // drifted stayed unroutable until someone deleted the row. - if err := modeladmin.ResyncModelConfigRevisions(options.Context, - application.ModelConfigLoader(), - modeladmin.NewRevisionStore(distSvc.Registry, modelRevisionLifecycle), - ); err != nil { - xlog.Warn("Failed to resync model config revisions", "error", err) - } + // Captured here, used after the model configs are loaded below: the + // resync reads the loader, which is still empty at this point. + revisionStore = modeladmin.NewRevisionStore(distSvc.Registry, modelRevisionLifecycle) gs.OnModelsChanged = func(evt messaging.CacheInvalidateEvent) { // ApplyRemoteChange honors the op: a "delete" prunes the element // (a reload-from-path is additive and cannot drop it), anything @@ -429,6 +426,18 @@ func New(opts ...config.AppOption) (*Application, error) { xlog.Error("error loading config files", "error", err) } + // Bring the controller's stored revisions back in line with the + // configuration just loaded. An inference request may only establish a + // revision, never replace one, so a model whose stored value has drifted + // stays unroutable until something republishes it. This has to run after + // the load above: the loader is empty until then, and a resync against an + // empty loader silently reconciles nothing. + if revisionStore != nil { + if err := modeladmin.ResyncModelConfigRevisions(options.Context, application.ModelConfigLoader(), revisionStore); err != nil { + xlog.Warn("Failed to resync model config revisions", "error", err) + } + } + if err := gallery.RegisterBackends(options.SystemState, application.ModelLoader()); err != nil { xlog.Error("error registering external backends", "error", err) } diff --git a/core/services/modeladmin/revision_resync.go b/core/services/modeladmin/revision_resync.go index 4724f60e03b0..1eceff3f9529 100644 --- a/core/services/modeladmin/revision_resync.go +++ b/core/services/modeladmin/revision_resync.go @@ -70,8 +70,17 @@ func ResyncModelConfigRevisions(ctx context.Context, loader *config.ModelConfigL return nil } + configs := loader.GetAllModelsConfigs() + if len(configs) == 0 { + // Reconciling nothing is indistinguishable from reconciling correctly, + // which is how a caller that ran this before the configs were loaded + // went unnoticed. Say so rather than report success. + xlog.Warn("Skipping model config revision resync: no model configurations are loaded") + return nil + } + var transitions []ModelRevisionTransition - for _, cfg := range loader.GetAllModelsConfigs() { + for _, cfg := range configs { want, err := config.ModelConfigRevision(&cfg) if err != nil { return fmt.Errorf("compute config revision for %q: %w", cfg.Name, err) diff --git a/core/services/modeladmin/revision_resync_test.go b/core/services/modeladmin/revision_resync_test.go index 3979ccf75298..12772544245a 100644 --- a/core/services/modeladmin/revision_resync_test.go +++ b/core/services/modeladmin/revision_resync_test.go @@ -140,3 +140,18 @@ var _ = Describe("ResyncModelConfigRevisions", func() { Expect(store.applied).To(BeEmpty()) }) }) + +// Running the resync before the model configs are loaded reconciled nothing +// while reporting success, which is how a mis-ordered startup call went +// unnoticed. An empty loader is now called out instead of looking like a +// clean run. +var _ = Describe("ResyncModelConfigRevisions with nothing loaded", func() { + It("does not touch stored revisions when no configs are loaded", func() { + dir := GinkgoT().TempDir() + loader := config.NewModelConfigLoader(dir) + store := &stubRevisionStore{stored: map[string]string{"served-before": "stale"}} + + Expect(ResyncModelConfigRevisions(context.Background(), loader, store)).To(Succeed()) + Expect(store.applied).To(BeEmpty()) + }) +})