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

add context argument to resource.nomad_csi_volume #503

Merged
merged 1 commit into from
Jan 21, 2025
Merged

Conversation

tgross
Copy link
Member

@tgross tgross commented Jan 16, 2025

The context output is missing from resource.nomad_csi_volume. This prevents successfully migrating from the deprecated resource.nomad_external_volume. Add it as a computed value that we get from the plugins.

Fixes: #494
Fixes: #417
Ref: https://hashicorp.atlassian.net/browse/NET-11851


Tested using the most recent available Docker release of democratic-csi (v1.9.3, not sure why the version shows as 1.9.0 below).

$ nomad plugin status org.democratic-csi.nfs
ID                   = org.democratic-csi.nfs
Provider             = org.democratic-csi.nfs
Version              = 1.9.0
Controllers Healthy  = 1
Controllers Expected = 1
Nodes Healthy        = 1
Nodes Expected       = 1

Allocations
No allocations placed
terraform configuration
provider "nomad" {
  address = "https://nomad3.local:4646"
  region  = "global"
}

data "nomad_plugin" "nfs" {
  plugin_id        = "org.democratic-csi.nfs"
  wait_for_healthy = true
}

resource "nomad_csi_volume" "example" {

  depends_on = [data.nomad_plugin.nfs]

  lifecycle {
    prevent_destroy = true
  }

  plugin_id = "org.democratic-csi.nfs"
  volume_id = "example1"
  name      = "example-volume"
  capacity_max = "500MiB"
  capacity_min = "200MiB"

  capability {
    access_mode     = "single-node-writer"
    attachment_mode = "file-system"
  }

}

output "message" {
  value = <<EOM

Volume: ${resource.nomad_csi_volume.example.id}

Computed properties:

  context:
%{for k, v in resource.nomad_csi_volume.example.context~}
    - ${k} => ${v}
%{endfor~}

  controller_required: ${resource.nomad_csi_volume.example.controller_required}
  controllers_expected: ${resource.nomad_csi_volume.example.controllers_expected}
  controllers_healthy: ${resource.nomad_csi_volume.example.controllers_healthy}
  nodes_expected: ${resource.nomad_csi_volume.example.nodes_expected}
  nodes_healthy: ${resource.nomad_csi_volume.example.nodes_healthy}
  plugin_provider: ${resource.nomad_csi_volume.example.plugin_provider}
  plugin_provider_version: ${resource.nomad_csi_volume.example.plugin_provider_version}
  schedulable: ${resource.nomad_csi_volume.example.schedulable}

EOM

}
terraform apply
$ terraform apply -auto-approve

Terraform will perform the following actions:

  # nomad_csi_volume.example will be created
  + resource "nomad_csi_volume" "example" {
      + capacity                = (known after apply)
      + capacity_max            = "500 MiB"
      + capacity_max_bytes      = (known after apply)
      + capacity_min            = "200 MiB"
      + capacity_min_bytes      = (known after apply)
      + context                 = (known after apply)
      + controller_required     = (known after apply)
      + controllers_expected    = (known after apply)
      + controllers_healthy     = (known after apply)
      + external_id             = (known after apply)
      + id                      = (known after apply)
      + name                    = "example-volume"
      + namespace               = "default"
      + nodes_expected          = (known after apply)
      + nodes_healthy           = (known after apply)
      + plugin_id               = "org.democratic-csi.nfs"
      + plugin_provider         = (known after apply)
      + plugin_provider_version = (known after apply)
      + schedulable             = (known after apply)
      + topologies              = (known after apply)
      + volume_id               = "example1"

      + capability {
          + access_mode     = "single-node-writer"
          + attachment_mode = "file-system"
        }
    }

Plan: 1 to add, 0 to change, 0 to destroy.

Changes to Outputs:
  ~ message = <<-EOT
        Volume: example1

        Computed properties:

          context:
            - node_attach_driver => nfs
            - provisioner_driver => nfs-client
            - server => 192.168.1.170
            - share => /srv/nfs_data/v/example-volume

          controller_required: true
          controllers_expected: 1
          controllers_healthy: 1
          nodes_expected: 1
          nodes_healthy: 1
          plugin_provider: org.democratic-csi.nfs
          plugin_provider_version: 1.9.0
          schedulable: true
    EOT -> (known after apply)
nomad_csi_volume.example: Creating...
nomad_csi_volume.example: Creation complete after 0s [id=example1]

Apply complete! Resources: 1 added, 0 changed, 0 destroyed.

Outputs:

message = <<EOT

Volume: example1

Computed properties:

  context:
    - node_attach_driver => nfs
    - provisioner_driver => nfs-client
    - server => 192.168.1.170
    - share => /srv/nfs_data/v/example-volume

  controller_required: true
  controllers_expected: 1
  controllers_healthy: 1
  nodes_expected: 1
  nodes_healthy: 1
  plugin_provider: org.democratic-csi.nfs
  plugin_provider_version: 1.9.0
  schedulable: true


EOT

terraform plan to verify no changes
$ terraform plan
╷
│ Warning: Provider development overrides are in effect
│
│ The following provider development overrides are set in the CLI configuration:
│  - hashicorp/nomad in /home/tim/go/bin
│
│ The behavior may therefore not match any released version of the provider and applying changes may cause
│ the state to become incompatible with published releases.
╵
data.nomad_plugin.nfs: Reading...
data.nomad_plugin.nfs: Read complete after 0s [id=org.democratic-csi.nfs]
nomad_csi_volume.example: Refreshing state... [id=example1]

