DSC_iSCSIInitiator: allow multiple targets for single iscsi session

[coatico]

Pull Request (PR) description

Allow the iSCSIInitiator resource for multiple iscsi targets to be defined for a single iscsi session (initiator).

This Pull Request (PR) fixes the following issues

None

Task list

• Added an entry to the change log under the Unreleased section of the
  file CHANGELOG.md. Entry should say what was changed and how that
  affects users (if applicable), and reference the issue being resolved
  (if applicable).
• Resource documentation added/updated in README.md.
• Resource parameter descriptions added/updated in README.md, schema.mof
  and comment-based help.
• Comment-based help added/updated.
• Localization strings added/updated in all localization files as appropriate.
• Examples appropriately added/updated.
• Unit tests added/updated. See DSC Community Testing Guidelines.
• Integration tests added/updated (where possible). See DSC Community Testing Guidelines.
• New/changed code adheres to DSC Community Style Guidelines.

---

This change is 

[PlagueHO]

Hi @coatico , thanks for submitting this. I've noticed that a lot of the code hasn't met styleguidelines and is possibly because it's forked from older code. There are a lot of changes that need to be reverted to get back to the style guidelines, so might be worth doing those first (which will reduce the size of the PR). This will also get the HQRM tests passing.

There is also functions that are added that already exist in DSCCommon- which is another thing that makes me think this was from an older version.

Also, with the new code, because the functions call out to a non-PS binary, we'll need to put a lot more rigor around these with unit testing etc- but ideally we'd not use an external CLI command to do this (it has many potential problems that we've run into elsewhere).

But again, thank you for taking the time to work on this and submit it - it looks like a super valuable addition.

Reviewed 4 of 8 files at r1.
Reviewable status: 4 of 8 files reviewed, 12 unresolved discussions (waiting on @coatico)

---

CHANGELOG.md line 1 at r1 (raw file):

# Change log for ComputerManagementDsc

I just noticed this is incorrect (for a long time) - can you change it to # Change log for iSCSIDsc

---

CHANGELOG.md line 65 at r1 (raw file):

- Removed unused ISNS feature from being installed for unit tests.

## [1.5.0.42] - 2024-10-18

Can you remove this block as it will get automatically added when a new version of the module is released?

---

Modules/iSCSIDsc.ResourceHelper/iSCSIDsc.ResourceHelper.psm1 line 1 at r1 (raw file):

<#

The first four functions are already in DSCREsource.Common shared library which this module depends on. So shouldn't be needed.

For the remaining functions, can you change the module to be named iSCSIDsc.Common (to align to other modules)?

For the remaining functions however, we will need to update them to meet style guidelines: General

Can you also create unit tests to cover these new functions?

---

Modules/iSCSIDsc.ResourceHelper/iSCSIDsc.ResourceHelper.psm1 line 172 at r1 (raw file):

    return $localizedData
}
function Get-ISCSIPersistentTarget() {

These functions need to be updated to meet style guidelines: General

---

Modules/iSCSIDsc.ResourceHelper/iSCSIDsc.ResourceHelper.psm1 line 189 at r1 (raw file):

    $escapedIP = $targetPortalIpAddress -replace '\.', '\.'
    #query iscscli for persistent targets
    $iscsi = "iscsicli listpersistenttargets | Select-String -Pattern `"\s*Address and Socket\s*:\s*($escapedIP)\s+(\d+)`" -Context 1, 10"

Is there any other way to get this information? Calling out to a non PS binary has a number of issues (e.g., translating results from text, not handling localized strings, detecting errors properly). We always try to avoid doing this if we can (but understand it is not possible).

If you have to call out to an external binary to do this, then recommend using this process: CertificateDsc/source/Modules/CertificateDsc.Common/CertificateDsc.Common.psm1 at main · dsccommunity/CertificateDsc

---

source/iSCSIDsc.psd1 line 6 at r1 (raw file):

    # Version number of this module.
    ModuleVersion         = '0.0.2'

Can you revert this as it doesn't need to be changed (the auto release process updates it)?

---

source/DSCResources/DSC_iSCSIInitiator/DSC_iSCSIInitiator.psm1 line 3 at r1 (raw file):

$modulePath = Join-Path -Path (Split-Path -Path (Split-Path -Path $PSScriptRoot -Parent) -Parent) -ChildPath 'Modules'

# Import the Networking Resource Helper Module

This comment isn't correct or needed.

---

source/DSCResources/DSC_iSCSIInitiator/DSC_iSCSIInitiator.psm1 line 7 at r1 (raw file):

# Import Localization Strings
$script:localizedData = Get-localizedData -DefaultUICulture 'en-US'

Can you revert this change as should be Get-LocalizedData?

---

source/DSCResources/DSC_iSCSIInitiator/DSC_iSCSIInitiator.psm1 line 53 at r1 (raw file):

    Write-Verbose -Message ( @(
            "$($MyInvocation.MyCommand): "
            $($LocalizedData.GettingiSCSIInitiatorMessage) `

Can you revert this change as needs to be $script:locatlizedData - and throughout.

---

source/DSCResources/DSC_iSCSIInitiator/DSC_iSCSIInitiator.psm1 line 191 at r1 (raw file):

    }

    $returnValue

Can revert this as to align we use return keyword to make clear intended return value (even though not strictly required)?

---

source/DSCResources/DSC_iSCSIInitiator/DSC_iSCSIInitiator.psm1 line 349 at r1 (raw file):

        [Boolean] $create = $false

        if ($targetPortal){

There are a lot of changes to the { style that need to be reverted as they don't meet the style guidelines: Style Guidelines

---

source/DSCResources/DSC_iSCSIInitiator/README.md line 1 at r1 (raw file):

# Description

This file is required - can it be restored?