Skip to content

Commit

Permalink
Version check for NVMe host (#243)
Browse files Browse the repository at this point in the history
Updating the NVMe host in PowerFlex versions
earlier than 4.6 is not supported

Fix empty NVMe host name

Empty host name should not be used for create and update.
  • Loading branch information
baoy1 authored Nov 16, 2024
1 parent 51ba597 commit 6952be4
Show file tree
Hide file tree
Showing 12 changed files with 169 additions and 33 deletions.
3 changes: 3 additions & 0 deletions docs/resources/nvme_host.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ description: |-

This resource is used to manager NVMe host from the PowerFlex array. We can Create, Update and Delete the PowerFlex NVMe host using this resource. We can also import an existing NVMe host from PowerFlex array.

Please note that due to certain limitations, updating the NVMe host in PowerFlex versions earlier than 4.6 is not supported

## Example Usage

```terraform
Expand All @@ -51,6 +53,7 @@ limitations under the License.
# To import , check import.sh for more info
# nqn is the required parameter to create
# To check which attributes can be updated, please refer Product Guide in the documentation
# Please note that due to certain limitations, updating the NVMe host in PowerFlex versions earlier than 4.6 is not supported
# Example for adding NVMe host.
resource "powerflex_nvme_host" "test-nvme-host" {
Expand Down
1 change: 1 addition & 0 deletions examples/resources/powerflex_nvme_host/resource.tf
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ limitations under the License.
# To import , check import.sh for more info
# nqn is the required parameter to create
# To check which attributes can be updated, please refer Product Guide in the documentation
# Please note that due to certain limitations, updating the NVMe host in PowerFlex versions earlier than 4.6 is not supported

# Example for adding NVMe host.
resource "powerflex_nvme_host" "test-nvme-host" {
Expand Down
4 changes: 2 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ toolchain go1.23.2
require (
github.com/bramvdbogaerde/go-scp v1.5.0
github.com/bytedance/mockey v1.2.13
github.com/dell/goscaleio v1.17.1-0.20241106124128-c895c8717b22
github.com/dell/goscaleio v1.17.1-0.20241116014606-58a85e67edac
github.com/hashicorp/terraform-plugin-framework v1.11.0
github.com/hashicorp/terraform-plugin-framework-validators v0.13.0
github.com/hashicorp/terraform-plugin-go v0.23.0
Expand Down Expand Up @@ -95,7 +95,7 @@ require (
github.com/vmihailenco/msgpack v4.0.4+incompatible // indirect
github.com/zclconf/go-cty v1.15.0 // indirect
golang.org/x/net v0.25.0 // indirect
golang.org/x/sys v0.26.0 // indirect
golang.org/x/sys v0.27.0 // indirect
golang.org/x/text v0.17.0 // indirect
google.golang.org/appengine v1.6.8 // indirect
google.golang.org/grpc v1.63.2 // indirect
Expand Down
8 changes: 4 additions & 4 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ github.com/cyphar/filepath-securejoin v0.2.4/go.mod h1:aPGpWjXOXUn2NCNjFvBE6aRxG
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/dell/goscaleio v1.17.1-0.20241106124128-c895c8717b22 h1:Ox4ra5FUj1LWL3M1ibMxPCzWLue2yRzYdhSuRsLYEDc=
github.com/dell/goscaleio v1.17.1-0.20241106124128-c895c8717b22/go.mod h1:dB1a2wXevGps25VAda+6WDp+NTUdgMZXvQVM0YOBpX8=
github.com/dell/goscaleio v1.17.1-0.20241116014606-58a85e67edac h1:6ujnbGEDbrnLZ1V9/aA4QihlAs7wzlbZoe5RUh8/75U=
github.com/dell/goscaleio v1.17.1-0.20241116014606-58a85e67edac/go.mod h1:OGFdDzRGhiAkXDTyrOUM42mxco5Y/vnhUyw/G7uXDzs=
github.com/dylanmei/iso8601 v0.1.0 h1:812NGQDBcqquTfH5Yeo7lwR0nzx/cKdsmf3qMjPURUI=
github.com/dylanmei/iso8601 v0.1.0/go.mod h1:w9KhXSgIyROl1DefbMYIE7UVSIvELTbMrCfx+QkYnoQ=
github.com/emirpasic/gods v1.18.1 h1:FXtiHYKDGKCW2KzwZKx0iC0PQmdlorYgdFG9jPXJ1Bc=
Expand Down Expand Up @@ -278,8 +278,8 @@ golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBc
golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.26.0 h1:KHjCJyddX0LoSTb3J+vWpupP9p0oznkqVk/IfjymZbo=
golang.org/x/sys v0.26.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
golang.org/x/sys v0.27.0 h1:wBqf8DvsY9Y/2P8gAfPDEYNuS30J4lPHJxXSb/nJZ+s=
golang.org/x/sys v0.27.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8=
golang.org/x/term v0.5.0/go.mod h1:jMB1sMXY+tzblOD4FWmEbocvup2/aLOaQEp7JmGp78k=
Expand Down
4 changes: 4 additions & 0 deletions powerflex/provider/POWERFLEX_TERRAFORM_TEST.env.sample
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ POWERFLEX_PROTECTION_DOMAIN_ID_SDS=
POWERFLEX_STORAGE_POOL_NAME=
POWERFLEX_SERVICE_ID=
POWERFLEX_SERVICE_NAME=
POWERFLEX_NVME_HOST_NAME=
POWERFLEX_NVME_HOST_NAME_CREATE=
POWERFLEX_NVME_HOST_NAME_UPDATE=
POWERFLEX_NVME_HOST_NQN=
POWERFLEX_NVME_TARGET_NAME=
POWERFLEX_NVME_TARGET_NAME_CREATE=
POWERFLEX_NVME_TARGET_NAME_UPDATE=
Expand Down
31 changes: 15 additions & 16 deletions powerflex/provider/nvme_host_datasource_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,15 @@ import (

var nvmeHostDatasourceConfig = `
data "powerflex_nvme_host" "nvme_host_datasource" {
filter {
name = ["nvme_acc_client1001"]
}
}
`
var nvmeHostDatasourceNoFilterConfig = `

var nvmeHostDatasourceConfigFilter = `
data "powerflex_nvme_host" "nvme_host_datasource" {
filter {
name = ["` + NVMeHostName + `"]
}
}
`

Expand All @@ -49,15 +50,15 @@ data "powerflex_nvme_host" "nvme_host_datasource" {
}
`

func TestAccDatasourceNvmeHost(t *testing.T) {
func TestAccNvmeHostDatasource(t *testing.T) {
resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
ProtoV6ProviderFactories: testAccProtoV6ProviderFactories,
Steps: []resource.TestStep{
{
Config: ProviderConfigForTesting + nvmeHostDatasourceNoFilterConfig,
Config: ProviderConfigForTesting + nvmeHostDatasourceConfig,
Check: resource.ComposeAggregateTestCheckFunc(
resource.TestCheckResourceAttr("data.powerflex_nvme_host.nvme_host_datasource", "nvme_host_details.#", "1"),
listCountGreaterThan("data.powerflex_nvme_host.nvme_host_datasource", "nvme_host_details", 0),
),
},
{
Expand All @@ -69,7 +70,7 @@ func TestAccDatasourceNvmeHost(t *testing.T) {
{ID: "mock-id", HostType: "NVMeHost", SystemID: "mock-system-id", Name: "", Nqn: "mock-nqn", MaxNumPaths: 4, MaxNumSysPorts: 10},
}, nil).Build()
},
Config: ProviderConfigForTesting + nvmeHostDatasourceNoFilterConfig,
Config: ProviderConfigForTesting + nvmeHostDatasourceConfig,
Check: resource.ComposeAggregateTestCheckFunc(
resource.TestCheckResourceAttr("data.powerflex_nvme_host.nvme_host_datasource", "nvme_host_details.#", "1"),
resource.TestCheckResourceAttr("data.powerflex_nvme_host.nvme_host_datasource", "nvme_host_details.0.name", "NVMeHost:mock-id"),
Expand All @@ -92,18 +93,16 @@ func TestAccDatasourceNvmeHost(t *testing.T) {
FunctionMocker.UnPatch()
}
},
Config: ProviderConfigForTesting + nvmeHostDatasourceConfig,
Config: ProviderConfigForTesting + nvmeHostDatasourceConfigFilter,
Check: resource.ComposeAggregateTestCheckFunc(
resource.TestCheckResourceAttr("data.powerflex_nvme_host.nvme_host_datasource", "nvme_host_details.0.name", "nvme_acc_client1001"),
resource.TestCheckResourceAttr("data.powerflex_nvme_host.nvme_host_datasource", "nvme_host_details.0.max_num_paths", "4"),
resource.TestCheckResourceAttr("data.powerflex_nvme_host.nvme_host_datasource", "nvme_host_details.0.max_num_sys_ports", "10"),
resource.TestCheckResourceAttr("data.powerflex_nvme_host.nvme_host_datasource", "nvme_host_details.0.name", NVMeHostName),
),
},
},
})
}

func TestAccDatasourceNvmeHostNegative(t *testing.T) {
func TestAccNvmeHostDatasourceNegative(t *testing.T) {
resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
ProtoV6ProviderFactories: testAccProtoV6ProviderFactories,
Expand All @@ -115,7 +114,7 @@ func TestAccDatasourceNvmeHostNegative(t *testing.T) {
}
FunctionMocker = Mock(helper.GetFirstSystem).Return(nil, fmt.Errorf("mock error")).Build()
},
Config: ProviderConfigForTesting + nvmeHostDatasourceConfig,
Config: ProviderConfigForTesting + nvmeHostDatasourceConfigFilter,
ExpectError: regexp.MustCompile(`.*Unable to Read Powerflex specific system*.`),
},
{
Expand All @@ -125,7 +124,7 @@ func TestAccDatasourceNvmeHostNegative(t *testing.T) {
}
FunctionMocker = Mock((*goscaleio.System).GetAllNvmeHosts).Return(nil, fmt.Errorf("mock error")).Build()
},
Config: ProviderConfigForTesting + nvmeHostDatasourceConfig,
Config: ProviderConfigForTesting + nvmeHostDatasourceConfigFilter,
ExpectError: regexp.MustCompile(`.*Unable to Read Powerflex NVMe Hosts*.`),
},
},
Expand Down
33 changes: 33 additions & 0 deletions powerflex/provider/nvme_host_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,14 @@ func (r *NvmeHostResource) Create(ctx context.Context, req resource.CreateReques
return
}

if !plan.Name.IsNull() && plan.Name.ValueString() == "" {
resp.Diagnostics.AddError(
"Name cannot be empty",
"Name cannot be empty",
)
return
}

hostResp, err := r.system.CreateNvmeHost(params)
if err != nil {
resp.Diagnostics.AddError(
Expand Down Expand Up @@ -224,6 +232,23 @@ func (r *NvmeHostResource) Update(ctx context.Context, req resource.UpdateReques
return
}

result, err := goscaleio.CheckPfmpVersion(r.client, "4.6")

if err != nil {
resp.Diagnostics.AddError(
"Error checking PowerFlex version",
err.Error(),
)
return
}
if result < 0 {
resp.Diagnostics.AddError(
"Updating NVMe host is not supported",
"Updating the NVMe host is not supported in PowerFlex versions earlier than 4.6",
)
return
}

if !plan.Nqn.Equal(state.Nqn) {
resp.Diagnostics.AddError(
"nqn cannot be modified after creation",
Expand All @@ -232,6 +257,14 @@ func (r *NvmeHostResource) Update(ctx context.Context, req resource.UpdateReques
return
}

if !plan.Name.IsNull() && plan.Name.ValueString() == "" {
resp.Diagnostics.AddError(
"Name cannot be empty",
"Name cannot be empty",
)
return
}

if !plan.Name.IsNull() && plan.Name.ValueString() != state.Name.ValueString() {
err := r.system.ChangeNvmeHostName(state.ID.ValueString(), plan.Name.ValueString())
if err != nil {
Expand Down
Loading

0 comments on commit 6952be4

Please sign in to comment.