New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP (do not merge): Upgrade to YAML v2 #2490

Closed
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
4 participants
@ghodss
Member

ghodss commented Nov 20, 2014

This is needed because of a serious issue with YAML v1 described at http://blog.labix.org/2014/09/22/announcing-yaml-v2-for-go#typeerrors - basically, YAML v1 does not return any errors if you try to unmarshal YAML or JSON into an incompatible object. This is making ObjectStream development more difficult than it should be.

At this point I am working on porting to v2 but getting some very strange errors from the object conversion code. The big change in v2 is that SetYAML/GetYAML have changed to UnmarshalYAML/MarshalYAML, but there may be another change that's now causing this issue. @lavalamp (or others), I would really appreciate if you could checkout this PR and try running:

go test -run TestEmbeddedObject -v github.com/GoogleCloudPlatform/kubernetes/pkg/runtime

And see the errors that come up. Here is the output, including the debug logger output as well as a bunch of my own haphazardly inserted print statements that are included in the PR.

$ go test -run TestEmbeddedObject -v github.com/GoogleCloudPlatform/kubernetes/pkg/runtime
=== RUN TestEmbeddedObject
Trying to convert '[runtime_test.EmbeddedTest runtime_test.EmbeddedTestExternal]' to '%!v(MISSING)'
Calling custom conversion of '[runtime.EmbeddedObject runtime.RawExtension]' to '%!v(MISSING)'
Trying to convert '[runtime_test.EmbeddedTest runtime_test.EmbeddedTestExternal]' to '%!v(MISSING)'
Calling custom conversion of '[runtime.EmbeddedObject runtime.RawExtension]' to '%!v(MISSING)'
Calling custom conversion of '[runtime.EmbeddedObject runtime.RawExtension]' to '%!v(MISSING)'
Trying to convert '[runtime.emptyPlugin runtime_test.EmbeddedTestExternal]' to '%!v(MISSING)'
Calling custom conversion of '[runtime.EmbeddedObject runtime.RawExtension]' to '%!v(MISSING)'
"apiVersion":"v1test","kind":"EmbeddedTest","name":"inner","creationTimestamp":null,"id":"inner","object":null,"emptyObject":null},"emptyObject":null}HELLO2: EmbeddedTestSTART-UNMARSHAL
Trying to convert '[runtime_test.EmbeddedTestExternal runtime_test.EmbeddedTest]' to '%!v(MISSING)'
Calling custom conversion of '[runtime.RawExtension runtime.EmbeddedObject]' to '%!v(MISSING)'
RAWEXTENSIONTEXT: apiVersion: v1test
creationTimestamp: null
emptyObject: null
id: inner
kind: EmbeddedTest
name: inner
object: null
HELLO1: apiVersion: v1test
creationTimestamp: null
emptyObject: null
id: inner
kind: EmbeddedTest
name: inner
object: null
HELLO2: EmbeddedTestHELLO1: apiVersion: v1test
creationTimestamp: null
emptyObject: null
id: inner
kind: EmbeddedTest
name: inner
object: null
HELLO2: EmbeddedTestTrying to convert '[runtime_test.EmbeddedTestExternal runtime_test.EmbeddedTest]' to '%!v(MISSING)'
Calling custom conversion of '[runtime.RawExtension runtime.EmbeddedObject]' to '%!v(MISSING)'
RAWEXTENSIONTEXT: HELLO1: HELLO2: /Users/sam/code/go/src/github.com/GoogleCloudPlatform/kubernetes/pkg/conversion/scheme.go:152 (0x107ac8)
        (*Scheme).NewObject: debug.PrintStack()
/Users/sam/code/go/src/github.com/GoogleCloudPlatform/kubernetes/pkg/runtime/scheme.go:193 (0xf61f0)
        (*Scheme).New: obj, err := s.raw.NewObject(versionName, typeName)
/Users/sam/code/go/src/github.com/GoogleCloudPlatform/kubernetes/pkg/runtime/scheme.go:112 (0xf5791)
        (*Scheme).rawExtensionToEmbeddedObject: inObj, err := scheme.New(inVersion, kind)
/Users/sam/code/go/src/github.com/GoogleCloudPlatform/kubernetes/pkg/runtime/scheme.go:151 (0xf6bf0)
        rawExtensionToEmbeddedObject).fm: s.rawExtensionToEmbeddedObject,
/usr/local/Cellar/go/1.3/libexec/src/pkg/runtime/asm_amd64.s:362 (0x2c2d2)
        call64: CALLFN(call64, 64)
/usr/local/Cellar/go/1.3/libexec/src/pkg/reflect/value.go:563 (0xe07c0)
        Value.call: call(fn, args, uint32(frametype.size), uint32(retOffset))
/usr/local/Cellar/go/1.3/libexec/src/pkg/reflect/value.go:411 (0xdf467)
        Value.Call: return v.call("Call", in)
/Users/sam/code/go/src/github.com/GoogleCloudPlatform/kubernetes/pkg/conversion/converter.go:253 (0x101d63)
        (*Converter).convert: ret := fv.Call(args)[0].Interface()
/Users/sam/code/go/src/github.com/GoogleCloudPlatform/kubernetes/pkg/conversion/converter.go:314 (0x103a7f)

/Users/sam/code/go/src/github.com/GoogleCloudPlatform/kubernetes/pkg/conversion/converter.go:241 (0x1017db)
        (*Converter).Convert: return c.convert(sv, dv, s)
/Users/sam/code/go/src/github.com/GoogleCloudPlatform/kubernetes/pkg/conversion/scheme.go:213 (0x108095)
        (*Scheme).Convert: return s.converter.Convert(in, out, AllowDifferentFieldTypeNames, s.generateConvertMeta(inVersion, outVersion))
/Users/sam/code/go/src/github.com/GoogleCloudPlatform/kubernetes/pkg/runtime/scheme.go:225 (0xf63a9)
        (*Scheme).Convert: return s.raw.Convert(in, out)
/Users/sam/code/go/src/github.com/GoogleCloudPlatform/kubernetes/pkg/runtime/scheme.go:127 (0xf5980)
        (*Scheme).rawExtensionToEmbeddedObject: err = scheme.Convert(inObj, outObj)
/Users/sam/code/go/src/github.com/GoogleCloudPlatform/kubernetes/pkg/runtime/scheme.go:151 (0xf6bf0)
        rawExtensionToEmbeddedObject).fm: s.rawExtensionToEmbeddedObject,
/usr/local/Cellar/go/1.3/libexec/src/pkg/runtime/asm_amd64.s:362 (0x2c2d2)
        call64: CALLFN(call64, 64)
/usr/local/Cellar/go/1.3/libexec/src/pkg/reflect/value.go:563 (0xe07c0)
        Value.call: call(fn, args, uint32(frametype.size), uint32(retOffset))
/usr/local/Cellar/go/1.3/libexec/src/pkg/reflect/value.go:411 (0xdf467)
        Value.Call: return v.call("Call", in)
/Users/sam/code/go/src/github.com/GoogleCloudPlatform/kubernetes/pkg/conversion/converter.go:253 (0x101d63)
        (*Converter).convert: ret := fv.Call(args)[0].Interface()
/Users/sam/code/go/src/github.com/GoogleCloudPlatform/kubernetes/pkg/conversion/converter.go:314 (0x103a7f)

/Users/sam/code/go/src/github.com/GoogleCloudPlatform/kubernetes/pkg/conversion/converter.go:241 (0x1017db)
        (*Converter).Convert: return c.convert(sv, dv, s)
/Users/sam/code/go/src/github.com/GoogleCloudPlatform/kubernetes/pkg/conversion/decode.go:66 (0x1048ad)
        (*Scheme).Decode: err = s.converter.Convert(obj, objOut, 0, s.generateConvertMeta(version, s.InternalVersion))
/Users/sam/code/go/src/github.com/GoogleCloudPlatform/kubernetes/pkg/runtime/scheme.go:282 (0xf67a0)
        (*Scheme).Decode: obj, err := s.raw.Decode(data)
/Users/sam/code/go/src/github.com/GoogleCloudPlatform/kubernetes/pkg/runtime/embedded_test.go:70 (0x46f23)
        TestEmbeddedObject: decoded, err := s.Decode(wire)
/usr/local/Cellar/go/1.3/libexec/src/pkg/testing/testing.go:422 (0x4186b)
        tRunner: test.F(t)
/usr/local/Cellar/go/1.3/libexec/src/pkg/runtime/proc.c:1445 (0x14b70)
        goexit: runtime·goexit(void)
--- FAIL: TestEmbeddedObject (0.00 seconds)
        embedded_test.go:68: Wire format is:
"apiVersion":"v1test","kind":"EmbeddedTest","name":"inner","creationTimestamp":null,"id":"inner","object":null,"emptyObject":null},"emptyObject":null}
        embedded_test.go:72: Unexpected decode error No type '' for version 'v1test'
FAIL
exit status 1
FAIL    github.com/GoogleCloudPlatform/kubernetes/pkg/runtime   0.025s
"apiVersion":"v1test","kind":"EmbeddedTest","name":"outer","creationTimestamp":null,"id":"outer","object":{}
                {}
        (*Converter).convert: if err := c.convert(sf, df, scope); err != nil {}
        (*Converter).convert: if err := c.convert(sf, df, scope); err != nil {}
"apiVersion":"v1test","kind":"EmbeddedTest","name":"outer","creationTimestamp":null,"id":"outer","object":{}
HELLO1: {}

The main test failure extracted from above is:

        embedded_test.go:68: Wire format is:
"apiVersion":"v1test","kind":"EmbeddedTest","name":"inner","creationTimestamp":null,"id":"inner","object":null,"emptyObject":null},"emptyObject":null}
        embedded_test.go:72: Unexpected decode error No type '' for version 'v1test'

There's something calling rawExtensionToEmbeddedObject but I can't figure out what is calling it and why the RawExtension object that is passed in is not valid. These lines are especially cryptic:

/Users/sam/code/go/src/github.com/GoogleCloudPlatform/kubernetes/pkg/runtime/scheme.go:151 (0xf6bf0)
        rawExtensionToEmbeddedObject).fm: s.rawExtensionToEmbeddedObject,

How is rawExtensionToEmbeddedObject).fm context for a call? Where is that call coming from?

@nictuku I think you tried this upgrade before in #1481, but ended up aborting? Was there a specific reason for that?

Anyways sorry this is a bit scatterbrained but I've been staring at this code for a few hours and still not sure of what's going on - any help would be appreciated. Thanks!

@ghodss

This comment has been minimized.

Show comment
Hide comment
@ghodss
Member

ghodss commented Nov 20, 2014

@smarterclayton

This comment has been minimized.

Show comment
Hide comment
@smarterclayton

smarterclayton Nov 20, 2014

Contributor

Hrm, that error looks like it's missing the first character. Is YAML eating the opening { when it calls UnmarshalYAML on RawExtension?

Contributor

smarterclayton commented Nov 20, 2014

Hrm, that error looks like it's missing the first character. Is YAML eating the opening { when it calls UnmarshalYAML on RawExtension?

@ghodss

This comment has been minimized.

Show comment
Hide comment
@ghodss

ghodss Nov 21, 2014

Member

I think something weird happened when I copy/pasted the above log... here is the correct output:

=== RUN TestEmbeddedObject
Trying to convert '[runtime_test.EmbeddedTest runtime_test.EmbeddedTestExternal]' to '%!v(MISSING)'
Calling custom conversion of '[runtime.EmbeddedObject runtime.RawExtension]' to '%!v(MISSING)'
Trying to convert '[runtime_test.EmbeddedTest runtime_test.EmbeddedTestExternal]' to '%!v(MISSING)'
Calling custom conversion of '[runtime.EmbeddedObject runtime.RawExtension]' to '%!v(MISSING)'
Calling custom conversion of '[runtime.EmbeddedObject runtime.RawExtension]' to '%!v(MISSING)'
Trying to convert '[runtime.emptyPlugin runtime_test.EmbeddedTestExternal]' to '%!v(MISSING)'
Calling custom conversion of '[runtime.EmbeddedObject runtime.RawExtension]' to '%!v(MISSING)'
HELLO1: {"apiVersion":"v1test","kind":"EmbeddedTest","name":"outer","creationTimestamp":null,"id":"outer","object":{"apiVersion":"v1test","kind":"EmbeddedTest","name":"inner","creationTimestamp":null,"id":"inner","object":null,"emptyObject":null},"emptyObject":null}HELLO2: EmbeddedTestSTART-UNMARSHAL
Trying to convert '[runtime_test.EmbeddedTestExternal runtime_test.EmbeddedTest]' to '%!v(MISSING)'
Calling custom conversion of '[runtime.RawExtension runtime.EmbeddedObject]' to '%!v(MISSING)'
RAWEXTENSIONTEXT: apiVersion: v1test
creationTimestamp: null
emptyObject: null
id: inner
kind: EmbeddedTest
name: inner
object: null
HELLO1: apiVersion: v1test
creationTimestamp: null
emptyObject: null
id: inner
kind: EmbeddedTest
name: inner
object: null
HELLO2: EmbeddedTestHELLO1: apiVersion: v1test
creationTimestamp: null
emptyObject: null
id: inner
kind: EmbeddedTest
name: inner
object: null
HELLO2: EmbeddedTestTrying to convert '[runtime_test.EmbeddedTestExternal runtime_test.EmbeddedTest]' to '%!v(MISSING)'
Calling custom conversion of '[runtime.RawExtension runtime.EmbeddedObject]' to '%!v(MISSING)'
RAWEXTENSIONTEXT: HELLO1: HELLO2: /Users/sam/code/go/src/github.com/GoogleCloudPlatform/kubernetes/pkg/conversion/scheme.go:152 (0x107ac8)
    (*Scheme).NewObject: debug.PrintStack()
/Users/sam/code/go/src/github.com/GoogleCloudPlatform/kubernetes/pkg/runtime/scheme.go:193 (0xf61f0)
    (*Scheme).New: obj, err := s.raw.NewObject(versionName, typeName)
/Users/sam/code/go/src/github.com/GoogleCloudPlatform/kubernetes/pkg/runtime/scheme.go:112 (0xf5791)
    (*Scheme).rawExtensionToEmbeddedObject: inObj, err := scheme.New(inVersion, kind)
/Users/sam/code/go/src/github.com/GoogleCloudPlatform/kubernetes/pkg/runtime/scheme.go:151 (0xf6bf0)
    rawExtensionToEmbeddedObject).fm: s.rawExtensionToEmbeddedObject,
/usr/local/Cellar/go/1.3/libexec/src/pkg/runtime/asm_amd64.s:362 (0x2c2d2)
    call64: CALLFN(call64, 64)
/usr/local/Cellar/go/1.3/libexec/src/pkg/reflect/value.go:563 (0xe07c0)
    Value.call: call(fn, args, uint32(frametype.size), uint32(retOffset))
/usr/local/Cellar/go/1.3/libexec/src/pkg/reflect/value.go:411 (0xdf467)
    Value.Call: return v.call("Call", in)
/Users/sam/code/go/src/github.com/GoogleCloudPlatform/kubernetes/pkg/conversion/converter.go:253 (0x101d63)
    (*Converter).convert: ret := fv.Call(args)[0].Interface()
