diff --git a/environs/config/config.go b/environs/config/config.go index e87ca9a466a..afafdb0cfa5 100644 --- a/environs/config/config.go +++ b/environs/config/config.go @@ -904,6 +904,14 @@ func Validate(cfg, old *Config) error { return errors.Trace(err) } + if err := cfg.validateNumProvisionWorkers(); err != nil { + return errors.Trace(err) + } + + if err := cfg.validateNumContainerProvisionWorkers(); err != nil { + return errors.Trace(err) + } + if old != nil { // Check the immutable config values. These can't change for _, attr := range immutableAttributes { @@ -1480,6 +1488,24 @@ func (c *Config) NumProvisionWorkers() int { return value } +const ( + MaxNumProvisionWorkers = 100 + MaxNumContainerProvisionWorkers = 25 +) + +// validateNumProvisionWorkers ensures the number cannot be set to +// more than 100. +// TODO: (hml) 26-Feb-2024 +// Once we can better link the controller config and the model config, +// allow the max value to be set in the controller config. +func (c *Config) validateNumProvisionWorkers() error { + value, ok := c.defined[NumProvisionWorkersKey].(int) + if ok && value > MaxNumProvisionWorkers { + return errors.Errorf("%s: must be less than %d", NumProvisionWorkersKey, MaxNumProvisionWorkers) + } + return nil +} + // NumContainerProvisionWorkers returns the number of container provisioner // workers to use. func (c *Config) NumContainerProvisionWorkers() int { @@ -1487,6 +1513,19 @@ func (c *Config) NumContainerProvisionWorkers() int { return value } +// validateNumContainerProvisionWorkers ensures the number cannot be set to +// more than 25. +// TODO: (hml) 26-Feb-2024 +// Once we can better link the controller config and the model config, +// allow the max value to be set in the controller config. +func (c *Config) validateNumContainerProvisionWorkers() error { + value, ok := c.defined[NumContainerProvisionWorkersKey].(int) + if ok && value > MaxNumContainerProvisionWorkers { + return errors.Errorf("%s: must be less than %d", NumContainerProvisionWorkersKey, MaxNumContainerProvisionWorkers) + } + return nil +} + // ImageStream returns the simplestreams stream // used to identify which image ids to search // when starting an instance. @@ -1500,7 +1539,7 @@ func (c *Config) ImageStream() string { // AgentStream returns the simplestreams stream // used to identify which tools to use when -// when bootstrapping or upgrading an environment. +// bootstrapping or upgrading an environment. func (c *Config) AgentStream() string { v, _ := c.defined[AgentStreamKey].(string) if v != "" { diff --git a/environs/config/config_test.go b/environs/config/config_test.go index 202e7a9d244..8ae9181ee61 100644 --- a/environs/config/config_test.go +++ b/environs/config/config_test.go @@ -371,15 +371,31 @@ var configTests = []configTest{ }), err: `provisioner-harvest-mode: expected one of \[all none unknown destroyed], got "yes please"`, }, { - about: fmt.Sprintf( - "%s: %d", - "num-provision-workers", - 42, - ), + about: fmt.Sprintf("num-provision-workers: 42"), + useDefaults: config.UseDefaults, + attrs: minimalConfigAttrs.Merge(testing.Attrs{ + "num-provision-workers": 42, + }), + }, { + about: fmt.Sprintf("num-provision-workers: over max"), + useDefaults: config.UseDefaults, + attrs: minimalConfigAttrs.Merge(testing.Attrs{ + "num-provision-workers": 101, + }), + err: `num-provision-workers: must be less than 100`, + }, { + about: fmt.Sprintf("num-container-provision-workers: 17"), + useDefaults: config.UseDefaults, + attrs: minimalConfigAttrs.Merge(testing.Attrs{ + "num-container-provision-workers": 17, + }), + }, { + about: fmt.Sprintf("num-container-provision-workers: over max"), useDefaults: config.UseDefaults, attrs: minimalConfigAttrs.Merge(testing.Attrs{ - "num-provision-workers": "42", + "num-container-provision-workers": 26, }), + err: `num-container-provision-workers: must be less than 25`, }, { about: "default image stream", useDefaults: config.UseDefaults, @@ -707,10 +723,14 @@ func (test configTest) check(c *gc.C) { cfg, err := config.New(test.useDefaults, test.attrs) if test.err != "" { c.Check(cfg, gc.IsNil) - c.Assert(err, gc.ErrorMatches, test.err) + c.Check(err, gc.ErrorMatches, test.err) + return + } + if !c.Check(err, jc.ErrorIsNil, gc.Commentf("config.New failed")) { + // As we have a Check not an Assert so the test should not + // continue from here as it will result in a nil pointer panic. return } - c.Assert(err, jc.ErrorIsNil) typ, _ := test.attrs["type"].(string) // "null" has been deprecated in favour of "manual", @@ -719,23 +739,23 @@ func (test configTest) check(c *gc.C) { typ = "manual" } name, _ := test.attrs["name"].(string) - c.Assert(cfg.Type(), gc.Equals, typ) - c.Assert(cfg.Name(), gc.Equals, name) + c.Check(cfg.Type(), gc.Equals, typ) + c.Check(cfg.Name(), gc.Equals, name) agentVersion, ok := cfg.AgentVersion() if s := test.attrs["agent-version"]; s != nil { - c.Assert(ok, jc.IsTrue) - c.Assert(agentVersion, gc.Equals, version.MustParse(s.(string))) + c.Check(ok, jc.IsTrue) + c.Check(agentVersion, gc.Equals, version.MustParse(s.(string))) } else { - c.Assert(ok, jc.IsFalse) - c.Assert(agentVersion, gc.Equals, version.Zero) + c.Check(ok, jc.IsFalse) + c.Check(agentVersion, gc.Equals, version.Zero) } if expected, ok := test.attrs["uuid"]; ok { - c.Assert(cfg.UUID(), gc.Equals, expected) + c.Check(cfg.UUID(), gc.Equals, expected) } dev, _ := test.attrs["development"].(bool) - c.Assert(cfg.Development(), gc.Equals, dev) + c.Check(cfg.Development(), gc.Equals, dev) baseAttr, _ := test.attrs["default-base"].(string) defaultBase, ok := cfg.DefaultBase() @@ -748,67 +768,68 @@ func (test configTest) check(c *gc.C) { } if m, _ := test.attrs["firewall-mode"].(string); m != "" { - c.Assert(cfg.FirewallMode(), gc.Equals, m) + c.Check(cfg.FirewallMode(), gc.Equals, m) } if m, _ := test.attrs["default-space"].(string); m != "" { - c.Assert(cfg.DefaultSpace(), gc.Equals, m) + c.Check(cfg.DefaultSpace(), gc.Equals, m) } keys, _ := test.attrs["authorized-keys"].(string) - c.Assert(cfg.AuthorizedKeys(), gc.Equals, keys) + c.Check(cfg.AuthorizedKeys(), gc.Equals, keys) lfCfg, hasLogCfg := cfg.LogFwdSyslog() if v, ok := test.attrs["logforward-enabled"].(bool); ok { - c.Assert(hasLogCfg, jc.IsTrue) - c.Assert(lfCfg.Enabled, gc.Equals, v) + if c.Check(hasLogCfg, jc.IsTrue) { + c.Check(lfCfg.Enabled, gc.Equals, v) + } } if v, ok := test.attrs["syslog-ca-cert"].(string); v != "" { - c.Assert(hasLogCfg, jc.IsTrue) - c.Assert(lfCfg.CACert, gc.Equals, v) + c.Check(hasLogCfg, jc.IsTrue) + c.Check(lfCfg.CACert, gc.Equals, v) } else if ok { - c.Assert(hasLogCfg, jc.IsTrue) + c.Check(hasLogCfg, jc.IsTrue) c.Check(lfCfg.CACert, gc.Equals, "") } if v, ok := test.attrs["syslog-client-cert"].(string); v != "" { - c.Assert(hasLogCfg, jc.IsTrue) - c.Assert(lfCfg.ClientCert, gc.Equals, v) + c.Check(hasLogCfg, jc.IsTrue) + c.Check(lfCfg.ClientCert, gc.Equals, v) } else if ok { - c.Assert(hasLogCfg, jc.IsTrue) + c.Check(hasLogCfg, jc.IsTrue) c.Check(lfCfg.ClientCert, gc.Equals, "") } if v, ok := test.attrs["syslog-client-key"].(string); v != "" { - c.Assert(hasLogCfg, jc.IsTrue) - c.Assert(lfCfg.ClientKey, gc.Equals, v) + c.Check(hasLogCfg, jc.IsTrue) + c.Check(lfCfg.ClientKey, gc.Equals, v) } else if ok { - c.Assert(hasLogCfg, jc.IsTrue) + c.Check(hasLogCfg, jc.IsTrue) c.Check(lfCfg.ClientKey, gc.Equals, "") } if v, ok := test.attrs["ssl-hostname-verification"]; ok { - c.Assert(cfg.SSLHostnameVerification(), gc.Equals, v) + c.Check(cfg.SSLHostnameVerification(), gc.Equals, v) } if v, ok := test.attrs["provisioner-harvest-mode"]; ok { harvestMeth, err := config.ParseHarvestMode(v.(string)) - c.Assert(err, jc.ErrorIsNil) - c.Assert(cfg.ProvisionerHarvestMode(), gc.Equals, harvestMeth) + c.Check(err, jc.ErrorIsNil) + c.Check(cfg.ProvisionerHarvestMode(), gc.Equals, harvestMeth) } else { - c.Assert(cfg.ProvisionerHarvestMode(), gc.Equals, config.HarvestDestroyed) + c.Check(cfg.ProvisionerHarvestMode(), gc.Equals, config.HarvestDestroyed) } if v, ok := test.attrs["image-stream"]; ok { - c.Assert(cfg.ImageStream(), gc.Equals, v) + c.Check(cfg.ImageStream(), gc.Equals, v) } else { - c.Assert(cfg.ImageStream(), gc.Equals, "released") + c.Check(cfg.ImageStream(), gc.Equals, "released") } url, urlPresent := cfg.ImageMetadataURL() if v, _ := test.attrs["image-metadata-url"].(string); v != "" { - c.Assert(url, gc.Equals, v) - c.Assert(urlPresent, jc.IsTrue) + c.Check(url, gc.Equals, v) + c.Check(urlPresent, jc.IsTrue) } else { - c.Assert(urlPresent, jc.IsFalse) + c.Check(urlPresent, jc.IsFalse) } imageMetadataDefaultsDisabled := cfg.ImageMetadataDefaultsDisabled() @@ -821,9 +842,9 @@ func (test configTest) check(c *gc.C) { agentURL, urlPresent := cfg.AgentMetadataURL() expectedToolsURLValue := test.attrs["agent-metadata-url"] if urlPresent { - c.Assert(agentURL, gc.Equals, expectedToolsURLValue) + c.Check(agentURL, gc.Equals, expectedToolsURLValue) } else { - c.Assert(agentURL, gc.Equals, "") + c.Check(agentURL, gc.Equals, "") } // assertions for deprecated tools-stream attribute used with new agent-stream @@ -844,16 +865,16 @@ func (test configTest) check(c *gc.C) { containerURL, urlPresent := cfg.ContainerImageMetadataURL() if v, _ := test.attrs["container-image-metadata-url"].(string); v != "" { - c.Assert(containerURL, gc.Equals, v) - c.Assert(urlPresent, jc.IsTrue) + c.Check(containerURL, gc.Equals, v) + c.Check(urlPresent, jc.IsTrue) } else { - c.Assert(urlPresent, jc.IsFalse) + c.Check(urlPresent, jc.IsFalse) } if v, ok := test.attrs["container-image-stream"]; ok { - c.Assert(cfg.ContainerImageStream(), gc.Equals, v) + c.Check(cfg.ContainerImageStream(), gc.Equals, v) } else { - c.Assert(cfg.ContainerImageStream(), gc.Equals, "released") + c.Check(cfg.ContainerImageStream(), gc.Equals, "released") } containerImageMetadataDefaultsDisabled := cfg.ContainerImageMetadataDefaultsDisabled() @@ -864,20 +885,20 @@ func (test configTest) check(c *gc.C) { } resourceTags, cfgHasResourceTags := cfg.ResourceTags() - c.Assert(cfgHasResourceTags, jc.IsTrue) + c.Check(cfgHasResourceTags, jc.IsTrue) if tags, ok := test.attrs["resource-tags"]; ok { switch tags := tags.(type) { case []string: if len(tags) > 0 { - c.Assert(resourceTags, jc.DeepEquals, testResourceTagsMap) + c.Check(resourceTags, jc.DeepEquals, testResourceTagsMap) } case string: if tags != "" { - c.Assert(resourceTags, jc.DeepEquals, testResourceTagsMap) + c.Check(resourceTags, jc.DeepEquals, testResourceTagsMap) } } } else { - c.Assert(resourceTags, gc.HasLen, 0) + c.Check(resourceTags, gc.HasLen, 0) } xmit := cfg.TransmitVendorMetrics() diff --git a/state/modelconfig_test.go b/state/modelconfig_test.go index be855374c3d..3c51e540276 100644 --- a/state/modelconfig_test.go +++ b/state/modelconfig_test.go @@ -458,7 +458,7 @@ func (s *ModelConfigSourceSuite) TestUpdateModelConfigDefaults(c *gc.C) { attrs = map[string]interface{}{ "apt-mirror": "http://different-mirror", - "num-provision-workers": 666, + "num-provision-workers": 66, } err = s.State.UpdateModelConfigDefaultValues(attrs, []string{"http-proxy", "https-proxy"}, nil) c.Assert(err, jc.ErrorIsNil) @@ -487,7 +487,7 @@ func (s *ModelConfigSourceSuite) TestUpdateModelConfigDefaults(c *gc.C) { Value: "dummy-proxy", }}} expectedValues["num-provision-workers"] = config.AttributeDefaultValues{ - Controller: 666, + Controller: 66, Default: 16, } c.Assert(cfg, jc.DeepEquals, expectedValues) diff --git a/worker/provisioner/provisioner_test.go b/worker/provisioner/provisioner_test.go index b14a2dcd48c..1c600cc5de1 100644 --- a/worker/provisioner/provisioner_test.go +++ b/worker/provisioner/provisioner_test.go @@ -114,7 +114,7 @@ func (s *CommonProvisionerSuite) assertProvisionerObservesConfigChangesWorkerCou // Switch to reaping on All machines. attrs := map[string]interface{}{} if container { - attrs[config.NumContainerProvisionWorkersKey] = 42 + attrs[config.NumContainerProvisionWorkersKey] = 10 } else { attrs[config.NumProvisionWorkersKey] = 42 } @@ -130,7 +130,7 @@ func (s *CommonProvisionerSuite) assertProvisionerObservesConfigChangesWorkerCou select { case newCfg := <-cfgObserver: if container { - if newCfg.NumContainerProvisionWorkers() == 42 { + if newCfg.NumContainerProvisionWorkers() == 10 { return } received = append(received, newCfg.NumContainerProvisionWorkers())