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

Migration of nomad_external_volume to nomad_csi_volume omits values #494

Open
dcarbone opened this issue Dec 7, 2024 · 3 comments
Open

Comments

@dcarbone
Copy link

dcarbone commented Dec 7, 2024

Using:

$ terraform version
Terraform v1.10.1
on linux_amd64
+ provider registry.terraform.io/hashicorp/consul v2.21.0
+ provider registry.terraform.io/hashicorp/nomad v2.4.0
+ provider registry.terraform.io/hashicorp/random v3.6.3
+ provider registry.terraform.io/hashicorp/vault v4.5.0

Attempting to migrate an existing nomad_external_volume resource via this chain:

  1. Define appropriate nomad_csi_volume resource
  2. terraform import nomad_csi_volume.$NAME $VOLUME_ID@$NAMESPACE
  3. terraform rm nomad_external_volume.$NAME

The above always results in the following:

1. The capacity_min, capacity_max, secrets, and mount_options fields do not seem to be imported properly, their state always requires update on subsequent apply:

...
      + capacity_max            = "5.0 GiB"
      + capacity_min            = "5.0 GiB"
        id                      = $volume-id
        name                    = $volume-name
      + secrets                 = (sensitive value)
        # (16 unchanged attributes hidden)

      + mount_options {
          + fs_type     = "ext4"
          + mount_flags = [
              + "rw",
            ]
        }

        # (1 unchanged block hidden)
...

2. The node_attach_driver field seems to be un-definable:

From the task logs:

failed to setup alloc: pre-run hook "csi_hook" failed: mounting volumes: rpc error: code = InvalidArgument desc = unknown/unsupported node_attach_driver: undefined

I cannot find a way to specifically set this. Creating new nomad_csi_volume resources with this same plugin are working perfectly, as are ones created with the now-deprecated nomad_external_volume resource.

I think this is something missing within the import implementation for the "new" nomad_csi_volume type.

@tgross
Copy link
Member

tgross commented Dec 9, 2024

Hi @dcarbone!

  • It looks like capacity_min and capacity_max are indeed simply missing from the "new" resource definition (ref resource_volume.go. That can be fixed here in the provider. I'll mark this for roadmapping.
  • The secrets and mount_options fields are redacted by the Nomad API. This is likely going to need some thought as to how we handle this on the Nomad side to fix.
  • I'm not sure where you're getting node_attach_driver. That's not a field of nomad_external_volume or nomad_csi_volume, and doesn't appear in the CSI volume spec docs either.

@tgross tgross moved this from Needs Triage to Needs Roadmapping in Nomad - Community Issues Triage Dec 9, 2024
@dcarbone
Copy link
Author

dcarbone commented Dec 9, 2024

Howdy @tgross,

You'll have to forgive my relative ignorance of the CSI specification, however this field seems to be required by democratic-csi: https://github.com/democratic-csi/democratic-csi/blob/master/docs/nomad.md#creating-and-registering-the-volumes

I doubt that project is doing anything beyond the typical scope of what is typically done with CSI drivers, perhaps the context object is being defined with a nil value that is overriding the plugin defaults?

As mentioned in my initial post, I do not need to specify the context block, nor the node_attach_driver value directly with my older nomad_external_volume resources or my newer nomad_csi_volume resources. Therefore, I believe it must be a difference present in the Import flow vs. the New flow for nomad_csi_volume that is omitting / overriding this value.

Also, its a bit obvious in hindsight that secrets are not imported. Not importing secrets or mount_options isn't an actual issue for me, as I (obviously) still have both defined in the new resource. The actual problem is the missing node_attach_driver context value, which I think should still be importable (but please correct me if I am wrong about that).

@tgross
Copy link
Member

tgross commented Dec 9, 2024

I doubt that project is doing anything beyond the typical scope of what is typically done with CSI drivers, perhaps the context object is being defined with a nil value that is overriding the plugin defaults?

Ah, thanks for that clarification! The keys in the context object are driver specific -- to Nomad and Terraform it's just a map of string->string. (Ref https://developer.hashicorp.com/nomad/docs/other-specifications/volume#context). When you use the workflow nomad volume create, the plugin returns this context. When you use nomad volume register, you need to pass in the context manually. But for Terraform it looks like nomad_external_volume is missing the context field, but as far as I can tell the nomad_csi_volume resource should have it and it's not elided from the API.

Let me take a quick crack at seeing where the context might be getting dropped and report back.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Needs Roadmapping
Development

No branches or pull requests

2 participants