Fix controller config type coercion for time.Duration #15064
Conversation
…ed types." This reverts commit 13f41f9.
Juju 2.9.38 introduced type coercion on controller config before going over the wire or going into state. This had unexpected impact on two of the durations in controller config. This fix improves all handling of durations, and coerces to only basic types laid out controller.ConfigSchema.
| @@ -243,7 +248,15 @@ func (c *configCommand) setConfig(client controllerAPI, ctx *cmd.Context) error | |||
| values := make(map[string]interface{}) | |||
| for k := range attrs { | |||
| if controller.AllowedUpdateConfigAttributes.Contains(k) { | |||
| values[k] = parsed[k] | |||
| if field, ok := fields[k]; ok { | |||
There was a problem hiding this comment.
Should we log/print a warning here like we do for model config if there's a controller attribute that is mistyped. I would even argue that we go a step further and make it an error. Is there any circumstance where a controller config attribute name is not defined in the schema?
There was a problem hiding this comment.
There is two schemas, only time this could happen is if it isn't in both.
There was a problem hiding this comment.
also you can't edit a config key if it isn't in controller.AllowedUpdateConfigAttributes
| } | ||
|
|
||
| needsUpdate := false | ||
| for _, k := range []string{controller.AgentRateLimitRate, controller.MaxDebugLogDuration} { |
There was a problem hiding this comment.
I'm not sure why we don't need to include all the duration attributes here?
There was a problem hiding this comment.
these are the only ones that were coerced to time.Duration
|
/merge |
#15029 Merges: - #15014 - #15016 - #15024 - #15064 - #15063 Conflicts: - caas/kubernetes/provider/metadata.go - caas/kubernetes/provider/metadata_test.go - cmd/juju/controller/config_test.go - scripts/win-installer/setup.iss - snap/snapcraft.yaml - version/version.go Conflicts: - apiserver/admin_test.go - caas/kubernetes/provider/metadata.go - caas/kubernetes/provider/metadata_test.go - cmd/juju/controller/config.go - cmd/juju/controller/config_test.go - cmd/juju/controller/listmodels_test.go - provider/maas/maas_environ_whitebox_test.go - scripts/win-installer/setup.iss - snap/snapcraft.yaml - state/controller.go - state/upgrades.go - state/upgrades_test.go - upgrades/backend.go - upgrades/operations.go - upgrades/upgrade_test.go - version/version.go
Juju 2.9.38 introduced type coercion on controller config before
going over the wire or going into state. This had unexpected impact
on two of the durations in controller config.
This fix improves all handling of durations, and coerces to only basic
types laid out controller.ConfigSchema.
Introduced by #15016
QA steps
juju controller-config agent-ratelimit-rate=255msjuju controller-config max-debug-log-duration=23hjuju controller-config agent-ratelimit-rate=255msjuju controller-config max-debug-log-duration=23hjuju controller-config agent-ratelimit-rateshould return 255msjuju controller-config max-debug-log-duration=23hshould return 23h or 23h0m0sDocumentation changes
N/A
Bug reference
https://bugs.launchpad.net/juju/+bug/2003149