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

resource(volume): fix parsing mount_flags #269

Merged
merged 2 commits into from
Jun 7, 2022

Conversation

pop
Copy link
Contributor

@pop pop commented May 24, 2022

This fixes a bug where mount_flags were being silently dropped during resoruce creation.

I'm using Nomad v1.3.1 and terraform-provider-nomad v1.4.16.
I noticed that my nomad_volume mount_options.mount_flags weren't being set even though my fs_type was being set.
When creating the same volume directly with the nomad CLI mount_flags were being set so I knew it was a problem with the provider.

When the original error is not being ignored it causes a panic like so:

Stack trace from the terraform-provider-nomad_v1.4.16+dev plugin:

panic: interface conversion: interface {} is []interface {}, not []string

goroutine 55 [running]:
github.com/hashicorp/terraform-provider-nomad/nomad.resourceVolumeCreate(0xc00057f030, 0x1cbd860, 0xc00068c0c0, 0x24, 0x25a8540)
        /Users/pop/Projects/src/github.com/hashicorp/terraform-provider-nomad/nomad/resource_volume.go:353 +0xf77
github.com/hashicorp/terraform-plugin-sdk/helper/schema.(*Resource).Apply(0xc0003ec000, 0xc000612c30, 0xc00077d400, 0x1cbd860, 0xc00068c0c0, 0x1c10501, 0xc00084f8d0, 0xc00081e3f0)
        /Users/pop/go/1.16.2/pkg/mod/github.com/hashicorp/terraform-plugin-sdk@v1.17.2/helper/schema/resource.go:326 +0x273
github.com/hashicorp/terraform-plugin-sdk/helper/schema.(*Provider).Apply(0xc000316b80, 0xc00084fa38, 0xc000612c30, 0xc00077d400, 0xc000473288, 0xc000033170, 0x1c12820)
        /Users/pop/go/1.16.2/pkg/mod/github.com/hashicorp/terraform-plugin-sdk@v1.17.2/helper/schema/provider.go:294 +0x99
github.com/hashicorp/terraform-plugin-sdk/internal/helper/plugin.(*GRPCProviderServer).ApplyResourceChange(0xc000308890, 0x1f68090, 0xc0005d41b0, 0xc00057e1c0, 0xc000308890, 0xc0005d41b0, 0xc000860ba0)
        /Users/pop/go/1.16.2/pkg/mod/github.com/hashicorp/terraform-plugin-sdk@v1.17.2/internal/helper/plugin/grpc_provider.go:895 +0x8a5
github.com/hashicorp/terraform-plugin-sdk/internal/tfplugin5._Provider_ApplyResourceChange_Handler(0x1d1caa0, 0xc000308890, 0x1f68090, 0xc0005d41b0, 0xc0000ba6c0, 0x0, 0x1f68090, 0xc0005d41b0, 0xc0000f9800, 0x733)
        /Users/pop/go/1.16.2/pkg/mod/github.com/hashicorp/terraform-plugin-sdk@v1.17.2/internal/tfplugin5/tfplugin5.pb.go:3305 +0x214
google.golang.org/grpc.(*Server).processUnaryRPC(0xc000338000, 0x1f729b8, 0xc000626300, 0xc0000cc100, 0xc00032a510, 0x25670c0, 0x0, 0x0, 0x0)
        /Users/pop/go/1.16.2/pkg/mod/google.golang.org/grpc@v1.32.0/server.go:1194 +0x52b
google.golang.org/grpc.(*Server).handleStream(0xc000338000, 0x1f729b8, 0xc000626300, 0xc0000cc100, 0x0)
        /Users/pop/go/1.16.2/pkg/mod/google.golang.org/grpc@v1.32.0/server.go:1517 +0xd0c
google.golang.org/grpc.(*Server).serveStreams.func1.2(0xc0000c6240, 0xc000338000, 0x1f729b8, 0xc000626300, 0xc0000cc100)
        /Users/pop/go/1.16.2/pkg/mod/google.golang.org/grpc@v1.32.0/server.go:859 +0xab
created by google.golang.org/grpc.(*Server).serveStreams.func1
        /Users/pop/go/1.16.2/pkg/mod/google.golang.org/grpc@v1.32.0/server.go:857 +0x1fd

I have tested this fix for me and it resolve the issue, but I didn't have time to write automated tests to catch future regressions.

Code feedback is more than welcome.

This fixes a bug where mount_flags were being silently dropped during resoruce creation.
@pop
Copy link
Contributor Author

pop commented May 24, 2022

I guess this refs #266 and #260 but my code looks a little different and I didn't include a CHANGELOG item. Happy to add those if y'all want me to.

Note: this is not a dupe of those. Those fix the external_volume resource not the volume resource.

Copy link
Contributor

@lgfa29 lgfa29 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix @pop!

I tried adding tests like I did for #266 but it didn't quite work as I couldn't figure out a way to get the external_id. I think this would be easier with a real CSI cloud volume, but that would take a while to setup.

I tested it manually and does fix the problem 👍

@lgfa29 lgfa29 merged commit fcf2fbc into hashicorp:main Jun 7, 2022
@pop pop deleted the fix-parsing-mount_flags branch June 10, 2022 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants