hashicorp / nomad

Nomad is an easy-to-use, flexible, and performant workload orchestrator that can deploy a mix of microservice, batch, containerized, and non-containerized applications. Nomad is easy to operate and scale and has native Consul and Vault integrations.
https://www.nomadproject.io/
Other
14.76k stars 1.94k forks source link

helper package seems to be missing functions from 1.3.4 onwards #15074

Closed CarbonCollins closed 1 year ago

CarbonCollins commented 1 year ago

Nomad version

1.3.4 to 1.4.2

Operating system and Environment details

Go build environment for plugin building

Issue

From 1.3.4 onwards the github.com/hashicorp/nomad/helper go package seems to be missing several helper functions including:

helper.StringToPtr helper.Int64ToPtr

among a few others and I didn't see a change log entry detailing this change? have these been accidentally removed or did I just miss a deprecation message?

This also seems to be the case in the go helper package docs here: https://pkg.go.dev/github.com/hashicorp/nomad@v1.3.3/helper <- the helpers are present https://pkg.go.dev/github.com/hashicorp/nomad@v1.3.4/helper <- the helpers are no longer present

As some extra info I found this while attempting to debug an issue raised with the USB plugin here: https://gitlab.com/CarbonCollins/nomad-usb-device-plugin/-/issues/29

Reproduction steps

Within the USB plugin:

  1. Update github.com/hashicorp/nomad to v1.4.2 in the USB plugin dependencies
  2. Run make build
  3. Get a failed build with several undefined: helper.StringToPtr errors

Expected Result

When building the USB plugin using make build with the latest version of nomad I should get a working build with no other code changes.

Actual Result

When building the USB plugin using make build with the latest version of nomad as a dependnency I get:

# gitlab.com/CarbonCollins/nomad-usb-device-plugin/internal/plugin
internal/plugin/fingerprint.go:217:19: undefined: helper.StringToPtr
internal/plugin/fingerprint.go:220:19: undefined: helper.StringToPtr
internal/plugin/fingerprint.go:223:19: undefined: helper.StringToPtr
internal/plugin/fingerprint.go:226:19: undefined: helper.StringToPtr
internal/plugin/fingerprint.go:229:19: undefined: helper.StringToPtr
internal/plugin/fingerprint.go:234:16: undefined: helper.Int64ToPtr
internal/plugin/fingerprint.go:237:16: undefined: helper.Int64ToPtr
internal/plugin/fingerprint.go:240:16: undefined: helper.Int64ToPtr
internal/plugin/fingerprint.go:243:16: undefined: helper.Int64ToPtr
internal/plugin/fingerprint.go:246:16: undefined: helper.Int64ToPtr
internal/plugin/fingerprint.go:246:16: too many errors
make: *** [GNUmakefile:12: build] Error 2

Job file (if appropriate)

n/a

Nomad Server logs (if appropriate)

n/a

Nomad Client logs (if appropriate)

n/a

jrasell commented 1 year ago

Hi @CarbonCollins; Nomad v1.3.4 updated the version of Go used for building to v1.19. Among a number of features and fixes, this gave Nomad the ability to use generics which allowed us to remove the per-type pointer functions for a generic function.

I think you can either upgrade the tooling used to build the plugin to Go 1.19 and update the function calls to use the new generic function, or create your own helper functions to handle to pointer creation for each type needed. Apologies for causing a little extra work here. While are careful of all code modification, the helper package is not intended to be a general purpose SDK outside the Nomad codebase.

CarbonCollins commented 1 year ago

Hi @jrasell Thats good to know! ill update the plugin to use the Go 1.19 toolchain as a resolution to this issue there :)

Just as a followup question, I'm mainly using the helper package as from memory the driver skeleton used it :thinking:, my question being should I fully remove any dependency on the helper package to prevent things like this happening again?

jrasell commented 1 year ago

I think the pointer function within the helper package will be very stable and therefore safe to use.

github-actions[bot] commented 1 year ago

I'm going to lock this issue because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.