/Users/sam/code/go/src/github.com/GoogleCloudPlatform/kubernetes/pkg/conversion/converter.go:314 (0x103a7f)
    (*Converter).convert: if err := c.convert(sf, df, scope); err != nil {
/Users/sam/code/go/src/github.com/GoogleCloudPlatform/kubernetes/pkg/conversion/converter.go:241 (0x1017db)
    (*Converter).Convert: return c.convert(sv, dv, s)
/Users/sam/code/go/src/github.com/GoogleCloudPlatform/kubernetes/pkg/conversion/scheme.go:213 (0x108095)
    (*Scheme).Convert: return s.converter.Convert(in, out, AllowDifferentFieldTypeNames, s.generateConvertMeta(inVersion, outVersion))
/Users/sam/code/go/src/github.com/GoogleCloudPlatform/kubernetes/pkg/runtime/scheme.go:225 (0xf63a9)
    (*Scheme).Convert: return s.raw.Convert(in, out)
/Users/sam/code/go/src/github.com/GoogleCloudPlatform/kubernetes/pkg/runtime/scheme.go:127 (0xf5980)
    (*Scheme).rawExtensionToEmbeddedObject: err = scheme.Convert(inObj, outObj)
/Users/sam/code/go/src/github.com/GoogleCloudPlatform/kubernetes/pkg/runtime/scheme.go:151 (0xf6bf0)
    rawExtensionToEmbeddedObject).fm: s.rawExtensionToEmbeddedObject,
/usr/local/Cellar/go/1.3/libexec/src/pkg/runtime/asm_amd64.s:362 (0x2c2d2)
    call64: CALLFN(call64, 64)
/usr/local/Cellar/go/1.3/libexec/src/pkg/reflect/value.go:563 (0xe07c0)
    Value.call: call(fn, args, uint32(frametype.size), uint32(retOffset))
/usr/local/Cellar/go/1.3/libexec/src/pkg/reflect/value.go:411 (0xdf467)
    Value.Call: return v.call("Call", in)
/Users/sam/code/go/src/github.com/GoogleCloudPlatform/kubernetes/pkg/conversion/converter.go:253 (0x101d63)
    (*Converter).convert: ret := fv.Call(args)[0].Interface()
/Users/sam/code/go/src/github.com/GoogleCloudPlatform/kubernetes/pkg/conversion/converter.go:314 (0x103a7f)
    (*Converter).convert: if err := c.convert(sf, df, scope); err != nil {
/Users/sam/code/go/src/github.com/GoogleCloudPlatform/kubernetes/pkg/conversion/converter.go:241 (0x1017db)
    (*Converter).Convert: return c.convert(sv, dv, s)
/Users/sam/code/go/src/github.com/GoogleCloudPlatform/kubernetes/pkg/conversion/decode.go:66 (0x1048ad)
    (*Scheme).Decode: err = s.converter.Convert(obj, objOut, 0, s.generateConvertMeta(version, s.InternalVersion))
/Users/sam/code/go/src/github.com/GoogleCloudPlatform/kubernetes/pkg/runtime/scheme.go:282 (0xf67a0)
    (*Scheme).Decode: obj, err := s.raw.Decode(data)
/Users/sam/code/go/src/github.com/GoogleCloudPlatform/kubernetes/pkg/runtime/embedded_test.go:70 (0x46f23)
    TestEmbeddedObject: decoded, err := s.Decode(wire)
/usr/local/Cellar/go/1.3/libexec/src/pkg/testing/testing.go:422 (0x4186b)
    tRunner: test.F(t)
/usr/local/Cellar/go/1.3/libexec/src/pkg/runtime/proc.c:1445 (0x14b70)
    goexit: runtime·goexit(void)
--- FAIL: TestEmbeddedObject (0.00 seconds)
    embedded_test.go:68: Wire format is:
        {"apiVersion":"v1test","kind":"EmbeddedTest","name":"outer","creationTimestamp":null,"id":"outer","object":{"apiVersion":"v1test","kind":"EmbeddedTest","name":"inner","creationTimestamp":null,"id":"inner","object":null,"emptyObject":null},"emptyObject":null}
    embedded_test.go:72: Unexpected decode error No type '' for version 'v1test'
FAIL
exit status 1
FAIL    github.com/GoogleCloudPlatform/kubernetes/pkg/runtime   0.024s
Member

ghodss commented Nov 21, 2014

I think something weird happened when I copy/pasted the above log... here is the correct output:

=== RUN TestEmbeddedObject
Trying to convert '[runtime_test.EmbeddedTest runtime_test.EmbeddedTestExternal]' to '%!v(MISSING)'
Calling custom conversion of '[runtime.EmbeddedObject runtime.RawExtension]' to '%!v(MISSING)'
Trying to convert '[runtime_test.EmbeddedTest runtime_test.EmbeddedTestExternal]' to '%!v(MISSING)'
Calling custom conversion of '[runtime.EmbeddedObject runtime.RawExtension]' to '%!v(MISSING)'
Calling custom conversion of '[runtime.EmbeddedObject runtime.RawExtension]' to '%!v(MISSING)'
Trying to convert '[runtime.emptyPlugin runtime_test.EmbeddedTestExternal]' to '%!v(MISSING)'
Calling custom conversion of '[runtime.EmbeddedObject runtime.RawExtension]' to '%!v(MISSING)'
HELLO1: {"apiVersion":"v1test","kind":"EmbeddedTest","name":"outer","creationTimestamp":null,"id":"outer","object":{"apiVersion":"v1test","kind":"EmbeddedTest","name":"inner","creationTimestamp":null,"id":"inner","object":null,"emptyObject":null},"emptyObject":null}HELLO2: EmbeddedTestSTART-UNMARSHAL
Trying to convert '[runtime_test.EmbeddedTestExternal runtime_test.EmbeddedTest]' to '%!v(MISSING)'
Calling custom conversion of '[runtime.RawExtension runtime.EmbeddedObject]' to '%!v(MISSING)'
RAWEXTENSIONTEXT: apiVersion: v1test
creationTimestamp: null
emptyObject: null
id: inner
kind: EmbeddedTest
name: inner
object: null
HELLO1: apiVersion: v1test
creationTimestamp: null
emptyObject: null
id: inner
kind: EmbeddedTest
name: inner
object: null
HELLO2: EmbeddedTestHELLO1: apiVersion: v1test
creationTimestamp: null
emptyObject: null
id: inner
kind: EmbeddedTest
name: inner
object: null
HELLO2: EmbeddedTestTrying to convert '[runtime_test.EmbeddedTestExternal runtime_test.EmbeddedTest]' to '%!v(MISSING)'
Calling custom conversion of '[runtime.RawExtension runtime.EmbeddedObject]' to '%!v(MISSING)'
RAWEXTENSIONTEXT: HELLO1: HELLO2: /Users/sam/code/go/src/github.com/GoogleCloudPlatform/kubernetes/pkg/conversion/scheme.go:152 (0x107ac8)
    (*Scheme).NewObject: debug.PrintStack()
/Users/sam/code/go/src/github.com/GoogleCloudPlatform/kubernetes/pkg/runtime/scheme.go:193 (0xf61f0)
    (*Scheme).New: obj, err := s.raw.NewObject(versionName, typeName)
/Users/sam/code/go/src/github.com/GoogleCloudPlatform/kubernetes/pkg/runtime/scheme.go:112 (0xf5791)
    (*Scheme).rawExtensionToEmbeddedObject: inObj, err := scheme.New(inVersion, kind)
/Users/sam/code/go/src/github.com/GoogleCloudPlatform/kubernetes/pkg/runtime/scheme.go:151 (0xf6bf0)
    rawExtensionToEmbeddedObject).fm: s.rawExtensionToEmbeddedObject,
/usr/local/Cellar/go/1.3/libexec/src/pkg/runtime/asm_amd64.s:362 (0x2c2d2)
    call64: CALLFN(call64, 64)
/usr/local/Cellar/go/1.3/libexec/src/pkg/reflect/value.go:563 (0xe07c0)
    Value.call: call(fn, args, uint32(frametype.size), uint32(retOffset))
/usr/local/Cellar/go/1.3/libexec/src/pkg/reflect/value.go:411 (0xdf467)
    Value.Call: return v.call("Call", in)
/Users/sam/code/go/src/github.com/GoogleCloudPlatform/kubernetes/pkg/conversion/converter.go:253 (0x101d63)
    (*Converter).convert: ret := fv.Call(args)[0].Interface()
/Users/sam/code/go/src/github.com/GoogleCloudPlatform/kubernetes/pkg/conversion/converter.go:314 (0x103a7f)
    (*Converter).convert: if err := c.convert(sf, df, scope); err != nil {
/Users/sam/code/go/src/github.com/GoogleCloudPlatform/kubernetes/pkg/conversion/converter.go:241 (0x1017db)
    (*Converter).Convert: return c.convert(sv, dv, s)
/Users/sam/code/go/src/github.com/GoogleCloudPlatform/kubernetes/pkg/conversion/scheme.go:213 (0x108095)
    (*Scheme).Convert: return s.converter.Convert(in, out, AllowDifferentFieldTypeNames, s.generateConvertMeta(inVersion, outVersion))
/Users/sam/code/go/src/github.com/GoogleCloudPlatform/kubernetes/pkg/runtime/scheme.go:225 (0xf63a9)
    (*Scheme).Convert: return s.raw.Convert(in, out)
/Users/sam/code/go/src/github.com/GoogleCloudPlatform/kubernetes/pkg/runtime/scheme.go:127 (0xf5980)
    (*Scheme).rawExtensionToEmbeddedObject: err = scheme.Convert(inObj, outObj)
/Users/sam/code/go/src/github.com/GoogleCloudPlatform/kubernetes/pkg/runtime/scheme.go:151 (0xf6bf0)
    rawExtensionToEmbeddedObject).fm: s.rawExtensionToEmbeddedObject,
/usr/local/Cellar/go/1.3/libexec/src/pkg/runtime/asm_amd64.s:362 (0x2c2d2)
    call64: CALLFN(call64, 64)
/usr/local/Cellar/go/1.3/libexec/src/pkg/reflect/value.go:563 (0xe07c0)
    Value.call: call(fn, args, uint32(frametype.size), uint32(retOffset))
/usr/local/Cellar/go/1.3/libexec/src/pkg/reflect/value.go:411 (0xdf467)
    Value.Call: return v.call("Call", in)
/Users/sam/code/go/src/github.com/GoogleCloudPlatform/kubernetes/pkg/conversion/converter.go:253 (0x101d63)
    (*Converter).convert: ret := fv.Call(args)[0].Interface()
/Users/sam/code/go/src/github.com/GoogleCloudPlatform/kubernetes/pkg/conversion/converter.go:314 (0x103a7f)
    (*Converter).convert: if err := c.convert(sf, df, scope); err != nil {
/Users/sam/code/go/src/github.com/GoogleCloudPlatform/kubernetes/pkg/conversion/converter.go:241 (0x1017db)
    (*Converter).Convert: return c.convert(sv, dv, s)
/Users/sam/code/go/src/github.com/GoogleCloudPlatform/kubernetes/pkg/conversion/decode.go:66 (0x1048ad)
    (*Scheme).Decode: err = s.converter.Convert(obj, objOut, 0, s.generateConvertMeta(version, s.InternalVersion))
/Users/sam/code/go/src/github.com/GoogleCloudPlatform/kubernetes/pkg/runtime/scheme.go:282 (0xf67a0)
    (*Scheme).Decode: obj, err := s.raw.Decode(data)
/Users/sam/code/go/src/github.com/GoogleCloudPlatform/kubernetes/pkg/runtime/embedded_test.go:70 (0x46f23)
    TestEmbeddedObject: decoded, err := s.Decode(wire)
/usr/local/Cellar/go/1.3/libexec/src/pkg/testing/testing.go:422 (0x4186b)
    tRunner: test.F(t)
/usr/local/Cellar/go/1.3/libexec/src/pkg/runtime/proc.c:1445 (0x14b70)
    goexit: runtime·goexit(void)
--- FAIL: TestEmbeddedObject (0.00 seconds)
    embedded_test.go:68: Wire format is:
        {"apiVersion":"v1test","kind":"EmbeddedTest","name":"outer","creationTimestamp":null,"id":"outer","object":{"apiVersion":"v1test","kind":"EmbeddedTest","name":"inner","creationTimestamp":null,"id":"inner","object":null,"emptyObject":null},"emptyObject":null}
    embedded_test.go:72: Unexpected decode error No type '' for version 'v1test'
FAIL
exit status 1
FAIL    github.com/GoogleCloudPlatform/kubernetes/pkg/runtime   0.024s
@ghodss

This comment has been minimized.

Show comment
Hide comment
@ghodss

ghodss Nov 21, 2014

Member

Okay I figured out what's going on. In the test TestEmbeddedObject, when you do Decode on the wire format:

{
  "apiVersion": "v1test",
  "kind": "EmbeddedTest",
  "name": "outer",
  "creationTimestamp": null,
  "id": "outer",
  "object": {
    "apiVersion": "v1test",
    "kind": "EmbeddedTest",
    "name": "inner",
    "creationTimestamp": null,
    "id": "inner",
    "object": null,
    "emptyObject": null
  },
  "emptyObject": null
}

...it calls yaml.Unmarshal before sending that returned object through the conversion process. The problem is in that yaml.Unmarshal call, located at https://github.com/GoogleCloudPlatform/kubernetes/blob/master/pkg/conversion/decode.go#L49 . Look at the resulting object with yaml v1:

&{{v1test EmbeddedTest  outer  0001-01-01 00:00:00 +0000 UTC  } outer {apiVersion: v1test
creationTimestamp: null
emptyObject: null
id: inner
kind: EmbeddedTest
name: inner
object: null
} {null}}

And look at the resulting object with yaml v2:

&{{v1test EmbeddedTest  outer  0001-01-01 00:00:00 +0000 UTC  } outer {apiVersion: v1test
creationTimestamp: null
emptyObject: null
id: inner
kind: EmbeddedTest
name: inner
object: null
} {}}

Notice the difference? The last field, "EmptyObject", is rendered as RawJSON="null" in the yaml v1 but RawJSON="" in yaml v2. What seems to be happening is that the yaml library now no longer calls UnmarshalYAML on pointers that point to nil - it just uses nil. So even though I have code in my new UnmarshalYAML function that creates an object with the YAML string "null" in the RawJSON field (as it does in the v1 SetYAML), it's never even getting called. The result is that our RawJSON magic is messed up by encountering "" as the RawJSON string instead of "null".

I have yet to figure out if we can workaround this in our code or if it's a blocker that we need fixed in go-yaml. @lavalamp @smarterclayton, would appreciate help in understanding if the conversion code can be changed so that having a blank RawJSON field could be interpreted as "null" instead of causing an error.

Member

ghodss commented Nov 21, 2014

Okay I figured out what's going on. In the test TestEmbeddedObject, when you do Decode on the wire format:

{
  "apiVersion": "v1test",
  "kind": "EmbeddedTest",
  "name": "outer",
  "creationTimestamp": null,
  "id": "outer",
  "object": {
    "apiVersion": "v1test",
    "kind": "EmbeddedTest",
    "name": "inner",
    "creationTimestamp": null,
    "id": "inner",
    "object": null,
    "emptyObject": null
  },
  "emptyObject": null
}

...it calls yaml.Unmarshal before sending that returned object through the conversion process. The problem is in that yaml.Unmarshal call, located at https://github.com/GoogleCloudPlatform/kubernetes/blob/master/pkg/conversion/decode.go#L49 . Look at the resulting object with yaml v1:

&{{v1test EmbeddedTest  outer  0001-01-01 00:00:00 +0000 UTC  } outer {apiVersion: v1test
creationTimestamp: null
emptyObject: null
id: inner
kind: EmbeddedTest
name: inner
object: null
} {null}}

And look at the resulting object with yaml v2:

&{{v1test EmbeddedTest  outer  0001-01-01 00:00:00 +0000 UTC  } outer {apiVersion: v1test
creationTimestamp: null
emptyObject: null
id: inner
kind: EmbeddedTest
name: inner
object: null
} {}}

Notice the difference? The last field, "EmptyObject", is rendered as RawJSON="null" in the yaml v1 but RawJSON="" in yaml v2. What seems to be happening is that the yaml library now no longer calls UnmarshalYAML on pointers that point to nil - it just uses nil. So even though I have code in my new UnmarshalYAML function that creates an object with the YAML string "null" in the RawJSON field (as it does in the v1 SetYAML), it's never even getting called. The result is that our RawJSON magic is messed up by encountering "" as the RawJSON string instead of "null".

I have yet to figure out if we can workaround this in our code or if it's a blocker that we need fixed in go-yaml. @lavalamp @smarterclayton, would appreciate help in understanding if the conversion code can be changed so that having a blank RawJSON field could be interpreted as "null" instead of causing an error.

@ghodss

This comment has been minimized.

Show comment
Hide comment
@ghodss

ghodss Nov 21, 2014

Member

So, I figured out the above issue. I just changed

       if len(in.RawJSON) == 4 && string(in.RawJSON) == "null" {

to

       if len(in.RawJSON) == 0 {

and that seems to have solved it.

I now have a new issue with this test:

go test -run TestSpecificKind  -v github.com/GoogleCloudPlatform/kubernetes/pkg/api

About 1 in every 3-4 times I run that command, this DeepEqual call https://github.com/ghodss/kubernetes/blob/master/pkg/api/serialization_test.go#L164 returns false for seemingly exactly similar structs. See an example diff of two structs that DeepEqual returned false for at http://i.imgur.com/1mOy3M8.png. I can't seem to figure out why they are returning false, though.

Member

ghodss commented Nov 21, 2014

So, I figured out the above issue. I just changed

       if len(in.RawJSON) == 4 && string(in.RawJSON) == "null" {

to

       if len(in.RawJSON) == 0 {

and that seems to have solved it.

I now have a new issue with this test:

go test -run TestSpecificKind  -v github.com/GoogleCloudPlatform/kubernetes/pkg/api

About 1 in every 3-4 times I run that command, this DeepEqual call https://github.com/ghodss/kubernetes/blob/master/pkg/api/serialization_test.go#L164 returns false for seemingly exactly similar structs. See an example diff of two structs that DeepEqual returned false for at http://i.imgur.com/1mOy3M8.png. I can't seem to figure out why they are returning false, though.

@lavalamp

This comment has been minimized.

Show comment
Hide comment
@lavalamp

lavalamp Nov 22, 2014

Member

DeepEqual makes a distinction between nil slices and 0 length slices, IIRC. There's an ObjectDiff function in pkg/util that might be able to help you figure it out-- it has both a json and a go %#v section, give one a try if the other's not working.

Member

lavalamp commented Nov 22, 2014

DeepEqual makes a distinction between nil slices and 0 length slices, IIRC. There's an ObjectDiff function in pkg/util that might be able to help you figure it out-- it has both a json and a go %#v section, give one a try if the other's not working.

@ghodss

This comment has been minimized.

Show comment
Hide comment
@ghodss

ghodss Nov 22, 2014

Member

go-spew seems to give different output for nil slices and 0 length slices - see http://play.golang.org/p/Uc__IeXO99 . So the two spew dumps should be different if the two objects were different. And ObjectDiff returns no differences between the two objects.

Member

ghodss commented Nov 22, 2014

go-spew seems to give different output for nil slices and 0 length slices - see http://play.golang.org/p/Uc__IeXO99 . So the two spew dumps should be different if the two objects were different. And ObjectDiff returns no differences between the two objects.

@lavalamp

This comment has been minimized.

Show comment
Hide comment
@lavalamp

lavalamp Nov 22, 2014

Member

Edit your ObjectDiff function-- if you go look at it there's two implementations. I bet if you switch away from the json one it will show you the diffrence.

I tend to think it's a bug that DeepEqual makes this distinction.

Member

lavalamp commented Nov 22, 2014

Edit your ObjectDiff function-- if you go look at it there's two implementations. I bet if you switch away from the json one it will show you the diffrence.

I tend to think it's a bug that DeepEqual makes this distinction.

@ghodss

This comment has been minimized.

Show comment
Hide comment
@ghodss

ghodss Nov 22, 2014

Member

I switched ObjectDiff to ObjectGoPrintDiff, but then this just shows me that there's a diff in the pointer addresses, which is expected and not the issue because DeepEqual follows pointer addresses to compare struct fields/values.

Member

ghodss commented Nov 22, 2014

I switched ObjectDiff to ObjectGoPrintDiff, but then this just shows me that there's a diff in the pointer addresses, which is expected and not the issue because DeepEqual follows pointer addresses to compare struct fields/values.

@ghodss

This comment has been minimized.

Show comment
Hide comment
@ghodss

ghodss Nov 22, 2014

Member

I think this has to do with time.Time not being equal in some cases. Haven't been able to repro this on its own but here are two dumped pieces of PodList that are returning reflect.DeepEqual as false:

KEYS EQUAL 嵏U欨P詐菳迋ǽŁ
(*api.ContainerStateRunning)(0xc2080ef020)({
 StartedAt: (time.Time) 0001-01-01 00:00:00 +0000 UTC
})
(*api.ContainerStateRunning)(0xc208169920)({
 StartedAt: (time.Time) 0001-01-01 00:00:00 +0000 UTC
})
STATUS INFO CSTATUS running NOT EQUAL
Member

ghodss commented Nov 22, 2014

I think this has to do with time.Time not being equal in some cases. Haven't been able to repro this on its own but here are two dumped pieces of PodList that are returning reflect.DeepEqual as false:

KEYS EQUAL 嵏U欨P詐菳迋ǽŁ
(*api.ContainerStateRunning)(0xc2080ef020)({
 StartedAt: (time.Time) 0001-01-01 00:00:00 +0000 UTC
})
(*api.ContainerStateRunning)(0xc208169920)({
 StartedAt: (time.Time) 0001-01-01 00:00:00 +0000 UTC
})
STATUS INFO CSTATUS running NOT EQUAL
@ghodss

This comment has been minimized.

Show comment
Hide comment
@ghodss

ghodss Nov 22, 2014

Member

Just pushed the changes in that enables that logging if you want to take a look.

Member

ghodss commented Nov 22, 2014

Just pushed the changes in that enables that logging if you want to take a look.

@ghodss

This comment has been minimized.

Show comment
Hide comment
@ghodss

ghodss Nov 22, 2014

Member

Okay yeah the time.Time.loc field is different for the two:

Running.StartedAt:
time.Time{sec:0, nsec:0x0, loc:(*time.Location)(nil)} // source object
time.Time{sec:0, nsec:0x0, loc:(*time.Location)(0x624ee0)} // encoded/decoded object
Member

ghodss commented Nov 22, 2014

Okay yeah the time.Time.loc field is different for the two:

Running.StartedAt:
time.Time{sec:0, nsec:0x0, loc:(*time.Location)(nil)} // source object
time.Time{sec:0, nsec:0x0, loc:(*time.Location)(0x624ee0)} // encoded/decoded object
@lavalamp

This comment has been minimized.

Show comment
Hide comment
@lavalamp

lavalamp Nov 22, 2014

Member

argh, that's super irritating. Maybe have to fool around with util.Time's encode/decode functions?

Member

lavalamp commented Nov 22, 2014

argh, that's super irritating. Maybe have to fool around with util.Time's encode/decode functions?

@ghodss

This comment has been minimized.

Show comment
Hide comment
@ghodss

ghodss Nov 22, 2014

Member

v1beta1 (the test I'm working on) encodes these values as time.Time, not util.Time.

I think I've found a root cause. See http://play.golang.org/p/6MKY5a68I6. When you create a blank time.Time{}, loc is nil. But if you unmarshal a time from JSON, time.Parse sets loc to an object representing UTC. I still don't know why yaml v1 results in a nil loc and yaml v2 results in a loc with an object - I can't even reproduce that behavior in isolation, but only in the k8s encode/decode context. Maybe yaml v2 now calls time.Parse but the only references I can find to the time package in the yaml package are references to Duration, not Parse or Time. In any case the bottom line is that the deepequal check for encode/decode does not work anymore.

I'm not sure how to resolve this. Probably doesn't make sense to hack the yaml library to set loc to nil, esp since if we ever switch to json library (e.g. for serverside things that don't need yaml anymore) this will break again. Maybe there's a way for encode/decode tests to always start with an object with loc filled? I can take a look to see if that's possible. Any other thoughts/suggestions/solutions would be welcome.

Member

ghodss commented Nov 22, 2014

v1beta1 (the test I'm working on) encodes these values as time.Time, not util.Time.

I think I've found a root cause. See http://play.golang.org/p/6MKY5a68I6. When you create a blank time.Time{}, loc is nil. But if you unmarshal a time from JSON, time.Parse sets loc to an object representing UTC. I still don't know why yaml v1 results in a nil loc and yaml v2 results in a loc with an object - I can't even reproduce that behavior in isolation, but only in the k8s encode/decode context. Maybe yaml v2 now calls time.Parse but the only references I can find to the time package in the yaml package are references to Duration, not Parse or Time. In any case the bottom line is that the deepequal check for encode/decode does not work anymore.

I'm not sure how to resolve this. Probably doesn't make sense to hack the yaml library to set loc to nil, esp since if we ever switch to json library (e.g. for serverside things that don't need yaml anymore) this will break again. Maybe there's a way for encode/decode tests to always start with an object with loc filled? I can take a look to see if that's possible. Any other thoughts/suggestions/solutions would be welcome.

@ghodss

This comment has been minimized.

Show comment
Hide comment
@ghodss

ghodss Nov 25, 2014

Member

I have pushed the latest here for posterity but this is still completely broken. See #2600 for recommended path forward.

Member

ghodss commented Nov 25, 2014

I have pushed the latest here for posterity but this is still completely broken. See #2600 for recommended path forward.

@kubernetes-bot

This comment has been minimized.

Show comment
Hide comment
@kubernetes-bot

kubernetes-bot Nov 26, 2014

Can one of the admins verify this patch?

kubernetes-bot commented Nov 26, 2014

Can one of the admins verify this patch?

@ghodss

This comment has been minimized.

Show comment
Hide comment
@ghodss

ghodss Dec 1, 2014

Member

Aborted in favor of the approach outlined in #2600 and its PR #2676.

Member

ghodss commented Dec 1, 2014

Aborted in favor of the approach outlined in #2600 and its PR #2676.

@ghodss ghodss closed this Dec 1, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment