From 9e9b82411d2fd0585c3faecb76bc0fffad506ff8 Mon Sep 17 00:00:00 2001 From: Ilya Mashchenko Date: Wed, 17 Jun 2026 17:49:52 +0300 Subject: [PATCH 1/2] feat(go.d): add single-instance collector framework support (#22752) --- .../SKILL.md | 13 + src/go/plugin/agent/discovery/sd/dyncfg.go | 6 +- .../plugin/agent/discovery/sd/dyncfg_test.go | 10 +- src/go/plugin/agent/discovery/sd/sd.go | 12 +- src/go/plugin/agent/discovery/sd/sim_test.go | 4 +- .../agent/discovery/sd/wait_decision_test.go | 2 +- .../plugin/agent/jobmgr/dyncfg_collector.go | 20 + .../jobmgr/dyncfg_collector_callbacks.go | 35 +- .../agent/jobmgr/dyncfg_collector_cmds.go | 68 ++- .../agent/jobmgr/dyncfg_collector_test.go | 407 +++++++++++++++++- .../plugin/agent/jobmgr/dyncfg_secretstore.go | 4 +- src/go/plugin/agent/jobmgr/instance_policy.go | 40 ++ src/go/plugin/agent/jobmgr/job_factory.go | 3 + src/go/plugin/agent/jobmgr/manager.go | 29 +- .../agent/jobmgr/manager_process_test.go | 124 ++++++ src/go/plugin/agent/jobmgr/manager_test.go | 16 +- src/go/plugin/agent/jobmgr/manager_v2_test.go | 93 ++++ .../plugin/agent/jobmgr/secretsctl/cache.go | 6 +- .../agent/jobmgr/secretsctl/callbacks.go | 6 +- .../agent/jobmgr/secretsctl/controller.go | 4 +- .../plugin/agent/jobmgr/secretsctl/dyncfg.go | 6 +- .../plugin/agent/jobmgr/secretsctl/initial.go | 2 +- .../plugin/framework/collectorapi/registry.go | 31 ++ .../framework/collectorapi/registry_test.go | 38 ++ src/go/plugin/framework/dyncfg/dyncfg.go | 1 + src/go/plugin/framework/dyncfg/handler.go | 130 +++--- .../plugin/framework/dyncfg/handler_test.go | 79 +++- .../framework/jobruntime/job_v1_test.go | 21 + .../framework/jobruntime/job_v2_test.go | 52 +++ 29 files changed, 1130 insertions(+), 132 deletions(-) create mode 100644 src/go/plugin/agent/jobmgr/instance_policy.go diff --git a/.agents/skills/project-writing-go-modules-framework-v2/SKILL.md b/.agents/skills/project-writing-go-modules-framework-v2/SKILL.md index 49277fa1485ca3..dc63fc020fe2eb 100644 --- a/.agents/skills/project-writing-go-modules-framework-v2/SKILL.md +++ b/.agents/skills/project-writing-go-modules-framework-v2/SKILL.md @@ -50,6 +50,8 @@ source files for evidence. embedded `charts.yaml` is RECOMMENDED. - `Collect(ctx)` MUST return `error` and write metrics to `metrix`; it MUST NOT return a V1 `map[string]int64`. +- Collector `Cleanup(ctx)` MUST be idempotent. The framework may call it more + than once, including after partial `Init` / `Check` setup. - Files SHOULD stay boring: public lifecycle methods in `collector.go`, setup helpers in `init.go` when needed, orchestration in `collect.go`, distinct upstream operations in `collect_.go`, metrics in `metrix.go` / @@ -60,6 +62,17 @@ source files for evidence. - If Functions exist, isolate them in a `func/` subpackage with a narrow `Deps` interface declared there. The Function package MUST NOT import the collector package or hold `*Collector`. +- `collectorapi.Creator.InstancePolicy` defaults to + `InstancePolicyPerJob`. Use `InstancePolicySingle` only for collectors that + are intentionally one canonical job per agent. Single-instance configs MUST + use `name == module` after defaults are applied. DynCfg exposes opted-in + single-instance configs as `single` objects with the module-level collector + config ID, no collector template, and no `add`/`remove`; updates target that + single object. +- Before opting in a production collector to `InstancePolicySingle`, decide how + its initial `single` object appears in DynCfg. The framework exposes a single + object only after a config exists; it does not publish a template placeholder, + and plain stock enable failures can remove the stock object. - Public config options SHOULD stay small and justified. A proposed config option MUST name the concrete operator decision it enables; "operators may want to tune it" is not enough. Internal tuning SHOULD use constants unless diff --git a/src/go/plugin/agent/discovery/sd/dyncfg.go b/src/go/plugin/agent/discovery/sd/dyncfg.go index 74573271cc8422..0cac012ba5e1f7 100644 --- a/src/go/plugin/agent/discovery/sd/dyncfg.go +++ b/src/go/plugin/agent/discovery/sd/dyncfg.go @@ -83,7 +83,7 @@ func (cb *sdCallbacks) ExtractKey(fn dyncfg.Function) (key, name string, ok bool return dt + ":" + name, name, true } -func (cb *sdCallbacks) ValidateJobName(name string) error { +func (cb *sdCallbacks) ValidateConfigName(name string) error { return dyncfg.JobNameRuleAllowDots(name) } @@ -127,6 +127,10 @@ func (cb *sdCallbacks) ConfigID(cfg sdConfig) string { return cb.sd.dyncfgJobID(cfg.DiscovererType(), cfg.Name()) } +func (cb *sdCallbacks) ConfigType(sdConfig) dyncfg.ConfigType { + return dyncfg.ConfigTypeJob +} + // dyncfgConfig is the handler for dyncfg config commands. // Read-only commands (schema, get, userconfig) are executed directly. // State-changing commands are queued for serial execution. diff --git a/src/go/plugin/agent/discovery/sd/dyncfg_test.go b/src/go/plugin/agent/discovery/sd/dyncfg_test.go index 58fe258fa58fdb..64ffb74a4c7682 100644 --- a/src/go/plugin/agent/discovery/sd/dyncfg_test.go +++ b/src/go/plugin/agent/discovery/sd/dyncfg_test.go @@ -124,7 +124,7 @@ func (s *dyncfgSim) run(t *testing.T) { Path: fmt.Sprintf(dyncfgSDPath, testPluginName), EnableFailCode: 422, - JobCommands: []dyncfg.Command{ + ConfigCommands: []dyncfg.Command{ dyncfg.CommandSchema, dyncfg.CommandGet, dyncfg.CommandEnable, @@ -578,7 +578,7 @@ CONFIG test:sd:net_listeners:test-job status disabled }, wantDyncfg: ` FUNCTION_RESULT_BEGIN 1-enable 404 application/json -{"status":404,"errorMessage":"job not found."} +{"status":404,"errorMessage":"config not found."} FUNCTION_RESULT_END `, } @@ -720,7 +720,7 @@ CONFIG test:sd:net_listeners:test-job status accepted }, wantDyncfg: ` FUNCTION_RESULT_BEGIN 1-update 404 application/json -{"status":404,"errorMessage":"job not found."} +{"status":404,"errorMessage":"config not found."} FUNCTION_RESULT_END `, } @@ -831,7 +831,7 @@ CONFIG test:sd:net_listeners:test-job delete }, wantDyncfg: ` FUNCTION_RESULT_BEGIN 1-remove 404 application/json -{"status":404,"errorMessage":"job not found."} +{"status":404,"errorMessage":"config not found."} FUNCTION_RESULT_END `, } @@ -1034,7 +1034,7 @@ func TestServiceDiscovery_DyncfgFileConfig(t *testing.T) { }, wantDyncfg: ` FUNCTION_RESULT_BEGIN 1-remove 405 application/json -{"status":405,"errorMessage":"removing jobs of type 'file' is not supported, only 'dyncfg' jobs can be removed."} +{"status":405,"errorMessage":"removing configurations of source type 'file' is not supported, only 'dyncfg' configurations can be removed."} FUNCTION_RESULT_END `, } diff --git a/src/go/plugin/agent/discovery/sd/sd.go b/src/go/plugin/agent/discovery/sd/sd.go index d948bd112fee99..8ca95ebb62804a 100644 --- a/src/go/plugin/agent/discovery/sd/sd.go +++ b/src/go/plugin/agent/discovery/sd/sd.go @@ -76,7 +76,7 @@ func NewServiceDiscovery(cfg Config) (*ServiceDiscovery, error) { Path: fmt.Sprintf(dyncfgSDPath, cfg.PluginName), EnableFailCode: 422, - JobCommands: []dyncfg.Command{ + ConfigCommands: []dyncfg.Command{ dyncfg.CommandSchema, dyncfg.CommandGet, dyncfg.CommandEnable, @@ -229,7 +229,7 @@ func (d *ServiceDiscovery) removePipeline(conf confFile) { d.mgr.Stop(scfg.PipelineKey()) } - d.handler.NotifyJobRemove(scfg) + d.handler.NotifyConfigRemove(scfg) } } @@ -288,7 +288,7 @@ func (d *ServiceDiscovery) addConfig(ctx context.Context, scfg sdConfig) { // No existing config - expose this one d.handler.AddDiscoveredConfig(scfg, dyncfg.StatusAccepted) - d.handler.NotifyJobCreate(scfg, dyncfg.StatusAccepted) + d.handler.NotifyConfigCreate(scfg, dyncfg.StatusAccepted) if d.runModePolicy.AutoEnableDiscovered || d.fnReg == nil || d.dyncfgCh == nil { // Auto-enable in terminal mode and tests. // Also auto-enable when no function registry is attached, because @@ -322,8 +322,8 @@ func (d *ServiceDiscovery) addConfig(ctx context.Context, scfg sdConfig) { d.handler.AddDiscoveredConfig(scfg, dyncfg.StatusAccepted) // Update dyncfg (remove old, create new with new source) - d.handler.NotifyJobRemove(entry.Cfg) - d.handler.NotifyJobCreate(scfg, dyncfg.StatusAccepted) + d.handler.NotifyConfigRemove(entry.Cfg) + d.handler.NotifyConfigCreate(scfg, dyncfg.StatusAccepted) if d.runModePolicy.AutoEnableDiscovered || d.fnReg == nil || d.dyncfgCh == nil { d.autoEnableConfig(scfg) @@ -356,7 +356,7 @@ func (d *ServiceDiscovery) removeOldConfigsFromSource(source, newKey string) { // If it was exposed, remove from exposed cache and dyncfg. // But DON'T stop the pipeline - let the new config's enable handle that if _, ok := d.handler.RemoveDiscoveredConfig(oldCfg); ok { - d.handler.NotifyJobRemove(oldCfg) + d.handler.NotifyConfigRemove(oldCfg) } } } diff --git a/src/go/plugin/agent/discovery/sd/sim_test.go b/src/go/plugin/agent/discovery/sd/sim_test.go index 3ae60db2e1eed6..233096af690fbe 100644 --- a/src/go/plugin/agent/discovery/sd/sim_test.go +++ b/src/go/plugin/agent/discovery/sd/sim_test.go @@ -74,7 +74,7 @@ func (sim *discoverySimExt) run(t *testing.T) { Path: fmt.Sprintf(dyncfgSDPath, testPluginName), EnableFailCode: 422, - JobCommands: []dyncfg.Command{ + ConfigCommands: []dyncfg.Command{ dyncfg.CommandSchema, dyncfg.CommandGet, dyncfg.CommandEnable, @@ -151,7 +151,7 @@ func (sim *discoverySim) run(t *testing.T) { Path: fmt.Sprintf(dyncfgSDPath, testPluginName), EnableFailCode: 422, - JobCommands: []dyncfg.Command{ + ConfigCommands: []dyncfg.Command{ dyncfg.CommandSchema, dyncfg.CommandGet, dyncfg.CommandEnable, diff --git a/src/go/plugin/agent/discovery/sd/wait_decision_test.go b/src/go/plugin/agent/discovery/sd/wait_decision_test.go index 08b7fcb59c1282..d7ff585661b9b0 100644 --- a/src/go/plugin/agent/discovery/sd/wait_decision_test.go +++ b/src/go/plugin/agent/discovery/sd/wait_decision_test.go @@ -152,7 +152,7 @@ func newWaitTestServiceDiscovery(t *testing.T) (*ServiceDiscovery, chan confFile Path: fmt.Sprintf(dyncfgSDPath, testPluginName), EnableFailCode: 422, - JobCommands: []dyncfg.Command{ + ConfigCommands: []dyncfg.Command{ dyncfg.CommandSchema, dyncfg.CommandGet, dyncfg.CommandEnable, diff --git a/src/go/plugin/agent/jobmgr/dyncfg_collector.go b/src/go/plugin/agent/jobmgr/dyncfg_collector.go index 58d4ac2a9c73a8..895a40bd022c05 100644 --- a/src/go/plugin/agent/jobmgr/dyncfg_collector.go +++ b/src/go/plugin/agent/jobmgr/dyncfg_collector.go @@ -27,6 +27,13 @@ func (m *Manager) dyncfgJobID(cfg confgroup.Config) string { return fmt.Sprintf("%s%s:%s", m.dyncfgCollectorPrefixValue(), cfg.Module(), cfg.Name()) } +func (m *Manager) dyncfgConfigID(cfg confgroup.Config) string { + if m.isSingleInstanceCollector(cfg.Module()) { + return m.dyncfgModID(cfg.Module()) + } + return m.dyncfgJobID(cfg) +} + func dyncfgCollectorModCmds() string { return dyncfg.JoinCommands( dyncfg.CommandAdd, @@ -37,6 +44,19 @@ func dyncfgCollectorModCmds() string { dyncfg.CommandUserconfig) } +func dyncfgCollectorConfigCmds() []dyncfg.Command { + return []dyncfg.Command{ + dyncfg.CommandSchema, + dyncfg.CommandGet, + dyncfg.CommandEnable, + dyncfg.CommandDisable, + dyncfg.CommandUpdate, + dyncfg.CommandRestart, + dyncfg.CommandTest, + dyncfg.CommandUserconfig, + } +} + func (m *Manager) dyncfgCollectorModuleCreate(name string) { m.dyncfgResponder.ConfigCreate(netdataapi.ConfigOpts{ ID: m.dyncfgModID(name), diff --git a/src/go/plugin/agent/jobmgr/dyncfg_collector_callbacks.go b/src/go/plugin/agent/jobmgr/dyncfg_collector_callbacks.go index b0954d8bee53a9..96087ed32a9a30 100644 --- a/src/go/plugin/agent/jobmgr/dyncfg_collector_callbacks.go +++ b/src/go/plugin/agent/jobmgr/dyncfg_collector_callbacks.go @@ -40,16 +40,36 @@ func (cb *collectorCallbacks) ExtractKey(fn dyncfg.Function) (key, name string, if !ok { return "", "", false } + if cb.mgr.isSingleInstanceCollector(mn) { + cb.mgr.Warningf("dyncfg: %s: single-instance collector %s does not support add", fn.Command(), mn) + return "", "", false + } jn = fn.JobName() if jn == "" { return "", "", false } } else { - // For other commands: ID contains module:job. - mn, jn, ok = cb.mgr.extractModuleJobName(fn.ID()) + mn, ok = cb.mgr.extractModuleName(fn.ID()) if !ok { return "", "", false } + if cb.mgr.isSingleInstanceCollector(mn) { + if fn.ID() != cb.mgr.dyncfgModID(mn) { + cb.mgr.Warningf("dyncfg: %s: single-instance collector %s must use config ID %s", fn.Command(), mn, cb.mgr.dyncfgModID(mn)) + return "", "", false + } + jn = mn + } else { + // For per-job commands: ID contains module:job. + mn, jn, ok = cb.mgr.extractModuleJobName(fn.ID()) + if !ok { + return "", "", false + } + } + } + if err := cb.mgr.validateDyncfgCollectorIdentity(mn, jn); err != nil { + cb.mgr.Warningf("dyncfg: %s: %v", fn.Command(), err) + return "", "", false } key = mn + "_" + jn @@ -59,7 +79,7 @@ func (cb *collectorCallbacks) ExtractKey(fn dyncfg.Function) (key, name string, return key, jn, true } -func (cb *collectorCallbacks) ValidateJobName(name string) error { +func (cb *collectorCallbacks) ValidateConfigName(name string) error { return dyncfg.JobNameRuleStrict(name) } @@ -156,7 +176,14 @@ func (cb *collectorCallbacks) OnStatusChange(entry *dyncfg.Entry[confgroup.Confi } func (cb *collectorCallbacks) ConfigID(cfg confgroup.Config) string { - return cb.mgr.dyncfgJobID(cfg) + return cb.mgr.dyncfgConfigID(cfg) +} + +func (cb *collectorCallbacks) ConfigType(cfg confgroup.Config) dyncfg.ConfigType { + if cb.mgr.isSingleInstanceCollector(cfg.Module()) { + return dyncfg.ConfigTypeSingle + } + return dyncfg.ConfigTypeJob } // codedError wraps an error with an HTTP status code for the handler. diff --git a/src/go/plugin/agent/jobmgr/dyncfg_collector_cmds.go b/src/go/plugin/agent/jobmgr/dyncfg_collector_cmds.go index 0f0806545c43f8..8c5705432d6896 100644 --- a/src/go/plugin/agent/jobmgr/dyncfg_collector_cmds.go +++ b/src/go/plugin/agent/jobmgr/dyncfg_collector_cmds.go @@ -69,20 +69,63 @@ func (m *Manager) requireModuleJob(fn dyncfg.Function) (string, string, collecto m.dyncfgResponder.SendCodef(fn, 404, "The specified module '%s' is not registered.", moduleName) return "", "", collectorapi.Creator{}, false } + if err := validateCollectorInstanceIdentity(moduleName, jobName, creator.InstancePolicy); err != nil { + m.Warningf("dyncfg: %s: %v", cmd, err) + m.dyncfgResponder.SendCodef(fn, 400, "%v.", err) + return "", "", collectorapi.Creator{}, false + } return moduleName, jobName, creator, true } +func (m *Manager) requireSingleConfigID(fn dyncfg.Function, moduleName string, creator collectorapi.Creator) bool { + if creator.InstancePolicy != collectorapi.InstancePolicySingle { + return true + } + if fn.ID() == m.dyncfgModID(moduleName) { + return true + } + m.Warningf("dyncfg: %s: single-instance collector %s must use config ID %s", fn.Command(), moduleName, m.dyncfgModID(moduleName)) + m.dyncfgResponder.SendCodef(fn, 400, "Single-instance collector %s must use config ID %s.", moduleName, m.dyncfgModID(moduleName)) + return false +} + +func (m *Manager) requireConfigModuleJob(fn dyncfg.Function) (string, string, collectorapi.Creator, bool) { + moduleName, creator, ok := m.requireModule(fn) + if !ok { + return "", "", collectorapi.Creator{}, false + } + if creator.InstancePolicy == collectorapi.InstancePolicySingle { + if !m.requireSingleConfigID(fn, moduleName, creator) { + return "", "", collectorapi.Creator{}, false + } + return moduleName, moduleName, creator, true + } + return m.requireModuleJob(fn) +} + func (m *Manager) dyncfgCmdUserconfig(fn dyncfg.Function) { jn := fn.JobName() - if jn == "" { - jn = "test" - } mn, creator, ok := m.requireTemplateModule(fn) if !ok { return } + if !m.requireSingleConfigID(fn, mn, creator) { + return + } + if jn == "" { + if creator.InstancePolicy == collectorapi.InstancePolicySingle { + jn = mn + } else { + jn = "test" + } + } + if err := validateCollectorInstanceIdentity(mn, jn, creator.InstancePolicy); err != nil { + m.Warningf("dyncfg: %s: %v", fn.Command(), err) + m.dyncfgResponder.SendCodef(fn, 400, "%v.", err) + return + } if creator.Config == nil || creator.Config() == nil { m.Warningf("dyncfg: %s: module %s: configuration not found", fn.Command(), mn) @@ -107,10 +150,17 @@ func (m *Manager) dyncfgCmdTest(fn dyncfg.Function) { if !ok { return } + if !m.requireSingleConfigID(fn, mn, creator) { + return + } jn := fn.JobName() if jn == "" { - jn = "test" + if creator.InstancePolicy == collectorapi.InstancePolicySingle { + jn = mn + } else { + jn = "test" + } } m.Infof("dyncfg: %s: %s/%s job by user '%s'", cmd, mn, jn, fn.User()) @@ -120,6 +170,11 @@ func (m *Manager) dyncfgCmdTest(fn dyncfg.Function) { m.dyncfgResponder.SendCodef(fn, 400, "Unacceptable job name '%s': %v.", jn, err) return } + if err := validateCollectorInstanceIdentity(mn, jn, creator.InstancePolicy); err != nil { + m.Warningf("dyncfg: %s: %v", cmd, err) + m.dyncfgResponder.SendCodef(fn, 400, "%v.", err) + return + } if !fn.HasPayload() { m.Warningf("dyncfg: %s: module %s: missing configuration payload", cmd, mn) m.dyncfgResponder.SendCodef(fn, 400, "Missing configuration payload.") @@ -234,6 +289,9 @@ func (m *Manager) dyncfgCmdSchema(fn dyncfg.Function) { if !ok { return } + if !m.requireSingleConfigID(fn, mn, mod) { + return + } m.Infof("dyncfg: %s: %s module by user '%s'", fn.Command(), mn, fn.User()) @@ -249,7 +307,7 @@ func (m *Manager) dyncfgCmdSchema(fn dyncfg.Function) { func (m *Manager) dyncfgCmdGet(fn dyncfg.Function) { cmd := fn.Command() - mn, jn, creator, ok := m.requireModuleJob(fn) + mn, jn, creator, ok := m.requireConfigModuleJob(fn) if !ok { return } diff --git a/src/go/plugin/agent/jobmgr/dyncfg_collector_test.go b/src/go/plugin/agent/jobmgr/dyncfg_collector_test.go index 9454d15980afc3..afd463a768d635 100644 --- a/src/go/plugin/agent/jobmgr/dyncfg_collector_test.go +++ b/src/go/plugin/agent/jobmgr/dyncfg_collector_test.go @@ -210,6 +210,209 @@ func TestDyncfgCmdTest_PassesJobNameToV2Collector(t *testing.T) { assert.Equal(t, float64(200), resp["status"]) } +func TestDyncfgCmdTest_SingleInstancePolicyUsesCanonicalName(t *testing.T) { + var buf bytes.Buffer + mod := &namedTestV1Module{} + + mgr := newCollectorTestManager() + mgr.SetDyncfgResponder(dyncfg.NewResponder(netdataapi.New(safewriter.New(&buf)))) + mgr.modules.Register("singletest", collectorapi.Creator{ + InstancePolicy: collectorapi.InstancePolicySingle, + Create: func() collectorapi.CollectorV1 { + return mod + }, + }) + + fn := dyncfg.NewFunction(functions.Function{ + UID: "single-test-default-name", + ContentType: "application/json", + Payload: mustMarshalCollectorConfigPayload(t, prepareDyncfgCfg("singletest", "payload-name")), + Args: collectorTestArgs(mgr, "singletest", string(dyncfg.CommandTest)), + }) + + mgr.dyncfgCmdTest(fn) + mgr.cmdTestWG.Wait() + + var resp map[string]any + mustDecodeFunctionPayload(t, buf.String(), "single-test-default-name", &resp) + assert.Equal(t, float64(200), resp["status"]) + assert.Equal(t, "singletest", mod.jobName) +} + +func TestDyncfgCmdTest_SingleInstancePolicyRejectsNonCanonicalName(t *testing.T) { + var buf bytes.Buffer + created := false + + mgr := newCollectorTestManager() + mgr.SetDyncfgResponder(dyncfg.NewResponder(netdataapi.New(safewriter.New(&buf)))) + mgr.modules.Register("singletest", collectorapi.Creator{ + InstancePolicy: collectorapi.InstancePolicySingle, + Create: func() collectorapi.CollectorV1 { + created = true + return &testV1Module{} + }, + }) + + fn := dyncfg.NewFunction(functions.Function{ + UID: "single-test-custom-name", + ContentType: "application/json", + Payload: mustMarshalCollectorConfigPayload(t, prepareDyncfgCfg("singletest", "payload-name")), + Args: collectorTestArgs(mgr, "singletest", string(dyncfg.CommandTest), "custom"), + }) + + mgr.dyncfgCmdTest(fn) + + out := buf.String() + assert.Equal(t, 1, strings.Count(out, "FUNCTION_RESULT_BEGIN single-test-custom-name")) + assert.Contains(t, out, "\"status\":400") + assert.Contains(t, out, "single-instance collector singletest") + assert.False(t, created) +} + +func TestDyncfgCmdUserconfig_SingleInstancePolicyUsesCanonicalName(t *testing.T) { + var buf bytes.Buffer + + mgr := newCollectorTestManager() + mgr.SetDyncfgResponder(dyncfg.NewResponder(netdataapi.New(safewriter.New(&buf)))) + mgr.modules.Register("singleuserconfig", collectorapi.Creator{ + InstancePolicy: collectorapi.InstancePolicySingle, + Create: func() collectorapi.CollectorV1 { return &testV1Module{} }, + Config: func() any { return &collectorapi.MockConfiguration{} }, + }) + + fn := dyncfg.NewFunction(functions.Function{ + UID: "single-userconfig-default-name", + Payload: []byte(`{}`), + Args: collectorTestArgs(mgr, "singleuserconfig", string(dyncfg.CommandUserconfig)), + }) + + mgr.dyncfgCmdUserconfig(fn) + + out := buf.String() + assert.Equal(t, 1, strings.Count(out, "FUNCTION_RESULT_BEGIN single-userconfig-default-name")) + assert.Contains(t, out, "name: singleuserconfig") +} + +func TestDyncfgCmdUserconfig_SingleInstancePolicyRejectsNonCanonicalName(t *testing.T) { + var buf bytes.Buffer + + mgr := newCollectorTestManager() + mgr.SetDyncfgResponder(dyncfg.NewResponder(netdataapi.New(safewriter.New(&buf)))) + mgr.modules.Register("singleuserconfig", collectorapi.Creator{ + InstancePolicy: collectorapi.InstancePolicySingle, + Create: func() collectorapi.CollectorV1 { return &testV1Module{} }, + Config: func() any { return &collectorapi.MockConfiguration{} }, + }) + + fn := dyncfg.NewFunction(functions.Function{ + UID: "single-userconfig-custom-name", + Payload: []byte(`{}`), + Args: collectorTestArgs(mgr, "singleuserconfig", string(dyncfg.CommandUserconfig), "custom"), + }) + + mgr.dyncfgCmdUserconfig(fn) + + out := buf.String() + assert.Equal(t, 1, strings.Count(out, "FUNCTION_RESULT_BEGIN single-userconfig-custom-name")) + assert.Contains(t, out, "\"status\":400") + assert.Contains(t, out, "single-instance collector singleuserconfig") +} + +func TestDyncfgCmdGet_SingleInstancePolicyUsesSingleConfigID(t *testing.T) { + var buf bytes.Buffer + + mgr := newCollectorTestManager() + mgr.SetDyncfgResponder(dyncfg.NewResponder(netdataapi.New(safewriter.New(&buf)))) + mgr.modules.Register("singleget", collectorapi.Creator{ + InstancePolicy: collectorapi.InstancePolicySingle, + Create: func() collectorapi.CollectorV1 { return &collectorapi.MockCollectorV1{} }, + }) + + cfg := prepareDyncfgCfg("singleget", "singleget"). + Set("option_str", "one"). + Set("option_int", 2) + seedCollectorEntry(mgr, cfg, dyncfg.StatusDisabled) + + fn := dyncfg.NewFunction(functions.Function{ + UID: "single-get", + Args: collectorTestArgs(mgr, "singleget", string(dyncfg.CommandGet)), + }) + + mgr.dyncfgCmdGet(fn) + + var resp map[string]any + mustDecodeFunctionPayload(t, buf.String(), "single-get", &resp) + assert.Equal(t, "one", resp["option_str"]) + assert.Equal(t, float64(2), resp["option_int"]) +} + +func TestDyncfgCmdLifecycle_SingleInstancePolicyUsesSingleConfigID(t *testing.T) { + var buf bytes.Buffer + + mgr := newCollectorTestManager() + mgr.SetDyncfgResponder(dyncfg.NewResponder(netdataapi.New(safewriter.New(&buf)))) + mgr.modules.Register("singlelife", collectorapi.Creator{ + InstancePolicy: collectorapi.InstancePolicySingle, + Create: func() collectorapi.CollectorV1 { + return &collectorapi.MockCollectorV1{ + ChartsFunc: func() *collectorapi.Charts { + return &collectorapi.Charts{ + &collectorapi.Chart{ + ID: "id", + Title: "title", + Units: "units", + Dims: collectorapi.Dims{{ID: "id1"}}, + }, + } + }, + CollectFunc: func(context.Context) map[string]int64 { + return map[string]int64{"id1": 1} + }, + } + }, + }) + + cfg := prepareDyncfgCfg("singlelife", "singlelife") + seedCollectorEntry(mgr, cfg, dyncfg.StatusAccepted) + + mgr.dyncfgCollectorSeqExec(dyncfg.NewFunction(functions.Function{ + UID: "single-enable", + Args: collectorTestArgs(mgr, "singlelife", string(dyncfg.CommandEnable)), + })) + entry, ok := mgr.collectorExposed.LookupByKey("singlelife") + require.True(t, ok) + assert.Equal(t, dyncfg.StatusRunning, entry.Status) + _, running := mgr.runningJobs.lookup("singlelife") + assert.True(t, running) + + mgr.dyncfgCollectorSeqExec(dyncfg.NewFunction(functions.Function{ + UID: "single-restart", + Args: collectorTestArgs(mgr, "singlelife", string(dyncfg.CommandRestart)), + })) + entry, ok = mgr.collectorExposed.LookupByKey("singlelife") + require.True(t, ok) + assert.Equal(t, dyncfg.StatusRunning, entry.Status) + _, running = mgr.runningJobs.lookup("singlelife") + assert.True(t, running) + + mgr.dyncfgCollectorSeqExec(dyncfg.NewFunction(functions.Function{ + UID: "single-disable", + Args: collectorTestArgs(mgr, "singlelife", string(dyncfg.CommandDisable)), + })) + entry, ok = mgr.collectorExposed.LookupByKey("singlelife") + require.True(t, ok) + assert.Equal(t, dyncfg.StatusDisabled, entry.Status) + _, running = mgr.runningJobs.lookup("singlelife") + assert.False(t, running) + + out := buf.String() + assert.Equal(t, 1, strings.Count(out, "FUNCTION_RESULT_BEGIN single-enable")) + assert.Equal(t, 1, strings.Count(out, "FUNCTION_RESULT_BEGIN single-restart")) + assert.Equal(t, 1, strings.Count(out, "FUNCTION_RESULT_BEGIN single-disable")) + assert.Contains(t, out, "CONFIG test:collector:singlelife status running") + assert.Contains(t, out, "CONFIG test:collector:singlelife status disabled") +} + func TestDyncfgCmdTest_CodedInitAndCheckErrorsPreserveCode(t *testing.T) { tests := map[string]struct { register func(*Manager, string) @@ -357,6 +560,158 @@ func TestDyncfgCollectorSeqExec_SyncsSecretStoreDepsForMutatingCommands(t *testi } } +func TestDyncfgCollectorSeqExec_SingleInstancePolicySyncsSecretStoreDepsByModuleKey(t *testing.T) { + mgr := newCollectorTestManager() + mgr.modules.Register("singledeps", collectorapi.Creator{ + InstancePolicy: collectorapi.InstancePolicySingle, + Create: func() collectorapi.CollectorV1 { return &testV1Module{} }, + }) + + oldCfg := prepareUserCfg("singledeps", "singledeps"). + Set("password", "${store:vault:vault_old:secret/data/mysql#password}") + newCfg := prepareDyncfgCfg("singledeps", "singledeps"). + Set("password", "${store:vault:vault_new:secret/data/mysql#password}") + + cb := &collectorSeqTestCallbacks{ + mgr: mgr, + parsed: map[dyncfg.Command]confgroup.Config{dyncfg.CommandUpdate: newCfg}, + } + mgr.collectorHandler = newCollectorTestHandler(mgr, cb) + + seedCollectorEntry(mgr, oldCfg, dyncfg.StatusDisabled) + mgr.syncSecretStoreDepsForConfig(oldCfg) + + beforeExposed, beforeRunning := mgr.secretStoreDeps.Impacted("vault:vault_old") + require.Len(t, beforeExposed, 1) + assert.Equal(t, "singledeps", beforeExposed[0].ID) + assert.Equal(t, "singledeps:singledeps", beforeExposed[0].Display) + assert.Empty(t, beforeRunning) + + fn := dyncfg.NewFunction(functions.Function{ + UID: "single-deps-update", + ContentType: "application/json", + Payload: mustMarshalCollectorConfigPayload(t, newCfg), + Args: collectorTestArgs(mgr, "singledeps", string(dyncfg.CommandUpdate)), + }) + + mgr.dyncfgCollectorSeqExec(fn) + + oldExposed, oldRunning := mgr.secretStoreDeps.Impacted("vault:vault_old") + assert.Empty(t, oldExposed) + assert.Empty(t, oldRunning) + + newExposed, newRunning := mgr.secretStoreDeps.Impacted("vault:vault_new") + require.Len(t, newExposed, 1) + assert.Equal(t, "singledeps", newExposed[0].ID) + assert.Equal(t, "singledeps:singledeps", newExposed[0].Display) + assert.Empty(t, newRunning) +} + +func TestDyncfgCmdUpdate_SingleInstancePolicyReplacesLowerPriorityWithoutStarting(t *testing.T) { + var buf bytes.Buffer + + mgr := newCollectorTestManager() + mgr.SetDyncfgResponder(dyncfg.NewResponder(netdataapi.New(safewriter.New(&buf)))) + mgr.modules.Register("singleupdate", collectorapi.Creator{ + InstancePolicy: collectorapi.InstancePolicySingle, + Create: func() collectorapi.CollectorV1 { return &testV1Module{} }, + }) + + userCfg := prepareUserCfg("singleupdate", "singleupdate") + seedCollectorEntry(mgr, userCfg, dyncfg.StatusDisabled) + + newCfg := prepareDyncfgCfg("singleupdate", "singleupdate") + fn := dyncfg.NewFunction(functions.Function{ + UID: "single-update", + ContentType: "application/json", + Payload: mustMarshalCollectorConfigPayload(t, newCfg), + Args: collectorTestArgs(mgr, "singleupdate", string(dyncfg.CommandUpdate)), + }) + + mgr.collectorHandler.CmdUpdate(fn) + + out := buf.String() + assert.Equal(t, 1, strings.Count(out, "FUNCTION_RESULT_BEGIN single-update")) + assert.Contains(t, out, "\"status\":200") + assert.Contains(t, out, "CONFIG test:collector:singleupdate create disabled single /collectors/test/Jobs dyncfg") + assert.Contains(t, out, "'schema get enable disable update restart test userconfig'") + assert.NotContains(t, out, "remove") + + entry, ok := mgr.collectorExposed.LookupByKey("singleupdate") + require.True(t, ok) + assert.Equal(t, confgroup.TypeDyncfg, entry.Cfg.SourceType()) + assert.Equal(t, dyncfg.StatusDisabled, entry.Status) + + _, running := mgr.runningJobs.lookup("singleupdate") + assert.False(t, running) +} + +func TestDyncfgCmdUpdate_SingleInstancePolicyReplacesRunningLowerPriority(t *testing.T) { + var buf bytes.Buffer + + mgr := newCollectorTestManager() + mgr.SetDyncfgResponder(dyncfg.NewResponder(netdataapi.New(safewriter.New(&buf)))) + mgr.modules.Register("singleupdate", collectorapi.Creator{ + InstancePolicy: collectorapi.InstancePolicySingle, + Create: func() collectorapi.CollectorV1 { + return &collectorapi.MockCollectorV1{ + ChartsFunc: func() *collectorapi.Charts { + return &collectorapi.Charts{ + &collectorapi.Chart{ + ID: "id", + Title: "title", + Units: "units", + Dims: collectorapi.Dims{{ID: "id1"}}, + }, + } + }, + CollectFunc: func(context.Context) map[string]int64 { + return map[string]int64{"id1": 1} + }, + } + }, + }) + + userCfg := prepareUserCfg("singleupdate", "singleupdate") + seedCollectorEntry(mgr, userCfg, dyncfg.StatusRunning) + oldJob := &collectorProbeJob{ + fullName: "singleupdate", + moduleName: "singleupdate", + name: "singleupdate", + } + mgr.runningJobs.lock() + mgr.runningJobs.add(oldJob.FullName(), oldJob) + mgr.runningJobs.unlock() + t.Cleanup(func() { mgr.stopRunningJob("singleupdate") }) + + newCfg := prepareDyncfgCfg("singleupdate", "singleupdate") + fn := dyncfg.NewFunction(functions.Function{ + UID: "single-update-running", + ContentType: "application/json", + Payload: mustMarshalCollectorConfigPayload(t, newCfg), + Args: collectorTestArgs(mgr, "singleupdate", string(dyncfg.CommandUpdate)), + }) + + mgr.collectorHandler.CmdUpdate(fn) + + out := buf.String() + assert.Equal(t, 1, strings.Count(out, "FUNCTION_RESULT_BEGIN single-update-running")) + assert.Contains(t, out, "\"status\":200") + assert.Contains(t, out, "CONFIG test:collector:singleupdate create running single /collectors/test/Jobs dyncfg") + assert.Contains(t, out, "CONFIG test:collector:singleupdate status running") + assert.NotContains(t, out, "remove") + + entry, ok := mgr.collectorExposed.LookupByKey("singleupdate") + require.True(t, ok) + assert.Equal(t, confgroup.TypeDyncfg, entry.Cfg.SourceType()) + assert.Equal(t, dyncfg.StatusRunning, entry.Status) + assert.True(t, oldJob.stopped) + + runningJob, running := mgr.runningJobs.lookup("singleupdate") + require.True(t, running) + assert.NotSame(t, oldJob, runningJob) +} + func TestDyncfgCollectorSeqExec_DoesNotSyncSecretStoreDepsForNonMutatingCommands(t *testing.T) { tests := map[string]struct { command dyncfg.Command @@ -483,6 +838,48 @@ func TestCollectorCallbacks_ParseAndValidate(t *testing.T) { } } +func TestCollectorCallbacks_ExtractKeySingleInstancePolicy(t *testing.T) { + tests := map[string]struct { + args []string + wantKey string + wantName string + wantOK bool + }{ + "add is rejected": { + args: []string{"single", string(dyncfg.CommandAdd), "single"}, + }, + "canonical enable uses module key": { + args: []string{"single", string(dyncfg.CommandEnable)}, + wantKey: "single", + wantName: "single", + wantOK: true, + }, + "job-style enable is rejected": { + args: []string{"single:single", string(dyncfg.CommandEnable)}, + }, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + mgr := newCollectorTestManager() + mgr.modules.Register("single", collectorapi.Creator{ + InstancePolicy: collectorapi.InstancePolicySingle, + Create: func() collectorapi.CollectorV1 { return &testV1Module{} }, + }) + cb := &collectorCallbacks{mgr: mgr} + fn := dyncfg.NewFunction(functions.Function{ + UID: name, + Args: collectorTestArgs(mgr, tc.args...), + }) + + key, gotName, ok := cb.ExtractKey(fn) + assert.Equal(t, tc.wantOK, ok) + assert.Equal(t, tc.wantKey, key) + assert.Equal(t, tc.wantName, gotName) + }) + } +} + func TestCollectorCallbacks_ParseAndValidate_SuppressesAuditSideEffects(t *testing.T) { tempDir := t.TempDir() analyzer := &auditAnalyzerSpy{} @@ -1242,7 +1639,7 @@ func newCollectorTestHandler(mgr *Manager, cb dyncfg.Callbacks[confgroup.Config] Path: "/collectors/test/Jobs", EnableFailCode: 200, RemoveStockOnEnableFail: true, - JobCommands: []dyncfg.Command{ + ConfigCommands: []dyncfg.Command{ dyncfg.CommandSchema, dyncfg.CommandGet, dyncfg.CommandEnable, @@ -1297,7 +1694,7 @@ func (cb *collectorSeqTestCallbacks) ExtractKey(fn dyncfg.Function) (key, name s return cb.mgr.collectorCallbacks.ExtractKey(fn) } -func (cb *collectorSeqTestCallbacks) ValidateJobName(name string) error { +func (cb *collectorSeqTestCallbacks) ValidateConfigName(name string) error { return dyncfg.JobNameRuleStrict(name) } @@ -1319,7 +1716,11 @@ func (cb *collectorSeqTestCallbacks) OnStatusChange(*dyncfg.Entry[confgroup.Conf } func (cb *collectorSeqTestCallbacks) ConfigID(cfg confgroup.Config) string { - return cb.mgr.dyncfgJobID(cfg) + return cb.mgr.dyncfgConfigID(cfg) +} + +func (cb *collectorSeqTestCallbacks) ConfigType(cfg confgroup.Config) dyncfg.ConfigType { + return cb.mgr.collectorCallbacks.ConfigType(cfg) } type collectorProbeJob struct { diff --git a/src/go/plugin/agent/jobmgr/dyncfg_secretstore.go b/src/go/plugin/agent/jobmgr/dyncfg_secretstore.go index 2f5cb210953fe2..38a7dec82a5d60 100644 --- a/src/go/plugin/agent/jobmgr/dyncfg_secretstore.go +++ b/src/go/plugin/agent/jobmgr/dyncfg_secretstore.go @@ -44,13 +44,13 @@ func (m *Manager) restartDependentCollectorJob(fullName string) error { if err := m.collectorCallbacks.Start(entry.Cfg); err != nil { entry.Status = dyncfg.StatusFailed - m.collectorHandler.NotifyJobStatus(entry.Cfg, dyncfg.StatusFailed) + m.collectorHandler.NotifyConfigStatus(entry.Cfg, dyncfg.StatusFailed) m.collectorCallbacks.OnStatusChange(entry, oldStatus, dyncfg.NewFunction(functions.Function{})) return fmt.Errorf("job '%s' restart failed: %w", fullName, err) } entry.Status = dyncfg.StatusRunning - m.collectorHandler.NotifyJobStatus(entry.Cfg, dyncfg.StatusRunning) + m.collectorHandler.NotifyConfigStatus(entry.Cfg, dyncfg.StatusRunning) m.collectorCallbacks.OnStatusChange(entry, oldStatus, dyncfg.NewFunction(functions.Function{})) return nil } diff --git a/src/go/plugin/agent/jobmgr/instance_policy.go b/src/go/plugin/agent/jobmgr/instance_policy.go new file mode 100644 index 00000000000000..10b8d7926bd792 --- /dev/null +++ b/src/go/plugin/agent/jobmgr/instance_policy.go @@ -0,0 +1,40 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +package jobmgr + +import ( + "fmt" + + "github.com/netdata/netdata/go/plugins/plugin/framework/collectorapi" + "github.com/netdata/netdata/go/plugins/plugin/framework/confgroup" +) + +func validateCollectorInstanceIdentity(module, name string, policy collectorapi.InstancePolicy) error { + if policy != collectorapi.InstancePolicySingle { + return nil + } + if name == module { + return nil + } + return fmt.Errorf("single-instance collector %s must use config name %q, got %q", module, module, name) +} + +func validateCollectorConfigIdentity(cfg confgroup.Config, creator collectorapi.Creator) error { + return validateCollectorInstanceIdentity(cfg.Module(), cfg.Name(), creator.InstancePolicy) +} + +func (m *Manager) collectorInstancePolicy(module string) collectorapi.InstancePolicy { + creator, ok := m.modules.Lookup(module) + if !ok { + return collectorapi.InstancePolicyPerJob + } + return creator.InstancePolicy +} + +func (m *Manager) isSingleInstanceCollector(module string) bool { + return m.collectorInstancePolicy(module) == collectorapi.InstancePolicySingle +} + +func (m *Manager) validateDyncfgCollectorIdentity(module, name string) error { + return validateCollectorInstanceIdentity(module, name, m.collectorInstancePolicy(module)) +} diff --git a/src/go/plugin/agent/jobmgr/job_factory.go b/src/go/plugin/agent/jobmgr/job_factory.go index a0245c0553a05b..8f0edc19aefdf0 100644 --- a/src/go/plugin/agent/jobmgr/job_factory.go +++ b/src/go/plugin/agent/jobmgr/job_factory.go @@ -91,6 +91,9 @@ func (f *jobFactory) create(cfg confgroup.Config) (runtimeJob, error) { if !ok { return nil, fmt.Errorf("can not find %s module", cfg.Module()) } + if err := validateCollectorConfigIdentity(cfg, creator); err != nil { + return nil, err + } functionOnly := creator.FunctionOnly || cfg.FunctionOnly() if cfg.FunctionOnly() && creator.Methods == nil && creator.JobMethods == nil { diff --git a/src/go/plugin/agent/jobmgr/manager.go b/src/go/plugin/agent/jobmgr/manager.go index a8269c3753d442..e0872e41c7f4a0 100644 --- a/src/go/plugin/agent/jobmgr/manager.go +++ b/src/go/plugin/agent/jobmgr/manager.go @@ -146,16 +146,7 @@ func New(cfg Config) *Manager { Path: fmt.Sprintf(dyncfgCollectorPath, cfg.PluginName), EnableFailCode: 200, RemoveStockOnEnableFail: true, - JobCommands: []dyncfg.Command{ - dyncfg.CommandSchema, - dyncfg.CommandGet, - dyncfg.CommandEnable, - dyncfg.CommandDisable, - dyncfg.CommandUpdate, - dyncfg.CommandRestart, - dyncfg.CommandTest, - dyncfg.CommandUserconfig, - }, + ConfigCommands: dyncfgCollectorConfigCmds(), }) mgr.vnodesCtl = vnodectl.New(vnodectl.Options{ Logger: mgr.Logger, @@ -262,7 +253,10 @@ func (m *Manager) Run(ctx context.Context, in chan []*confgroup.Group) { m.secretsCtl.PublishExisting() m.fnReg.RegisterPrefix("config", m.dyncfgCollectorPrefixValue(), dyncfg.WrapHandler(m.dyncfgConfig)) - for name := range m.modules { + for name, creator := range m.modules { + if creator.InstancePolicy == collectorapi.InstancePolicySingle { + continue + } m.dyncfgCollectorModuleCreate(name) } m.funcCtl.RegisterModules(m.modules) @@ -356,7 +350,12 @@ func (m *Manager) run() { } func (m *Manager) addConfig(cfg confgroup.Config) { - if _, ok := m.modules.Lookup(cfg.Module()); !ok { + creator, ok := m.modules.Lookup(cfg.Module()) + if !ok { + return + } + if err := validateCollectorConfigIdentity(cfg, creator); err != nil { + m.Warningf("ignoring %s[%s] config: %v", cfg.Module(), cfg.Name(), err) return } @@ -380,10 +379,10 @@ func (m *Manager) addConfig(cfg confgroup.Config) { } m.syncSecretStoreDepsForConfig(entry.Cfg) - m.collectorHandler.NotifyJobCreate(entry.Cfg, entry.Status) + m.collectorHandler.NotifyConfigCreate(entry.Cfg, entry.Status) if m.runModePolicy.AutoEnableDiscovered { - m.collectorHandler.CmdEnable(dyncfg.NewFunction(functions.Function{Args: []string{m.dyncfgJobID(entry.Cfg), "enable"}})) + m.collectorHandler.CmdEnable(dyncfg.NewFunction(functions.Function{Args: []string{m.dyncfgConfigID(entry.Cfg), "enable"}})) } else { m.collectorHandler.WaitForDecision(entry.Cfg) } @@ -402,7 +401,7 @@ func (m *Manager) removeConfig(cfg confgroup.Config) { m.fileStatus.remove(cfg) if !isStock(cfg) || entry.Status == dyncfg.StatusRunning { - m.collectorHandler.NotifyJobRemove(cfg) + m.collectorHandler.NotifyConfigRemove(cfg) } } diff --git a/src/go/plugin/agent/jobmgr/manager_process_test.go b/src/go/plugin/agent/jobmgr/manager_process_test.go index 5fbe454c3f68ae..118fab45b3a679 100644 --- a/src/go/plugin/agent/jobmgr/manager_process_test.go +++ b/src/go/plugin/agent/jobmgr/manager_process_test.go @@ -121,6 +121,88 @@ func TestRunNotifyRunningJobs_TickOutsideLock(t *testing.T) { } } +func TestManagerAddConfigSingleInstancePolicy(t *testing.T) { + tests := map[string]struct { + run func(t *testing.T, mgr *Manager) + }{ + "non-canonical config is rejected before exposure": { + run: func(t *testing.T, mgr *Manager) { + cfg := prepareUserCfg("single", "custom") + + mgr.addConfig(cfg) + + assert.Equal(t, 0, mgr.collectorSeen.Count()) + _, exposed := mgr.collectorExposed.LookupByKey(cfg.FullName()) + assert.False(t, exposed) + }, + }, + "defaulted empty name is accepted as canonical": { + run: func(t *testing.T, mgr *Manager) { + cfg := prepareUserCfg("single", "") + cfg.ApplyDefaults(confgroup.Default{}) + + mgr.addConfig(cfg) + + entry, ok := mgr.collectorExposed.LookupByKey("single") + require.True(t, ok) + assert.Equal(t, "single", entry.Cfg.Name()) + }, + }, + "canonical higher-priority config replaces lower-priority config": { + run: func(t *testing.T, mgr *Manager) { + stockCfg := prepareStockCfg("single", "single") + userCfg := prepareUserCfg("single", "single") + + mgr.addConfig(stockCfg) + entry, ok := mgr.collectorExposed.LookupByKey("single") + require.True(t, ok) + assert.Equal(t, confgroup.TypeStock, entry.Cfg.SourceType()) + + mgr.addConfig(userCfg) + entry, ok = mgr.collectorExposed.LookupByKey("single") + require.True(t, ok) + assert.Equal(t, confgroup.TypeUser, entry.Cfg.SourceType()) + }, + }, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + mgr := New(Config{PluginName: testPluginName}) + mgr.modules = collectorapi.Registry{ + "single": { + InstancePolicy: collectorapi.InstancePolicySingle, + Create: func() collectorapi.CollectorV1 { return &collectorapi.MockCollectorV1{} }, + }, + } + + tc.run(t, mgr) + }) + } +} + +func TestManagerAddConfigSingleInstancePolicyPublishesDyncfgSingle(t *testing.T) { + var buf bytes.Buffer + mgr := New(Config{ + PluginName: testPluginName, + Out: &buf, + Modules: collectorapi.Registry{ + "single": { + InstancePolicy: collectorapi.InstancePolicySingle, + Create: func() collectorapi.CollectorV1 { return &collectorapi.MockCollectorV1{} }, + }, + }, + }) + + cfg := prepareUserCfg("single", "single") + mgr.addConfig(cfg) + + out := buf.String() + assert.Contains(t, out, "CONFIG "+mgr.dyncfgModID("single")+" create accepted single /collectors/"+testPluginName+"/Jobs") + assert.Contains(t, out, "'schema get enable disable update restart test userconfig'") + assert.NotContains(t, out, " remove") +} + func TestRun_DoesNotRegisterModuleMethodsBeforeAnyJobStarts(t *testing.T) { fnReg := &recordingFunctionRegistry{} mgr := New(Config{PluginName: testPluginName, FnReg: fnReg}) @@ -281,6 +363,48 @@ func TestRun_PublishesVnodesAndSecretstoresBeforeCollectorTemplates(t *testing.T assert.Less(t, secretIdx, collectorIdx, "secretstore publication must happen before collector template publication") } +func TestRun_DoesNotPublishCollectorTemplateForSingleInstanceModules(t *testing.T) { + var buf bytes.Buffer + + mgr := New(Config{ + PluginName: testPluginName, + Out: &buf, + Modules: collectorapi.Registry{ + "single": collectorapi.Creator{ + InstancePolicy: collectorapi.InstancePolicySingle, + }, + "perjob": collectorapi.Creator{}, + }, + }) + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + in := make(chan []*confgroup.Group) + done := make(chan struct{}) + go func() { + mgr.Run(ctx, in) + close(done) + }() + + waitCtx, waitCancel := context.WithTimeout(context.Background(), time.Second) + defer waitCancel() + require.True(t, mgr.WaitStarted(waitCtx), "manager did not report started") + + cancel() + close(in) + + select { + case <-done: + case <-time.After(2 * time.Second): + t.Fatal("manager did not stop after cancel") + } + + output := buf.String() + assert.Contains(t, output, "CONFIG "+mgr.dyncfgModID("perjob")+" create accepted template /collectors/"+testPluginName+"/Jobs") + assert.NotContains(t, output, "CONFIG "+mgr.dyncfgModID("single")+" create accepted template /collectors/"+testPluginName+"/Jobs") +} + type lockProbeJob struct { fullName string moduleName string diff --git a/src/go/plugin/agent/jobmgr/manager_test.go b/src/go/plugin/agent/jobmgr/manager_test.go index f4eea57dbcb53a..27e2ff6a4c5536 100644 --- a/src/go/plugin/agent/jobmgr/manager_test.go +++ b/src/go/plugin/agent/jobmgr/manager_test.go @@ -862,7 +862,7 @@ func TestManager_Run_Dyncfg_Enable(t *testing.T) { wantDyncfg: ` FUNCTION_RESULT_BEGIN 1-enable 404 application/json -{"status":404,"errorMessage":"job not found."} +{"status":404,"errorMessage":"config not found."} FUNCTION_RESULT_END `, } @@ -1088,7 +1088,7 @@ func TestManager_Run_Dyncfg_Disable(t *testing.T) { wantDyncfg: ` FUNCTION_RESULT_BEGIN 1-disable 404 application/json -{"status":404,"errorMessage":"job not found."} +{"status":404,"errorMessage":"config not found."} FUNCTION_RESULT_END `, } @@ -1314,7 +1314,7 @@ func TestManager_Run_Dyncfg_Restart(t *testing.T) { wantDyncfg: ` FUNCTION_RESULT_BEGIN 1-restart 404 application/json -{"status":404,"errorMessage":"job not found."} +{"status":404,"errorMessage":"config not found."} FUNCTION_RESULT_END `, } @@ -1560,7 +1560,7 @@ func TestManager_Run_Dyncfg_Remove(t *testing.T) { wantDyncfg: ` FUNCTION_RESULT_BEGIN 1-remove 404 application/json -{"status":404,"errorMessage":"job not found."} +{"status":404,"errorMessage":"config not found."} FUNCTION_RESULT_END `, } @@ -1647,15 +1647,15 @@ FUNCTION_RESULT_END CONFIG test:collector:success:discovered status running FUNCTION_RESULT_BEGIN 1-remove 405 application/json -{"status":405,"errorMessage":"removing jobs of type 'stock' is not supported, only 'dyncfg' jobs can be removed."} +{"status":405,"errorMessage":"removing configurations of source type 'stock' is not supported, only 'dyncfg' configurations can be removed."} FUNCTION_RESULT_END FUNCTION_RESULT_BEGIN 2-remove 405 application/json -{"status":405,"errorMessage":"removing jobs of type 'user' is not supported, only 'dyncfg' jobs can be removed."} +{"status":405,"errorMessage":"removing configurations of source type 'user' is not supported, only 'dyncfg' configurations can be removed."} FUNCTION_RESULT_END FUNCTION_RESULT_BEGIN 3-remove 405 application/json -{"status":405,"errorMessage":"removing jobs of type 'discovered' is not supported, only 'dyncfg' jobs can be removed."} +{"status":405,"errorMessage":"removing configurations of source type 'discovered' is not supported, only 'dyncfg' configurations can be removed."} FUNCTION_RESULT_END `, } @@ -1780,7 +1780,7 @@ func TestManager_Run_Dyncfg_Update(t *testing.T) { wantDyncfg: ` FUNCTION_RESULT_BEGIN 1-update 404 application/json -{"status":404,"errorMessage":"job not found."} +{"status":404,"errorMessage":"config not found."} FUNCTION_RESULT_END `, } diff --git a/src/go/plugin/agent/jobmgr/manager_v2_test.go b/src/go/plugin/agent/jobmgr/manager_v2_test.go index d14535a22b47bf..90af6a480f2b75 100644 --- a/src/go/plugin/agent/jobmgr/manager_v2_test.go +++ b/src/go/plugin/agent/jobmgr/manager_v2_test.go @@ -140,6 +140,29 @@ func TestManagerCreateCollectorJobV2Branching(t *testing.T) { } } +func TestManagerCreateCollectorJobSingleInstancePolicyAllowsV2FunctionOnly(t *testing.T) { + mod := &namedTestV2Module{} + mgr := New(Config{PluginName: testPluginName}) + mgr.modules = collectorapi.Registry{ + "testmod": { + InstancePolicy: collectorapi.InstancePolicySingle, + CreateV2: func() collectorapi.CollectorV2 { return mod }, + JobMethods: func(_ collectorapi.RuntimeJob) []funcapi.MethodConfig { return nil }, + }, + } + + job, err := mgr.createCollectorJob(prepareFunctionOnlyCfg("testmod", "testmod")) + require.NoError(t, err) + require.NotNil(t, job) + require.NoError(t, job.AutoDetection()) + + _, isV2 := job.(*jobruntime.JobV2) + assert.True(t, isV2) + assert.Same(t, mod, job.Collector()) + assert.Equal(t, "testmod", mod.jobName) + assert.Equal(t, "testmod", job.FullName()) +} + func TestManagerCreateCollectorJobSetsJobNameV2(t *testing.T) { mod := &namedTestV2Module{testV2Module: testV2Module{store: metrix.NewCollectorStore()}} mgr := New(Config{PluginName: testPluginName}) @@ -169,3 +192,73 @@ func TestManagerCreateCollectorJobSetsJobNameV1(t *testing.T) { assert.Same(t, mod, job.Collector()) assert.Equal(t, "job1", mod.jobName) } + +func TestManagerCreateCollectorJobSingleInstancePolicyRequiresCanonicalName(t *testing.T) { + tests := map[string]struct { + creator func(*bool) collectorapi.Creator + jobName string + wantErr string + }{ + "v1 canonical name is accepted": { + creator: func(created *bool) collectorapi.Creator { + return collectorapi.Creator{ + InstancePolicy: collectorapi.InstancePolicySingle, + Create: func() collectorapi.CollectorV1 { + *created = true + return &testV1Module{} + }, + } + }, + jobName: "testmod", + }, + "v1 non-canonical name is rejected before create": { + creator: func(created *bool) collectorapi.Creator { + return collectorapi.Creator{ + InstancePolicy: collectorapi.InstancePolicySingle, + Create: func() collectorapi.CollectorV1 { + *created = true + return &testV1Module{} + }, + } + }, + jobName: "job1", + wantErr: `single-instance collector testmod must use config name "testmod", got "job1"`, + }, + "v2 non-canonical name is rejected before create": { + creator: func(created *bool) collectorapi.Creator { + return collectorapi.Creator{ + InstancePolicy: collectorapi.InstancePolicySingle, + CreateV2: func() collectorapi.CollectorV2 { + *created = true + return &testV2Module{store: metrix.NewCollectorStore()} + }, + } + }, + jobName: "job1", + wantErr: `single-instance collector testmod must use config name "testmod", got "job1"`, + }, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + created := false + mgr := New(Config{PluginName: testPluginName}) + mgr.modules = collectorapi.Registry{ + "testmod": tc.creator(&created), + } + + job, err := mgr.createCollectorJob(prepareUserCfg("testmod", tc.jobName)) + if tc.wantErr != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tc.wantErr) + assert.False(t, created) + assert.Nil(t, job) + return + } + + require.NoError(t, err) + assert.True(t, created) + assert.NotNil(t, job) + }) + } +} diff --git a/src/go/plugin/agent/jobmgr/secretsctl/cache.go b/src/go/plugin/agent/jobmgr/secretsctl/cache.go index 6301a06db2d0ce..d44bbb293b9bda 100644 --- a/src/go/plugin/agent/jobmgr/secretsctl/cache.go +++ b/src/go/plugin/agent/jobmgr/secretsctl/cache.go @@ -51,7 +51,7 @@ func (c *Controller) rememberDiscoveredConfig(cfg secretstore.Config) (*dyncfg.E entry, ok := c.lookupInternal(cfg.ExposedKey()) if !ok { entry = c.handler.AddDiscoveredConfig(cfg, dyncfg.StatusAccepted) - c.handler.NotifyJobCreate(cfg, dyncfg.StatusAccepted) + c.handler.NotifyConfigCreate(cfg, dyncfg.StatusAccepted) return entry, true, nil } @@ -66,7 +66,7 @@ func (c *Controller) rememberDiscoveredConfig(cfg secretstore.Config) (*dyncfg.E } entry = c.handler.AddDiscoveredConfig(cfg, dyncfg.StatusAccepted) - c.handler.NotifyJobCreate(cfg, dyncfg.StatusAccepted) + c.handler.NotifyConfigCreate(cfg, dyncfg.StatusAccepted) return entry, true, nil } @@ -86,7 +86,7 @@ func (c *Controller) removeDiscoveredConfig(cfg secretstore.Config) (*dyncfg.Ent c.cb.Stop(entry.Cfg) c.cb.TakeCommandMessage() - c.handler.NotifyJobRemove(entry.Cfg) + c.handler.NotifyConfigRemove(entry.Cfg) return entry, true } diff --git a/src/go/plugin/agent/jobmgr/secretsctl/callbacks.go b/src/go/plugin/agent/jobmgr/secretsctl/callbacks.go index eb1d2c93829148..2d5b18dc32a88d 100644 --- a/src/go/plugin/agent/jobmgr/secretsctl/callbacks.go +++ b/src/go/plugin/agent/jobmgr/secretsctl/callbacks.go @@ -132,7 +132,7 @@ func (cb *secretStoreCallbacks) ExtractKey(fn dyncfg.Function) (key, name string return key, name, true } -func (cb *secretStoreCallbacks) ValidateJobName(name string) error { +func (cb *secretStoreCallbacks) ValidateConfigName(name string) error { return dyncfg.JobNameRuleAllowDots(name) } @@ -232,3 +232,7 @@ func (cb *secretStoreCallbacks) TakeCommandMessage() string { func (cb *secretStoreCallbacks) ConfigID(cfg secretstore.Config) string { return cb.deps.dyncfgSecretStoreID(cfg.ExposedKey()) } + +func (cb *secretStoreCallbacks) ConfigType(secretstore.Config) dyncfg.ConfigType { + return dyncfg.ConfigTypeJob +} diff --git a/src/go/plugin/agent/jobmgr/secretsctl/controller.go b/src/go/plugin/agent/jobmgr/secretsctl/controller.go index ecdffee97587b9..a49b1d0c94e84e 100644 --- a/src/go/plugin/agent/jobmgr/secretsctl/controller.go +++ b/src/go/plugin/agent/jobmgr/secretsctl/controller.go @@ -88,7 +88,7 @@ func New(opts Options) *Controller { Exposed: c.exposed, Callbacks: c.cb, Path: fmt.Sprintf(dyncfgSecretStorePath, c.pluginName), - JobCommands: []dyncfg.Command{ + ConfigCommands: []dyncfg.Command{ dyncfg.CommandSchema, dyncfg.CommandGet, dyncfg.CommandUpdate, @@ -130,7 +130,7 @@ func (c *Controller) PublishExisting() { } c.exposed.ForEach(func(_ string, entry *dyncfg.Entry[secretstore.Config]) bool { - c.handler.NotifyJobCreate(entry.Cfg, entry.Status) + c.handler.NotifyConfigCreate(entry.Cfg, entry.Status) return true }) } diff --git a/src/go/plugin/agent/jobmgr/secretsctl/dyncfg.go b/src/go/plugin/agent/jobmgr/secretsctl/dyncfg.go index c8dac43687ee0e..65f7c6bc44a1ec 100644 --- a/src/go/plugin/agent/jobmgr/secretsctl/dyncfg.go +++ b/src/go/plugin/agent/jobmgr/secretsctl/dyncfg.go @@ -49,7 +49,7 @@ func (c *Controller) dyncfgCmdAdd(fn dyncfg.Function) { key, name, ok := c.cb.ExtractKey(fn) if !ok { - c.api.SendCodef(fn, 400, "invalid job ID format.") + c.api.SendCodef(fn, 400, "invalid config ID format.") return } if _, exists := c.lookup(key); exists { @@ -61,7 +61,7 @@ func (c *Controller) dyncfgCmdAdd(fn dyncfg.Function) { return } if err := dyncfg.JobNameRuleAllowDots(name); err != nil { - c.api.SendCodef(fn, 400, "invalid job name '%s': %v.", name, err) + c.api.SendCodef(fn, 400, "invalid config name '%s': %v.", name, err) return } @@ -102,7 +102,7 @@ func (c *Controller) dyncfgCmdAdd(fn dyncfg.Function) { } c.api.SendCodef(fn, code, "%s", msg) - c.handler.NotifyJobCreate(cfg, entry.Status) + c.handler.NotifyConfigCreate(cfg, entry.Status) } func (c *Controller) dyncfgCmdSchema(fn dyncfg.Function) { diff --git a/src/go/plugin/agent/jobmgr/secretsctl/initial.go b/src/go/plugin/agent/jobmgr/secretsctl/initial.go index 1140c3eda0c692..2c5db420b9b14b 100644 --- a/src/go/plugin/agent/jobmgr/secretsctl/initial.go +++ b/src/go/plugin/agent/jobmgr/secretsctl/initial.go @@ -29,7 +29,7 @@ func (c *Controller) publishInitialConfig(rawCfg secretstore.Config) { } c.exposed.Add(entry) - c.handler.NotifyJobCreate(cfg, entry.Status) + c.handler.NotifyConfigCreate(cfg, entry.Status) } func (c *Controller) prepareConfigCandidate(cfg secretstore.Config) (secretstore.Config, error) { diff --git a/src/go/plugin/framework/collectorapi/registry.go b/src/go/plugin/framework/collectorapi/registry.go index 79bfdce9cda432..e8bb375e9a67ea 100644 --- a/src/go/plugin/framework/collectorapi/registry.go +++ b/src/go/plugin/framework/collectorapi/registry.go @@ -23,6 +23,31 @@ type Defaults struct { Disabled bool } +// InstancePolicy controls how many runtime instances a collector module may +// have in a single go.d process. +type InstancePolicy int + +const ( + // InstancePolicyPerJob preserves the existing model: every accepted job + // config may create its own runtime collector instance. + InstancePolicyPerJob InstancePolicy = iota + + // InstancePolicySingle allows exactly one canonical runtime collector + // instance per module. Configs for single-instance collectors must resolve + // to name == module after defaults are applied; dyncfg exposes them as a + // module-level single config, not as a template plus jobs. + InstancePolicySingle +) + +func (p InstancePolicy) valid() bool { + switch p { + case InstancePolicyPerJob, InstancePolicySingle: + return true + default: + return false + } +} + type ( // Creator is a Job builder. // Optional function fields (Methods/MethodHandler) enable the FunctionProvider pattern: @@ -34,6 +59,9 @@ type ( JobConfigSchema string Config func() any + // InstancePolicy defaults to InstancePolicyPerJob when omitted. + InstancePolicy InstancePolicy + // Optional: FunctionProvider fields for exposing data functions // If Methods is non-nil, this module provides functions Methods func() []funcapi.MethodConfig @@ -73,6 +101,9 @@ func (r Registry) Register(name string, creator Creator) { if _, ok := r[name]; ok { panic(fmt.Sprintf("%s is already in registry", name)) } + if !creator.InstancePolicy.valid() { + panic(fmt.Sprintf("%s has invalid InstancePolicy %d", name, creator.InstancePolicy)) + } if creator.Methods != nil && creator.JobMethods != nil { panic(fmt.Sprintf("%s has both Methods and JobMethods defined (mutually exclusive)", name)) } diff --git a/src/go/plugin/framework/collectorapi/registry_test.go b/src/go/plugin/framework/collectorapi/registry_test.go index e3d58795d5bf50..c202ac6c0961a4 100644 --- a/src/go/plugin/framework/collectorapi/registry_test.go +++ b/src/go/plugin/framework/collectorapi/registry_test.go @@ -75,3 +75,41 @@ func TestRegisterPanicOnMethodsAndJobMethodsConflict(t *testing.T) { }) } } + +func TestRegister_InstancePolicy(t *testing.T) { + tests := map[string]struct { + creator Creator + wantPolicy InstancePolicy + wantPanic string + }{ + "default is per-job": { + creator: Creator{}, + wantPolicy: InstancePolicyPerJob, + }, + "single-instance is accepted": { + creator: Creator{InstancePolicy: InstancePolicySingle}, + wantPolicy: InstancePolicySingle, + }, + "unknown policy panics": { + creator: Creator{InstancePolicy: InstancePolicy(99)}, + wantPanic: "invalid has invalid InstancePolicy 99", + }, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + registry := make(Registry) + if tc.wantPanic != "" { + assert.PanicsWithValue(t, tc.wantPanic, func() { + registry.Register("invalid", tc.creator) + }) + return + } + + registry.Register("mod", tc.creator) + got, ok := registry.Lookup("mod") + require.True(t, ok) + assert.Equal(t, tc.wantPolicy, got.InstancePolicy) + }) + } +} diff --git a/src/go/plugin/framework/dyncfg/dyncfg.go b/src/go/plugin/framework/dyncfg/dyncfg.go index f4caf5736fdcf9..ae51cb88afbd9a 100644 --- a/src/go/plugin/framework/dyncfg/dyncfg.go +++ b/src/go/plugin/framework/dyncfg/dyncfg.go @@ -25,6 +25,7 @@ func (s Status) String() string { type ConfigType string const ( + ConfigTypeSingle ConfigType = "single" ConfigTypeTemplate ConfigType = "template" ConfigTypeJob ConfigType = "job" ) diff --git a/src/go/plugin/framework/dyncfg/handler.go b/src/go/plugin/framework/dyncfg/handler.go index a9f2301f5596a2..93c09690ec4ee3 100644 --- a/src/go/plugin/framework/dyncfg/handler.go +++ b/src/go/plugin/framework/dyncfg/handler.go @@ -14,16 +14,16 @@ import ( // Callbacks defines component-specific operations for the handler. type Callbacks[C Config] interface { - // ExtractKey parses dyncfg function ID into cache key + job name. + // ExtractKey parses dyncfg function ID into cache key + config name. ExtractKey(fn Function) (key, name string, ok bool) // ParseAndValidate parses payload into a config with dyncfg metadata set. // Includes all validation (including heavy checks like module instantiation). ParseAndValidate(fn Function, name string) (C, error) - // ValidateJobName enforces the domain's job-name policy. Called before + // ValidateConfigName enforces the domain's config-name policy. Called before // ParseAndValidate so cheap name-format rejections happen without parsing payload. - ValidateJobName(name string) error + ValidateConfigName(name string) error // Start creates a work unit and starts it. Owns the full start lifecycle // including pre-start cleanup and post-fail retry scheduling. @@ -46,6 +46,9 @@ type Callbacks[C Config] interface { // ConfigID returns the dyncfg wire protocol ID for a config. ConfigID(cfg C) string + + // ConfigType returns the dyncfg type for a config. + ConfigType(cfg C) ConfigType } // CodedError allows callbacks to override the default response code. @@ -86,7 +89,7 @@ type HandlerOpts[C Config] struct { Path string // dyncfg path (e.g. "/collectors/go.d/Jobs") EnableFailCode int // response code for enable failure (jobmgr: 200, SD: 422) RemoveStockOnEnableFail bool // remove stock config from exposed on enable failure - JobCommands []Command // base commands for jobs; CommandRemove is added implicitly for dyncfg configs + ConfigCommands []Command // base commands for non-template configs; CommandRemove is added implicitly only for dyncfg ConfigTypeJob configs } // Handler implements the shared dyncfg command state machine. @@ -101,7 +104,7 @@ type Handler[C Config] struct { path string enableFailCode int removeStockOnEnableFail bool - jobCommands []Command + configCommands []Command waitGate *waitGate[C] } @@ -303,7 +306,7 @@ func NewHandler[C Config](opts HandlerOpts[C]) *Handler[C] { path: opts.Path, enableFailCode: opts.EnableFailCode, removeStockOnEnableFail: opts.RemoveStockOnEnableFail, - jobCommands: opts.JobCommands, + configCommands: opts.ConfigCommands, waitGate: newWaitGate(opts.WaitKey, opts.WaitTimeout), } } @@ -403,34 +406,39 @@ func (h *Handler[C]) SyncDecision(fn Function) { h.waitGate.clearIfMatch(waitKey) } -// NotifyJobCreate registers/updates a config in the dyncfg API (upsert). -func (h *Handler[C]) NotifyJobCreate(cfg C, status Status) { +// NotifyConfigCreate registers/updates a config in the dyncfg API (upsert). +func (h *Handler[C]) NotifyConfigCreate(cfg C, status Status) { isDyncfg := cfg.SourceType() == "dyncfg" h.api.ConfigCreate(netdataapi.ConfigOpts{ ID: h.cb.ConfigID(cfg), Status: status.String(), - ConfigType: ConfigTypeJob.String(), + ConfigType: h.cb.ConfigType(cfg).String(), Path: h.path, SourceType: cfg.SourceType(), Source: cfg.Source(), - SupportedCommands: h.jobSupportedCommands(isDyncfg), + SupportedCommands: h.configSupportedCommands(cfg, isDyncfg), }) } -// NotifyJobStatus sends a status update for a config. -func (h *Handler[C]) NotifyJobStatus(cfg C, status Status) { +// NotifyConfigStatus sends a status update for a config. +func (h *Handler[C]) NotifyConfigStatus(cfg C, status Status) { h.api.ConfigStatus(h.cb.ConfigID(cfg), status) } -// NotifyJobRemove removes a config from the dyncfg API. -func (h *Handler[C]) NotifyJobRemove(cfg C) { +// NotifyConfigRemove removes a config from the dyncfg API. +func (h *Handler[C]) NotifyConfigRemove(cfg C) { h.api.ConfigDelete(h.cb.ConfigID(cfg)) } -func (h *Handler[C]) jobSupportedCommands(isDyncfg bool) string { - cmds := make([]Command, len(h.jobCommands)) - copy(cmds, h.jobCommands) - if isDyncfg { +func (h *Handler[C]) configSupportedCommands(cfg C, isDyncfg bool) string { + cmds := make([]Command, 0, len(h.configCommands)+1) + for _, cmd := range h.configCommands { + if cmd == CommandAdd || cmd == CommandRemove { + continue + } + cmds = append(cmds, cmd) + } + if isDyncfg && h.cb.ConfigType(cfg) == ConfigTypeJob { cmds = append(cmds, CommandRemove) } return JoinCommands(cmds...) @@ -445,7 +453,7 @@ func (h *Handler[C]) CmdAdd(fn Function) { key, name, ok := h.cb.ExtractKey(fn) if !ok { - h.api.SendCodef(fn, 400, "invalid job ID format.") + h.api.SendCodef(fn, 400, "invalid config ID format.") return } @@ -454,8 +462,8 @@ func (h *Handler[C]) CmdAdd(fn Function) { return } - if err := h.cb.ValidateJobName(name); err != nil { - h.api.SendCodef(fn, 400, "invalid job name '%s': %v.", name, err) + if err := h.cb.ValidateConfigName(name); err != nil { + h.api.SendCodef(fn, 400, "invalid config name '%s': %v.", name, err) return } @@ -464,6 +472,10 @@ func (h *Handler[C]) CmdAdd(fn Function) { h.api.SendCodef(fn, 400, "%v", err) return } + if h.cb.ConfigType(newCfg) != ConfigTypeJob { + h.api.SendCodef(fn, 405, "adding configurations of type '%s' is not supported, only 'job' configurations can be added.", h.cb.ConfigType(newCfg)) + return + } // Replace existing config at the same key, if any. if existing, ok := h.exposed.LookupByKey(key); ok { @@ -479,20 +491,20 @@ func (h *Handler[C]) CmdAdd(fn Function) { h.exposed.Add(newEntry) h.api.SendCodef(fn, 202, "") - h.NotifyJobCreate(newCfg, StatusAccepted) + h.NotifyConfigCreate(newCfg, StatusAccepted) } // CmdEnable handles the "enable" command. func (h *Handler[C]) CmdEnable(fn Function) { key, _, ok := h.cb.ExtractKey(fn) if !ok { - h.api.SendCodef(fn, 400, "invalid job ID format.") + h.api.SendCodef(fn, 400, "invalid config ID format.") return } entry, ok := h.exposed.LookupByKey(key) if !ok { - h.api.SendCodef(fn, 404, "job not found.") + h.api.SendCodef(fn, 404, "config not found.") return } @@ -501,13 +513,13 @@ func (h *Handler[C]) CmdEnable(fn Function) { switch entry.Status { case StatusRunning: h.api.SendCodef(fn, 200, "") - h.NotifyJobStatus(entry.Cfg, StatusRunning) + h.NotifyConfigStatus(entry.Cfg, StatusRunning) return case StatusAccepted, StatusDisabled, StatusFailed: // proceed to start default: h.api.SendCodef(fn, 405, "enabling is not allowed in '%s' state.", entry.Status) - h.NotifyJobStatus(entry.Cfg, entry.Status) + h.NotifyConfigStatus(entry.Cfg, entry.Status) return } @@ -528,9 +540,9 @@ func (h *Handler[C]) CmdEnable(fn Function) { // failure semantics and whether the job should be retried. if h.removeStockOnEnableFail && !isCodedError(err) && entry.Cfg.SourceType() == "stock" { h.exposed.Remove(entry.Cfg) - h.NotifyJobRemove(entry.Cfg) + h.NotifyConfigRemove(entry.Cfg) } else { - h.NotifyJobStatus(entry.Cfg, StatusFailed) + h.NotifyConfigStatus(entry.Cfg, StatusFailed) } h.cb.OnStatusChange(entry, oldStatus, fn) @@ -539,7 +551,7 @@ func (h *Handler[C]) CmdEnable(fn Function) { entry.Status = StatusRunning h.api.SendCodef(fn, 200, "%s", takeCommandMessage(h.cb)) - h.NotifyJobStatus(entry.Cfg, StatusRunning) + h.NotifyConfigStatus(entry.Cfg, StatusRunning) h.cb.OnStatusChange(entry, oldStatus, fn) } @@ -547,13 +559,13 @@ func (h *Handler[C]) CmdEnable(fn Function) { func (h *Handler[C]) CmdDisable(fn Function) { key, _, ok := h.cb.ExtractKey(fn) if !ok { - h.api.SendCodef(fn, 400, "invalid job ID format.") + h.api.SendCodef(fn, 400, "invalid config ID format.") return } entry, ok := h.exposed.LookupByKey(key) if !ok { - h.api.SendCodef(fn, 404, "job not found.") + h.api.SendCodef(fn, 404, "config not found.") return } @@ -561,7 +573,7 @@ func (h *Handler[C]) CmdDisable(fn Function) { if entry.Status == StatusDisabled { h.api.SendCodef(fn, 200, "") - h.NotifyJobStatus(entry.Cfg, StatusDisabled) + h.NotifyConfigStatus(entry.Cfg, StatusDisabled) return } @@ -570,7 +582,7 @@ func (h *Handler[C]) CmdDisable(fn Function) { entry.Status = StatusDisabled h.api.SendCodef(fn, 200, "%s", takeCommandMessage(h.cb)) - h.NotifyJobStatus(entry.Cfg, StatusDisabled) + h.NotifyConfigStatus(entry.Cfg, StatusDisabled) h.cb.OnStatusChange(entry, oldStatus, fn) } @@ -578,18 +590,22 @@ func (h *Handler[C]) CmdDisable(fn Function) { func (h *Handler[C]) CmdRemove(fn Function) { key, _, ok := h.cb.ExtractKey(fn) if !ok { - h.api.SendCodef(fn, 400, "invalid job ID format.") + h.api.SendCodef(fn, 400, "invalid config ID format.") return } entry, ok := h.exposed.LookupByKey(key) if !ok { - h.api.SendCodef(fn, 404, "job not found.") + h.api.SendCodef(fn, 404, "config not found.") return } if entry.Cfg.SourceType() != "dyncfg" { - h.api.SendCodef(fn, 405, "removing jobs of type '%s' is not supported, only 'dyncfg' jobs can be removed.", entry.Cfg.SourceType()) + h.api.SendCodef(fn, 405, "removing configurations of source type '%s' is not supported, only 'dyncfg' configurations can be removed.", entry.Cfg.SourceType()) + return + } + if h.cb.ConfigType(entry.Cfg) != ConfigTypeJob { + h.api.SendCodef(fn, 405, "removing configurations of type '%s' is not supported, only 'job' configurations can be removed.", h.cb.ConfigType(entry.Cfg)) return } @@ -598,20 +614,20 @@ func (h *Handler[C]) CmdRemove(fn Function) { h.cb.Stop(entry.Cfg) h.api.SendCodef(fn, 200, "%s", takeCommandMessage(h.cb)) - h.NotifyJobRemove(entry.Cfg) + h.NotifyConfigRemove(entry.Cfg) } // CmdUpdate handles the "update" command. func (h *Handler[C]) CmdUpdate(fn Function) { key, name, ok := h.cb.ExtractKey(fn) if !ok { - h.api.SendCodef(fn, 400, "invalid job ID format.") + h.api.SendCodef(fn, 400, "invalid config ID format.") return } entry, ok := h.exposed.LookupByKey(key) if !ok { - h.api.SendCodef(fn, 404, "job not found.") + h.api.SendCodef(fn, 404, "config not found.") return } @@ -623,7 +639,7 @@ func (h *Handler[C]) CmdUpdate(fn Function) { newCfg, err := h.cb.ParseAndValidate(fn, name) if err != nil { h.api.SendCodef(fn, 400, "%v", err) - h.NotifyJobStatus(entry.Cfg, entry.Status) + h.NotifyConfigStatus(entry.Cfg, entry.Status) return } @@ -632,20 +648,20 @@ func (h *Handler[C]) CmdUpdate(fn Function) { // No-op: running dyncfg config with same hash. if !isConversion && entry.Status == StatusRunning && entry.Cfg.Hash() == newCfg.Hash() { h.api.SendCodef(fn, 200, "") - h.NotifyJobStatus(entry.Cfg, StatusRunning) + h.NotifyConfigStatus(entry.Cfg, StatusRunning) return } if entry.Status == StatusAccepted { h.api.SendCodef(fn, 403, "updating is not allowed in '%s' state.", entry.Status) - h.NotifyJobStatus(entry.Cfg, StatusAccepted) + h.NotifyConfigStatus(entry.Cfg, StatusAccepted) return } oldStatus := entry.Status oldCfg := entry.Cfg - // For conversion: stop old before cache update (matching jobmgr line 681). + // For conversion, stop the old work before publishing replacement config state. if isConversion { h.cb.Stop(oldCfg) } @@ -662,10 +678,10 @@ func (h *Handler[C]) CmdUpdate(fn Function) { if oldStatus == StatusDisabled { newEntry.Status = StatusDisabled if isConversion { - h.NotifyJobCreate(newCfg, StatusDisabled) + h.NotifyConfigCreate(newCfg, StatusDisabled) } h.api.SendCodef(fn, 200, "%s", takeCommandMessage(h.cb)) - h.NotifyJobStatus(newCfg, StatusDisabled) + h.NotifyConfigStatus(newCfg, StatusDisabled) h.cb.OnStatusChange(newEntry, oldStatus, fn) return } @@ -685,14 +701,14 @@ func (h *Handler[C]) CmdUpdate(fn Function) { h.exposed.Add(entry) h.api.SendCodef(fn, 200, "%v", err) - h.NotifyJobStatus(oldCfg, oldStatus) + h.NotifyConfigStatus(oldCfg, oldStatus) // No OnStatusChange call here: effective state did not change. return } newEntry.Status = StatusFailed if isConversion { - h.NotifyJobCreate(newCfg, StatusFailed) + h.NotifyConfigCreate(newCfg, StatusFailed) } code := 200 var ce CodedError @@ -700,17 +716,17 @@ func (h *Handler[C]) CmdUpdate(fn Function) { code = ce.DyncfgCode() } h.api.SendCodef(fn, code, "%v", err) - h.NotifyJobStatus(newCfg, StatusFailed) + h.NotifyConfigStatus(newCfg, StatusFailed) h.cb.OnStatusChange(newEntry, oldStatus, fn) return } newEntry.Status = StatusRunning if isConversion { - h.NotifyJobCreate(newCfg, StatusRunning) + h.NotifyConfigCreate(newCfg, StatusRunning) } h.api.SendCodef(fn, 200, "%s", takeCommandMessage(h.cb)) - h.NotifyJobStatus(newCfg, StatusRunning) + h.NotifyConfigStatus(newCfg, StatusRunning) h.cb.OnStatusChange(newEntry, oldStatus, fn) } @@ -720,26 +736,26 @@ func (h *Handler[C]) CmdUpdate(fn Function) { func (h *Handler[C]) CmdRestart(fn Function) { key, _, ok := h.cb.ExtractKey(fn) if !ok { - h.api.SendCodef(fn, 400, "invalid job ID format.") + h.api.SendCodef(fn, 400, "invalid config ID format.") return } entry, ok := h.exposed.LookupByKey(key) if !ok { - h.api.SendCodef(fn, 404, "job not found.") + h.api.SendCodef(fn, 404, "config not found.") return } switch entry.Status { case StatusAccepted, StatusDisabled: h.api.SendCodef(fn, 405, "restarting is not allowed in '%s' state.", entry.Status) - h.NotifyJobStatus(entry.Cfg, entry.Status) + h.NotifyConfigStatus(entry.Cfg, entry.Status) return case StatusRunning, StatusFailed: // proceed default: h.api.SendCodef(fn, 405, "restarting is not allowed in '%s' state.", entry.Status) - h.NotifyJobStatus(entry.Cfg, entry.Status) + h.NotifyConfigStatus(entry.Cfg, entry.Status) return } @@ -756,15 +772,15 @@ func (h *Handler[C]) CmdRestart(fn Function) { if errors.As(err, &ce) { code = ce.DyncfgCode() } - h.api.SendCodef(fn, code, "job restart failed: %v", err) - h.NotifyJobStatus(entry.Cfg, StatusFailed) + h.api.SendCodef(fn, code, "config restart failed: %v", err) + h.NotifyConfigStatus(entry.Cfg, StatusFailed) h.cb.OnStatusChange(entry, oldStatus, fn) return } entry.Status = StatusRunning h.api.SendCodef(fn, 200, "") - h.NotifyJobStatus(entry.Cfg, StatusRunning) + h.NotifyConfigStatus(entry.Cfg, StatusRunning) h.cb.OnStatusChange(entry, oldStatus, fn) } diff --git a/src/go/plugin/framework/dyncfg/handler_test.go b/src/go/plugin/framework/dyncfg/handler_test.go index a49f0463ba7549..840912d4351fc4 100644 --- a/src/go/plugin/framework/dyncfg/handler_test.go +++ b/src/go/plugin/framework/dyncfg/handler_test.go @@ -38,6 +38,7 @@ type mockCallbacks struct { stopFn func(cfg testConfig) onStatusChangeFn func(entry *Entry[testConfig], oldStatus Status, fn Function) configIDFn func(cfg testConfig) string + configTypeFn func(cfg testConfig) ConfigType startCalls []testConfig updateCalls []updateCall @@ -73,7 +74,7 @@ func (m *mockCallbacks) ParseAndValidate(fn Function, name string) (testConfig, return testConfig{uid: "dyncfg:" + name, key: name, sourceType: "dyncfg", source: "test"}, nil } -func (m *mockCallbacks) ValidateJobName(name string) error { +func (m *mockCallbacks) ValidateConfigName(name string) error { return JobNameRuleStrict(name) } @@ -114,6 +115,13 @@ func (m *mockCallbacks) ConfigID(cfg testConfig) string { return "test:" + cfg.ExposedKey() } +func (m *mockCallbacks) ConfigType(cfg testConfig) ConfigType { + if m.configTypeFn != nil { + return m.configTypeFn(cfg) + } + return ConfigTypeJob +} + func newTestHandler(cb *mockCallbacks) *Handler[testConfig] { return newTestHandlerWithWaitTimeout(cb, 5*time.Second) } @@ -140,7 +148,7 @@ func newTestHandlerWithOutput(cb *mockCallbacks, waitTimeout time.Duration) (*Ha Path: "/test/path", EnableFailCode: 200, RemoveStockOnEnableFail: true, - JobCommands: []Command{ + ConfigCommands: []Command{ CommandSchema, CommandGet, CommandEnable, @@ -509,7 +517,7 @@ func TestCmdAdd_NoPayload(t *testing.T) { assert.Equal(t, 0, h.exposed.Count()) } -func TestCmdAdd_InvalidJobName(t *testing.T) { +func TestCmdAdd_InvalidConfigName(t *testing.T) { cb := &mockCallbacks{} h := newTestHandler(cb) @@ -523,6 +531,21 @@ func TestCmdAdd_InvalidJobName(t *testing.T) { assert.Equal(t, 0, h.exposed.Count()) } +func TestCmdAdd_NonJobConfigTypeRejected(t *testing.T) { + cb := &mockCallbacks{ + configTypeFn: func(testConfig) ConfigType { return ConfigTypeSingle }, + } + h, out := newTestHandlerWithOutput(cb, 5*time.Second) + + fn := newTestFn("test:job1", "add", "job1", []byte(`{}`)) + h.CmdAdd(fn) + + assert.Equal(t, 0, h.seen.Count()) + assert.Equal(t, 0, h.exposed.Count()) + assert.Contains(t, out.String(), "405") + assert.Contains(t, out.String(), "adding configurations of type 'single' is not supported, only 'job' configurations can be added.") +} + func TestCmdAdd_ParseError(t *testing.T) { cb := &mockCallbacks{} cb.parseAndValidateFn = func(_ Function, _ string) (testConfig, error) { @@ -764,6 +787,27 @@ func TestCmdRemove_NonDyncfg_Rejected(t *testing.T) { assert.Len(t, cb.stopCalls, 0) } +func TestCmdRemove_DyncfgSingleRejected(t *testing.T) { + cb := &mockCallbacks{ + configTypeFn: func(testConfig) ConfigType { return ConfigTypeSingle }, + } + h := newTestHandler(cb) + + cfg := testConfig{uid: "dyncfg:job1", key: "job1", sourceType: "dyncfg"} + h.seen.Add(cfg) + h.exposed.Add(&Entry[testConfig]{Cfg: cfg, Status: StatusRunning}) + + fn := newTestFn("test:job1", "remove", "", nil) + h.CmdRemove(fn) + + _, ok := h.seen.LookupByUID("dyncfg:job1") + assert.True(t, ok, "dyncfg single should not be removed") + + _, ok = h.exposed.LookupByKey("job1") + assert.True(t, ok, "dyncfg single should remain exposed") + assert.Len(t, cb.stopCalls, 0) +} + func TestCmdRemove_NotFound(t *testing.T) { cb := &mockCallbacks{} h := newTestHandler(cb) @@ -1179,7 +1223,7 @@ func TestCmdRestart_NotFound(t *testing.T) { func TestCmdRestart_StartFails(t *testing.T) { cb := &mockCallbacks{} cb.startFn = func(_ testConfig) error { return errors.New("restart failed") } - h := newTestHandler(cb) + h, out := newTestHandlerWithOutput(cb, 5*time.Second) cfg := testConfig{uid: "dyncfg:job1", key: "job1", sourceType: "dyncfg"} h.exposed.Add(&Entry[testConfig]{Cfg: cfg, Status: StatusRunning}) @@ -1192,6 +1236,8 @@ func TestCmdRestart_StartFails(t *testing.T) { entry, _ := h.exposed.LookupByKey("job1") assert.Equal(t, StatusFailed, entry.Status) + assert.Contains(t, out.String(), `"status":422`) + assert.Contains(t, out.String(), "config restart failed: restart failed") require.Len(t, cb.statusCalls, 1) assert.Equal(t, StatusRunning, cb.statusCalls[0].oldStatus) } @@ -1201,7 +1247,7 @@ func TestCmdRestart_StartFails_CodedError(t *testing.T) { cb.startFn = func(_ testConfig) error { return &codedErr{err: errors.New("bad config"), code: 400} } - h := newTestHandler(cb) + h, out := newTestHandlerWithOutput(cb, 5*time.Second) cfg := testConfig{uid: "dyncfg:job1", key: "job1", sourceType: "dyncfg"} h.exposed.Add(&Entry[testConfig]{Cfg: cfg, Status: StatusRunning}) @@ -1211,30 +1257,37 @@ func TestCmdRestart_StartFails_CodedError(t *testing.T) { entry, _ := h.exposed.LookupByKey("job1") assert.Equal(t, StatusFailed, entry.Status) + assert.Contains(t, out.String(), `"status":400`) + assert.Contains(t, out.String(), "config restart failed: bad config") } // --- Notify Tests --- -func TestNotifyJobCreate_SupportedCommands(t *testing.T) { +func TestNotifyConfigCreate_SupportedCommands(t *testing.T) { tests := []struct { name string commands []Command sourceType string + configType ConfigType wantRemove bool }{ - {"dyncfg with restart", []Command{CommandSchema, CommandGet, CommandRestart}, "dyncfg", true}, - {"dyncfg no restart", []Command{CommandSchema, CommandGet}, "dyncfg", true}, - {"stock with restart", []Command{CommandSchema, CommandGet, CommandRestart}, "stock", false}, - {"stock no restart", []Command{CommandSchema, CommandGet}, "stock", false}, + {"dyncfg job with restart", []Command{CommandSchema, CommandGet, CommandRestart}, "dyncfg", ConfigTypeJob, true}, + {"dyncfg job no restart", []Command{CommandSchema, CommandGet}, "dyncfg", ConfigTypeJob, true}, + {"stock job with restart", []Command{CommandSchema, CommandGet, CommandRestart}, "stock", ConfigTypeJob, false}, + {"stock job no restart", []Command{CommandSchema, CommandGet}, "stock", ConfigTypeJob, false}, + {"dyncfg single no remove", []Command{CommandSchema, CommandGet, CommandUpdate}, "dyncfg", ConfigTypeSingle, false}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - cb := &mockCallbacks{} + cb := &mockCallbacks{ + configTypeFn: func(testConfig) ConfigType { return tt.configType }, + } h := newTestHandler(cb) - h.jobCommands = tt.commands + h.configCommands = tt.commands - cmds := h.jobSupportedCommands(tt.sourceType == "dyncfg") + cfg := testConfig{sourceType: tt.sourceType} + cmds := h.configSupportedCommands(cfg, tt.sourceType == "dyncfg") // Base commands should always be present. for _, cmd := range tt.commands { diff --git a/src/go/plugin/framework/jobruntime/job_v1_test.go b/src/go/plugin/framework/jobruntime/job_v1_test.go index 2c00f4f08b3827..69d76cb91cb10a 100644 --- a/src/go/plugin/framework/jobruntime/job_v1_test.go +++ b/src/go/plugin/framework/jobruntime/job_v1_test.go @@ -427,6 +427,27 @@ func TestJob_AutoDetection_FunctionOnly_NilCharts(t *testing.T) { assert.NoError(t, job.AutoDetection()) } +func TestJob_AutoDetection_FunctionOnlyFailCheckCleansUp(t *testing.T) { + job := newTestFunctionOnlyJob() + cleanupCalls := 0 + m := &collectorapi.MockCollectorV1{ + InitFunc: func(context.Context) error { + return nil + }, + CheckFunc: func(context.Context) error { + return errors.New("check error") + }, + CleanupFunc: func(context.Context) { + cleanupCalls++ + }, + } + job.module = m + + assert.Error(t, job.AutoDetection()) + assert.True(t, m.CleanupDone) + assert.Equal(t, 1, cleanupCalls) +} + func TestJob_Start_FunctionOnly(t *testing.T) { collectCalled := false m := &collectorapi.MockCollectorV1{ diff --git a/src/go/plugin/framework/jobruntime/job_v2_test.go b/src/go/plugin/framework/jobruntime/job_v2_test.go index ebd33894dfb7c9..65510d4242ee3e 100644 --- a/src/go/plugin/framework/jobruntime/job_v2_test.go +++ b/src/go/plugin/framework/jobruntime/job_v2_test.go @@ -775,6 +775,34 @@ END`, chartengine.Priority, chartengine.Priority)) assert.Equal(t, 0, collectCalls) }, }, + "function-only autodetection check failure cleans up": { + run: func(t *testing.T) { + cleanupCalls := 0 + mod := &mockModuleV2{ + checkFunc: func(context.Context) error { + return errors.New("check failed") + }, + cleanupFunc: func(context.Context) { + cleanupCalls++ + }, + } + job := NewJobV2(JobV2Config{ + PluginName: pluginName, + Name: jobName, + ModuleName: modName, + FullName: modName + "_" + jobName, + Module: mod, + Out: &bytes.Buffer{}, + UpdateEvery: 1, + AutoDetectEvery: 1, + FunctionOnly: true, + }) + + require.Error(t, job.AutoDetection()) + assert.True(t, mod.cleaned) + assert.Equal(t, 1, cleanupCalls) + }, + }, } for name, tc := range tests { @@ -782,6 +810,30 @@ END`, chartengine.Priority, chartengine.Priority)) } } +func TestJobV2_CleanupCanBeCalledRepeatedly(t *testing.T) { + cleanupCalls := 0 + mod := &mockModuleV2{ + cleanupFunc: func(context.Context) { + cleanupCalls++ + }, + } + job := NewJobV2(JobV2Config{ + PluginName: pluginName, + Name: jobName, + ModuleName: modName, + FullName: modName + "_" + jobName, + Module: mod, + Out: &bytes.Buffer{}, + }) + + assert.NotPanics(t, func() { + job.Cleanup() + job.Cleanup() + }) + assert.Equal(t, 2, cleanupCalls) + assert.True(t, mod.cleaned) +} + func TestJobV2_StartMarksNotRunningBeforeCleanup(t *testing.T) { cleanupStarted := make(chan struct{}) cleanupRelease := make(chan struct{}) From d2d17893342f38e807d339f68ae3f14e8d9f0d7f Mon Sep 17 00:00:00 2001 From: thiagoftsm Date: Wed, 17 Jun 2026 15:22:04 +0000 Subject: [PATCH 2/2] Fix MSI issues (#22751) --- packaging/windows/netdata.wxs.in | 45 +++++++++++++++++++++++++++++--- 1 file changed, 42 insertions(+), 3 deletions(-) diff --git a/packaging/windows/netdata.wxs.in b/packaging/windows/netdata.wxs.in index fcac0d1be76088..b5b7f572ecfd95 100644 --- a/packaging/windows/netdata.wxs.in +++ b/packaging/windows/netdata.wxs.in @@ -250,7 +250,20 @@ - + + @@ -272,12 +285,38 @@ - + + - + +