feat: allow a machine health check schedule of Never

pkg/machinepolicies/duration_formatter.go

@@ -3,6 +3,7 @@ package machinepolicies
 import (
 	"fmt"
 	"strconv"
+	"strings"
 	"time"
 )
 
@@ -30,20 +31,67 @@ func ToTimeSpan(duration time.Duration) string {
 	return fmt.Sprintf("%02d.%02d:%02d:%02d.%05d", days, hours, minutes, seconds, secondsFraction)
 }
 
+// FromTimeSpan parses a .NET time span, "[d.]hh:mm:ss[.fffffff]", into a duration. The day and
+// fractional-second components are optional, and the server does not pad the day component to a
+// fixed width, so the fields cannot be read at fixed offsets. Input that does not parse yields a
+// zero duration.
 func FromTimeSpan(timeSpan string) time.Duration {
-	if len(timeSpan) == 8 {
-		hours, _ := strconv.ParseInt(timeSpan[0:2], 10, 64)
-		minutes, _ := strconv.ParseInt(timeSpan[3:5], 10, 64)
-		seconds, _ := strconv.ParseInt(timeSpan[6:8], 10, 64)
-		duration, _ := time.ParseDuration(fmt.Sprintf("%dh%dm%ds", hours, minutes, seconds))
-		return duration
-	}
-
-	days, _ := strconv.ParseInt(timeSpan[0:0], 10, 32)
-	hours, _ := strconv.ParseInt(timeSpan[2:4], 10, 64)
-	hours += (days * 24)
-	minutes, _ := strconv.ParseInt(timeSpan[5:7], 10, 64)
-	seconds, _ := strconv.ParseInt(timeSpan[8:10], 10, 64)
-	duration, _ := time.ParseDuration(fmt.Sprintf("%dh%dm%ds", hours, minutes, seconds))
-	return duration
+	var days int64
+	remainder := timeSpan
+
+	// Both the day separator and the fractional-second separator are ".", so a leading segment is
+	// only the day component when the rest still holds a complete "hh:mm:ss".
+	if index := strings.Index(remainder, "."); index >= 0 && strings.Count(remainder[index+1:], ":") == 2 {
+		parsedDays, err := strconv.ParseInt(remainder[:index], 10, 64)
+		if err != nil {
+			return 0
+		}
+		days = parsedDays
+		remainder = remainder[index+1:]
+	}
+
+	var fraction time.Duration
+	if index := strings.Index(remainder, "."); index >= 0 {
+		digits := remainder[index+1:]
+		parsedFraction, err := strconv.ParseInt(digits, 10, 64)
+		if err != nil {
+			return 0
+		}
+		// The digits are a decimal fraction of a second, however many of them there are.
+		scale := pow10(len(digits))
+		fraction = time.Duration(parsedFraction * int64(time.Second) / scale)
+		remainder = remainder[:index]
+	}
+
+	fields := strings.Split(remainder, ":")
+	if len(fields) != 3 {
+		return 0
+	}
+
+	hours, err := strconv.ParseInt(fields[0], 10, 64)
+	if err != nil {
+		return 0
+	}
+	minutes, err := strconv.ParseInt(fields[1], 10, 64)
+	if err != nil {
+		return 0
+	}
+	seconds, err := strconv.ParseInt(fields[2], 10, 64)
+	if err != nil {
+		return 0
+	}
+
+	return time.Duration(days)*24*time.Hour +
+		time.Duration(hours)*time.Hour +
+		time.Duration(minutes)*time.Minute +
+		time.Duration(seconds)*time.Second +
+		fraction
+}
+
+func pow10(exponent int) int64 {
+	result := int64(1)
+	for i := 0; i < exponent; i++ {
+		result *= 10
+	}
+	return result
 }

pkg/machinepolicies/duration_formatter_test.go

@@ -0,0 +1,94 @@
+package machinepolicies
+
+import (
+	"testing"
+	"time"
+)
+
+func TestToTimeSpan(t *testing.T) {
+	halfSecond, _ := time.ParseDuration("0.5s")
+	second, _ := time.ParseDuration("1111ms")
+	twoHours, _ := time.ParseDuration("120m")
+	fourtySevenHours, _ := time.ParseDuration("47h")
+	twoDays, _ := time.ParseDuration("48h")
+
+	testCases := []struct {
+		duration time.Duration
+		expected string
+	}{
+		{halfSecond, "00:00:00.50000"},
+		{second, "00:00:01.11100"},
+		{time.Second, "00:00:01"},
+		{time.Minute, "00:01:00"},
+		{time.Hour, "01:00:00"},
+		{twoHours, "02:00:00"},
+		{fourtySevenHours, "01.23:00:00"},
+		{twoDays, "02.00:00:00"},
+	}
+
+	for _, testCase := range testCases {
+		if actual := ToTimeSpan(testCase.duration); actual != testCase.expected {
+			t.Errorf("ToTimeSpan(%s) = %q, want %q", testCase.duration, actual, testCase.expected)
+		}
+	}
+}
+
+func TestFromTimeSpan(t *testing.T) {
+	testCases := []struct {
+		timeSpan string
+		expected time.Duration
+	}{
+		// hh:mm:ss
+		{"00:00:00", 0},
+		{"00:00:01", time.Second},
+		{"00:01:00", time.Minute},
+		{"01:00:00", time.Hour},
+		{"02:00:00", 2 * time.Hour},
+		// d.hh:mm:ss, as written by ToTimeSpan
+		{"00.00:00:01", time.Second},
+		{"00.47:00:00", 47 * time.Hour},
+		{"01.23:00:00", 47 * time.Hour},
+		{"02.00:00:00", 48 * time.Hour},
+		// d.hh:mm:ss, as returned by the Octopus server, which does not pad the days
+		{"1.00:00:00", 24 * time.Hour},
+		{"7.12:30:00", 7*24*time.Hour + 12*time.Hour + 30*time.Minute},
+		{"37500.00:00:00", 900000 * time.Hour},
+		// fractional seconds
+		{"00:00:00.50000", 500 * time.Millisecond},
+		{"00:00:01.11100", 1111 * time.Millisecond},
+		{"01.00:00:00.50000", 24*time.Hour + 500*time.Millisecond},
+		// .NET renders fractional seconds as seven digits
+		{"00:00:00.5000000", 500 * time.Millisecond},
+		// malformed input yields a zero duration rather than a panic
+		{"", 0},
+		{"not-a-timespan", 0},
+		{"00:00", 0},
+	}
+
+	for _, testCase := range testCases {
+		if actual := FromTimeSpan(testCase.timeSpan); actual != testCase.expected {
+			t.Errorf("FromTimeSpan(%q) = %s, want %s", testCase.timeSpan, actual, testCase.expected)
+		}
+	}
+}
+
+func TestTimeSpanRoundTrip(t *testing.T) {
+	durations := []time.Duration{
+		0,
+		time.Second,
+		time.Minute,
+		time.Hour,
+		47 * time.Hour,
+		48 * time.Hour,
+		7 * 24 * time.Hour,
+		900000 * time.Hour,
+		500 * time.Millisecond,
+		24*time.Hour + 12*time.Hour + 30*time.Minute + 15*time.Second,
+	}
+
+	for _, duration := range durations {
+		if actual := FromTimeSpan(ToTimeSpan(duration)); actual != duration {
+			t.Errorf("FromTimeSpan(ToTimeSpan(%s)) = %s, want %s", duration, actual, duration)
+		}
+	}
+}

pkg/machinepolicies/machine_health_check_policy.go

@@ -28,6 +28,14 @@ func NewMachineHealthCheckPolicy() *MachineHealthCheckPolicy {
 
 // MarshalJSON returns a machine health check policy as its JSON encoding.
 func (m *MachineHealthCheckPolicy) MarshalJSON() ([]byte, error) {
+	// A health check schedule of "Never" is the absence of both an interval and a cron
+	// expression, so a zero interval has to be left out of the payload entirely. Writing
+	// "00:00:00" is read back as an interval of zero rather than as "Never".
+	healthCheckInterval := ""
+	if m.HealthCheckInterval != 0 {
+		healthCheckInterval = ToTimeSpan(m.HealthCheckInterval)
+	}
+
 	machineHealthCheckPolicy := struct {
 		BashHealthCheckPolicy       *MachineScriptPolicy `json:"BashHealthCheckPolicy,omitempty"`
 		HealthCheckCron             string               `json:"HealthCheckCron,omitempty"`
@@ -39,7 +47,7 @@ func (m *MachineHealthCheckPolicy) MarshalJSON() ([]byte, error) {
 		BashHealthCheckPolicy:       m.BashHealthCheckPolicy,
 		HealthCheckCron:             m.HealthCheckCron,
 		HealthCheckCronTimezone:     m.HealthCheckCronTimezone,
-		HealthCheckInterval:         ToTimeSpan(m.HealthCheckInterval),
+		HealthCheckInterval:         healthCheckInterval,
 		HealthCheckType:             m.HealthCheckType,
 		PowerShellHealthCheckPolicy: m.PowerShellHealthCheckPolicy,
 	}

pkg/machinepolicies/machine_health_check_policy_test.go

@@ -0,0 +1,67 @@
+package machinepolicies
+
+import (
+	"encoding/json"
+	"strings"
+	"testing"
+	"time"
+)
+
+func TestMachineHealthCheckPolicyMarshalJSON(t *testing.T) {
+	testCases := []struct {
+		name     string
+		interval time.Duration
+		expected string
+	}{
+		{"interval is written as a time span", 24 * time.Hour, `"HealthCheckInterval":"01.00:00:00"`},
+		{"sub-day interval is written as a time span", time.Hour, `"HealthCheckInterval":"01:00:00"`},
+	}
+
+	for _, testCase := range testCases {
+		t.Run(testCase.name, func(t *testing.T) {
+			policy := NewMachineHealthCheckPolicy()
+			policy.HealthCheckInterval = testCase.interval
+
+			data, err := json.Marshal(policy)
+			if err != nil {
+				t.Fatalf("unexpected error: %v", err)
+			}
+			if !strings.Contains(string(data), testCase.expected) {
+				t.Errorf("marshalled policy %s does not contain %s", data, testCase.expected)
+			}
+		})
+	}
+}
+
+// A zero interval means the health check schedule is "Never", which the server represents by the
+// absence of both HealthCheckInterval and HealthCheckCron. Writing "00:00:00" instead is read back
+// as an interval of zero minutes, not as Never.
+func TestMachineHealthCheckPolicyMarshalJSONOmitsZeroInterval(t *testing.T) {
+	policy := NewMachineHealthCheckPolicy()
+	policy.HealthCheckInterval = 0
+
+	data, err := json.Marshal(policy)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if strings.Contains(string(data), "HealthCheckInterval") {
+		t.Errorf("marshalled policy %s should not contain HealthCheckInterval", data)
+	}
+}
+
+func TestMachineHealthCheckPolicyUnmarshalJSONWithoutInterval(t *testing.T) {
+	data := `{
+		"PowerShellHealthCheckPolicy": {"RunType": "Inline", "ScriptBody": ""},
+		"BashHealthCheckPolicy": {"RunType": "Inline", "ScriptBody": ""},
+		"HealthCheckCronTimezone": "UTC",
+		"HealthCheckType": "RunScript"
+	}`
+
+	policy := &MachineHealthCheckPolicy{}
+	if err := json.Unmarshal([]byte(data), policy); err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if policy.HealthCheckInterval != 0 {
+		t.Errorf("HealthCheckInterval = %s, want 0", policy.HealthCheckInterval)
+	}
+}

pkg/machines/duration_formatter.go

@@ -3,6 +3,7 @@ package machines
 import (
 	"fmt"
 	"strconv"
+	"strings"
 	"time"
 )
 
@@ -30,20 +31,67 @@ func ToTimeSpan(duration time.Duration) string {
 	return fmt.Sprintf("%02d.%02d:%02d:%02d.%05d", days, hours, minutes, seconds, secondsFraction)
 }
 
+// FromTimeSpan parses a .NET time span, "[d.]hh:mm:ss[.fffffff]", into a duration. The day and
+// fractional-second components are optional, and the server does not pad the day component to a
+// fixed width, so the fields cannot be read at fixed offsets. Input that does not parse yields a
+// zero duration.
 func FromTimeSpan(timeSpan string) time.Duration {
-	if len(timeSpan) == 8 {
-		hours, _ := strconv.ParseInt(timeSpan[0:2], 10, 64)
-		minutes, _ := strconv.ParseInt(timeSpan[3:5], 10, 64)
-		seconds, _ := strconv.ParseInt(timeSpan[6:8], 10, 64)
-		duration, _ := time.ParseDuration(fmt.Sprintf("%dh%dm%ds", hours, minutes, seconds))
-		return duration
-	}
-
-	days, _ := strconv.ParseInt(timeSpan[0:0], 10, 32)
-	hours, _ := strconv.ParseInt(timeSpan[2:4], 10, 64)
-	hours += (days * 24)
-	minutes, _ := strconv.ParseInt(timeSpan[5:7], 10, 64)
-	seconds, _ := strconv.ParseInt(timeSpan[8:10], 10, 64)
-	duration, _ := time.ParseDuration(fmt.Sprintf("%dh%dm%ds", hours, minutes, seconds))
-	return duration
+	var days int64
+	remainder := timeSpan
+
+	// Both the day separator and the fractional-second separator are ".", so a leading segment is
+	// only the day component when the rest still holds a complete "hh:mm:ss".
+	if index := strings.Index(remainder, "."); index >= 0 && strings.Count(remainder[index+1:], ":") == 2 {
+		parsedDays, err := strconv.ParseInt(remainder[:index], 10, 64)
+		if err != nil {
+			return 0
+		}
+		days = parsedDays
+		remainder = remainder[index+1:]
+	}
+
+	var fraction time.Duration
+	if index := strings.Index(remainder, "."); index >= 0 {
+		digits := remainder[index+1:]
+		parsedFraction, err := strconv.ParseInt(digits, 10, 64)
+		if err != nil {
+			return 0
+		}
+		// The digits are a decimal fraction of a second, however many of them there are.
+		scale := pow10(len(digits))
+		fraction = time.Duration(parsedFraction * int64(time.Second) / scale)
+		remainder = remainder[:index]
+	}
+
+	fields := strings.Split(remainder, ":")
+	if len(fields) != 3 {
+		return 0
+	}
+
+	hours, err := strconv.ParseInt(fields[0], 10, 64)
+	if err != nil {
+		return 0
+	}
+	minutes, err := strconv.ParseInt(fields[1], 10, 64)
+	if err != nil {
+		return 0
+	}
+	seconds, err := strconv.ParseInt(fields[2], 10, 64)
+	if err != nil {
+		return 0
+	}
+
+	return time.Duration(days)*24*time.Hour +
+		time.Duration(hours)*time.Hour +
+		time.Duration(minutes)*time.Minute +
+		time.Duration(seconds)*time.Second +
+		fraction
+}
+
+func pow10(exponent int) int64 {
+	result := int64(1)
+	for i := 0; i < exponent; i++ {
+		result *= 10
+	}
+	return result
 }