# Subsecond Probe Options

Thanks @sftim and @johnbelamaric for your reviews! Trying to summarize the options in one place (since it's a bit messy in the KEP at the moment), here's some options that have been discussed for configuring a probe to run on a subsecond interval:

(to make editing the KEP easier, here's a link to a [hackmd version](https://hackmd.io/nsGZk3bySKOstryPoPCcag))

## `*Milliseconds`

This would add new fields, `periodMilliseconds`, `initialDelayMilliseconds`, and `timeoutMilliseconds` to the Probe struct. For example,

  periodSeconds: 1
  periodMilliseconds: 500

would result in an effective period value of `periodSeconds + periodMilliseconds`, or for our example `1.5s`. Milliseconds fields would be required to be less than one second (as anything seconds values could be done in the existing field).

The `*Milliseconds` fields can be either unsigned or signed, neither of which is perfect.

One corner case for unsigned is the following: `periodSeconds: 0` and `periodMilliseconds: 500` for an effective period of `500ms`. On legacy systems where `periodMilliseconds` is not recognized, this would lead to `periodSeconds: 0`, which gets reset to `periodSeconds: 10` in [`defaults.go`](https://github.com/kubernetes/kubernetes/blob/a41f9e976da10af28169cbbfebbce5ad4ba965f0/pkg/apis/core/v1/defaults.go#L214-L215). I can imagine users not being happy with that... :) , though this can probably be handled when converting from one API version to another using the logic John suggested [here](https://github.com/kubernetes/enhancements/pull/3067#discussion_r822207500).

Using signed could result in an ambiguous round-trip scenario as Tim was [describing](https://github.com/kubernetes/enhancements/pull/3067#discussion_r806052051) for a possible future v2 API: if we have `1.5s` stored in a single field, what were the original values? `1s` and `+500ms` or `2s` and `-500ms`?

This pattern could be extended for smaller durations as well, i.e. `*Microseconds` and `*Nanoseconds` (and potentially others if they are ever added to Go).

## `early*Offset`

(copying directly from [Tim's comment](https://github.com/kubernetes/enhancements/pull/3067#issuecomment-1039311016))

Add an extra field to specify “early failure”: an integer quantity of milliseconds (or microseconds - we should leave room for that). That early failure offset is something that legacy clients can ignore without a big problem and would fully support round-tripping to a future Pod v2 API that unifies these fields.

Using a negative offset makes the distinction between (1s - 995ms) more obvious to a legacy client that's unaware of the new fields. A positive offset (0s + 5ms, or maybe 0s + 5000μs) could get interpreted quite differently.

If we do that, we should absolutely disallow a negative offset ≥ 1.0s seconds. Otherwise we have a challenge around unambiguous round-tripping (eg from a future Pod v2 API back to Pod v1).

## `*Duration`

This would be very similar to the previous option, except that it would only require one field instead of multiple ones for each unit type. For example,

  periodDuration: 1.5s

This could be used in one of two ways:

It could be added to the existing `*Seconds` value, as done in the previous example (i.e. `periodSeconds + periodDuration`). The same caveats listed in the previous option would also apply in this case (signed vs. unsigned, adding vs. subtracting). In this case, the value would be restricted to less than one second.

Alternatively, it could _replace_ the existing `*Seconds` value, using the normalizing pattern described [here](https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api_changes.md#making-a-singular-field-plural).

As Tim noted, the current API guidelines [require using integers](https://github.com/kubernetes/community/blob/489de956d7b601fd23c8f48a87b501e0de4a9c7f/contributors/devel/sig-architecture/api-conventions.md?plain=1#L878-L880) for durations. That said, the preceding line also mentions that the best approach is [still TBD](https://github.com/kubernetes/community/blob/489de956d7b601fd23c8f48a87b501e0de4a9c7f/contributors/devel/sig-architecture/api-conventions.md?plain=1#L873-L876), so there's some ambiguity in the docs on this topic.

## A new struct, `ProbeOffset`

This one kind of splits the difference between the two previous options, using a `struct` to allow for specifying a time unit and value:

type ProbeOffset struct {
  value uint32
  unit string
}

which would be used as follows

  periodOffset:
    value: 500
    unit: milliseconds

This requires introducing a new type for duration, which in hindsight does not meet [either of the API conventions](https://github.com/kubernetes/community/blob/489de956d7b601fd23c8f48a87b501e0de4a9c7f/contributors/devel/sig-architecture/api-conventions.md#units) mentioned above.

Usage would be similar to the `*Duration` option, i.e. either replace or add to existing field (and if adding to the existing would be restricted to less than 1 second).

## Setting the time units in a different field, `ReadSecondsAs`

The last option would be specify a specific time unit value that would override, in a sense, the "seconds" in all the `*Seconds` fields. For example

  periodSeconds: 100
  readSecondsAs: milliseconds

would result in an effective period of `500ms`.

Note that this would apply for all `*Seconds` values: if `readSecondsAs` is used, all probe times would need to be rendered in milliseconds. If field level granuality was required (i.e. one value in seconds, one in milliseconds), may as well create `*Milliseconds` fields.

For more examples of how this would work, see [this WIP PR](https://github.com/kubernetes/kubernetes/pull/107958).

Round-tripping issues are handled in a [drop function](https://github.com/kubernetes/kubernetes/blob/39a3c5c880d79fead1ae4dc80f462cac4a33878f/pkg/api/pod/util.go#L607-L636) (using similar logic as would be needed in the first two options).

While this would reduce the number of new fields one has to add, it is to some degree counterintuitive to have a `*Seconds` field in milliseconds.