Add Remove-NsxServiceGroupMember cmdlet #448

ruchg1337 · 2018-01-17T09:06:43Z

Hi,

As said in the commit message : Add Remove-NsxServiceGroupMember cmdlet

Best regards,

vmwclabot · 2018-01-17T09:06:49Z

@ruchg1337, you must sign our contributor license agreement before your changes are merged. Click here to sign the agreement. If you are a VMware employee, read this for further instruction.

vmwclabot · 2018-01-18T02:17:36Z

@ruchg1337, VMware has approved your signed contributor license agreement.

alagoutte · 2018-01-19T21:33:05Z

module/PowerNSX.psm1

+    Group for specific or all Service Groups
+
+    .EXAMPLE
+    PS C:\> Get-NsxServiceGroup Heartbeat | Remove-NsxServiceGroupMember -Member $Service1


no need PS c:>
and missing a text for explain example

alagoutte · 2018-01-19T21:33:13Z

module/PowerNSX.psm1

+    .EXAMPLE
+    PS C:\> Get-NsxServiceGroup Heartbeat | Remove-NsxServiceGroupMember -Member $Service1
+
+    PS C:\> get-nsxservicegroup Service-Group-4 | Remove-NsxServiceGroupMember $Service1,$Service2


same issue here

alagoutte · 2018-01-19T21:33:52Z

module/PowerNSX.psm1

