Merge lp:~axwalk/gwacl/add-delete-role into lp:gwacl

[Andrew Wilkins]

Introduce AddRole, DeleteRole; add missing Role fields

Two more management service API methods are added: AddRole
and DeleteRole (operating on deployments). These are
required when working with Availability Sets, where a role
must be added to an existing Cloud Service.

PersistentVMRole has been changed to be an alias for Role,
which has been expanded to include missing fields.
PersistentVMRole continues to have its own serialisation
methods, so it gets the correct XML tag name and namespace.

Roles can only have a single OS disk, so I've changed the
signature of NewRole to reflect this. Roles may have
additional data disks.

Introduce AddRole, DeleteRole; add missing Role fields

Two more management service API methods are added: AddRole
and DeleteRole (operating on deployments). These are
required when working with Availability Sets, where a role
must be added to an existing Cloud Service.

PersistentVMRole has been changed to be an alias for Role,
which has been expanded to include missing fields.
PersistentVMRole continues to have its own serialisation
methods, so it gets the correct XML tag name and namespace.

Roles can only have a single OS disk, so I've changed the
signature of NewRole to reflect this. Roles may have
additional data disks.

[Ian Booth (wallyworld)]

I think the AddRole test accidentally uses the update role stuff.

Not sure if we want to put this

"services/hostedservices/" + request.ServiceName + "/deployments/" + request.DeploymentName + "/roles/"

in a helper function

[Andrew Wilkins (axwalk)]

Doh, thanks, updated.

[Ian Booth (wallyworld)]

With test fix

review: Approve

[Go Bot (go-bot)]

The prerequisite https://code.launchpad.net/~axwalk/gwacl/retry-rewind-request-body has not yet been merged into https://code.launchpad.net/~gwacl-hackers/gwacl/trunk.

=== modified file 'example/management/run.go'
--- example/management/run.go	2013-10-31 08:10:35 +0000
+++ example/management/run.go	2014-03-11 03:33:04 +0000
@@ -214,9 +214,8 @@
     diskLabel := makeRandomIdentifier("gwacl", 64)
     vhd := gwacl.NewOSVirtualHardDisk("", diskLabel, diskName, mediaLink, sourceImageName, "Linux")
     roleName := gwacl.MakeRandomRoleName("gwaclrole")
-    role := gwacl.NewRole("ExtraSmall", roleName,
-        []gwacl.ConfigurationSet{*linuxConfigurationSet, *networkConfigurationSet},
-        []gwacl.OSVirtualHardDisk{*vhd})
+    role := gwacl.NewRole("ExtraSmall", roleName, vhd,
+        []gwacl.ConfigurationSet{*linuxConfigurationSet, *networkConfigurationSet})
     machineName := makeRandomIdentifier("gwaclmachine", 20)
     deployment := gwacl.NewDeploymentForCreateVMDeployment(
         machineName, "Production", machineName, []gwacl.Role{*role}, virtualNetworkName)

=== modified file 'management.go'
--- management.go	2013-08-28 00:13:04 +0000
+++ management.go	2014-03-11 03:33:04 +0000
@@ -140,9 +140,7 @@
     // 1. Get the list of the VM disks.
     diskNameMap := make(map[string]bool)
     for _, role := range deployment.RoleList {
-        for _, osVHD := range role.OSVirtualHardDisk {
-            diskNameMap[osVHD.DiskName] = true
-        }
+        diskNameMap[role.OSVirtualHardDisk.DiskName] = true
     }
     // 2. Delete deployment.  This will delete all the role instances inside
     // this deployment as a side effect.

=== modified file 'management_base.go'
--- management_base.go	2013-10-31 04:16:38 +0000
+++ management_base.go	2014-03-11 03:33:04 +0000
@@ -474,6 +474,57 @@
     return api.blockUntilCompleted(response)
 }
 
