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 filter by cluster uuid in subnet datasource #323

Merged

Conversation

shreevari
Copy link
Contributor

@shreevari shreevari commented Jan 12, 2022

Fixes #308

@shreevari shreevari marked this pull request as ready for review January 13, 2022 09:13
Copy link
Collaborator

@yannickstruyf3 yannickstruyf3 left a comment

Choose a reason for hiding this comment

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

Can we create a generic "filter" param so it is easy to extend it with additional parameters down the road? For example filter subnets based on VLAN ID?
This same filter logic can be applied to other data sources as well. Similar example: https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/ami

@shreevari
Copy link
Contributor Author

AWS provider uses a common ec2 filter struct and transparently passes them to the SDK, thus being handled by SDK/API.

An equivalent of this in Nutanix V3 APIs is the filter param in the metadata, which we already expose for list type data sources.
Do you suggest that I add a filter param for single data sources like nutanix_subnet, nutanix_cluster, etc and pass the filter through to the API call?

@shreevari
Copy link
Contributor Author

Here's an example of filtering for list type data sources that I tried out for filtering by vlan_id


data "nutanix_clusters" "clusters" {}

output "sbnet" {
  value = data.nutanix_subnets.test
}

data "nutanix_subnets" "test" {
  metadata {
    filter = "vlan_id==154"
  }
}

@shreevari shreevari force-pushed the bugfix/fix-308-subnet-datasource-filter-by-cluster-id branch from 8761ef4 to 285d6e0 Compare February 3, 2022 15:38
@siddharth-nutanix
Copy link
Collaborator

@shreevari Can you also add an example and update the datasource documentation for this change?

client/client.go Outdated Show resolved Hide resolved
client/client.go Show resolved Hide resolved
client/client.go Show resolved Hide resolved
client/v3/v3_service.go Outdated Show resolved Hide resolved
@siddharth-nutanix
Copy link
Collaborator

siddharth-nutanix commented Feb 9, 2022

/ok-to-test

Acceptance test run status: success
Line code coverage is 60.2

Copy link
Collaborator

@yannickstruyf3 yannickstruyf3 left a comment

Choose a reason for hiding this comment

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

Can we also add a test case for filters in data_source_nutanix_subnet_test.go?

client/client.go Outdated Show resolved Hide resolved
client/client.go Show resolved Hide resolved
client/client.go Outdated Show resolved Hide resolved
@shreevari shreevari merged commit 9654fa4 into master Feb 10, 2022
@siddharth-nutanix siddharth-nutanix deleted the bugfix/fix-308-subnet-datasource-filter-by-cluster-id branch February 16, 2022 14:38
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.

Allow Subnet Datasources to filter based on PE ID
4 participants