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 resource nsxt_ip_pool_allocation_ip_address #190

Merged

Conversation

MartinWeindel
Copy link
Contributor

For allocating and releasing IP addresses from an IP pool, the resource nsxt_ip_pool_allocation_ip_address and the data source nsxt_ip_pool have been added.

Acceptance tests and documentation have also been added. For the data source and the allocation tests, an existing IP pool is needed. A new environmental variable NSXT_TEST_IP_POOL has been defined to specify it.

@annakhm
Copy link
Collaborator

annakhm commented Oct 9, 2019

Hi @MartinWeindel, thank you for this contribution and sorry for the delay. We will review in coming days.

Copy link
Collaborator

@annakhm annakhm left a comment

Choose a reason for hiding this comment

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

Thanks again for filing this enhancement! Some comments below.

nsxt/resource_nsxt_ip_pool_allocation_ip_address_test.go Outdated Show resolved Hide resolved
nsxt/data_source_nsxt_ip_pool.go Show resolved Hide resolved
nsxt/data_source_nsxt_ip_pool.go Outdated Show resolved Hide resolved
nsxt/resource_nsxt_ip_pool_allocation_ip_address_test.go Outdated Show resolved Hide resolved
func getIpPoolName() string {
name := os.Getenv("NSXT_TEST_IP_POOL")
if name == "" {
name = ipPoolDefaultName
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think nsx provides default IP pool. We need to skip the test if pool is not provided for the test in the environment (otherwise test fails)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Test is now skipped if NSXT_TEST_IP_POOL is not set

},
Steps: []resource.TestStep{
{
Config: testAccNSXIpPoolAllocationIPAddressCreateTemplate(poolName),
Copy link
Collaborator

Choose a reason for hiding this comment

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

I notice this test is failing with 409 (conflict) during create. Hope to provide more details on this later.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't see this issue and have no clue about its cause. Please provide some suggestions how to deal with it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for the changes. 409 Conflict was due to my pool not having IP to allocate, so this is fine.

ignoring test TestAccResourceNsxtIPPoolAllocationIPAddress_basic if
NSXT_TEST_IP_POOL not set, fixing lint spelling issues, reordered
error handling
@annakhm annakhm merged commit cfef1d5 into vmware:master Oct 15, 2019
@MartinWeindel MartinWeindel deleted the nsxt_ip_pool_allocation_ip_address branch October 18, 2019 07:38
@MartinWeindel
Copy link
Contributor Author

@annakhm: Thanks for the review and merging. BTW, will there be a release in the very near future?

@annakhm
Copy link
Collaborator

annakhm commented Oct 22, 2019

@MartinWeindel hopefully, we are working on it

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.

None yet

2 participants