changing ip_address param to expect list

[daarrn]

Description

changing spec options, ip_address parameter is being changed from a 'str' type to a 'list' type

fixes issue #245

How Has This Been Tested?

validated the change using a local collection of lowlydba and was able to successfully create a multi subnet ag listener when the ip_address parameter was expecting a list

Types of changes

• [x ] Bug fix (non-breaking change which fixes an issue) - Fixes #
• New feature (non-breaking change which adds functionality)

Checklist:

• [x ] I have read/followed the CONTRIBUTING document.
• [x ] I have read/followed the PR Quick Start Guide
• [x ] I have added tests to cover my changes or they are N/A.
• [x ] I have added a changelog fragment describing the changes.
• New module options/parameters include a version_added property.

[daarrn]

Hello, very first time attempting to contribute, please don't be shy to let me know if I missed performing something correctly in the contribution process. Thanks for your work!

[DorBreger]

I think you will need to update the type of the subnet_mask too.

[daarrn]

I think you will need to update the type of the subnet_mask too.

I think you are right that the change will be needed there as well. I do not have a use case to test that change and left it out of my request due to that

[lowlydba]

Hi @daarrn ! Thank you for the pull request. I'll try to take a look at it this weekend and provide feedback.

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.69%. Comparing base (de0aac7) to head (ead4087).
Report is 1 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff             @@
##              main     #246      +/-   ##
===========================================
- Coverage   100.00%   93.69%   -6.31%     
===========================================
  Files           33       64      +31     
  Lines          106     2205    +2099     
===========================================
+ Hits           106     2066    +1960     
- Misses           0      139     +139     

Flag	Coverage Δ
sanity	?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[DorBreger]

@daarrn I tested this on a couple of VMs on my laptop on two different subnets with different subnet masks, and I can confirm you would have to change the subnet mask parameter to a list too.

[lowlydba]

In addition to the comments:

• The documentation in plugins\modules\ag_listener.py will need to be updated to match the type changes (failed sanity check here).
• Can you update the tests to use lists in addition to single strings (tests\integration\targets\win_ag_listener\tasks\main.yml)

[daarrn]

I think you will need to update the type of the subnet_mask too.

I think you are right that the change will be needed there as well. I do not have a use case to test that change and left it out of my request due to that

Yeah, all three of these should be lists to be consistent with the inputs in DBATools:

• IP Address
• Subnet IP
• Subnet Mask

most recent commit contains changes to include all parameters expecting lists to be defined as lists

[daarrn]

    ip_address = @{type = 'list'; elements='str'; required = $false }

Per the [sanity check](https://github.com/lowlydba/lowlydba.sqlserver/actions/runs/9006650802/job/25120714271?pr=246#step:4:935), the [type of elements](https://github.com/ansible/ansible/pull/66386#issuecomment-589717074) should be specified too (I assume this is the right syntax, but verify) 

most recent commit includes defining the element type when using list

[daarrn]

• fixed the expected type of the ip_address, subnet_ip, and subnet_mask parameters to be lists instead of strings (lowlydba.sqlserver.ag_listener)

added suggested changes

[lowlydba]

The sanity checks are still failing due to the list changes not being updated in the ag_listener.py documentation file.

[daarrn]

The sanity checks are still failing due to the list changes not being updated in the ag_listener.py documentation file.

whoops, yup. Updated document file, thank you!

[github-actions]

Docs Build 📝

Thank you for contribution!✨

This PR has been merged and the docs are now incorporated into main:
https://lowlydba.github.io/lowlydba.sqlserver/branch/main

[lowlydba]

Looks good as far as passing tests, just missing using the lists the tests:

• Can you update the tests to use lists in addition to single strings (tests\integration\targets\win_ag_listener\tasks\main.yml)

[daarrn]

@lowlydba I updated the tests as well as some examples of the command call to properly use lists instead of single strings or comma separated strings

[lowlydba]

Looks great! Thanks again for the enhancement, this is a very nice upgrade.