MicroTime should use microsecond percision in serialization

[peterhuene]

See related issue in deislabs/krustlet#5.

It appears that the serialization of MicroTime may result in 3, 6, or 9 digits for the subsecond precision.

I believe this is because it simply serializes the inner DateTime<Utc>, which will format as if to_rfc3339 was called; I believe this behaves as if it were being formatted with SecondsFormat::AutoSi resulting in 3, 6, or 9 digits depending on the nanoseconds value in the DateTime.

Unfortunately, k8s appears to require exactly six digits of subsecond precision and will error when parsing the datetimes containing 3 or 9 digits of precision.

I think the fix here would be to change MicroTime's serialization such that it serializes the result of self.0.to_rfc3339(SecondsFormat::Micro, true). This should guarantee six digits of subsecond precision.

[peterhuene]

See this comment on the linked issue. We believe it might be a bug in how k8s should be parsing these RFC 3339 date times rather than a bug in k8s-openapi. Still, serializing with a fixed-6 microsecond precision from k8s-openapi would workaround the issue.

[Arnavion]

Is there a situation where the regular io.k8s.apimachinery.pkg.apis.meta.v1.Time should not be serialized with 9 6 decimals? I'm asking because it would mean I can just do that for all DateTime newtypes, ie both Time and MicroTime, rather than add a special type for MicroTime.

Edit: 9 -> 6

[peterhuene]

@technosophos might be able to answer that, he's much more knowledgeable about the k8s API than I.

[Arnavion]

https://github.com/kubernetes/apimachinery/blob/b789a6e6b31f49e34cd0b0876791677a85bbb375/pkg/apis/meta/v1/time.go#L112 parses with time.RFC3339, which is defined as "2006-01-02T15:04:05Z07:00", so it has no decimal parts. However this does succeed and parse the decimals:

package main

import (
    "fmt"
    "time"
)

func main() {
    dfmt := time.RFC3339
    str := "2020-03-05T20:23:12.878787Z"
    out, err := time.Parse(dfmt, str)
    if err != nil {
        fmt.Println(err)
    }
    fmt.Printf("Date is: %s\n", out)
}

... so it seems it would be fine to always serialize with six decimals.

[Arnavion]

Never mind, it's basically the same amount of work to special-case it for MicroTime as for both Time and MicroTime. Also, I confirmed that even if Kubernetes might accept decimals in Time serialized form, it definitely doesn't emit them. So just to be safe, I'm going to truncate Time to zero decimals as well.

[technosophos]

I did file an issue against Kubernetes APIMachinery about this, as I believe it is a typo in their date format: https://github.com/kubernetes/apimachinery/issues/88

But, yes, as their date format currently stands, they require exactly this precision: 2006-01-02T15:04:05.000000Z07:00 (https://github.com/kubernetes/apimachinery/blob/master/pkg/apis/meta/v1/micro_time.go#L26)

More or less than 6 places will cause APIMachinery's Go parser to fail.

[Arnavion]

@technosophos The question was about Time, not MicroTime, but as I said in the comment right before yours it doesn't matter any more.

[technosophos]

Sorry, yeah -- I see that now. Thanks for working on this so quickly. And I see you figured out Time already, too!

[Arnavion]

Can either of you test with the fix-63 branch ? It should be a drop-in replacement for k8s-openapi = "0.7"

[peterhuene]

I'll test it now.

[peterhuene]

Unfortunately it looks like krustlet is a little out of date wrt kube and k8s-openapi versions, and there's been breaking changes in kube since the version it is currently using, so it'll take longer for me to test out (I just started with krustlet today).

[Arnavion]

You can manually edit the time.rs and micro_time.rs in your ~/.cargo/registry/src/github.com-*/k8s-openapi-*/... path with the changes from that branch.

[technosophos]

I just created a quick project using the latest version, and tested the MicroTime fix, and it is looking good. I am now definitely seeing timestamps with trailing zeros.

[peterhuene]

I tested with just the change to MicroTime and 👍

[technosophos]

@Arnavion Thank you so much for going out of your way to fix this so quickly. I really, really appreciate that.

[radu-matei]

Unfortunately it looks like krustlet is a little out of date wrt kube and k8s-openapi versions.

There is a work in progress branch that updates to the latest release.