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

new datasource azurerm_elastic_san_volume_snapshot #26439

Merged
merged 6 commits into from
Jul 9, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,147 @@
package elasticsan

import (
"context"
"fmt"
"time"

"github.com/hashicorp/go-azure-helpers/lang/pointer"
"github.com/hashicorp/go-azure-helpers/lang/response"
"github.com/hashicorp/go-azure-sdk/resource-manager/elasticsan/2023-01-01/snapshots"
"github.com/hashicorp/go-azure-sdk/resource-manager/elasticsan/2023-01-01/volumes"
"github.com/hashicorp/terraform-provider-azurerm/internal/sdk"
"github.com/hashicorp/terraform-provider-azurerm/internal/services/elasticsan/validate"
"github.com/hashicorp/terraform-provider-azurerm/internal/tf/pluginsdk"
)

type ElasticSANVolumeSnapshotDataSource struct{}

var _ sdk.DataSource = ElasticSANVolumeSnapshotDataSource{}

type ElasticSANVolumeSnapshotDataSourceModel struct {
Name string `tfschema:"name"`
SourceVolumeSizeGiB int64 `tfschema:"source_volume_size_gib"`
Copy link
Member

Choose a reason for hiding this comment

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

In the other elastic san resources we're using something_in_gib, can we rename this to follow that pattern as well

Suggested change
SourceVolumeSizeGiB int64 `tfschema:"source_volume_size_gib"`
SourceVolumeSizeGiB int64 `tfschema:"source_volume_size_in_gib"`

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed

CreateSource []ElasticSANVolumeSnapshotCreateSource `tfschema:"creation_source"`
VolumeGroupId string `tfschema:"volume_group_id"`
VolumeName string `tfschema:"volume_name"`
}

type ElasticSANVolumeSnapshotCreateSource struct {
SourceId string `tfschema:"source_id"`
}

func (r ElasticSANVolumeSnapshotDataSource) ResourceType() string {
return "azurerm_elastic_san_volume_snapshot"
}

func (r ElasticSANVolumeSnapshotDataSource) ModelObject() interface{} {
return &ElasticSANVolumeSnapshotDataSourceModel{}
}

func (r ElasticSANVolumeSnapshotDataSource) Arguments() map[string]*pluginsdk.Schema {
return map[string]*pluginsdk.Schema{
"name": {
Type: pluginsdk.TypeString,
Required: true,
ValidateFunc: validate.ElasticSanSnapshotName,
},

"volume_group_id": {
Copy link
Member

Choose a reason for hiding this comment

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

Can we add some validation here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added

Type: pluginsdk.TypeString,
Required: true,
ForceNew: true,
},
}
}

func (r ElasticSANVolumeSnapshotDataSource) Attributes() map[string]*pluginsdk.Schema {
return map[string]*pluginsdk.Schema{
"creation_source": {
Type: pluginsdk.TypeList,
Computed: true,
Elem: &pluginsdk.Resource{
Schema: map[string]*pluginsdk.Schema{
"source_id": {
Type: pluginsdk.TypeString,
Computed: true,
},
},
},
},
Copy link
Member

Choose a reason for hiding this comment

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

Can we flatten this to

Suggested change
"creation_source": {
Type: pluginsdk.TypeList,
Computed: true,
Elem: &pluginsdk.Resource{
Schema: map[string]*pluginsdk.Schema{
"source_id": {
Type: pluginsdk.TypeString,
Computed: true,
},
},
},
},
"source_id": {
Type: pluginsdk.TypeString,
Computed: true,
},

Copy link
Contributor Author

Choose a reason for hiding this comment

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

make sense, changed

"source_volume_size_gib": {
Computed: true,
Type: pluginsdk.TypeInt,
},
"volume_name": {
Computed: true,
Type: pluginsdk.TypeString,
},
}
}