+type DeleteRoleRequest struct {
+    ServiceName    string
+    DeploymentName string
+    RoleName       string
+    DeleteMedia    bool
+}
+
+// DeleteRole deletes a named Role from within the specified Cloud Service
+// and Deployment.
+// See http://msdn.microsoft.com/en-us/library/windowsazure/jj157187.aspx
+func (api *ManagementAPI) DeleteRole(request *DeleteRoleRequest) error {
+    checkPathComponents(request.ServiceName, request.DeploymentName, request.RoleName)
+    url := ("services/hostedservices/" + request.ServiceName +
+        "/deployments/" + request.DeploymentName + "/roles/" + request.RoleName)
+    if request.DeleteMedia {
+        url = addURLQueryParams(url, "comp", "media")
+    }
+    response, err := api.session.delete(url, "2013-08-01")
+    if err != nil {
+        if IsNotFoundError(err) {
+            return nil
+        }
+        return err
+    }
+    return api.blockUntilCompleted(response)
+}
+
+type AddRoleRequest struct {
+    ServiceName      string
+    DeploymentName   string
+    PersistentVMRole *PersistentVMRole
+}
+
+// AddRole creates a new Role within the specified Cloud Service
+// and Deployment.
+// See http://msdn.microsoft.com/en-us/library/windowsazure/jj157187.aspx
+func (api *ManagementAPI) AddRole(request *AddRoleRequest) error {
+    checkPathComponents(request.ServiceName, request.DeploymentName)
+    url := ("services/hostedservices/" + request.ServiceName +
+        "/deployments/" + request.DeploymentName + "/roles")
+    role, err := request.PersistentVMRole.Serialize()
+    if err != nil {
+        return err
+    }
+    response, err := api.session.post(url, "2013-10-01", []byte(role), "application/xml")
+    if err != nil {
+        return err
+    }
+    return api.blockUntilCompleted(response)
+}
+
 type CreateAffinityGroupRequest struct {
     CreateAffinityGroup *CreateAffinityGroup
 }

=== modified file 'management_base_test.go'
--- management_base_test.go	2013-10-31 04:16:38 +0000
+++ management_base_test.go	2014-03-11 03:33:04 +0000
@@ -681,7 +681,7 @@
     serviceName := "serviceName"
     configurationSet := NewLinuxProvisioningConfigurationSet("testHostname12345", "test", "test123#@!", "user-data", "false")
     vhd := NewOSVirtualHardDisk("hostCaching", "diskLabel", "diskName", "http://mediaLink", "sourceImageName", "os")
-    role := NewRole("ExtraSmall", "test-role-123", []ConfigurationSet{*configurationSet}, []OSVirtualHardDisk{*vhd})
+    role := NewRole("ExtraSmall", "test-role-123", vhd, []ConfigurationSet{*configurationSet})
     deployment := NewDeploymentForCreateVMDeployment("test-machine-name", "Staging", "testLabel", []Role{*role}, "testNetwork")
     err := api.AddDeployment(deployment, serviceName)
 
@@ -1109,6 +1109,76 @@
     c.Check(recordedRequests[1].URL, Matches, ".*/operations/foobar")
 }
 