No changes. Your infrastructure matches the configuration.

Terraform has compared your real infrastructure against your configuration and found no differences, so no
changes are needed.

Results from Nomad API:

$ nomad volume status
Container Storage Interface
ID        Name            Namespace  Plugin ID               Schedulable  Access Mode
example1  example-volume  default    org.democratic-csi.nfs  true         <none>

$ nomad volume status example1
ID                   = example1
Name                 = example-volume
Namespace            = default
External ID          = example-volume
Plugin ID            = org.democratic-csi.nfs
Provider             = org.democratic-csi.nfs
Version              = 1.9.0
Capacity             = 0 B
Schedulable          = true
Controllers Healthy  = 1
Controllers Expected = 1
Nodes Healthy        = 1
Nodes Expected       = 1
Access Mode          = <none>
Attachment Mode      = <none>
Mount Options        = <none>

Allocations
No allocations placed

$ nomad operator api "/v1/volume/csi/example1" | jq .Context
{
  "provisioner_driver": "nfs-client",
  "node_attach_driver": "nfs",
  "server": "192.168.1.170",
  "share": "/srv/nfs_data/v/example-volume"
}

@tgross tgross requested review from a team as code owners January 16, 2025 21:24
@tgross tgross marked this pull request as draft January 16, 2025 21:24
@tgross tgross force-pushed the csi-missing-context branch from 7533140 to 5c36efa Compare January 16, 2025 21:26
@tgross tgross force-pushed the csi-missing-context branch 2 times, most recently from cc795db to c4e0492 Compare January 16, 2025 21:45
@tgross
Copy link
Member Author

tgross commented Jan 16, 2025

Failing test is an unrelated flake and should be fixed by #502 once that merges Done.

The `context` output is missing from `resource.nomad_csi_volume`. This prevents
successfully migrating from the deprecated `resource.nomad_external_volume`.

Fixes: #494
Ref: https://hashicorp.atlassian.net/browse/NET-11851
@tgross tgross force-pushed the csi-missing-context branch from c4e0492 to 7629241 Compare January 17, 2025 13:45
@tgross tgross marked this pull request as ready for review January 17, 2025 15:49
Copy link
Contributor

@pkazmierczak pkazmierczak left a comment

Choose a reason for hiding this comment

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

LGTM!

@tgross tgross merged commit 04fdb22 into main Jan 21, 2025
3 checks passed
@tgross tgross deleted the csi-missing-context branch January 21, 2025 19:09
tgross added a commit to hashicorp/nomad that referenced this pull request Jan 23, 2025
When a volume is updated, we merge the new definition to the old. But the
volume's context comes from the plugin and is likely not present in the user's
volume specification. Which means that if the user re-submits the volume
specification to make an adjustment to the volume, we wipe out the context field
which might be required for subsequent operations by the CSI plugin. This was
discovered to be a problem with the Terraform provider and fixed there, but it's
also a problem for users of the `volume create` and `volume register` commands.

Update the merge so that we only overwrite the value of the context if it's been
explictly set by the user. We still need to support user-driven updates to
context for the `volume register` workflow.

Ref: hashicorp/terraform-provider-nomad#503
Fixes: democratic-csi/democratic-csi#438
tgross added a commit to hashicorp/nomad that referenced this pull request Jan 23, 2025
When a volume is updated, we merge the new definition to the old. But the
volume's context comes from the plugin and is likely not present in the user's
volume specification. Which means that if the user re-submits the volume
specification to make an adjustment to the volume, we wipe out the context field
which might be required for subsequent operations by the CSI plugin. This was
discovered to be a problem with the Terraform provider and fixed there, but it's
also a problem for users of the `volume create` and `volume register` commands.

Update the merge so that we only overwrite the value of the context if it's been
explictly set by the user. We still need to support user-driven updates to
context for the `volume register` workflow.

Ref: hashicorp/terraform-provider-nomad#503
Fixes: democratic-csi/democratic-csi#438
tgross added a commit to hashicorp/nomad that referenced this pull request Jan 23, 2025
When a volume is updated, we merge the new definition to the old. But the
volume's context comes from the plugin and is likely not present in the user's
volume specification. Which means that if the user re-submits the volume
specification to make an adjustment to the volume, we wipe out the context field
which might be required for subsequent operations by the CSI plugin. This was
discovered to be a problem with the Terraform provider and fixed there, but it's
also a problem for users of the `volume create` and `volume register` commands.

Update the merge so that we only overwrite the value of the context if it's been
explictly set by the user. We still need to support user-driven updates to
context for the `volume register` workflow.

Ref: hashicorp/terraform-provider-nomad#503
Fixes: democratic-csi/democratic-csi#438
tgross added a commit to hashicorp/nomad that referenced this pull request Jan 23, 2025
When a volume is updated, we merge the new definition to the old. But the
volume's context comes from the plugin and is likely not present in the user's
volume specification. Which means that if the user re-submits the volume
specification to make an adjustment to the volume, we wipe out the context field
which might be required for subsequent operations by the CSI plugin. This was
discovered to be a problem with the Terraform provider and fixed there, but it's
also a problem for users of the `volume create` and `volume register` commands.

Update the merge so that we only overwrite the value of the context if it's been
explictly set by the user. We still need to support user-driven updates to
context for the `volume register` workflow.

Ref: hashicorp/terraform-provider-nomad#503
Fixes: democratic-csi/democratic-csi#438
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Migration of nomad_external_volume to nomad_csi_volume omits values nomad_csi_volume missing context block
2 participants