pillar/updateTaskTimer: switch to less-error prone

time.Duration

Signed-off-by: Christoph Ostarek <[email protected]>

Author: christoph-zededa

pkg/pillar/cmd/zedagent/handlecertconfig.go

@@ -200,13 +200,13 @@ func controllerCertsTask(ctx *zedagentContext, triggerCerts <-chan struct{}) {
 	// Run a timer for extra safety to handle controller certificates updates
 	// If we failed with the initial we have a short timer, otherwise
 	// the configurable one.
-	const shortTime = 120 // Two minutes
-	certInterval := ctx.globalConfig.GlobalValueInt(types.CertInterval)
+	const shortTime = 2 * time.Minute
+	certInterval := ctx.globalConfig.GlobalValueDuration(types.CertInterval)
 	if retry {
 		log.Noticef("Initial getCertsFromController failed; switching to short timer")
 		certInterval = shortTime
 	}
-	interval := time.Duration(certInterval) * time.Second
+	interval := certInterval
 	max := float64(interval)
 	min := max * 0.3
 	periodicTicker := flextimer.NewRangeTicker(time.Duration(min),
@@ -232,14 +232,13 @@ func controllerCertsTask(ctx *zedagentContext, triggerCerts <-chan struct{}) {
 		ctx.ps.StillRunning(wdName, warningTime, errorTime)
 		if retry && success {
 			log.Noticef("getCertsFromController succeeded; switching to long timer %d seconds",
-				ctx.globalConfig.GlobalValueInt(types.CertInterval))
-			updateTaskTimer(ctx.globalConfig.GlobalValueInt(types.CertInterval),
+				ctx.globalConfig.GlobalValueDuration(types.CertInterval))
+			updateTaskTimer(ctx.globalConfig.GlobalValueDuration(types.CertInterval),
 				ctx.getconfigCtx.certTickerHandle)
 			retry = false
 		} else if !retry && !success {
 			log.Noticef("getCertsFromController failed; switching to short timer")
-			updateTaskTimer(shortTime,
-				ctx.getconfigCtx.certTickerHandle)
+			updateTaskTimer(shortTime, ctx.getconfigCtx.certTickerHandle)
 			retry = true
 		}
 	}

pkg/pillar/cmd/zedagent/handleconfig.go

@@ -516,16 +516,15 @@ func updateConfigTimer(configInterval uint32, tickerHandle interface{}) {
 
 // updateTaskTimer updates the given tickerHandle to use a new interval based on configInterval.
 // It adjusts the ticker's range and triggers an immediate tick to ensure the new interval takes effect promptly.
-func updateTaskTimer(configInterval uint32, tickerHandle interface{}) {
+func updateTaskTimer(configInterval time.Duration, tickerHandle interface{}) {
 
 	if tickerHandle == nil {
 		// Happens if the tickerHandle has not been initialized yet.
 		log.Warnf("updateConfigTimer: no tickerHandle yet")
 		return
 	}
-	interval := time.Duration(configInterval) * time.Second
-	log.Functionf("updateTaskTimer() change to %v", interval)
-	max := float64(interval)
+	log.Functionf("updateTaskTimer() change to %v", configInterval)
+	max := float64(configInterval)
 	min := max * 0.3
 	// Update the ticker to use the new interval range.
 	flextimer.UpdateRangeTicker(tickerHandle,

pkg/pillar/cmd/zedagent/handlemetrics.go

@@ -1013,16 +1013,16 @@ func hardwareHealthTimerTask(ctx *zedagentContext, handleChannel chan interface{
 	// Run a timer for extra safety to send hardwarehealth updates
 	// If we failed with the initial we have a short timer, otherwise
 	// the configurable one.
-	const shortTimeSecs = 120 // Short time: two minutes
-	hardwareHealthInterval := ctx.globalConfig.GlobalValueInt(types.HardwareHealthInterval)
-	interval := time.Duration(hardwareHealthInterval)
+	const shortTime = 2 * time.Minute
+	hardwareHealthInterval := ctx.globalConfig.GlobalValueDuration(types.HardwareHealthInterval)
+	interval := hardwareHealthInterval
 	if retry {
 		log.Noticef("Initial publishHardwareHealth failed; switching to short timer")
-		interval = shortTimeSecs
+		interval = shortTime
 	}
-	max := float64(interval * time.Second)
-	min := max * 0.3
-	ticker := flextimer.NewRangeTicker(time.Duration(min), time.Duration(max))
+	max := interval
+	min := time.Duration(max.Seconds() * 0.3 * float64(time.Second))
+	ticker := flextimer.NewRangeTicker(min, max)
 	// Return handle to caller
 	handleChannel <- ticker
 
@@ -1049,7 +1049,7 @@ func hardwareHealthTimerTask(ctx *zedagentContext, handleChannel chan interface{
 				retry = false
 			} else if !retry && !success {
 				log.Noticef("Hardwarehealth failed; switching to short timer")
-				updateTaskTimer(shortTimeSecs, ticker)
+				updateTaskTimer(shortTime, ticker)
 				retry = true
 			}
 		case <-stillRunning.C:

pkg/pillar/cmd/zedagent/parseconfig.go

@@ -2854,8 +2854,8 @@ func parseConfigItems(ctx *getconfigContext, config *zconfig.EdgeDevConfig,
 		// Set GlobalStatus Values from GlobalConfig.
 		oldConfigInterval := oldGlobalConfig.GlobalValueInt(types.ConfigInterval)
 		newConfigInterval := newGlobalConfig.GlobalValueInt(types.ConfigInterval)
-		oldCertInterval := oldGlobalConfig.GlobalValueInt(types.CertInterval)
-		newCertInterval := newGlobalConfig.GlobalValueInt(types.CertInterval)
+		oldCertInterval := oldGlobalConfig.GlobalValueDuration(types.CertInterval)
+		newCertInterval := newGlobalConfig.GlobalValueDuration(types.CertInterval)
 
 		oldMetricInterval := oldGlobalConfig.GlobalValueInt(types.MetricInterval)
 		newMetricInterval := newGlobalConfig.GlobalValueInt(types.MetricInterval)

pkg/pillar/types/global.go

@@ -9,6 +9,7 @@ import (
 	"regexp"
 	"strconv"
 	"strings"
+	"time"
 
 	"github.com/sirupsen/logrus" // OK for logrus.Fatal
 )
@@ -34,6 +35,9 @@ const (
 	SenderStatusDebug                                  // Not a failure
 )
 
+// see also https://brandur.org/fragments/go-max-time-duration
+const maxDuration time.Duration = 1<<63 - 1
+
 // String prints ASCII
 func (status SenderStatus) String() string {
 	switch status {
@@ -434,6 +438,8 @@ const (
 	ConfigItemTypeString
 	// ConfigItemTypeTriState - for config item's who's value is a tristate
 	ConfigItemTypeTriState
+	// ConfigItemTypeDuration - for config item's who's value is a duration
+	ConfigItemTypeDuration
 )
 
 var (
@@ -485,6 +491,10 @@ type ConfigItemSpec struct {
 	IntMax     uint32
 	IntDefault uint32
 
+	DurationMin     time.Duration
+	DurationMax     time.Duration
+	DurationDefault time.Duration
+
 	StringValidator Validator
 	StringDefault   string
 	BoolDefault     bool
@@ -505,6 +515,8 @@ func (configSpec ConfigItemSpec) DefaultValue() ConfigItemValue {
 		item.StrValue = configSpec.StringDefault
 	case ConfigItemTypeTriState:
 		item.TriStateValue = configSpec.TriStateDefault
+	case ConfigItemTypeDuration:
+		item.DurationValue = configSpec.DurationDefault
 	}
 	return item
 }
@@ -520,6 +532,24 @@ type ConfigItemSpecMap struct {
 	AgentSettings map[AgentSettingKey]ConfigItemSpec
 }
 
+// AddDurationItem - Adds integer item to specMap
+func (specMap *ConfigItemSpecMap) AddDurationItem(key GlobalSettingKey,
+	defaultDuration, minDuration, maxDuration time.Duration) {
+
+	if defaultDuration < minDuration || defaultDuration > maxDuration {
+		logrus.Fatalf("Adding int item %s failed. Value does not meet given min/max criteria", key)
+	}
+
+	configItem := ConfigItemSpec{
+		ItemType:        ConfigItemTypeDuration,
+		Key:             string(key),
+		DurationDefault: defaultDuration,
+		DurationMin:     minDuration,
+		DurationMax:     maxDuration,
+	}
+	specMap.GlobalSettings[key] = configItem
+}
+
 // AddIntItem - Adds integer item to specMap
 func (specMap *ConfigItemSpecMap) AddIntItem(key GlobalSettingKey,
 	defaultInt uint32, min uint32, max uint32) {
@@ -691,6 +721,7 @@ type ConfigItemValue struct {
 	StrValue      string
 	BoolValue     bool
 	TriStateValue TriState
+	DurationValue time.Duration
 }
 
 // StringValue - Returns the value in String Format
@@ -704,6 +735,8 @@ func (val ConfigItemValue) StringValue() string {
 		return val.StrValue
 	case ConfigItemTypeTriState:
 		return FormatTriState(val.TriStateValue)
+	case ConfigItemTypeDuration:
+		return fmt.Sprintf("%d", uint64(val.DurationValue.Seconds()))
 	default:
 		return fmt.Sprintf("UnknownType(%d)", val.ItemType)
 	}
@@ -771,6 +804,17 @@ func (configPtr *ConfigItemValueMap) GlobalValueInt(key GlobalSettingKey) uint32
 	}
 }
 
+// GlobalValueDuration - Gets a duration global setting value
+func (configPtr *ConfigItemValueMap) GlobalValueDuration(key GlobalSettingKey) time.Duration {
+	val := configPtr.globalConfigItemValue(key)
+	if val.ItemType == ConfigItemTypeDuration {
+		return val.DurationValue
+	} else {
+		logrus.Fatalf("***Key(%s) is of Type(%d) NOT time.Duration", key, val.ItemType)
+		return 0
+	}
+}
+
 // GlobalValueString - Gets a string global setting value
 func (configPtr *ConfigItemValueMap) GlobalValueString(key GlobalSettingKey) string {
 	val := configPtr.globalConfigItemValue(key)
@@ -842,7 +886,7 @@ func (configPtr *ConfigItemValueMap) DelAgentValue(key AgentSettingKey, agentNam
 	}
 }
 
-// SetGlobalValueInt - sets a int value for a key
+// SetGlobalValueInt - sets an int value for a key
 func (configPtr *ConfigItemValueMap) SetGlobalValueInt(key GlobalSettingKey, value uint32) {
 	if configPtr.GlobalSettings == nil {
 		configPtr.GlobalSettings = make(map[GlobalSettingKey]ConfigItemValue)
@@ -896,22 +940,29 @@ func (configPtr *ConfigItemValueMap) ResetGlobalValue(key GlobalSettingKey) {
 	configPtr.GlobalSettings[key] = specMap.GlobalSettings[key].DefaultValue()
 }
 
+func (configSpec ConfigItemSpec) parseUint(itemValue string) (uint32, error) {
+	i64, err := strconv.ParseUint(itemValue, 10, 32)
+	if err == nil {
+		val := uint32(i64)
+		if val > configSpec.IntMax || val < configSpec.IntMin {
+			retErr := fmt.Errorf("value out of bounds. Parsed value: %d, Max: %d, Min: %d",
+				val, configSpec.IntMax, configSpec.IntMin)
+			return val, retErr
+		} else {
+			return val, err
+		}
+	}
+
+	return 0, err
+}
+
 func (configSpec ConfigItemSpec) parseValue(itemValue string) (ConfigItemValue, error) {
 	value := configSpec.DefaultValue()
 	var retErr error
 	if configSpec.ItemType == ConfigItemTypeInt {
-		i64, err := strconv.ParseUint(itemValue, 10, 32)
-		if err == nil {
-			val := uint32(i64)
-			if val > configSpec.IntMax || val < configSpec.IntMin {
-				retErr = fmt.Errorf("value out of bounds. Parsed value: %d, Max: %d, Min: %d",
-					val, configSpec.IntMax, configSpec.IntMin)
-			} else {
-				value.IntValue = val
-			}
-		} else {
+		value.IntValue, retErr = configSpec.parseUint(itemValue)
+		if retErr != nil {
 			value.IntValue = configSpec.IntDefault
-			retErr = err
 		}
 	} else if configSpec.ItemType == ConfigItemTypeTriState {
 		newTs, err := ParseTriState(itemValue)
@@ -936,6 +987,19 @@ func (configSpec ConfigItemSpec) parseValue(itemValue string) (ConfigItemValue,
 		} else {
 			return value, err
 		}
+	} else if configSpec.ItemType == ConfigItemTypeDuration {
+		val, err := configSpec.parseUint(itemValue)
+		if err == nil {
+			if val > configSpec.IntMax || val < configSpec.IntMin {
+				retErr = fmt.Errorf("value out of bounds. Parsed value: %d, Max: %d, Min: %d",
+					val, configSpec.IntMax, configSpec.IntMin)
+			} else {
+				value.DurationValue = time.Duration(val * uint32(time.Second))
+			}
+		} else {
+			value.DurationValue = configSpec.DurationDefault
+			retErr = err
+		}
 	}
 	return value, retErr
 }
@@ -959,7 +1023,7 @@ func NewConfigItemSpecMap() ConfigItemSpecMap {
 	configItemSpecMap.AddIntItem(ConfigInterval, 60, 5, HourInSec)
 	// Additional safety to periodically fetch the controller certificate
 	// Useful for odd cases when the triggered updates do not work.
-	configItemSpecMap.AddIntItem(CertInterval, 24*HourInSec, 60, 0xFFFFFFFF)
+	configItemSpecMap.AddDurationItem(CertInterval, 24*time.Hour, 60*time.Second, maxDuration)
 	// timer.metric.diskscan.interval (seconds)
 	// Shorter interval can lead to device scanning the disk frequently which is a costly operation.
 	configItemSpecMap.AddIntItem(DiskScanMetricInterval, 300, 5, HourInSec)
@@ -969,7 +1033,7 @@ func NewConfigItemSpecMap() ConfigItemSpecMap {
 	configItemSpecMap.AddIntItem(MetricInterval, 60, 5, HourInSec)
 	// timer.metric.hardwarehealth.interval (seconds)
 	// Default value 12 hours minimum value 6 hours.
-	configItemSpecMap.AddIntItem(HardwareHealthInterval, 43200, 21600, 0xFFFFFFFF)
+	configItemSpecMap.AddDurationItem(HardwareHealthInterval, 43200*time.Second, 21600*time.Second, maxDuration)
 	// timer.reboot.no.network (seconds) - reboot after no controller connectivity
 	// Max designed to allow the option of never rebooting even if device
 	//  can't connect to the cloud

pkg/pillar/types/global_test.go

@@ -23,6 +23,8 @@ func TestDefaultValue(t *testing.T) {
 			assert.Equal(t, item.TriStateDefault, defaultValue.TriStateValue)
 		} else if item.ItemType == ConfigItemTypeString {
 			assert.Equal(t, item.StringDefault, defaultValue.StrValue)
+		} else if item.ItemType == ConfigItemTypeDuration {
+			assert.Equal(t, item.DurationDefault, defaultValue.DurationValue)
 		}
 	}