+func assertDeleteRoleRequest(c *C, api *ManagementAPI, httpRequest *X509Request, serviceName, deploymentName, roleName string, deleteMedia bool) {
+    expectedURL := (defaultManagement + api.session.subscriptionId +
+        "/services/hostedservices/" +
+        serviceName + "/deployments/" + deploymentName + "/roles/" + roleName)
+    if deleteMedia {
+        expectedURL += "?comp=media"
+    }
+    checkRequest(
+        c, httpRequest, expectedURL, "2013-08-01", nil, "DELETE")
+}
+
+func (suite *managementBaseAPISuite) TestDeleteRole(c *C) {
+    suite.testDeleteRole(c, false)
+}
+
+func (suite *managementBaseAPISuite) TestDeleteRoleDeleteMedia(c *C) {
+    suite.testDeleteRole(c, true)
+}
+
+func (suite *managementBaseAPISuite) testDeleteRole(c *C, deleteMedia bool) {
+    api := makeAPI(c)
+    request := &DeleteRoleRequest{
+        ServiceName:    "serviceName",
+        DeploymentName: "deploymentName",
+        RoleName:       "roleName",
+        DeleteMedia:    deleteMedia,
+    }
+    rigFixedResponseDispatcher(&x509Response{StatusCode: http.StatusOK})
+    recordedRequests := make([]*X509Request, 0)
+    rigRecordingDispatcher(&recordedRequests)
+
+    err := api.DeleteRole(request)
+    c.Assert(err, IsNil)
+    assertDeleteRoleRequest(
+        c, api, recordedRequests[0], request.ServiceName,
+        request.DeploymentName, request.RoleName, deleteMedia)
+}
+
+func assertAddRoleRequest(c *C, api *ManagementAPI, httpRequest *X509Request, serviceName, deploymentName, expectedXML string) {
+    expectedURL := (defaultManagement + api.session.subscriptionId +
+        "/services/hostedservices/" +
+        serviceName + "/deployments/" + deploymentName + "/roles")
+    checkRequest(
+        c, httpRequest, expectedURL, "2013-10-01", []byte(expectedXML), "POST")
+    c.Assert(httpRequest.ContentType, Equals, "application/xml")
+}
+
+func (suite *managementBaseAPISuite) TestAddRole(c *C) {
+    api := makeAPI(c)
+    request := &AddRoleRequest{
+        ServiceName:    "serviceName",
+        DeploymentName: "deploymentName",
+        PersistentVMRole: &PersistentVMRole{
+            RoleName: "newRoleNamePerhaps",
+        },
+    }
+    rigFixedResponseDispatcher(&x509Response{StatusCode: http.StatusOK})
+    recordedRequests := make([]*X509Request, 0)
+    rigRecordingDispatcher(&recordedRequests)
+
+    err := api.AddRole(request)
+    c.Assert(err, IsNil)
+
+    expectedXML, err := request.PersistentVMRole.Serialize()
+    c.Assert(err, IsNil)
+    assertAddRoleRequest(
+        c, api, recordedRequests[0], request.ServiceName,
+        request.DeploymentName, expectedXML)
+}
+
 func (suite *managementBaseAPISuite) TestCreateAffinityGroup(c *C) {
     api := makeAPI(c)
     cag := NewCreateAffinityGroup(

=== modified file 'management_test.go'
--- management_test.go	2013-08-28 00:13:04 +0000
+++ management_test.go	2014-03-11 03:33:04 +0000
@@ -265,10 +265,8 @@
     return &Deployment{
         RoleInstanceList: makeNamedRoleInstances("one", "two"),
         RoleList: []Role{
-            {OSVirtualHardDisk: []OSVirtualHardDisk{
-                {DiskName: "disk1"}, {DiskName: "disk2"}}},
-            {OSVirtualHardDisk: []OSVirtualHardDisk{
-                {DiskName: "disk1"}, {DiskName: "disk3"}}},
+            {OSVirtualHardDisk: &OSVirtualHardDisk{DiskName: "disk1"}},
+            {OSVirtualHardDisk: &OSVirtualHardDisk{DiskName: "disk2"}},
         },
     }
 }
@@ -292,14 +290,13 @@
     }
     err := api.DestroyDeployment(request)
     c.Assert(err, IsNil)
-    c.Check(record, HasLen, 5)
+    c.Check(record, HasLen, 4)
     assertGetDeploymentRequest(c, api, &GetDeploymentRequest{
         request.ServiceName, request.DeploymentName}, record[0])
     assertDeleteDeploymentRequest(c, api, request.ServiceName,
         request.DeploymentName, record[1])
     assertDeleteDiskRequest(c, api, "disk1", record[2], true)
     assertDeleteDiskRequest(c, api, "disk2", record[3], true)
-    assertDeleteDiskRequest(c, api, "disk3", record[4], true)
 }
 
 func (suite *suiteDestroyDeployment) TestOkayWhenDeploymentNotFound(c *C) {
@@ -338,14 +335,13 @@
     }
     err := api.DestroyDeployment(request)
     c.Assert(err, IsNil)
-    c.Check(record, HasLen, 5)
+    c.Check(record, HasLen, 4)
     assertGetDeploymentRequest(c, api, &GetDeploymentRequest{
         request.ServiceName, request.DeploymentName}, record[0])
     assertDeleteDeploymentRequest(c, api, request.ServiceName,
         request.DeploymentName, record[1])
     assertDeleteDiskRequest(c, api, "disk1", record[2], true)
     assertDeleteDiskRequest(c, api, "disk2", record[3], true)
-    assertDeleteDiskRequest(c, api, "disk3", record[4], true)
 }
 
 func (suite *suiteDestroyDeployment) TestFailsGettingDeployment(c *C) {
@@ -393,7 +389,7 @@
     exampleDeployment := suite.makeExampleDeployment()
     responses = append(responses, makeOKXMLResponse(c, exampleDeployment)...)
     // For deleting disks.
-    responses = append(responses, exampleOkayResponse, exampleOkayResponse, exampleOkayResponse)
+    responses = append(responses, exampleOkayResponse, exampleOkayResponse)
     // For other requests.
     responses = append(responses, exampleFailResponse)
     record := []*X509Request{}
@@ -407,7 +403,7 @@
     err := api.DestroyDeployment(request)
     c.Assert(err, NotNil)
     c.Check(err, ErrorMatches, "DELETE request failed [(]500: Internal Server Error[)]")
-    c.Check(record, HasLen, 5)
+    c.Check(record, HasLen, 4)
 }
 
 type suiteDestroyHostedService struct{}

=== modified file 'xmlobjects.go'
--- xmlobjects.go	2013-10-31 07:59:14 +0000
+++ xmlobjects.go	2014-03-11 03:33:04 +0000
@@ -109,7 +109,7 @@
 //
 
 type LoadBalancerProbe struct {
-    Path     string `xml:"Path"`
+    Path     string `xml:"Path,omitempty"`
     Port     int    `xml:"Port"` // Not uint16; see https://bugs.launchpad.net/juju-core/+bug/1201880
     Protocol string `xml:"Protocol"`
 }
@@ -310,11 +310,16 @@
 }
 
 type Role struct {
-    RoleName          string              `xml:"RoleName"`
-    RoleType          string              `xml:"RoleType"` // Always "PersistentVMRole"
-    ConfigurationSets []ConfigurationSet  `xml:"ConfigurationSets>ConfigurationSet"`
-    OSVirtualHardDisk []OSVirtualHardDisk `xml:"OSVirtualHardDisk"`
-    RoleSize          string              `xml:"RoleSize"`
+    XMLNS                             string                 `xml:"xmlns,attr,omitempty"`
+    RoleName                          string                 `xml:"RoleName"`
+    OsVersion                         string                 `xml:"OsVersion,omitempty"`
+    RoleType                          string                 `xml:"RoleType"` // Always "PersistentVMRole"
+    ConfigurationSets                 []ConfigurationSet     `xml:"ConfigurationSets>ConfigurationSet"`
+    AvailabilitySetName               string                 `xml:"AvailabilitySetName,omitempty"`
+    DataVirtualHardDisks              *[]DataVirtualHardDisk `xml:"DataVirtualHardDisks>DataVirtualHardDisk,omitempty"`
+    OSVirtualHardDisk                 *OSVirtualHardDisk     `xml:"OSVirtualHardDisk,omitempty"`
+    RoleSize                          string                 `xml:"RoleSize"`
+    DefaultWinRmCertificateThumbprint string                 `xml:"DefaultWinRmCertificateThumbprint,omitempty"`
 }
 
 //
@@ -325,14 +330,13 @@
     return toxml(c)
 }
 
-func NewRole(RoleSize string, RoleName string,
-    ConfigurationSets []ConfigurationSet, vhds []OSVirtualHardDisk) *Role {
+func NewRole(RoleSize, RoleName string, vhd *OSVirtualHardDisk, ConfigurationSets []ConfigurationSet) *Role {
     return &Role{
         RoleSize:          RoleSize,
         RoleName:          RoleName,
         RoleType:          "PersistentVMRole",
         ConfigurationSets: ConfigurationSets,
-        OSVirtualHardDisk: vhds,
+        OSVirtualHardDisk: vhd,
     }
 }
 
@@ -600,24 +604,18 @@
 //
 // PersistentVMRole, as used by GetRole, UpdateRole, etc.
 //
-type PersistentVMRole struct {
-    XMLNS                             string                 `xml:"xmlns,attr"`
-    RoleName                          string                 `xml:"RoleName"`
-    OsVersion                         string                 `xml:"OsVersion"`
-    RoleType                          string                 `xml:"RoleType"` // Always PersistentVMRole
-    ConfigurationSets                 []ConfigurationSet     `xml:"ConfigurationSets>ConfigurationSet"`
-    AvailabilitySetName               string                 `xml:"AvailabilitySetName"`
-    DataVirtualHardDisks              *[]DataVirtualHardDisk `xml:"DataVirtualHardDisks>DataVirtualHardDisk,omitempty"`
-    OSVirtualHardDisk                 OSVirtualHardDisk      `xml:"OSVirtualHardDisk"`
-    RoleSize                          string                 `xml:"RoleSize"`
-    DefaultWinRmCertificateThumbprint string                 `xml:"DefaultWinRmCertificateThumbprint"`
-}
+type PersistentVMRole Role
 
 func (role *PersistentVMRole) Deserialize(data []byte) error {
     return xml.Unmarshal(data, role)
 }
 
 func (role *PersistentVMRole) Serialize() (string, error) {
+    if role.XMLNS == "" {
+        clone := *role
+        clone.XMLNS = XMLNS
+        role = &clone
+    }
     return toxml(role)
 }
 

=== modified file 'xmlobjects_test.go'
--- xmlobjects_test.go	2013-10-31 07:59:14 +0000
+++ xmlobjects_test.go	2014-03-11 03:33:04 +0000
@@ -291,7 +291,7 @@
                 MediaLink:           "path-to-vhd",
             },
         },
