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

Proposed by Andrew Wilkins on 2014-03-10
Status: Merged
Approved by: Andrew Wilkins on 2014-03-11
Approved revision: 234
Merged at revision: 233
Proposed branch: lp:~axwalk/gwacl/add-delete-role
Merge into: lp:gwacl
Prerequisite: lp:~axwalk/gwacl/retry-rewind-request-body
Diff against target: 424 lines (+173/-51)
7 files modified
example/management/run.go (+2/-3)
management.go (+1/-3)
management_base.go (+51/-0)
management_base_test.go (+71/-1)
management_test.go (+6/-10)
xmlobjects.go (+19/-21)
xmlobjects_test.go (+23/-13)
To merge this branch: bzr merge lp:~axwalk/gwacl/add-delete-role
Reviewer Review Type Date Requested Status
Ian Booth 2014-03-10 Approve on 2014-03-11
Review via email: mp+210121@code.launchpad.net

This proposal supersedes a proposal from 2014-03-10.

Commit message

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.

Description of the change

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.

To post a comment you must log in.
Ian Booth (wallyworld) wrote :

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) wrote :

Doh, thanks, updated.

Ian Booth (wallyworld) wrote :

With test fix

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'example/management/run.go'
2--- example/management/run.go 2013-10-31 08:10:35 +0000
3+++ example/management/run.go 2014-03-11 03:33:04 +0000
4@@ -214,9 +214,8 @@
5 diskLabel := makeRandomIdentifier("gwacl", 64)
6 vhd := gwacl.NewOSVirtualHardDisk("", diskLabel, diskName, mediaLink, sourceImageName, "Linux")
7 roleName := gwacl.MakeRandomRoleName("gwaclrole")
8- role := gwacl.NewRole("ExtraSmall", roleName,
9- []gwacl.ConfigurationSet{*linuxConfigurationSet, *networkConfigurationSet},
10- []gwacl.OSVirtualHardDisk{*vhd})
11+ role := gwacl.NewRole("ExtraSmall", roleName, vhd,
12+ []gwacl.ConfigurationSet{*linuxConfigurationSet, *networkConfigurationSet})
13 machineName := makeRandomIdentifier("gwaclmachine", 20)
14 deployment := gwacl.NewDeploymentForCreateVMDeployment(
15 machineName, "Production", machineName, []gwacl.Role{*role}, virtualNetworkName)
16
17=== modified file 'management.go'
18--- management.go 2013-08-28 00:13:04 +0000
19+++ management.go 2014-03-11 03:33:04 +0000
20@@ -140,9 +140,7 @@
21 // 1. Get the list of the VM disks.
22 diskNameMap := make(map[string]bool)
23 for _, role := range deployment.RoleList {
24- for _, osVHD := range role.OSVirtualHardDisk {
25- diskNameMap[osVHD.DiskName] = true
26- }
27+ diskNameMap[role.OSVirtualHardDisk.DiskName] = true
28 }
29 // 2. Delete deployment. This will delete all the role instances inside
30 // this deployment as a side effect.
31
32=== modified file 'management_base.go'
33--- management_base.go 2013-10-31 04:16:38 +0000
34+++ management_base.go 2014-03-11 03:33:04 +0000
35@@ -474,6 +474,57 @@
36 return api.blockUntilCompleted(response)
37 }
38
39+type DeleteRoleRequest struct {
40+ ServiceName string
41+ DeploymentName string
42+ RoleName string
43+ DeleteMedia bool
44+}
45+
46+// DeleteRole deletes a named Role from within the specified Cloud Service
47+// and Deployment.
48+// See http://msdn.microsoft.com/en-us/library/windowsazure/jj157187.aspx
49+func (api *ManagementAPI) DeleteRole(request *DeleteRoleRequest) error {
50+ checkPathComponents(request.ServiceName, request.DeploymentName, request.RoleName)
51+ url := ("services/hostedservices/" + request.ServiceName +
52+ "/deployments/" + request.DeploymentName + "/roles/" + request.RoleName)
53+ if request.DeleteMedia {
54+ url = addURLQueryParams(url, "comp", "media")
55+ }
56+ response, err := api.session.delete(url, "2013-08-01")
57+ if err != nil {
58+ if IsNotFoundError(err) {
59+ return nil
60+ }
61+ return err
62+ }
63+ return api.blockUntilCompleted(response)
64+}
65+
66+type AddRoleRequest struct {
67+ ServiceName string
68+ DeploymentName string
69+ PersistentVMRole *PersistentVMRole
70+}
71+
72+// AddRole creates a new Role within the specified Cloud Service
73+// and Deployment.
74+// See http://msdn.microsoft.com/en-us/library/windowsazure/jj157187.aspx
75+func (api *ManagementAPI) AddRole(request *AddRoleRequest) error {
76+ checkPathComponents(request.ServiceName, request.DeploymentName)
77+ url := ("services/hostedservices/" + request.ServiceName +
78+ "/deployments/" + request.DeploymentName + "/roles")
79+ role, err := request.PersistentVMRole.Serialize()
80+ if err != nil {
81+ return err
82+ }
83+ response, err := api.session.post(url, "2013-10-01", []byte(role), "application/xml")
84+ if err != nil {
85+ return err
86+ }
87+ return api.blockUntilCompleted(response)
88+}
89+
90 type CreateAffinityGroupRequest struct {
91 CreateAffinityGroup *CreateAffinityGroup
92 }
93
94=== modified file 'management_base_test.go'
95--- management_base_test.go 2013-10-31 04:16:38 +0000
96+++ management_base_test.go 2014-03-11 03:33:04 +0000
97@@ -681,7 +681,7 @@
98 serviceName := "serviceName"
99 configurationSet := NewLinuxProvisioningConfigurationSet("testHostname12345", "test", "test123#@!", "user-data", "false")
100 vhd := NewOSVirtualHardDisk("hostCaching", "diskLabel", "diskName", "http://mediaLink", "sourceImageName", "os")
101- role := NewRole("ExtraSmall", "test-role-123", []ConfigurationSet{*configurationSet}, []OSVirtualHardDisk{*vhd})
102+ role := NewRole("ExtraSmall", "test-role-123", vhd, []ConfigurationSet{*configurationSet})
103 deployment := NewDeploymentForCreateVMDeployment("test-machine-name", "Staging", "testLabel", []Role{*role}, "testNetwork")
104 err := api.AddDeployment(deployment, serviceName)
105
106@@ -1109,6 +1109,76 @@
107 c.Check(recordedRequests[1].URL, Matches, ".*/operations/foobar")
108 }
109
110+func assertDeleteRoleRequest(c *C, api *ManagementAPI, httpRequest *X509Request, serviceName, deploymentName, roleName string, deleteMedia bool) {
111+ expectedURL := (defaultManagement + api.session.subscriptionId +
112+ "/services/hostedservices/" +
113+ serviceName + "/deployments/" + deploymentName + "/roles/" + roleName)
114+ if deleteMedia {
115+ expectedURL += "?comp=media"
116+ }
117+ checkRequest(
118+ c, httpRequest, expectedURL, "2013-08-01", nil, "DELETE")
119+}
120+
121+func (suite *managementBaseAPISuite) TestDeleteRole(c *C) {
122+ suite.testDeleteRole(c, false)
123+}
124+
125+func (suite *managementBaseAPISuite) TestDeleteRoleDeleteMedia(c *C) {
126+ suite.testDeleteRole(c, true)
127+}
128+
129+func (suite *managementBaseAPISuite) testDeleteRole(c *C, deleteMedia bool) {
130+ api := makeAPI(c)
131+ request := &DeleteRoleRequest{
132+ ServiceName: "serviceName",
133+ DeploymentName: "deploymentName",
134+ RoleName: "roleName",
135+ DeleteMedia: deleteMedia,
136+ }
137+ rigFixedResponseDispatcher(&x509Response{StatusCode: http.StatusOK})
138+ recordedRequests := make([]*X509Request, 0)
139+ rigRecordingDispatcher(&recordedRequests)
140+
141+ err := api.DeleteRole(request)
142+ c.Assert(err, IsNil)
143+ assertDeleteRoleRequest(
144+ c, api, recordedRequests[0], request.ServiceName,
145+ request.DeploymentName, request.RoleName, deleteMedia)
146+}
147+
148+func assertAddRoleRequest(c *C, api *ManagementAPI, httpRequest *X509Request, serviceName, deploymentName, expectedXML string) {
149+ expectedURL := (defaultManagement + api.session.subscriptionId +
150+ "/services/hostedservices/" +
151+ serviceName + "/deployments/" + deploymentName + "/roles")
152+ checkRequest(
153+ c, httpRequest, expectedURL, "2013-10-01", []byte(expectedXML), "POST")
154+ c.Assert(httpRequest.ContentType, Equals, "application/xml")
155+}
156+
157+func (suite *managementBaseAPISuite) TestAddRole(c *C) {
158+ api := makeAPI(c)
159+ request := &AddRoleRequest{
160+ ServiceName: "serviceName",
161+ DeploymentName: "deploymentName",
162+ PersistentVMRole: &PersistentVMRole{
163+ RoleName: "newRoleNamePerhaps",
164+ },
165+ }
166+ rigFixedResponseDispatcher(&x509Response{StatusCode: http.StatusOK})
167+ recordedRequests := make([]*X509Request, 0)
168+ rigRecordingDispatcher(&recordedRequests)
169+
170+ err := api.AddRole(request)
171+ c.Assert(err, IsNil)
172+
173+ expectedXML, err := request.PersistentVMRole.Serialize()
174+ c.Assert(err, IsNil)
175+ assertAddRoleRequest(
176+ c, api, recordedRequests[0], request.ServiceName,
177+ request.DeploymentName, expectedXML)
178+}
179+
180 func (suite *managementBaseAPISuite) TestCreateAffinityGroup(c *C) {
181 api := makeAPI(c)
182 cag := NewCreateAffinityGroup(
183
184=== modified file 'management_test.go'
185--- management_test.go 2013-08-28 00:13:04 +0000
186+++ management_test.go 2014-03-11 03:33:04 +0000
187@@ -265,10 +265,8 @@
188 return &Deployment{
189 RoleInstanceList: makeNamedRoleInstances("one", "two"),
190 RoleList: []Role{
191- {OSVirtualHardDisk: []OSVirtualHardDisk{
192- {DiskName: "disk1"}, {DiskName: "disk2"}}},
193- {OSVirtualHardDisk: []OSVirtualHardDisk{
194- {DiskName: "disk1"}, {DiskName: "disk3"}}},
195+ {OSVirtualHardDisk: &OSVirtualHardDisk{DiskName: "disk1"}},
196+ {OSVirtualHardDisk: &OSVirtualHardDisk{DiskName: "disk2"}},
197 },
198 }
199 }
200@@ -292,14 +290,13 @@
201 }
202 err := api.DestroyDeployment(request)
203 c.Assert(err, IsNil)
204- c.Check(record, HasLen, 5)
205+ c.Check(record, HasLen, 4)
206 assertGetDeploymentRequest(c, api, &GetDeploymentRequest{
207 request.ServiceName, request.DeploymentName}, record[0])
208 assertDeleteDeploymentRequest(c, api, request.ServiceName,
209 request.DeploymentName, record[1])
210 assertDeleteDiskRequest(c, api, "disk1", record[2], true)
211 assertDeleteDiskRequest(c, api, "disk2", record[3], true)
212- assertDeleteDiskRequest(c, api, "disk3", record[4], true)
213 }
214
215 func (suite *suiteDestroyDeployment) TestOkayWhenDeploymentNotFound(c *C) {
216@@ -338,14 +335,13 @@
217 }
218 err := api.DestroyDeployment(request)
219 c.Assert(err, IsNil)
220- c.Check(record, HasLen, 5)
221+ c.Check(record, HasLen, 4)
222 assertGetDeploymentRequest(c, api, &GetDeploymentRequest{
223 request.ServiceName, request.DeploymentName}, record[0])
224 assertDeleteDeploymentRequest(c, api, request.ServiceName,
225 request.DeploymentName, record[1])
226 assertDeleteDiskRequest(c, api, "disk1", record[2], true)
227 assertDeleteDiskRequest(c, api, "disk2", record[3], true)
228- assertDeleteDiskRequest(c, api, "disk3", record[4], true)
229 }
230
231 func (suite *suiteDestroyDeployment) TestFailsGettingDeployment(c *C) {
232@@ -393,7 +389,7 @@
233 exampleDeployment := suite.makeExampleDeployment()
234 responses = append(responses, makeOKXMLResponse(c, exampleDeployment)...)
235 // For deleting disks.
236- responses = append(responses, exampleOkayResponse, exampleOkayResponse, exampleOkayResponse)
237+ responses = append(responses, exampleOkayResponse, exampleOkayResponse)
238 // For other requests.
239 responses = append(responses, exampleFailResponse)
240 record := []*X509Request{}
241@@ -407,7 +403,7 @@
242 err := api.DestroyDeployment(request)
243 c.Assert(err, NotNil)
244 c.Check(err, ErrorMatches, "DELETE request failed [(]500: Internal Server Error[)]")
245- c.Check(record, HasLen, 5)
246+ c.Check(record, HasLen, 4)
247 }
248
249 type suiteDestroyHostedService struct{}
250
251=== modified file 'xmlobjects.go'
252--- xmlobjects.go 2013-10-31 07:59:14 +0000
253+++ xmlobjects.go 2014-03-11 03:33:04 +0000
254@@ -109,7 +109,7 @@
255 //
256
257 type LoadBalancerProbe struct {
258- Path string `xml:"Path"`
259+ Path string `xml:"Path,omitempty"`
260 Port int `xml:"Port"` // Not uint16; see https://bugs.launchpad.net/juju-core/+bug/1201880
261 Protocol string `xml:"Protocol"`
262 }
263@@ -310,11 +310,16 @@
264 }
265
266 type Role struct {
267- RoleName string `xml:"RoleName"`
268- RoleType string `xml:"RoleType"` // Always "PersistentVMRole"
269- ConfigurationSets []ConfigurationSet `xml:"ConfigurationSets>ConfigurationSet"`
270- OSVirtualHardDisk []OSVirtualHardDisk `xml:"OSVirtualHardDisk"`
271- RoleSize string `xml:"RoleSize"`
272+ XMLNS string `xml:"xmlns,attr,omitempty"`
273+ RoleName string `xml:"RoleName"`
274+ OsVersion string `xml:"OsVersion,omitempty"`
275+ RoleType string `xml:"RoleType"` // Always "PersistentVMRole"
276+ ConfigurationSets []ConfigurationSet `xml:"ConfigurationSets>ConfigurationSet"`
277+ AvailabilitySetName string `xml:"AvailabilitySetName,omitempty"`
278+ DataVirtualHardDisks *[]DataVirtualHardDisk `xml:"DataVirtualHardDisks>DataVirtualHardDisk,omitempty"`
279+ OSVirtualHardDisk *OSVirtualHardDisk `xml:"OSVirtualHardDisk,omitempty"`
280+ RoleSize string `xml:"RoleSize"`
281+ DefaultWinRmCertificateThumbprint string `xml:"DefaultWinRmCertificateThumbprint,omitempty"`
282 }
283
284 //
285@@ -325,14 +330,13 @@
286 return toxml(c)
287 }
288
289-func NewRole(RoleSize string, RoleName string,
290- ConfigurationSets []ConfigurationSet, vhds []OSVirtualHardDisk) *Role {
291+func NewRole(RoleSize, RoleName string, vhd *OSVirtualHardDisk, ConfigurationSets []ConfigurationSet) *Role {
292 return &Role{
293 RoleSize: RoleSize,
294 RoleName: RoleName,
295 RoleType: "PersistentVMRole",
296 ConfigurationSets: ConfigurationSets,
297- OSVirtualHardDisk: vhds,
298+ OSVirtualHardDisk: vhd,
299 }
300 }
301
302@@ -600,24 +604,18 @@
303 //
304 // PersistentVMRole, as used by GetRole, UpdateRole, etc.
305 //
306-type PersistentVMRole struct {
307- XMLNS string `xml:"xmlns,attr"`
308- RoleName string `xml:"RoleName"`
309- OsVersion string `xml:"OsVersion"`
310- RoleType string `xml:"RoleType"` // Always PersistentVMRole
311- ConfigurationSets []ConfigurationSet `xml:"ConfigurationSets>ConfigurationSet"`
312- AvailabilitySetName string `xml:"AvailabilitySetName"`
313- DataVirtualHardDisks *[]DataVirtualHardDisk `xml:"DataVirtualHardDisks>DataVirtualHardDisk,omitempty"`
314- OSVirtualHardDisk OSVirtualHardDisk `xml:"OSVirtualHardDisk"`
315- RoleSize string `xml:"RoleSize"`
316- DefaultWinRmCertificateThumbprint string `xml:"DefaultWinRmCertificateThumbprint"`
317-}
318+type PersistentVMRole Role
319
320 func (role *PersistentVMRole) Deserialize(data []byte) error {
321 return xml.Unmarshal(data, role)
322 }
323
324 func (role *PersistentVMRole) Serialize() (string, error) {
325+ if role.XMLNS == "" {
326+ clone := *role
327+ clone.XMLNS = XMLNS
328+ role = &clone
329+ }
330 return toxml(role)
331 }
332
333
334=== modified file 'xmlobjects_test.go'
335--- xmlobjects_test.go 2013-10-31 07:59:14 +0000
336+++ xmlobjects_test.go 2014-03-11 03:33:04 +0000
337@@ -291,7 +291,7 @@
338 MediaLink: "path-to-vhd",
339 },
340 },
341- OSVirtualHardDisk: OSVirtualHardDisk{
342+ OSVirtualHardDisk: &OSVirtualHardDisk{
343 HostCaching: "host-caching-mode-of-os-disk",
344 DiskName: "name-of-os-disk",
345 MediaLink: "path-to-vhd",
346@@ -360,7 +360,7 @@
347 MediaLink: "path-to-vhd",
348 },
349 },
350- OSVirtualHardDisk: OSVirtualHardDisk{
351+ OSVirtualHardDisk: &OSVirtualHardDisk{
352 HostCaching: "host-caching-mode-of-os-disk",
353 DiskName: "name-of-os-disk",
354 MediaLink: "path-to-vhd",
355@@ -922,7 +922,8 @@
356 UpgradeDomainCount: "number-of-upgrade-domains-in-deployment",
357 RoleList: []Role{
358 {
359- RoleName: "name-of-role",
360+ RoleName: "name-of-role",
361+ OsVersion: "operating-system-version",
362 ConfigurationSets: []ConfigurationSet{
363 {
364 ConfigurationSetType: "LinuxProvisioningConfiguration",
365@@ -931,8 +932,9 @@
366 },
367 },
368 {
369- RoleName: "name-of-role",
370- RoleType: "PersistentVMRole",
371+ RoleName: "name-of-role",
372+ OsVersion: "operating-system-version",
373+ RoleType: "PersistentVMRole",
374 ConfigurationSets: []ConfigurationSet{
375 {
376 ConfigurationSetType: CONFIG_SET_NETWORK,
377@@ -947,15 +949,23 @@
378 SubnetNames: &[]string{"name-of-subnet"},
379 },
380 },
381- OSVirtualHardDisk: []OSVirtualHardDisk{
382+ AvailabilitySetName: "name-of-availability-set",
383+ DataVirtualHardDisks: &[]DataVirtualHardDisk{
384 {
385- HostCaching: "host-caching-mode-of-os-disk",
386- DiskName: "name-of-os-disk",
387- MediaLink: "path-to-vhd",
388- SourceImageName: "image-used-to-create-os-disk",
389- OS: "operating-system-on-os-disk",
390+ HostCaching: "host-caching-mode-of-data-disk",
391+ DiskName: "name-of-data-disk",
392+ LUN: "logical-unit-number-of-data-disk",
393+ LogicalDiskSizeInGB: "size-of-data-disk",
394+ MediaLink: "path-to-vhd",
395 },
396 },
397+ OSVirtualHardDisk: &OSVirtualHardDisk{
398+ HostCaching: "host-caching-mode-of-os-disk",
399+ DiskName: "name-of-os-disk",
400+ MediaLink: "path-to-vhd",
401+ SourceImageName: "image-used-to-create-os-disk",
402+ OS: "operating-system-on-os-disk",
403+ },
404 RoleSize: "size-of-instance",
405 },
406 },
407@@ -1030,7 +1040,7 @@
408 deploymentSlot := "staging"
409 label := "deploymentLabel"
410 vhd := NewOSVirtualHardDisk("hostCaching", "diskLabel", "diskName", "mediaLink", "sourceImageName", "os")
411- roles := []Role{*NewRole("size", "name", []ConfigurationSet{}, []OSVirtualHardDisk{*vhd})}
412+ roles := []Role{*NewRole("size", "name", vhd, []ConfigurationSet{})}
413 virtualNetworkName := "network"
414
415 deployment := NewDeploymentForCreateVMDeployment(name, deploymentSlot, label, roles, virtualNetworkName)
416@@ -1303,7 +1313,7 @@
417 configset := []ConfigurationSet{*config}
418 vhd := NewOSVirtualHardDisk("hostCaching", "diskLabel", "diskName", "mediaLink", "sourceImageName", "os")
419
420- role := NewRole(rolesize, rolename, configset, []OSVirtualHardDisk{*vhd})
421+ role := NewRole(rolesize, rolename, vhd, configset)
422 c.Check(role.RoleSize, Equals, rolesize)
423 c.Check(role.RoleName, Equals, rolename)
424 c.Check(role.ConfigurationSets, DeepEquals, configset)

Subscribers

People subscribed via source and target branches

to all changes: