diff --git a/pkg/machinepolicies/duration_formatter.go b/pkg/machinepolicies/duration_formatter.go index 6f09bdc1..f6f12a5a 100644 --- a/pkg/machinepolicies/duration_formatter.go +++ b/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 } diff --git a/pkg/machinepolicies/duration_formatter_test.go b/pkg/machinepolicies/duration_formatter_test.go new file mode 100644 index 00000000..cc3e7fa0 --- /dev/null +++ b/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) + } + } +} diff --git a/pkg/machinepolicies/machine_health_check_policy.go b/pkg/machinepolicies/machine_health_check_policy.go index f6c12fb9..06f76713 100644 --- a/pkg/machinepolicies/machine_health_check_policy.go +++ b/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, } diff --git a/pkg/machinepolicies/machine_health_check_policy_test.go b/pkg/machinepolicies/machine_health_check_policy_test.go new file mode 100644 index 00000000..734165b5 --- /dev/null +++ b/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) + } +} diff --git a/pkg/machines/duration_formatter.go b/pkg/machines/duration_formatter.go index b6cc8038..55cacf16 100644 --- a/pkg/machines/duration_formatter.go +++ b/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 } diff --git a/pkg/machines/duration_formatter_test.go b/pkg/machines/duration_formatter_test.go index 9484a95e..06032f37 100644 --- a/pkg/machines/duration_formatter_test.go +++ b/pkg/machines/duration_formatter_test.go @@ -11,22 +11,84 @@ func TestToTimeSpan(t *testing.T) { twoHours, _ := time.ParseDuration("120m") fourtySevenHours, _ := time.ParseDuration("47h") twoDays, _ := time.ParseDuration("48h") - t.Logf("500ms: %s", ToTimeSpan(halfSecond)) - t.Logf("1000ms: %s", ToTimeSpan(second)) - t.Logf("1s: %s", ToTimeSpan(time.Second)) - t.Logf("1m: %s", ToTimeSpan(time.Minute)) - t.Logf("1h: %s", ToTimeSpan(time.Hour)) - t.Logf("120m: %s", ToTimeSpan(twoHours)) - t.Logf("47h: %s", ToTimeSpan(fourtySevenHours)) - t.Logf("48h: %s", ToTimeSpan(twoDays)) + + 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) { - t.Logf("1s: %s", FromTimeSpan("00.00:00:01")) - t.Logf("1m: %s", FromTimeSpan("00.00:01:00")) - t.Logf("1h: %s", FromTimeSpan("00.01:00:00")) - t.Logf("120m: %s", FromTimeSpan("00.02:00:00")) - t.Logf("47h: %s", FromTimeSpan("00.47:00:00")) - t.Logf("48h: %s", FromTimeSpan("00.48:00:00")) - t.Logf("2d: %s", FromTimeSpan("02.00:00:00")) + 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) + } + } } diff --git a/pkg/machines/machine_health_check_policy.go b/pkg/machines/machine_health_check_policy.go index 08f3ea41..b3f09fc2 100644 --- a/pkg/machines/machine_health_check_policy.go +++ b/pkg/machines/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, } diff --git a/pkg/machines/machine_health_check_policy_test.go b/pkg/machines/machine_health_check_policy_test.go new file mode 100644 index 00000000..914ca669 --- /dev/null +++ b/pkg/machines/machine_health_check_policy_test.go @@ -0,0 +1,67 @@ +package machines + +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) + } +}