+    begin {}
+    process {
+
+        foreach ($Mem in $Member){


May be add a confirm check ?

alagoutte · 2018-01-19T21:35:34Z

Hi @ruchg1337 look not to bad but see inline comment

also, there is a lot of indent/trailing whitespace change, i think need a separate commit for fix this..
i will be nice also to add test on testsuite !

ruchg1337 · 2018-01-23T09:21:24Z

Hi @alagoutte,

Thank you for your comments. I am fixing it.

About the tests, for you info, I just gave it a go and there are already tests which are not working.

PS C:\work\2018\powernsx-master> Start-Test ServiceGroups
Executing tests against VC gvavcenterva01p and NSX 10.3.150.19 from connection file : C:\work\2018\
powernsx-master\tests\Test.cxn
Executing all tests in '.' with Tags Environment

Executing script C:\work\2018\powernsx-master\tests\integration\09.ServiceGroups.Tests.ps1
Tests completed in 0ms
Tests Passed: 0, Failed: 0, Skipped: 0, Pending: 0, Inconclusive: 0 
Executing all tests in '.' matching test name ServiceGroups

Executing script C:\work\2018\powernsx-master\tests\integration\09.ServiceGroups.Tests.ps1

  Describing ServiceGroups
Using existing PowerCLI connection to 10.3.150.20

    Context Local ServiceGroup retrieval
      [+] Can retrieve all local ServiceGroups 6.15s
      [+] Can retrieve both local and universal ServiceGroup 3.2s
      [+] Can retreive a group by name 3.22s
      [+] Can retreive a local service group by name with scopeid 1.87s
      [+] Can retreive a local service group by id 601ms

    Context Universal ServiceGroup retrieval
      [!] Can retrieve all universal ServiceGroups 488ms
      [!] Can retrieve both local and universal ServiceGroup 31ms
      [!] Can retreive all service groups by name 5ms
      [!] Can retreive a universal service group by name with scopeid 6ms
      [!] Can retreive a universal service group by id 6ms

    Context Successful Local Service Group Creation
      [+] Can create a service group 2.26s
      [+] Can create a service group with inheritance 2.13s
      [+] Can create a service group and return an objectId only 278ms
      [-] Creates only a single servicegroup when used as the first part of a pipeline (#347) 2.4s
        WebException: The remote server returned an error: (400) Bad Request.
        InternalNsxApiException: Invoke-NsxWebRequest : The NSX API response received indicates a f
ailure. 400 : Bad Request : Response Body: <?xml version="1.0" encoding="UTF-8"?>
        <error><details>Universal objects cannot be created when NSX Manager is in STANDALONE role.
</details><errorCode>261</errorCode><moduleName>core-services</moduleName></error>
        at Invoke-NsxWebRequest, C:\work\2018\powernsx-master\PowerNSX.psm1: line 4330
        at New-NsxServiceGroup<Process>, C:\work\2018\powernsx-master\PowerNSX.psm1: line 26597
        at <ScriptBlock>, C:\work\2018\powernsx-master\tests\integration\09.ServiceGroups.Tests.ps1
: line 214
        at Get-NsxServiceGroup<Process>, C:\work\2018\powernsx-master\PowerNSX.psm1: line 26331
        at <ScriptBlock>, C:\work\2018\powernsx-master\tests\integration\09.ServiceGroups.Tests.ps1
: line 214

    Context Successful Universal Service Group Creation
      [-] Can create a universal service group 6.34s
        WebException: The remote server returned an error: (400) Bad Request.
        InternalNsxApiException: Invoke-NsxWebRequest : The NSX API response received indicates a f
ailure. 400 : Bad Request : Response Body: <?xml version="1.0" encoding="UTF-8"?>
        <error><details>Universal objects cannot be created when NSX Manager is in STANDALONE role.
</details><errorCode>261</errorCode><moduleName>core-services</moduleName></error>
        at Invoke-NsxWebRequest, C:\work\2018\powernsx-master\PowerNSX.psm1: line 4330
        at New-NsxServiceGroup<Process>, C:\work\2018\powernsx-master\PowerNSX.psm1: line 26597
        at <ScriptBlock>, C:\work\2018\powernsx-master\tests\integration\09.ServiceGroups.Tests.ps1
: line 230

    Context Service Group Deletion
      [+] Can delete a servicegroup by object 2.03s

    Context Applicable Members
      [-] Error occurred in Context block 10.15s
        WebException: The remote server returned an error: (400) Bad Request.
        InternalNsxApiException: Invoke-NsxWebRequest : The NSX API response received indicates a f
ailure. 400 : Bad Request : Response Body: <?xml version="1.0" encoding="UTF-8"?>
        <error><details>Universal objects cannot be created when NSX Manager is in STANDALONE role.
</details><errorCode>261</errorCode><moduleName>core-services</moduleName></error>
        at Invoke-NsxWebRequest, C:\work\2018\powernsx-master\PowerNSX.psm1: line 4330
        at New-NsxServiceGroup<Process>, C:\work\2018\powernsx-master\PowerNSX.psm1: line 26597
        at <ScriptBlock>, C:\work\2018\powernsx-master\tests\integration\09.ServiceGroups.Tests.ps1
: line 277
        at Invoke-Blocks, C:\Program Files\WindowsPowerShell\Modules\Pester\4.1.1\Functions\SetupTe
ardown.ps1: line 140
        at Invoke-TestGroupSetupBlocks, C:\Program Files\WindowsPowerShell\Modules\Pester\4.1.1\Fun
ctions\SetupTeardown.ps1: line 125
        at DescribeImpl, C:\Program Files\WindowsPowerShell\Modules\Pester\4.1.1\Functions\Describe
.ps1: line 157
Tests completed in 41.18s
Tests Passed: 9, Failed: 3, Skipped: 5, Pending: 0, Inconclusive: 0 

PS C:\work\2018\powernsx-master> ```

alagoutte · 2018-01-23T12:53:20Z

You need to pass you NSX Manager to Primary

nmbradford · 2018-01-23T23:12:27Z

adding to what @alagoutte said (he is correct, the manager needs to be configured as primary role) but there is something wrong with the way the tests are executing. The environment tests should detect and block this but I can see they arent being executed (nothing after Executing all tests in '.' with Tags Environment). Can you ensure you are in the tests directory when you import the module? (import-module ./test.psd1) and see if you get a failure in the environment tests about not having primary role?

nmbradford

Thanks for PR! mostly example/help text related comments, but a few things to change.

nmbradford · 2018-01-29T07:04:35Z

module/PowerNSX.psm1

+    <#
+    .SYNOPSIS
+    Removes a single Service, numerous Services, or a Service Group to a Service
+    Group


Reword : Removes one or more services or service groups from a service group.

nmbradford · 2018-01-29T07:05:10Z

module/PowerNSX.psm1

+    Group
+
+    .DESCRIPTION
+    Removes the defined Service or Service Group to an NSX Service Groups. Service


from an NSX Service Group_

nmbradford · 2018-01-29T07:06:56Z

module/PowerNSX.psm1

+    groups contain a mixture of selected ports to represent a potential
+    grouping of like ports.
+
+    This cmdlet removes the defined Services or Service Groups within a Service


This seems unclear (all service groups? If you are implying that user can pass all servicegroups on the pipeline, thats a given...) - reword?

nmbradford · 2018-01-29T07:08:31Z

module/PowerNSX.psm1

+    .EXAMPLE
+    Get-NsxServiceGroup Heartbeat | Remove-NsxServiceGroupMember -Member $Service1
+
+    Remove all saved Service Group Members named Heartbeat in Service Group in $Service1


description is not clear. and you probably should show how you get $service1. Reword to something like 'Gets the service fred and removes it from the service group Heartbeat.

nmbradford · 2018-01-29T07:09:27Z

module/PowerNSX.psm1

+
+    Remove all saved Service Group Members named Heartbeat in Service Group in $Service1
+
+    Get-NsxServiceGroup Service-Group-4 | Remove-NsxServiceGroupMember $Service1,$Service2 -Confirm:$false


Needs a new .EXAMPLE header.

nmbradford · 2018-01-29T07:10:07Z

module/PowerNSX.psm1

+
+    Get-NsxServiceGroup Service-Group-4 | Remove-NsxServiceGroupMember $Service1,$Service2 -Confirm:$false
+
+    Remove all saved Service Group Members named Heartbeat in Service Group in $Service1 and $Service2 without prompting for confirmation


again the comment is not so clear. There is no mention of heartbeat in the cli example?

nmbradford · 2018-01-29T07:13:15Z

module/PowerNSX.psm1

+            [switch]$Confirm=$true,
+        [Parameter (Mandatory=$true,Position=1)]
+            [ValidateScript({ ValidateServiceOrServiceGroup $_ })]
+            #The [] in XmlElement means it can expect more than one object!


Avoid programmer comments in the param block - these are visible to the user in get-help and should be used to explain what the param is. you can tell the user that you accept a collection if you want but do it in a user centric way.

nmbradford · 2018-01-29T07:15:42Z

module/PowerNSX.psm1

+            if ($decision -eq 0) {
+                $URI = "/api/2.0/services/applicationgroup/$($ServiceGroup.objectId)/members/$($Mem.objectId)"
+                $null = invoke-nsxwebrequest -method "DELETE" -uri $URI -connection $connection
+                Write-Progress -activity "Removing Service or Service Group $($Mem) to Service Group $($ServiceGroup)"


write-progress needs to have a 'before' and 'after' write-progress (with the latter specifying -completed) otherwise you get 'hanging' progress dialog.

Add Remove-NsxServiceGroupMember cmdlet

e4c63fc

alagoutte reviewed Jan 19, 2018

View reviewed changes

module/PowerNSX.psm1 Outdated

begin {}

process {

foreach ($Mem in $Member){

Copy link

Contributor

alagoutte Jan 19, 2018

Choose a reason for hiding this comment

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

May be add a confirm check ?

Fix confirmation issue

2f8dcbd

Ruch Grégory added 3 commits January 23, 2018 10:42

fix whitespace issue

cb0aac6

fix whitespace issue

eac4d43

fix whitespace issue

f58ba92

nmbradford suggested changes Jan 29, 2018

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Remove-NsxServiceGroupMember cmdlet #448

Add Remove-NsxServiceGroupMember cmdlet #448

ruchg1337 commented Jan 17, 2018

vmwclabot commented Jan 17, 2018

vmwclabot commented Jan 18, 2018

alagoutte Jan 19, 2018

alagoutte Jan 19, 2018

alagoutte Jan 19, 2018

alagoutte commented Jan 19, 2018

ruchg1337 commented Jan 23, 2018 •

edited

Loading

alagoutte commented Jan 23, 2018

nmbradford commented Jan 23, 2018

nmbradford left a comment

nmbradford Jan 29, 2018

nmbradford Jan 29, 2018

nmbradford Jan 29, 2018

nmbradford Jan 29, 2018

nmbradford Jan 29, 2018

nmbradford Jan 29, 2018

nmbradford Jan 29, 2018

nmbradford Jan 29, 2018


		Remove all saved Service Group Members named Heartbeat in Service Group in $Service1

		Get-NsxServiceGroup Service-Group-4 \| Remove-NsxServiceGroupMember $Service1,$Service2 -Confirm:$false

Add Remove-NsxServiceGroupMember cmdlet #448

Are you sure you want to change the base?

Add Remove-NsxServiceGroupMember cmdlet #448

Conversation

ruchg1337 commented Jan 17, 2018

vmwclabot commented Jan 17, 2018

vmwclabot commented Jan 18, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alagoutte commented Jan 19, 2018

ruchg1337 commented Jan 23, 2018 • edited Loading

alagoutte commented Jan 23, 2018

nmbradford commented Jan 23, 2018

nmbradford left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ruchg1337 commented Jan 23, 2018 •

edited

Loading