Skip to content

Commit

Permalink
Merge pull request juju#17014 from hmlanigan/merge-from-3.1
Browse files Browse the repository at this point in the history 
juju#17014

Contains:

* juju#17004 from hmlanigan/merge-from-2.9

Merge conflicts, deleted both files:

* CONFLICT (modify/delete): cmd/jujuc/main_test.go deleted in HEAD and modified in upstream/3.1. Version upstream/3.1 of cmd/jujuc/main_test.go left in tree.
CONFLICT (modify/delete): cmd/jujud/main_test.go deleted in HEAD and modified in upstream/3.1. Version upstream/3.1 of cmd/jujud/main_test.go left in tree.
  • Loading branch information
@jujubot
jujubot authored Mar 6, 2024
2 parents fbe9fa2 + 8dbc8d7 commit 46fd1c1
Show file tree
Hide file tree
Showing 4 changed files with 115 additions and 55 deletions.
41 changes: 40 additions & 1 deletion environs/config/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -1480,13 +1488,44 @@ 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 {
value, _ := c.defined[NumContainerProvisionWorkersKey].(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.
Expand All @@ -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 != "" {
Expand Down
121 changes: 71 additions & 50 deletions environs/config/config_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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",
Expand All @@ -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()
Expand All @@ -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()
Expand All @@ -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
Expand All @@ -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()
Expand All @@ -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()
Expand Down
4 changes: 2 additions & 2 deletions state/modelconfig_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions worker/provisioner/provisioner_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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())
Expand Down

0 comments on commit 46fd1c1

Please sign in to comment.