func (r ElasticSANVolumeSnapshotDataSource) Read() sdk.ResourceFunc {
return sdk.ResourceFunc{
Timeout: 5 * time.Minute,
Func: func(ctx context.Context, metadata sdk.ResourceMetaData) error {
client := metadata.Client.ElasticSan.Snapshots

var state ElasticSANVolumeSnapshotDataSourceModel
if err := metadata.Decode(&state); err != nil {
return fmt.Errorf("decoding: %+v", err)
}

volumeGroupId, err := snapshots.ParseVolumeGroupID(state.VolumeGroupId)
if err != nil {
return err
}

id := snapshots.NewSnapshotID(volumeGroupId.SubscriptionId, volumeGroupId.ResourceGroupName, volumeGroupId.ElasticSanName, volumeGroupId.VolumeGroupName, state.Name)

resp, err := client.VolumeSnapshotsGet(ctx, id)
if err != nil {
if response.WasNotFound(resp.HttpResponse) {
return fmt.Errorf("%s does not exist", id)
}

return fmt.Errorf("retrieving %s: %+v", id, err)
}

state.VolumeGroupId = volumeGroupId.ID()
state.Name = id.SnapshotName
if model := resp.Model; model != nil {
// these properties are not pointer so we don't need to check for nil
state.SourceVolumeSizeGiB = pointer.From(model.Properties.SourceVolumeSizeGiB)
state.VolumeName = pointer.From(model.Properties.VolumeName)

createSource, err := FlattenElasticSANVolumeSnapshotCreateSource(model.Properties.CreationData)
if err != nil {
return err
}
state.CreateSource = *createSource
}
metadata.SetID(id)

return metadata.Encode(&state)
},
}
}

func FlattenElasticSANVolumeSnapshotCreateSource(input snapshots.SnapshotCreationData) (*[]ElasticSANVolumeSnapshotCreateSource, error) {
Copy link
Member

Choose a reason for hiding this comment

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

Does this need to be a public function?

Suggested change
func FlattenElasticSANVolumeSnapshotCreateSource(input snapshots.SnapshotCreationData) (*[]ElasticSANVolumeSnapshotCreateSource, error) {
func flattenElasticSANVolumeSnapshotCreateSource(input snapshots.SnapshotCreationData) (*[]ElasticSANVolumeSnapshotCreateSource, error) {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the block is flattened, so this function is removed.

if input.SourceId == "" {
return &[]ElasticSANVolumeSnapshotCreateSource{}, nil
}

// for now source ID can only be ElasticSAN Volume ID
var sourceId string
parsedSourceId, err := volumes.ParseVolumeIDInsensitively(input.SourceId)
if err != nil {
return nil, fmt.Errorf("parsing source ID as ElasticSAN Volume ID: %+v", err)
}
sourceId = parsedSourceId.ID()

return &[]ElasticSANVolumeSnapshotCreateSource{
{
SourceId: sourceId,
},
}, nil
Copy link
Member

Choose a reason for hiding this comment

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

Since only ElasticSAN Volumes are supported for now can we simplify this for the time being

Suggested change
var sourceId string
parsedSourceId, err := volumes.ParseVolumeIDInsensitively(input.SourceId)
if err != nil {
return nil, fmt.Errorf("parsing source ID as ElasticSAN Volume ID: %+v", err)
}
sourceId = parsedSourceId.ID()
return &[]ElasticSANVolumeSnapshotCreateSource{
{
SourceId: sourceId,
},
}, nil
volumeId, err := volumes.ParseVolumeIDInsensitively(input.SourceId)
if err != nil {
return nil, err
}
return &[]ElasticSANVolumeSnapshotCreateSource{
{
SourceId: volumeId.ID(),
},
}, nil

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
package elasticsan_test

import (
"fmt"
"os"
"testing"

"github.com/hashicorp/terraform-provider-azurerm/internal/acceptance"
"github.com/hashicorp/terraform-provider-azurerm/internal/acceptance/check"
)

type ElasticSANVolumeSnapshotDataSource struct{}

// https://github.com/hashicorp/terraform-provider-azurerm/pull/25372#issuecomment-2022105240
// Elastic SAN Volume Snapshot is context-based and should not be regarded as the infrastructure managed by Terraform
// so we only onboard this as a data source instead of a resource. The acctest relys on Azure CLI to create/delete the snapshot.
func TestAccElasticSANVolumeSnapshotDataSource_basic(t *testing.T) {
data := acceptance.BuildTestData(t, "data.azurerm_elastic_san_volume_snapshot", "test")
d := ElasticSANVolumeSnapshotDataSource{}

const SnapshotTestRunEnv = "ARM_TEST_ELASTIC_SAN_VOLUME_SNAPSHOT_RUN"

if os.Getenv(SnapshotTestRunEnv) == "" {
t.Skipf("skip the test as one or more of below environment variables are not specified: %q", SnapshotTestRunEnv)
}
Copy link
Member

Choose a reason for hiding this comment

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

We should add the creation of the Snapshot as a test step in here instead of using the local-exec provisioner to run az cli. That way we don't need to set an environment variable to run this test. Here's an example of where we provision resources "outside" of Terraform in tests:

func TestAccDataSourceKubernetesNodePoolSnapshot_basic(t *testing.T) {
data := acceptance.BuildTestData(t, "data.azurerm_kubernetes_node_pool_snapshot", "test")
r := KubernetesNodePoolSnapshotDataSource{}
data.DataSourceTest(t, []acceptance.TestStep{
{
Config: r.snapshotSource(data),
Check: acceptance.ComposeTestCheckFunc(
data.CheckWithClientForResource(func(ctx context.Context, clients *clients.Client, state *terraform.InstanceState) error {
if _, ok := ctx.Deadline(); !ok {
var cancel context.CancelFunc
ctx, cancel = context.WithTimeout(ctx, 30*time.Minute)
defer cancel()
}
client := clients.Containers.SnapshotClient
poolId, err := agentpools.ParseAgentPoolID(state.ID)
if err != nil {
return err
}
id := snapshots.NewSnapshotID(poolId.SubscriptionId, poolId.ResourceGroupName, data.RandomString)
snapshot := snapshots.Snapshot{
Location: data.Locations.Primary,
Properties: &snapshots.SnapshotProperties{
CreationData: &snapshots.CreationData{
SourceResourceId: utils.String(poolId.ID()),
},
},
}
_, err = client.CreateOrUpdate(ctx, id, snapshot)
if err != nil {
return fmt.Errorf("creating %s: %+v", id, err)
}
return nil
}, "azurerm_kubernetes_cluster_node_pool.source"),
),
},

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed


data.DataSourceTestInSequence(t, []acceptance.TestStep{
{
Config: d.basic(data),
Check: acceptance.ComposeTestCheckFunc(
check.That(data.ResourceName).Key("creation_source.#").HasValue("1"),
check.That(data.ResourceName).Key("creation_source.0.source_id").IsNotEmpty(),
check.That(data.ResourceName).Key("source_volume_size_gib").IsNotEmpty(),
check.That(data.ResourceName).Key("volume_name").IsNotEmpty(),
),
},
})
}

func (d ElasticSANVolumeSnapshotDataSource) basic(data acceptance.TestData) string {
return fmt.Sprintf(`
variable "primary_location" {
default = %q
}
variable "random_integer" {
default = %d
}
variable "random_string" {
default = %q
}
Copy link
Member

Choose a reason for hiding this comment

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

This looks like a remnant from a resource/data source generated with Pandora, please remove these

Suggested change
variable "primary_location" {
default = %q
}
variable "random_integer" {
default = %d
}
variable "random_string" {
default = %q
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed


provider "azurerm" {
features {}
}

resource "azurerm_resource_group" "test" {
name = "acctestrg-esvg-${var.random_integer}"
location = var.primary_location
}

resource "azurerm_elastic_san" "test" {
name = "acctestes-${var.random_string}"
resource_group_name = azurerm_resource_group.test.name
location = azurerm_resource_group.test.location
base_size_in_tib = 1
sku {
name = "Premium_LRS"
}
}

resource "azurerm_elastic_san_volume_group" "test" {
name = "acctestesvg-${var.random_string}"
elastic_san_id = azurerm_elastic_san.test.id
}

resource "azurerm_elastic_san_volume" "test" {
name = "acctestesv-${var.random_string}"
volume_group_id = azurerm_elastic_san_volume_group.test.id
size_in_gib = 1
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
name = "acctestrg-esvg-${var.random_integer}"
location = var.primary_location
}
resource "azurerm_elastic_san" "test" {
name = "acctestes-${var.random_string}"
resource_group_name = azurerm_resource_group.test.name
location = azurerm_resource_group.test.location
base_size_in_tib = 1
sku {
name = "Premium_LRS"
}
}
resource "azurerm_elastic_san_volume_group" "test" {
name = "acctestesvg-${var.random_string}"
elastic_san_id = azurerm_elastic_san.test.id
}
resource "azurerm_elastic_san_volume" "test" {
name = "acctestesv-${var.random_string}"
volume_group_id = azurerm_elastic_san_volume_group.test.id
size_in_gib = 1
name = "acctestrg-esvg-%[2]d"
location = %[1]s
}
resource "azurerm_elastic_san" "test" {
name = "acctestes-%[3]s"
resource_group_name = azurerm_resource_group.test.name
location = azurerm_resource_group.test.location
base_size_in_tib = 1
sku {
name = "Premium_LRS"
}
}
resource "azurerm_elastic_san_volume_group" "test" {
name = "acctestesvg-%[3]s"
elastic_san_id = azurerm_elastic_san.test.id
}
resource "azurerm_elastic_san_volume" "test" {
name = "acctestesv-%[3]s"
volume_group_id = azurerm_elastic_san_volume_group.test.id
size_in_gib = 1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed

}

locals {
snapshot_name = "acctest-ess-${var.random_string}"
}

resource "terraform_data" "test" {
input = {
resource_group_name = azurerm_resource_group.test.name
elastic_san_name = azurerm_elastic_san.test.name
volume_group_name = azurerm_elastic_san_volume_group.test.name
snapshot_name = local.snapshot_name
}

provisioner "local-exec" {
command = <<COMMAND
az elastic-san volume snapshot create -g ${self.input.resource_group_name} -e ${self.input.elastic_san_name} -v ${self.input.volume_group_name} -n ${self.input.snapshot_name} --creation-data '{source-id:${azurerm_elastic_san_volume.test.id}}'
COMMAND
}

provisioner "local-exec" {
when = destroy
command = <<COMMAND
az elastic-san volume snapshot delete -g ${self.input.resource_group_name} -e ${self.input.elastic_san_name} -v ${self.input.volume_group_name} -n ${self.input.snapshot_name} -y
COMMAND
}
}

data "azurerm_elastic_san_volume_snapshot" "test" {
name = local.snapshot_name
volume_group_id = azurerm_elastic_san_volume_group.test.id
depends_on = [terraform_data.test]
}
`, data.Locations.Primary, data.RandomInteger, data.RandomString)
}
1 change: 1 addition & 0 deletions internal/services/elasticsan/registration.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ func (Registration) DataSources() []sdk.DataSource {
return []sdk.DataSource{
ElasticSANDataSource{},
ElasticSANVolumeGroupDataSource{},
ElasticSANVolumeSnapshotDataSource{},
}
}

Expand Down
24 changes: 24 additions & 0 deletions internal/services/elasticsan/validate/elastic_san_snapshot_name.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package validate

import (
"fmt"
"regexp"
)

func ElasticSanSnapshotName(i interface{}, k string) (warnings []string, errors []error) {
v, ok := i.(string)
if !ok {
errors = append(errors, fmt.Errorf("expected %q to be a string but it wasn't", k))
return
}

if matched := regexp.MustCompile(`^[a-z0-9][a-z0-9_-]{1,61}[a-z0-9]$`).Match([]byte(v)); !matched {
errors = append(errors, fmt.Errorf("%q must be between 3 and 63 characters. It can contain only lowercase letters, numbers, underscores (_) and hyphens (-). It must start and end with a lowercase letter or number", k))
}

if matched := regexp.MustCompile(`[_-][_-]`).Match([]byte(v)); matched {
errors = append(errors, fmt.Errorf("%q must have hyphens and underscores be surrounded by alphanumeric character", k))
}

return warnings, errors
}
Loading
Loading