-        OSVirtualHardDisk: OSVirtualHardDisk{
+        OSVirtualHardDisk: &OSVirtualHardDisk{
             HostCaching:     "host-caching-mode-of-os-disk",
             DiskName:        "name-of-os-disk",
             MediaLink:       "path-to-vhd",
@@ -360,7 +360,7 @@
                 MediaLink:           "path-to-vhd",
             },
         },
-        OSVirtualHardDisk: OSVirtualHardDisk{
+        OSVirtualHardDisk: &OSVirtualHardDisk{
             HostCaching:     "host-caching-mode-of-os-disk",
             DiskName:        "name-of-os-disk",
             MediaLink:       "path-to-vhd",
@@ -922,7 +922,8 @@
         UpgradeDomainCount: "number-of-upgrade-domains-in-deployment",
         RoleList: []Role{
             {
-                RoleName: "name-of-role",
+                RoleName:  "name-of-role",
+                OsVersion: "operating-system-version",
                 ConfigurationSets: []ConfigurationSet{
                     {
                         ConfigurationSetType:             "LinuxProvisioningConfiguration",
@@ -931,8 +932,9 @@
                 },
             },
             {
-                RoleName: "name-of-role",
-                RoleType: "PersistentVMRole",
+                RoleName:  "name-of-role",
+                OsVersion: "operating-system-version",
+                RoleType:  "PersistentVMRole",
                 ConfigurationSets: []ConfigurationSet{
                     {
                         ConfigurationSetType: CONFIG_SET_NETWORK,
@@ -947,15 +949,23 @@
                         SubnetNames: &[]string{"name-of-subnet"},
                     },
                 },
-                OSVirtualHardDisk: []OSVirtualHardDisk{
+                AvailabilitySetName: "name-of-availability-set",
+                DataVirtualHardDisks: &[]DataVirtualHardDisk{
                     {
-                        HostCaching:     "host-caching-mode-of-os-disk",
-                        DiskName:        "name-of-os-disk",
-                        MediaLink:       "path-to-vhd",
-                        SourceImageName: "image-used-to-create-os-disk",
-                        OS:              "operating-system-on-os-disk",
+                        HostCaching:         "host-caching-mode-of-data-disk",
+                        DiskName:            "name-of-data-disk",
+                        LUN:                 "logical-unit-number-of-data-disk",
+                        LogicalDiskSizeInGB: "size-of-data-disk",
+                        MediaLink:           "path-to-vhd",
                     },
                 },
+                OSVirtualHardDisk: &OSVirtualHardDisk{
+                    HostCaching:     "host-caching-mode-of-os-disk",
+                    DiskName:        "name-of-os-disk",
+                    MediaLink:       "path-to-vhd",
+                    SourceImageName: "image-used-to-create-os-disk",
+                    OS:              "operating-system-on-os-disk",
+                },
                 RoleSize: "size-of-instance",
             },
         },
@@ -1030,7 +1040,7 @@
     deploymentSlot := "staging"
     label := "deploymentLabel"
     vhd := NewOSVirtualHardDisk("hostCaching", "diskLabel", "diskName", "mediaLink", "sourceImageName", "os")
-    roles := []Role{*NewRole("size", "name", []ConfigurationSet{}, []OSVirtualHardDisk{*vhd})}
+    roles := []Role{*NewRole("size", "name", vhd, []ConfigurationSet{})}
     virtualNetworkName := "network"
 
     deployment := NewDeploymentForCreateVMDeployment(name, deploymentSlot, label, roles, virtualNetworkName)
@@ -1303,7 +1313,7 @@
     configset := []ConfigurationSet{*config}
     vhd := NewOSVirtualHardDisk("hostCaching", "diskLabel", "diskName", "mediaLink", "sourceImageName", "os")
 
-    role := NewRole(rolesize, rolename, configset, []OSVirtualHardDisk{*vhd})
+    role := NewRole(rolesize, rolename, vhd, configset)
     c.Check(role.RoleSize, Equals, rolesize)
     c.Check(role.RoleName, Equals, rolename)
     c.Check(role.ConfigurationSets, DeepEquals, configset)