Skip to content
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

Using retracted version v1.1.5 of hashicorp/go-msgpack #16808

Open
IamTheFij opened this issue Apr 5, 2023 · 2 comments · Fixed by #16810
Open

Using retracted version v1.1.5 of hashicorp/go-msgpack #16808

IamTheFij opened this issue Apr 5, 2023 · 2 comments · Fixed by #16810
Labels
stage/accepted Confirmed, and intend to work on. No timeline committment though. theme/dependencies Pull requests that update a dependency file type/bug

Comments

@IamTheFij
Copy link
Contributor

Nomad version

master

Operating system and Environment details

n/a

Issue

The current version listed in go.mod is a retracted version, v1.1.5. Apparently version v2.0.0 is identical and should be used instead. This prevents updating to new versions of github.com/hashicorp/nomad in other projects (such as github.com/hashicorp/terraform-provider-nomad

Reproduction steps

Check go.mod file for github.com/hashicorp/go-msgpack v1.1.5

Expected Result

Find github.com/hashicorp/go-msgpack v2.0.0 in go.mod

Actual Result

Find github.com/hashicorp/go-msgpack v1.1.5 in go.mod

Job file (if appropriate)

Nomad Server logs (if appropriate)

Nomad Client logs (if appropriate)

IamTheFij added a commit to IamTheFij/nomad that referenced this issue Apr 5, 2023
@lgfa29 lgfa29 added theme/dependencies Pull requests that update a dependency file stage/accepted Confirmed, and intend to work on. No timeline committment though. labels Apr 17, 2023
lgfa29 added a commit that referenced this issue Apr 17, 2023
* Upgrade from hashicorp/go-msgpack v1.1.5 to v2.1.0

Fixes #16808

* Update hashicorp/net-rpc-msgpackrpc to v2 to match go-msgpack

* deps: use go-msgpack v2.0.0

go-msgpack v2.1.0 includes some code changes that we will need to
investigate furthere to assess its impact on Nomad, so keeping this
dependency on v2.0.0 for now since it's no-op.

---------

Co-authored-by: Luiz Aoqui <luiz@hashicorp.com>
lgfa29 added a commit that referenced this issue Apr 17, 2023
* Upgrade from hashicorp/go-msgpack v1.1.5 to v2.1.0

Fixes #16808

* Update hashicorp/net-rpc-msgpackrpc to v2 to match go-msgpack

* deps: use go-msgpack v2.0.0

go-msgpack v2.1.0 includes some code changes that we will need to
investigate furthere to assess its impact on Nomad, so keeping this
dependency on v2.0.0 for now since it's no-op.

---------

Co-authored-by: Luiz Aoqui <luiz@hashicorp.com>
lgfa29 added a commit that referenced this issue Apr 17, 2023
* Upgrade from hashicorp/go-msgpack v1.1.5 to v2.1.0

Fixes #16808

* Update hashicorp/net-rpc-msgpackrpc to v2 to match go-msgpack

* deps: use go-msgpack v2.0.0

go-msgpack v2.1.0 includes some code changes that we will need to
investigate furthere to assess its impact on Nomad, so keeping this
dependency on v2.0.0 for now since it's no-op.

---------

Co-authored-by: Luiz Aoqui <luiz@hashicorp.com>
lgfa29 added a commit that referenced this issue Apr 17, 2023
* Upgrade from hashicorp/go-msgpack v1.1.5 to v2.1.0

Fixes #16808

* Update hashicorp/net-rpc-msgpackrpc to v2 to match go-msgpack

* deps: use go-msgpack v2.0.0

go-msgpack v2.1.0 includes some code changes that we will need to
investigate furthere to assess its impact on Nomad, so keeping this
dependency on v2.0.0 for now since it's no-op.

---------

Co-authored-by: Luiz Aoqui <luiz@hashicorp.com>
lgfa29 added a commit that referenced this issue Apr 17, 2023
* Upgrade from hashicorp/go-msgpack v1.1.5 to v2.1.0

Fixes #16808

* Update hashicorp/net-rpc-msgpackrpc to v2 to match go-msgpack

* deps: use go-msgpack v2.0.0

go-msgpack v2.1.0 includes some code changes that we will need to
investigate furthere to assess its impact on Nomad, so keeping this
dependency on v2.0.0 for now since it's no-op.

---------

Co-authored-by: Luiz Aoqui <luiz@hashicorp.com>
lgfa29 added a commit that referenced this issue Apr 17, 2023
* Upgrade from hashicorp/go-msgpack v1.1.5 to v2.1.0

Fixes #16808

* Update hashicorp/net-rpc-msgpackrpc to v2 to match go-msgpack

* deps: use go-msgpack v2.0.0

go-msgpack v2.1.0 includes some code changes that we will need to
investigate furthere to assess its impact on Nomad, so keeping this
dependency on v2.0.0 for now since it's no-op.

---------

Co-authored-by: Ian Fijolek <ian@iamthefij.com>
Co-authored-by: Luiz Aoqui <luiz@hashicorp.com>
lgfa29 added a commit that referenced this issue Apr 17, 2023
* Upgrade from hashicorp/go-msgpack v1.1.5 to v2.1.0

Fixes #16808

* Update hashicorp/net-rpc-msgpackrpc to v2 to match go-msgpack

* deps: use go-msgpack v2.0.0

go-msgpack v2.1.0 includes some code changes that we will need to
investigate furthere to assess its impact on Nomad, so keeping this
dependency on v2.0.0 for now since it's no-op.

---------

Co-authored-by: Ian Fijolek <ian@iamthefij.com>
Co-authored-by: Luiz Aoqui <luiz@hashicorp.com>
lgfa29 added a commit that referenced this issue Apr 17, 2023
* Upgrade from hashicorp/go-msgpack v1.1.5 to v2.1.0

Fixes #16808

* Update hashicorp/net-rpc-msgpackrpc to v2 to match go-msgpack

* deps: use go-msgpack v2.0.0

go-msgpack v2.1.0 includes some code changes that we will need to
investigate furthere to assess its impact on Nomad, so keeping this
dependency on v2.0.0 for now since it's no-op.

---------

Co-authored-by: Ian Fijolek <ian@iamthefij.com>
Co-authored-by: Luiz Aoqui <luiz@hashicorp.com>
@lgfa29
Copy link
Contributor

lgfa29 commented May 1, 2023

Reopening the issue because this upgrade causes Nomad to fail (#17047)

@tgross
Copy link
Member

tgross commented Oct 13, 2023

Adding to the mystery on this one, in hashicorp/raft#575 (comment) @banks pointed out that it should be safe to upgrade so long as you're using the old time encoding. But as it turns out, the code suggests that we think already are (ref structs.go#L12468-L12485):

// msgpackHandle is a shared handle for encoding/decoding of structs
var MsgpackHandle = func() *codec.MsgpackHandle {
	h := &codec.MsgpackHandle{}
	h.RawToString = true

	// maintain binary format from time prior to upgrading latest ugorji
	h.BasicHandle.TimeNotBuiltin = true

	// Sets the default type for decoding a map into a nil interface{}.
	// This is necessary in particular because we store the driver configs as a
	// nil interface{}.
	h.MapType = reflect.TypeOf(map[string]interface{}(nil))

	// only review struct codec tags
	h.TypeInfos = codec.NewTypeInfos([]string{"codec"})

	return h
}()

That handle gets used everywhere in our RPC transport. That suggests to me that we may have misconfigured the transport for Raft specifically. I haven't hunted that down yet but that feels like a clue if anyone else on the Nomad team finds themselves with time to dig into it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stage/accepted Confirmed, and intend to work on. No timeline committment though. theme/dependencies Pull requests that update a dependency file type/bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants