Merge lp:~allenap/gwacl/remove-role-endpoints into lp:gwacl

Proposed by Gavin Panella on 2013-07-19
Status: Merged
Approved by: Gavin Panella on 2013-07-25
Approved revision: 218
Merged at revision: 201
Proposed branch: lp:~allenap/gwacl/remove-role-endpoints
Merge into: lp:gwacl
Diff against target: 994 lines (+554/-88)
8 files modified
example/management/run.go (+78/-47)
management.go (+98/-3)
management_base.go (+2/-2)
management_base_test.go (+41/-0)
management_test.go (+307/-7)
x509dispatcher.go (+17/-23)
xmlobjects.go (+7/-2)
xmlobjects_test.go (+4/-4)
To merge this branch: bzr merge lp:~allenap/gwacl/remove-role-endpoints
Reviewer Review Type Date Requested Status
Julian Edwards (community) Approve on 2013-07-23
Raphaël Badin (community) 2013-07-19 Approve on 2013-07-19
Review via email: mp+175799@code.launchpad.net

Commit message

New method RemoveRoleEndpoints.

To post a comment you must log in.
Raphaël Badin (rvb) wrote :

Looks good… but please have a look at [2].

[0]

The method is called 'RemoveRoleEndpoints' but the parameter type is called 'RemoveRoleEndpointRequest', I think it should be 'RemoveRoleEndpoint*s*Request'

[1]

As always, I'd like to see this method exercised in the real world (i.e. in the example script).

[2]

21 + InputEndpoints []InputEndpoint
22 +}

40 + if reflect.DeepEqual(endpoint, existingEndpoint) {

This is clearly okay for the provider but I wonder if it would not be more straightforward, instead of passing whole endpoints objects, to pass just a list of the ports (the external ports) corresponding to the endpoints to be removed… because the code we have here imposes to create InputEndpoints structures where only the "natural key" (i.e. the external port) could be sufficient to identify the endpoints to be deleted. What do you think?

review: Approve
Julian Edwards (julian-edwards) wrote :

On Friday 19 Jul 2013 13:52:26 Raphaël Badin wrote:
> This is clearly okay for the provider but I wonder if it would not be more
> straightforward, instead of passing whole endpoints objects, to pass just a
> list of the ports (the external ports) corresponding to the endpoints to be
> removed… because the code we have here imposes to create InputEndpoints
> structures where only the "natural key" (i.e. the external port) could be
> sufficient to identify the endpoints to be deleted. What do you think?

We talked the other day of wrapping add/remove-role-endpoints with another
func that does just take simple port numbers.

I am still not sure what Azure considers to be the primary key for an
endpoint, the documentation is not clear enough.

195. By Gavin Panella on 2013-07-22

Fix typo.

Gavin Panella (allenap) wrote :

> Looks good… but please have a look at [2].
>
> [0]
>
> The method is called 'RemoveRoleEndpoints' but the parameter type is called
> 'RemoveRoleEndpointRequest', I think it should be
> 'RemoveRoleEndpoint*s*Request'

Good spot, fixed.

>
> [1]
>
> As always, I'd like to see this method exercised in the real world (i.e. in
> the example script).

Good point! I'll do that before I land.

>
> [2]
>
> 21      +    InputEndpoints []InputEndpoint
> 22      +}
> …
> 40      +                        if reflect.DeepEqual(endpoint,
> existingEndpoint) {
>
> This is clearly okay for the provider but I wonder if it would not be more
> straightforward, instead of passing whole endpoints objects, to pass just a
> list of the ports (the external ports) corresponding to the endpoints to be
> removed… because the code we have here imposes to create InputEndpoints
> structures where only the "natural key" (i.e. the external port) could be
> sufficient to identify the endpoints to be deleted.  What do you think?

My thinking goes something like this:

- Always ignore LoadBalancerProbe for the purposes of comparison.

- If LoadBalancedEndpointSetName is defined, then LocalPort, Port,
  Protocol and VIP should be used for comparison.

- If LoadBalancedEndpointSetName is *not* defined, then Port, Protocol
  and VIP should be used for comparison.

- This leaves Name. I'm not sure what purpose this serves, other than
  as a pretty name for the UI. If so, it's irrelevant for the purposes
  of comparison.

It may be worth having a FilterRoleEndpoints function, which behaves
like RemoveRoleEndpoints, but accepts a func(*InputEndpoint) bool
function which can either mutate the endpoint or remove it (by
returning false). I think this might better support the desired
behaviour in juju-core, and make us more flexible to change.

What do you think?

Gavin Panella (allenap) wrote :

> - If LoadBalancedEndpointSetName is defined, then LocalPort, Port,
> Protocol and VIP should be used for comparison.

... in *addition* to LoadBalancedEndpointSetName.

196. By Gavin Panella on 2013-07-22

Switch to consts for LinuxProvisioningConfiguration and NetworkConfiguration.

197. By Gavin Panella on 2013-07-22

Break out comparison of InputEndpoints into a separate function.

198. By Gavin Panella on 2013-07-22

Comment on RemoveRoleEndpoint.

199. By Gavin Panella on 2013-07-22

Redefine RemoveRoleEndpoints in terms of new method FilterRoleEndpoints.

200. By Gavin Panella on 2013-07-22

Reorder.

201. By Gavin Panella on 2013-07-22

Document the filter function.

202. By Gavin Panella on 2013-07-22

Don't make the role filtering function public yet.

203. By Gavin Panella on 2013-07-22

Simplify filter definition.

204. By Gavin Panella on 2013-07-22

Explain why filterRoleEndpoints() is private.

205. By Gavin Panella on 2013-07-22

Fix typo.

206. By Gavin Panella on 2013-07-22

Add use of {Add,Remove}RoleEndpoints to the management example.

207. By Gavin Panella on 2013-07-22

Fix typo.

Gavin Panella (allenap) wrote :

All done, please take another look! Thanks.

Julian Edwards (julian-edwards) wrote :

Approved, but please fix [1].

[0]
+// CompareInputEndpoints attempts to compare two InputEndpoint objects in a
+// way that's congruent with how Windows's Azure considers them. The name is
+// ignored, as is LoadBalancerProbe. The other fields are used for comparison,
+// except when LoadBalancedEndpointSetName is set, in which case LocalPort is
+// ignored.

>blink< !

I had not grokked any of that (but could be Lyme brain) from the docs. It's madness.

[1]

+ panic("wtf") // Safe to remove in Go 1.1

Should be OK to remove now, we are all supposed to be using Go 1.1 (add https://launchpad.net/~james-page/+archive/golang-backports) as it's the only supported version of Go.

[2]

+// Returns true to keep the endpoint defined in the role's configuration,
+// false to remove it. It is also welcome to mutate endpoints; they are passed
+// by reference.

By pointer, strictly speaking.

Very nice readable tests for the new comparison func, BTW!

review: Approve
208. By Gavin Panella on 2013-07-23

Improve doc for CompareInputEndpoints.

Raphaël Badin (rvb) wrote :

> > [2]
> >
> My thinking goes something like this:
>
> - Always ignore LoadBalancerProbe for the purposes of comparison.
>
> - If LoadBalancedEndpointSetName is defined, then LocalPort, Port,
>  Protocol and VIP should be used for comparison.
>
> - If LoadBalancedEndpointSetName is *not* defined, then Port, Protocol
>  and VIP should be used for comparison.

Why omitting LocalPort in this case? I must say I'm a bit confused about what LoadBalancedEndpointSetName means… even after reading the doc :).
>
> - This leaves Name. I'm not sure what purpose this serves, other than
>  as a pretty name for the UI. If so, it's irrelevant for the purposes
>  of comparison.
>
> It may be worth having a FilterRoleEndpoints function, which behaves
> like RemoveRoleEndpoints, but accepts a func(*InputEndpoint) bool
> function which can either mutate the endpoint or remove it (by
> returning false). I think this might better support the desired
> behaviour in juju-core, and make us more flexible to change.

Seems a bit overkill *if* we can figure out what the "natural key" of an endpoint is…

Raphaël Badin (rvb) wrote :

> Seems a bit overkill *if* we can figure out what the "natural key" of an
> endpoint is…

I think (name), (protocol, local port) and (protocol, port) are natural keys for an endpoint.

Gavin Panella (allenap) wrote :

On 23 July 2013 16:53, Raphaël Badin <email address hidden> wrote:
> I think (name), (protocol, local port) and (protocol, port) are
> natural keys for an endpoint.

So a comparison would go something like, in order:

- If Name matches, they're the same,

- If Protocol and LocalPort match, they're the same,

- If Protocol and Port match, they're the same,

- Otherwise they're not the same.

What about LoadBalancedEndpointSetName?

  Specifies a name for a set of load-balanced endpoints. Specifying
  this element for a given endpoint adds it to the set.

That sounds like if it's set to a non-empty string then the natural
key would be (set-name, protocol, port, local-port).

This opens up more questions:

- For a load-balanced endpoint set, does the public port have to be
  the same for all endpoints therein?

- What happens if a load-balanced set uses the same port as a
  non-load-balanced endpoint? I assume they conflict. So, when
  comparing endpoints we have to consider if they're the same
  definition, or in conflict.

Also, I've seen some example XML for InputEndpoint that includes a Vip
tag (VIP in the GWACL structs). This ist shown in the docs for
GetRole, but not UpdateRole. I guess we can ignore this as Azure only
supports one VIP per deployment at present.

I'm still working on this branch... I think I'm going to land as soon
as I can with a whopping FIXME in CompareInputEndpoints, and then do
some experimentation to figure all the above out, at least that of it
which is relevant to us.

Gavin Panella (allenap) wrote :

> [0]
...
> I had not grokked any of that (but could be Lyme brain) from the docs.  It's
> madness.

Ha :) I've tried to improve the comment here. However, as you've
probably seen, the approach is still being discussed, so don't pay too
much attention to this yet.

>
> [1]
>
> +    panic("wtf") // Safe to remove in Go 1.1
>
> Should be OK to remove now, we are all supposed to be using Go 1.1 (add
> https://launchpad.net/~james-page/+archive/golang-backports) as it's the only
> supported version of Go.

Done.

We can also upgrade the code underlying fork/http and fork/tls now;
it's based on 1.0.2 right now, for compatibility. Rapheël has already
tried this and it works fine, iirc, we just need to get round to doing
it.

>
> [2]
>
> +// Returns true to keep the endpoint defined in the role's configuration,
> +// false to remove it. It is also welcome to mutate endpoints; they are
> passed
> +// by reference.
>
> By pointer, strictly speaking.

Is this a distinction that only makes a difference to a former C++
programmer, or is there a tangible difference in meaning?

>
> Very nice readable tests for the new comparison func, BTW!

Ta :) Very nice review :)

Gavin Panella (allenap) wrote :

> > It may be worth having a FilterRoleEndpoints function, which behaves
> > like RemoveRoleEndpoints, but accepts a func(*InputEndpoint) bool
> > function which can either mutate the endpoint or remove it (by
> > returning false). I think this might better support the desired
> > behaviour in juju-core, and make us more flexible to change.
>
> Seems a bit overkill *if* we can figure out what the "natural key" of an
> endpoint is…

I did it, and it was a good move, imo. The filterRoleEndpoints
function is not public, and is only called by RemoveRoleEndpoints, but
splitting up the function has made testing and reasoning easier.
Don't dismiss divide and conquer :)

209. By Gavin Panella on 2013-07-23

Merge trunk.

210. By Gavin Panella on 2013-07-23

Remove Go 1.0 compatibility hack.

Julian Edwards (julian-edwards) wrote :

On 24/07/13 07:58, Gavin Panella wrote:
> We can also upgrade the code underlying fork/http and fork/tls now;
> it's based on 1.0.2 right now, for compatibility. Rapheël has already
> tried this and it works fine, iirc, we just need to get round to doing
> it.

Wicked. JFDI. :)

> Is this a distinction that only makes a difference to a former C++
> programmer, or is there a tangible difference in meaning?

Ah, ummm, maybe :)

Pointers are definitely different beasts to references in my head, but
Go blurs the distinction by eschewing the -> operator in favour of a .
for struct property access regardless of it being a pointer.

Does Go talk about references anywhere? I thought it just mentioned
pointer types.

>> Very nice readable tests for the new comparison func, BTW!
>
> Ta :) Very nice review :)

We're going to need to get a room at this rate.

Raphaël Badin (rvb) wrote :

> On 23 July 2013 16:53, Raphaël Badin <email address hidden> wrote:
> > I think (name), (protocol, local port) and (protocol, port) are
> > natural keys for an endpoint.
>
> So a comparison would go something like, in order:
>
> - If Name matches, they're the same,

Why not stop here? The name is a mandatory (well, I think) identifier.

> I'm still working on this branch... I think I'm going to land as soon
> as I can with a whopping FIXME in CompareInputEndpoints, and then do
> some experimentation to figure all the above out, at least that of it
> which is relevant to us.

Yeah, we need this ASAP to do the work on the provider so it seems like a good idea to land this sooner rather than later.

Raphaël Badin (rvb) wrote :

> On 23 July 2013 16:53, Raphaël Badin <email address hidden> wrote:
> > I think (name), (protocol, local port) and (protocol, port) are
> > natural keys for an endpoint.
>
> So a comparison would go something like, in order:
>
> - If Name matches, they're the same,
>
> - If Protocol and LocalPort match, they're the same,
>
> - If Protocol and Port match, they're the same,
>
> - Otherwise they're not the same.

On second thought that's probably a good solution. At first I thought we could only give it a list of names. That would be indeed much simpler but your solution is more flexible and will allow a user to remove endpoints based on the {port, protocol} and that's probably the best option.

> What about LoadBalancedEndpointSetName?
>
> Specifies a name for a set of load-balanced endpoints. Specifying
> this element for a given endpoint adds it to the set.
>
> That sounds like if it's set to a non-empty string then the natural
> key would be (set-name, protocol, port, local-port).
>
> This opens up more questions:
>
> - For a load-balanced endpoint set, does the public port have to be
> the same for all endpoints therein?

AFAIUI this is meant to be used to load-balance things between machine and so I don't think it adds any meaning when it comes to deciding what the natural keys are… but this is all surmise of course because the documentation is — once again — not very clear.

211. By Gavin Panella on 2013-07-25

Change content type to application/xml.

212. By Gavin Panella on 2013-07-25

Improve logging experience for requests.

213. By Gavin Panella on 2013-07-25

Replace all fmt.Print* calls in the management example with log calls.

214. By Gavin Panella on 2013-07-25

Logging tweaks.

215. By Gavin Panella on 2013-07-25

Fix formatting.

216. By Gavin Panella on 2013-07-25

Fix the example endpoint so that it can be created.

217. By Gavin Panella on 2013-07-25

Couple of FIXMEs.

218. By Gavin Panella on 2013-07-25

UpdateRole needs to block until complete.

Gavin Panella (allenap) wrote :

This _seems_ to work now. I'm landing it, and will fix the comparison algorithm separately.

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-07-18 08:19:43 +0000
3+++ example/management/run.go 2013-07-25 21:52:25 +0000
4@@ -13,6 +13,7 @@
5 "flag"
6 "fmt"
7 "launchpad.net/gwacl"
8+ . "launchpad.net/gwacl/logging"
9 "math/rand"
10 "os"
11 "time"
12@@ -20,13 +21,11 @@
13
14 var certFile string
15 var subscriptionID string
16-var verbose bool
17 var pause bool
18
19 func getParams() error {
20 flag.StringVar(&certFile, "cert", "", "Name of your management certificate file (in PEM format).")
21 flag.StringVar(&subscriptionID, "subscriptionid", "", "Your Azure subscription ID.")
22- flag.BoolVar(&verbose, "verbose", false, "Print debugging output")
23 flag.BoolVar(&pause, "pause", false, "Wait for user input after the VM is brought up (useful for further testing)")
24
25 flag.Parse()
26@@ -70,12 +69,11 @@
27
28 err := getParams()
29 if err != nil {
30- fmt.Println(err)
31+ Info(err)
32 os.Exit(1)
33 }
34
35 api, err := gwacl.NewManagementAPI(subscriptionID, certFile)
36- gwacl.SetVerbose(verbose)
37 checkError(err)
38
39 // Exercise the API.
40@@ -83,7 +81,7 @@
41 // Exercise hosted services API.
42 ExerciseHostedServicesAPI(api)
43
44- fmt.Println("All done.")
45+ Info("All done.")
46 }
47
48 func ExerciseHostedServicesAPI(api *gwacl.ManagementAPI) {
49@@ -92,23 +90,23 @@
50 release := "13.04"
51
52 affinityGroupName := gwacl.MakeRandomHostname("affinitygroup")
53- fmt.Println("Creating an affinity group...")
54+ Info("Creating an affinity group...")
55 cag := gwacl.NewCreateAffinityGroup(affinityGroupName, "affinity-label", "affinity-description", location)
56 err = api.CreateAffinityGroup(&gwacl.CreateAffinityGroupRequest{
57 CreateAffinityGroup: cag})
58 checkError(err)
59- fmt.Printf("Created affinity group %s\n", affinityGroupName)
60+ Infof("Created affinity group %s\n", affinityGroupName)
61
62 defer func() {
63- fmt.Printf("Deleting affinity group %s\n", affinityGroupName)
64+ Infof("Deleting affinity group %s\n", affinityGroupName)
65 err := api.DeleteAffinityGroup(&gwacl.DeleteAffinityGroupRequest{
66 Name: affinityGroupName})
67 checkError(err)
68- fmt.Printf("Done deleting affinity group %s\n", affinityGroupName)
69+ Infof("Done deleting affinity group %s\n", affinityGroupName)
70 }()
71
72 virtualNetworkName := gwacl.MakeRandomVirtualNetworkName("virtual-net-")
73- fmt.Printf("Creating virtual network %s...\n", virtualNetworkName)
74+ Infof("Creating virtual network %s...\n", virtualNetworkName)
75 virtualNetwork := gwacl.VirtualNetworkSite{
76 Name: virtualNetworkName,
77 AffinityGroup: affinityGroupName,
78@@ -118,69 +116,70 @@
79 }
80 err = api.AddVirtualNetworkSite(&virtualNetwork)
81 checkError(err)
82- fmt.Println("Done creating virtual network")
83+ Info("Done creating virtual network")
84
85 defer func() {
86- fmt.Printf("Deleting virtual network %s...\n", virtualNetworkName)
87+ Infof("Deleting virtual network %s...\n", virtualNetworkName)
88 err := api.RemoveVirtualNetworkSite(virtualNetworkName)
89 checkError(err)
90- fmt.Printf("Done deleting virtual network %s\n", virtualNetworkName)
91+ Infof("Done deleting virtual network %s\n", virtualNetworkName)
92 }()
93
94 networkConfig, err := api.GetNetworkConfiguration()
95 checkError(err)
96 if networkConfig == nil {
97- fmt.Println("No network configuration is set")
98+ Info("No network configuration is set")
99 } else {
100 xml, err := networkConfig.Serialize()
101 checkError(err)
102- fmt.Println(xml)
103+ Info(xml)
104 }
105
106- fmt.Printf("Getting OS Image for release '%s' and location '%s'...\n", release, location)
107+ Infof("Getting OS Image for release '%s' and location '%s'...\n", release, location)
108 images, err := api.ListOSImages()
109 checkError(err)
110 preciseImage, err := images.GetLatestUbuntuImage(release, location)
111 checkError(err)
112 sourceImageName := preciseImage.Name
113- fmt.Printf("Got image named '%s'\n", sourceImageName)
114- fmt.Println("Done getting OS Image\n")
115+ Infof("Got image named '%s'\n", sourceImageName)
116+ Info("Done getting OS Image\n")
117
118 hostServiceName := gwacl.MakeRandomHostedServiceName("gwacl")
119- fmt.Printf("Creating a hosted service: '%s'...\n", hostServiceName)
120+ Infof("Creating a hosted service: '%s'...\n", hostServiceName)
121 createHostedService := gwacl.NewCreateHostedServiceWithLocation(hostServiceName, "testLabel", location)
122 err = api.AddHostedService(createHostedService)
123 checkError(err)
124- fmt.Println("Done creating a hosted service\n")
125+ Info("Done creating a hosted service\n")
126
127 defer func() {
128- fmt.Println("Destroying hosted service...")
129+ Info("Destroying hosted service...")
130+ // FIXME: Check error
131 api.DestroyHostedService(&gwacl.DestroyHostedServiceRequest{
132 ServiceName: hostServiceName})
133- fmt.Println("Done destroying hosted service\n")
134+ Info("Done destroying hosted service\n")
135 }()
136
137- fmt.Println("Listing hosted services...")
138+ Info("Listing hosted services...")
139 hostedServices, err := api.ListHostedServices()
140 checkError(err)
141- fmt.Printf("Got %d hosted service(s)\n", len(hostedServices))
142+ Infof("Got %d hosted service(s)\n", len(hostedServices))
143 if len(hostedServices) > 0 {
144 hostedService := hostedServices[0]
145 detailedHostedService, err := api.GetHostedServiceProperties(hostedService.ServiceName, true)
146 checkError(err)
147- fmt.Printf("Hosted service '%s' contains %d deployments\n", hostedService.ServiceName, len(detailedHostedService.Deployments))
148+ Infof("Hosted service '%s' contains %d deployments\n", hostedService.ServiceName, len(detailedHostedService.Deployments))
149 // Do the same again with ListAllDeployments.
150 deployments, err := api.ListAllDeployments(&gwacl.ListAllDeploymentsRequest{ServiceName: hostedService.ServiceName})
151 checkError(err)
152 if len(deployments) != len(detailedHostedService.Deployments) {
153- fmt.Printf(
154+ Infof(
155 "Mismatch in reported deployments: %d != %d",
156 len(deployments), len(detailedHostedService.Deployments))
157 }
158 }
159- fmt.Println("Done listing hosted services\n")
160+ Info("Done listing hosted services\n")
161
162- fmt.Println("Adding VM deployment...")
163+ Info("Adding VM deployment...")
164 hostname := gwacl.MakeRandomHostname("gwaclhost")
165 // Random passwords are no use to man nor beast here if you want to
166 // test with your instance, so we'll use a fixed one. It's not really a
167@@ -194,16 +193,17 @@
168
169 storageAccount := makeRandomIdentifier("gwacl", 24)
170 storageLabel := makeRandomIdentifier("gwacl", 64)
171- fmt.Printf("Requesting storage account with name '%s' and label '%s'...\n", storageAccount, storageLabel)
172+ Infof("Requesting storage account with name '%s' and label '%s'...\n", storageAccount, storageLabel)
173 cssi := gwacl.NewCreateStorageServiceInputWithLocation(storageAccount, storageLabel, location, "false")
174 err = api.AddStorageAccount(cssi)
175 checkError(err)
176- fmt.Println("Done requesting storage account\n")
177+ Info("Done requesting storage account\n")
178
179 defer func() {
180- fmt.Printf("Deleting storage account %s...\n", storageAccount)
181+ Infof("Deleting storage account %s...\n", storageAccount)
182+ // FIXME: Check error
183 api.DeleteStorageAccount(storageAccount)
184- fmt.Println("Done deleting storage account\n")
185+ Info("Done deleting storage account\n")
186 }()
187
188 mediaLink := gwacl.CreateVirtualHardDiskMediaLink(storageAccount, fmt.Sprintf("vhds/%s.vhd", vhdName))
189@@ -216,36 +216,67 @@
190 deployment := gwacl.NewDeploymentForCreateVMDeployment(machineName, "Staging", machineName, []gwacl.Role{*role}, "")
191 err = api.AddDeployment(deployment, hostServiceName)
192 checkError(err)
193- fmt.Println("Done adding VM deployment\n")
194+ Info("Done adding VM deployment\n")
195
196- fmt.Println("Starting VM...")
197+ Info("Starting VM...")
198 err = api.StartRole(&gwacl.StartRoleRequest{hostServiceName, deployment.Name, role.RoleName})
199 checkError(err)
200- fmt.Println("Done starting VM\n")
201+ Info("Done starting VM\n")
202
203- fmt.Println("Listing VM...")
204+ Info("Listing VM...")
205 instances, err := api.ListInstances(&gwacl.ListInstancesRequest{hostServiceName})
206 checkError(err)
207- fmt.Printf("Got %d instance(s)\n", len(instances))
208- fmt.Println("Done listing VM\n")
209+ Infof("Got %d instance(s)\n", len(instances))
210+ Info("Done listing VM\n")
211
212- fmt.Println("Getting deployment info...")
213+ Info("Getting deployment info...")
214 request := &gwacl.GetDeploymentRequest{ServiceName: hostServiceName, DeploymentName: machineName}
215 deploy, err := api.GetDeployment(request)
216 checkError(err)
217 fqdn, err := deploy.GetFQDN()
218 checkError(err)
219- fmt.Println("Got deployment info\n")
220-
221- fmt.Println("host:", fqdn)
222- fmt.Println("username:", username)
223- fmt.Println("password:", password)
224- fmt.Println("")
225+ Info("Got deployment info\n")
226+
227+ Info("host:", fqdn)
228+ Info("username:", username)
229+ Info("password:", password)
230+ Info("")
231+
232+ Info("Adding role input endpoint...")
233+ endpoint := gwacl.InputEndpoint{
234+ Name: gwacl.MakeRandomHostname("endpoint-"),
235+ Port: 1080,
236+ LocalPort: 80,
237+ Protocol: "TCP",
238+ }
239+ err = api.AddRoleEndpoints(&gwacl.AddRoleEndpointsRequest{
240+ ServiceName: hostServiceName,
241+ DeploymentName: deployment.Name,
242+ RoleName: role.RoleName,
243+ InputEndpoints: []gwacl.InputEndpoint{endpoint},
244+ })
245+ checkError(err)
246+ Info("Added role input endpoint\n")
247+
248+ defer func() {
249+ Info("Removing role input endpoint...")
250+ // FIXME: Something above - AddRoleEndpoints seems a prime suspect -
251+ // causes Azure to do something asynchronously, which we must wait for
252+ // before attempting this call, or it will blow up with a 409.
253+ err := api.RemoveRoleEndpoints(&gwacl.RemoveRoleEndpointsRequest{
254+ ServiceName: hostServiceName,
255+ DeploymentName: deployment.Name,
256+ RoleName: role.RoleName,
257+ InputEndpoints: []gwacl.InputEndpoint{endpoint},
258+ })
259+ checkError(err)
260+ Info("Removed role input endpoint\n")
261+ }()
262
263 if pause {
264 var wait string
265- fmt.Println("Pausing so you can play with the newly-created VM")
266- fmt.Println("To clear up, type something and press enter to continue")
267+ Info("Pausing so you can play with the newly-created VM")
268+ Info("To clear up, type something and press enter to continue")
269 fmt.Scan(&wait)
270 }
271 }
272
273=== modified file 'management.go'
274--- management.go 2013-07-18 09:52:59 +0000
275+++ management.go 2013-07-25 21:52:25 +0000
276@@ -255,7 +255,7 @@
277 return nil
278 }
279
280-type AddRoleEndpointRequest struct {
281+type AddRoleEndpointsRequest struct {
282 ServiceName string
283 DeploymentName string
284 RoleName string
285@@ -265,7 +265,7 @@
286 // AddRoleEndpoints appends the supplied endpoints to the existing endpoints
287 // for the named service/deployment/role name. Note that the Azure API
288 // leaves this open to a race condition between fetching and updating the role.
289-func (api *ManagementAPI) AddRoleEndpoints(request *AddRoleEndpointRequest) error {
290+func (api *ManagementAPI) AddRoleEndpoints(request *AddRoleEndpointsRequest) error {
291 var err error
292 vmRole, err := api.GetRole(&GetRoleRequest{
293 ServiceName: request.ServiceName,
294@@ -278,7 +278,7 @@
295
296 for i, configSet := range vmRole.ConfigurationSets {
297 // TODO: Is NetworkConfiguration always present?
298- if configSet.ConfigurationSetType == "NetworkConfiguration" {
299+ if configSet.ConfigurationSetType == CONFIG_SET_NETWORK {
300 endpointsP := vmRole.ConfigurationSets[i].InputEndpoints
301 if endpointsP == nil {
302 // No endpoints set at all, initialise it to be empty.
303@@ -306,3 +306,98 @@
304 }
305 return nil
306 }
307+
308+// CompareInputEndpoints attempts to compare two InputEndpoint objects in a
309+// way that's congruent with how Windows's Azure considers them. The name is
310+// always ignored, as is LoadBalancerProbe. When LoadBalancedEndpointSetName
311+// is set (not the empty string), all the fields - LocalPort, Port, Protocol,
312+// VIP and LoadBalancedEndpointSetName - are used for the comparison. When
313+// LoadBalancedEndpointSetName is the empty string, all except LocalPort -
314+// effectively Port, Protocol and VIP - are used for comparison.
315+func CompareInputEndpoints(a, b *InputEndpoint) bool {
316+ if a.LoadBalancedEndpointSetName == "" {
317+ return a.Port == b.Port && a.Protocol == b.Protocol && a.VIP == b.VIP
318+ } else {
319+ return (a.LoadBalancedEndpointSetName == b.LoadBalancedEndpointSetName &&
320+ a.LocalPort == b.LocalPort && a.Port == b.Port &&
321+ a.Protocol == b.Protocol && a.VIP == b.VIP)
322+ }
323+}
324+
325+type RemoveRoleEndpointsRequest struct {
326+ ServiceName string
327+ DeploymentName string
328+ RoleName string
329+ InputEndpoints []InputEndpoint
330+}
331+
332+// RemoveRoleEndpoints attempts to remove the given endpoints from the
333+// specified role. It uses `CompareInputEndpoints` to determine when there's a
334+// match between the given endpoint and one already configured.
335+func (api *ManagementAPI) RemoveRoleEndpoints(request *RemoveRoleEndpointsRequest) error {
336+ filterRequest := filterRoleEndpointsRequest{
337+ ServiceName: request.ServiceName,
338+ DeploymentName: request.DeploymentName,
339+ RoleName: request.RoleName,
340+ Filter: func(a *InputEndpoint) bool {
341+ for _, b := range request.InputEndpoints {
342+ if CompareInputEndpoints(a, &b) {
343+ return false
344+ }
345+ }
346+ return true
347+ },
348+ }
349+ return api.filterRoleEndpoints(&filterRequest)
350+}
351+
352+// Returns true to keep the endpoint defined in the role's configuration,
353+// false to remove it. It is also welcome to mutate endpoints; they are passed
354+// by reference.
355+type inputEndpointFilter func(*InputEndpoint) bool
356+
357+type filterRoleEndpointsRequest struct {
358+ ServiceName string
359+ DeploymentName string
360+ RoleName string
361+ Filter inputEndpointFilter
362+}
363+
364+// filterRoleEndpoints is a general role endpoint filtering function. It is
365+// private because it is only a support function for RemoveRoleEndpoints, and
366+// is not tested directly.
367+func (api *ManagementAPI) filterRoleEndpoints(request *filterRoleEndpointsRequest) error {
368+ role, err := api.GetRole(&GetRoleRequest{
369+ ServiceName: request.ServiceName,
370+ DeploymentName: request.DeploymentName,
371+ RoleName: request.RoleName,
372+ })
373+ if err != nil {
374+ return err
375+ }
376+ for index, configSet := range role.ConfigurationSets {
377+ if configSet.ConfigurationSetType == CONFIG_SET_NETWORK {
378+ if configSet.InputEndpoints != nil {
379+ endpoints := []InputEndpoint{}
380+ for _, existingEndpoint := range *configSet.InputEndpoints {
381+ if request.Filter(&existingEndpoint) {
382+ endpoints = append(endpoints, existingEndpoint)
383+ }
384+ }
385+ if len(endpoints) == 0 {
386+ configSet.InputEndpoints = nil
387+ } else {
388+ configSet.InputEndpoints = &endpoints
389+ }
390+ }
391+ }
392+ // Update the role; implicit copying is a nuisance.
393+ role.ConfigurationSets[index] = configSet
394+ }
395+ return api.UpdateRole(&UpdateRoleRequest{
396+ ServiceName: request.ServiceName,
397+ DeploymentName: request.DeploymentName,
398+ RoleName: request.RoleName,
399+ PersistentVMRole: role,
400+ })
401+}
402
403=== modified file 'management_base.go'
404--- management_base.go 2013-07-25 05:32:02 +0000
405+++ management_base.go 2013-07-25 21:52:25 +0000
406@@ -452,11 +452,11 @@
407 if err != nil {
408 return err
409 }
410- _, err = api.session.put(url, "2012-03-01", []byte(role), "application/octet-stream")
411+ response, err := api.session.put(url, "2012-03-01", []byte(role), "application/xml")
412 if err != nil {
413 return err
414 }
415- return nil
416+ return api.blockUntilCompleted(response)
417 }
418
419 type CreateAffinityGroupRequest struct {
420
421=== modified file 'management_base_test.go'
422--- management_base_test.go 2013-07-25 05:32:02 +0000
423+++ management_base_test.go 2013-07-25 21:52:25 +0000
424@@ -1018,6 +1018,7 @@
425 serviceName + "/deployments/" + deploymentName + "/roles/" + roleName)
426 checkRequest(
427 c, httpRequest, expectedURL, "2012-03-01", []byte(expectedXML), "PUT")
428+ c.Assert(httpRequest.ContentType, Equals, "application/xml")
429 }
430
431 func (suite *managementBaseAPISuite) TestUpdateRole(c *C) {
432@@ -1044,6 +1045,46 @@
433 request.DeploymentName, request.RoleName, expectedXML)
434 }
435
436+func (suite *managementBaseAPISuite) TestUpdateRoleBlocksUntilComplete(c *C) {
437+ api := makeAPI(c)
438+ api.PollerInterval = time.Nanosecond
439+ request := &UpdateRoleRequest{
440+ ServiceName: "serviceName",
441+ DeploymentName: "deploymentName",
442+ RoleName: "roleName",
443+ PersistentVMRole: &PersistentVMRole{
444+ RoleName: "newRoleNamePerhaps",
445+ },
446+ }
447+ responses := []DispatcherResponse{
448+ // First response is 202 with an X-MS-Request-ID header.
449+ {makeX509ResponseWithOperationHeader("foobar"), nil},
450+ // Second response is XML to say that the request above has completed.
451+ {
452+ &x509Response{
453+ Body: []byte(fmt.Sprintf(operationXMLTemplate, "Succeeded")),
454+ StatusCode: http.StatusOK,
455+ },
456+ nil,
457+ },
458+ }
459+ recordedRequests := []*X509Request{}
460+ rigRecordingPreparedResponseDispatcher(&recordedRequests, responses)
461+
462+ err := api.UpdateRole(request)
463+ c.Assert(err, IsNil)
464+
465+ c.Assert(recordedRequests, HasLen, 2)
466+ expectedXML, err := request.PersistentVMRole.Serialize()
467+ c.Assert(err, IsNil)
468+ assertUpdateRoleRequest(
469+ c, api, recordedRequests[0], request.ServiceName,
470+ request.DeploymentName, request.RoleName, expectedXML)
471+ // Second request is to get status of operation "foobar".
472+ c.Check(recordedRequests[1].Method, Equals, "GET")
473+ c.Check(recordedRequests[1].URL, Matches, ".*/operations/foobar")
474+}
475+
476 func (suite *managementBaseAPISuite) TestCreateAffinityGroup(c *C) {
477 api := makeAPI(c)
478 cag := NewCreateAffinityGroup(
479
480=== modified file 'management_test.go'
481--- management_test.go 2013-07-18 07:04:50 +0000
482+++ management_test.go 2013-07-25 21:52:25 +0000
483@@ -853,7 +853,7 @@
484 existingRole := &PersistentVMRole{
485 ConfigurationSets: []ConfigurationSet{
486 {
487- ConfigurationSetType: "NetworkConfiguration",
488+ ConfigurationSetType: CONFIG_SET_NETWORK,
489 },
490 },
491 }
492@@ -873,7 +873,7 @@
493 },
494 }
495
496- request := &AddRoleEndpointRequest{
497+ request := &AddRoleEndpointsRequest{
498 ServiceName: "foo",
499 DeploymentName: "foo",
500 RoleName: "foo",
501@@ -900,7 +900,7 @@
502 existingRole := &PersistentVMRole{
503 ConfigurationSets: []ConfigurationSet{
504 {
505- ConfigurationSetType: "NetworkConfiguration",
506+ ConfigurationSetType: CONFIG_SET_NETWORK,
507 InputEndpoints: &[]InputEndpoint{
508 {
509 LocalPort: 123,
510@@ -927,7 +927,7 @@
511 },
512 }
513
514- request := &AddRoleEndpointRequest{
515+ request := &AddRoleEndpointsRequest{
516 ServiceName: "foo",
517 DeploymentName: "foo",
518 RoleName: "foo",
519@@ -960,7 +960,7 @@
520 rigRecordingPreparedResponseDispatcher(&record, responses)
521 api := makeAPI(c)
522
523- request := &AddRoleEndpointRequest{
524+ request := &AddRoleEndpointsRequest{
525 ServiceName: "foo",
526 DeploymentName: "foo",
527 RoleName: "foo"}
528@@ -979,7 +979,7 @@
529 existingRole := &PersistentVMRole{
530 ConfigurationSets: []ConfigurationSet{
531 {
532- ConfigurationSetType: "NetworkConfiguration",
533+ ConfigurationSetType: CONFIG_SET_NETWORK,
534 },
535 },
536 }
537@@ -999,7 +999,7 @@
538 },
539 }
540
541- request := &AddRoleEndpointRequest{
542+ request := &AddRoleEndpointsRequest{
543 ServiceName: "foo",
544 DeploymentName: "foo",
545 RoleName: "foo",
546@@ -1010,3 +1010,303 @@
547 c.Check(err, ErrorMatches, "PUT request failed [(]500: Internal Server Error[)]")
548 c.Check(record, HasLen, 2)
549 }
550+
551+type suiteRemoveRoleEndpoints struct{}
552+
553+var _ = Suite(&suiteRemoveRoleEndpoints{})
554+
555+func (suite *suiteRemoveRoleEndpoints) TestWhenNoPreexistingEndpoints(c *C) {
556+ var err error
557+ existingRole := &PersistentVMRole{
558+ ConfigurationSets: []ConfigurationSet{
559+ {
560+ ConfigurationSetType: CONFIG_SET_NETWORK,
561+ },
562+ },
563+ }
564+ responses := makeOKXMLResponse(c, existingRole)
565+ responses = append(responses, DispatcherResponse{
566+ // Accept upload of new endpoints
567+ response: &x509Response{StatusCode: http.StatusOK},
568+ })
569+ record := []*X509Request{}
570+ rigRecordingPreparedResponseDispatcher(&record, responses)
571+ api := makeAPI(c)
572+ endpoints := []InputEndpoint{
573+ {
574+ LocalPort: 999,
575+ Name: "test999",
576+ Port: 1999,
577+ },
578+ }
579+
580+ request := &RemoveRoleEndpointsRequest{
581+ ServiceName: "service-name",
582+ DeploymentName: "deployment-name",
583+ RoleName: "role-name",
584+ InputEndpoints: endpoints,
585+ }
586+ err = api.RemoveRoleEndpoints(request)
587+
588+ c.Assert(err, IsNil)
589+ c.Check(record, HasLen, 2)
590+ // Check GetRole was performed.
591+ assertGetRoleRequest(
592+ c, api, record[0], request.ServiceName, request.DeploymentName,
593+ request.RoleName)
594+ // Check UpdateRole was performed.
595+ expectedXML, err := existingRole.Serialize()
596+ c.Assert(err, IsNil)
597+ assertUpdateRoleRequest(
598+ c, api, record[1], request.ServiceName, request.DeploymentName,
599+ request.RoleName, expectedXML)
600+}
601+
602+func (suite *suiteRemoveRoleEndpoints) TestWhenEndpointIsDefined(c *C) {
603+ var err error
604+ existingRole := &PersistentVMRole{
605+ ConfigurationSets: []ConfigurationSet{
606+ {
607+ ConfigurationSetType: CONFIG_SET_NETWORK,
608+ InputEndpoints: &[]InputEndpoint{
609+ {
610+ LocalPort: 123,
611+ Name: "test123",
612+ Port: 1123,
613+ },
614+ {
615+ LocalPort: 456,
616+ Name: "test456",
617+ Port: 4456,
618+ },
619+ },
620+ },
621+ },
622+ }
623+ responses := makeOKXMLResponse(c, existingRole)
624+ responses = append(responses, DispatcherResponse{
625+ // Accept upload of new endpoints
626+ response: &x509Response{StatusCode: http.StatusOK},
627+ })
628+ record := []*X509Request{}
629+ rigRecordingPreparedResponseDispatcher(&record, responses)
630+ api := makeAPI(c)
631+
632+ request := &RemoveRoleEndpointsRequest{
633+ ServiceName: "service-name",
634+ DeploymentName: "deployment-name",
635+ RoleName: "role-name",
636+ // Remove the first of the existing endppints.
637+ InputEndpoints: (*existingRole.ConfigurationSets[0].InputEndpoints)[:1],
638+ }
639+ err = api.RemoveRoleEndpoints(request)
640+
641+ c.Assert(err, IsNil)
642+ c.Check(record, HasLen, 2)
643+ // Check GetRole was performed.
644+ assertGetRoleRequest(
645+ c, api, record[0], request.ServiceName, request.DeploymentName,
646+ request.RoleName)
647+ // Check UpdateRole was performed.
648+ expected := &PersistentVMRole{
649+ ConfigurationSets: []ConfigurationSet{
650+ {
651+ ConfigurationSetType: CONFIG_SET_NETWORK,
652+ InputEndpoints: &[]InputEndpoint{
653+ (*existingRole.ConfigurationSets[0].InputEndpoints)[1],
654+ },
655+ },
656+ },
657+ }
658+ expectedXML, err := expected.Serialize()
659+ c.Assert(err, IsNil)
660+ assertUpdateRoleRequest(
661+ c, api, record[1], request.ServiceName, request.DeploymentName,
662+ request.RoleName, expectedXML)
663+}
664+
665+func (suite *suiteRemoveRoleEndpoints) TestWhenAllEndpointsAreRemoved(c *C) {
666+ var err error
667+ existingRole := &PersistentVMRole{
668+ ConfigurationSets: []ConfigurationSet{
669+ {
670+ ConfigurationSetType: CONFIG_SET_NETWORK,
671+ InputEndpoints: &[]InputEndpoint{
672+ {
673+ LocalPort: 123,
674+ Name: "test123",
675+ Port: 1123,
676+ },
677+ },
678+ },
679+ },
680+ }
681+ responses := makeOKXMLResponse(c, existingRole)
682+ responses = append(responses, DispatcherResponse{
683+ // Accept upload of new endpoints
684+ response: &x509Response{StatusCode: http.StatusOK},
685+ })
686+ record := []*X509Request{}
687+ rigRecordingPreparedResponseDispatcher(&record, responses)
688+ api := makeAPI(c)
689+
690+ request := &RemoveRoleEndpointsRequest{
691+ ServiceName: "service-name",
692+ DeploymentName: "deployment-name",
693+ RoleName: "role-name",
694+ // Remove the first of the existing endppints.
695+ InputEndpoints: *existingRole.ConfigurationSets[0].InputEndpoints,
696+ }
697+ err = api.RemoveRoleEndpoints(request)
698+
699+ c.Assert(err, IsNil)
700+ c.Check(record, HasLen, 2)
701+ // Check GetRole was performed.
702+ assertGetRoleRequest(
703+ c, api, record[0], request.ServiceName, request.DeploymentName,
704+ request.RoleName)
705+ // Check UpdateRole was performed.
706+ expected := &PersistentVMRole{
707+ ConfigurationSets: []ConfigurationSet{
708+ {
709+ ConfigurationSetType: CONFIG_SET_NETWORK,
710+ // InputEndpoints is nil, not the empty slice.
711+ InputEndpoints: nil,
712+ },
713+ },
714+ }
715+ expectedXML, err := expected.Serialize()
716+ c.Assert(err, IsNil)
717+ assertUpdateRoleRequest(
718+ c, api, record[1], request.ServiceName, request.DeploymentName,
719+ request.RoleName, expectedXML)
720+}
721+
722+func (suite *suiteRemoveRoleEndpoints) TestWhenGetRoleFails(c *C) {
723+ responses := []DispatcherResponse{
724+ // No role found.
725+ {response: &x509Response{StatusCode: http.StatusNotFound}},
726+ }
727+ record := []*X509Request{}
728+ rigRecordingPreparedResponseDispatcher(&record, responses)
729+ api := makeAPI(c)
730+
731+ request := &RemoveRoleEndpointsRequest{
732+ ServiceName: "service-name",
733+ DeploymentName: "deployment-name",
734+ RoleName: "role-name",
735+ }
736+ err := api.RemoveRoleEndpoints(request)
737+
738+ c.Assert(err, NotNil)
739+ c.Check(err, ErrorMatches, "GET request failed [(]404: Not Found[)]")
740+ c.Check(record, HasLen, 1)
741+ assertGetRoleRequest(
742+ c, api, record[0], request.ServiceName, request.DeploymentName,
743+ request.RoleName)
744+}
745+
746+func (suite *suiteRemoveRoleEndpoints) TestWhenUpdateFails(c *C) {
747+ var err error
748+ existingRole := &PersistentVMRole{
749+ ConfigurationSets: []ConfigurationSet{
750+ {ConfigurationSetType: CONFIG_SET_NETWORK},
751+ },
752+ }
753+ responses := makeOKXMLResponse(c, existingRole)
754+ responses = append(responses, DispatcherResponse{
755+ // Cannot accept upload of new role endpoint
756+ response: &x509Response{StatusCode: http.StatusInternalServerError},
757+ })
758+ record := []*X509Request{}
759+ rigRecordingPreparedResponseDispatcher(&record, responses)
760+ api := makeAPI(c)
761+
762+ request := &RemoveRoleEndpointsRequest{
763+ ServiceName: "service-name",
764+ DeploymentName: "deployment-name",
765+ RoleName: "role-name",
766+ }
767+ err = api.RemoveRoleEndpoints(request)
768+
769+ c.Assert(err, NotNil)
770+ c.Check(err, ErrorMatches, "PUT request failed [(]500: Internal Server Error[)]")
771+ c.Check(record, HasLen, 2)
772+}
773+
774+type suiteCompareInputEndpoints struct{}
775+
776+var _ = Suite(&suiteCompareInputEndpoints{})
777+
778+func (suite *suiteCompareInputEndpoints) TestEqualWhenEmpty(c *C) {
779+ a := &InputEndpoint{}
780+ b := &InputEndpoint{}
781+ c.Assert(CompareInputEndpoints(a, b), Equals, true)
782+}
783+
784+func (suite *suiteCompareInputEndpoints) TestEquality(c *C) {
785+ checkComparison := func(a, b InputEndpoint, expected bool) {
786+ c.Check(CompareInputEndpoints(&a, &b), Equals, expected)
787+ }
788+ // Name has no influence on comparison.
789+ checkComparison(
790+ InputEndpoint{Name: "foo"},
791+ InputEndpoint{Name: "bar"},
792+ true)
793+ // LoadBalancerProbe has no influence on comparison.
794+ checkComparison(
795+ InputEndpoint{
796+ LoadBalancerProbe: &LoadBalancerProbe{Path: "foo"},
797+ },
798+ InputEndpoint{
799+ LoadBalancerProbe: &LoadBalancerProbe{Path: "bar"},
800+ },
801+ true,
802+ )
803+ // Port influences comparisons.
804+ checkComparison(
805+ InputEndpoint{Port: 1234},
806+ InputEndpoint{Port: 1234},
807+ true)
808+ checkComparison(
809+ InputEndpoint{Port: 1234},
810+ InputEndpoint{Port: 5678},
811+ false)
812+ // Protocol influences comparisons.
813+ checkComparison(
814+ InputEndpoint{Protocol: "TCP"},
815+ InputEndpoint{Protocol: "TCP"},
816+ true)
817+ checkComparison(
818+ InputEndpoint{Protocol: "TCP"},
819+ InputEndpoint{Protocol: "UDP"},
820+ false)
821+ // VIP influences comparisons.
822+ checkComparison(
823+ InputEndpoint{VIP: "1.2.3.4"},
824+ InputEndpoint{VIP: "1.2.3.4"},
825+ true)
826+ checkComparison(
827+ InputEndpoint{VIP: "1.2.3.4"},
828+ InputEndpoint{VIP: "5.6.7.8"},
829+ false)
830+ // LoadBalancedEndpointSetName influences comparisons.
831+ checkComparison(
832+ InputEndpoint{LoadBalancedEndpointSetName: "foo"},
833+ InputEndpoint{LoadBalancedEndpointSetName: "foo"},
834+ true)
835+ checkComparison(
836+ InputEndpoint{LoadBalancedEndpointSetName: "foo"},
837+ InputEndpoint{LoadBalancedEndpointSetName: "bar"},
838+ false)
839+ // LocalPort influences comparisons only when LoadBalancedEndpointSetName
840+ // is not the empty string.
841+ checkComparison(
842+ InputEndpoint{LocalPort: 1234},
843+ InputEndpoint{LocalPort: 5678},
844+ true)
845+ checkComparison(
846+ InputEndpoint{LoadBalancedEndpointSetName: "foo", LocalPort: 1234},
847+ InputEndpoint{LoadBalancedEndpointSetName: "foo", LocalPort: 5678},
848+ false)
849+}
850
851=== modified file 'x509dispatcher.go'
852--- x509dispatcher.go 2013-07-22 13:41:49 +0000
853+++ x509dispatcher.go 2013-07-25 21:52:25 +0000
854@@ -15,13 +15,6 @@
855 ContentType string
856 }
857
858-// Print debugging output.
859-var verbose = false
860-
861-func SetVerbose(newVerbose bool) {
862- verbose = newVerbose
863-}
864-
865 // newX509RequestGET initializes an X509Request for a GET. You may still need
866 // to set further values.
867 func newX509RequestGET(url, apiVersion string) *X509Request {
868@@ -68,24 +61,20 @@
869 type x509Response struct {
870 StatusCode int
871 // TODO: What exactly do we get back? How will we know its encoding?
872- Body []byte
873- RawHeader []byte
874- Header http.Header
875+ Body []byte
876+ Header http.Header
877 }
878
879 func newX509Response() *x509Response {
880 return &x509Response{
881- Body: make([]byte, 0),
882- RawHeader: make([]byte, 0),
883+ Body: make([]byte, 0),
884 }
885 }
886
887 func performX509Request(session *x509Session, request *X509Request) (*x509Response, error) {
888- if verbose {
889- Debug("Performing request")
890- Debug("Request url: " + request.URL)
891- Debug("Request method: " + request.Method)
892- Debugf("Request body: %s\n", request.Payload)
893+ Debugf("Request: %s %s", request.Method, request.URL)
894+ if len(request.Payload) > 0 {
895+ Debugf("Request body:\n%s", request.Payload)
896 }
897
898 bodyReader := ioutil.NopCloser(bytes.NewReader(request.Payload))
899@@ -107,11 +96,16 @@
900 return nil, err
901 }
902 response.Header = httpResponse.Header
903- if verbose {
904- Debug("Got response")
905- Debugf("Response status: %d\n", response.StatusCode)
906- Debugf("Response headers: %s\n", response.RawHeader)
907- Debugf("Response body: %s\n", response.Body)
908- }
909+
910+ Debugf("Response: %d %s", response.StatusCode, http.StatusText(response.StatusCode))
911+ if response.Header != nil {
912+ buf := bytes.Buffer{}
913+ response.Header.Write(&buf)
914+ Debugf("Response headers:\n%s", buf.String())
915+ }
916+ if len(response.Body) > 0 {
917+ Debugf("Response body:\n%s", response.Body)
918+ }
919+
920 return response, nil
921 }
922
923=== modified file 'xmlobjects.go'
924--- xmlobjects.go 2013-07-25 04:15:52 +0000
925+++ xmlobjects.go 2013-07-25 21:52:25 +0000
926@@ -39,6 +39,11 @@
927 // ConfigurationSet bits
928 //
929
930+const (
931+ CONFIG_SET_LINUX_PROVISIONING = "LinuxProvisioningConfiguration"
932+ CONFIG_SET_NETWORK = "NetworkConfiguration"
933+)
934+
935 // A ConfigurationSet object can be different things depending on its 'type'.
936 // The types we currently support are:
937 // - LinuxProvisioningConfigurationSet: configuration of a Linux VM
938@@ -80,7 +85,7 @@
939 Hostname, Username, Password, CustomData string,
940 DisableSSHPasswordAuthentication string) *ConfigurationSet {
941 return &ConfigurationSet{
942- ConfigurationSetType: "LinuxProvisioningConfiguration",
943+ ConfigurationSetType: CONFIG_SET_LINUX_PROVISIONING,
944 Hostname: Hostname,
945 Username: Username,
946 Password: Password,
947@@ -93,7 +98,7 @@
948 func NewNetworkConfigurationSet(
949 inputEndpoints []InputEndpoint, subnetNames []string) *ConfigurationSet {
950 return &ConfigurationSet{
951- ConfigurationSetType: "NetworkConfiguration",
952+ ConfigurationSetType: CONFIG_SET_NETWORK,
953 InputEndpoints: &inputEndpoints,
954 SubnetNames: &subnetNames,
955 }
956
957=== modified file 'xmlobjects_test.go'
958--- xmlobjects_test.go 2013-07-25 04:15:52 +0000
959+++ xmlobjects_test.go 2013-07-25 21:52:25 +0000
960@@ -249,7 +249,7 @@
961 RoleType: "PersistentVMRole",
962 ConfigurationSets: []ConfigurationSet{
963 {
964- ConfigurationSetType: "NetworkConfiguration",
965+ ConfigurationSetType: CONFIG_SET_NETWORK,
966 InputEndpoints: &[]InputEndpoint{
967 {
968 LoadBalancedEndpointSetName: "name-of-load-balanced-endpoint-set",
969@@ -318,7 +318,7 @@
970 RoleType: "PersistentVMRole",
971 ConfigurationSets: []ConfigurationSet{
972 {
973- ConfigurationSetType: "NetworkConfiguration",
974+ ConfigurationSetType: CONFIG_SET_NETWORK,
975 InputEndpoints: &[]InputEndpoint{
976 {
977 LoadBalancedEndpointSetName: "name-of-load-balanced-endpoint-set",
978@@ -935,7 +935,7 @@
979 RoleType: "PersistentVMRole",
980 ConfigurationSets: []ConfigurationSet{
981 {
982- ConfigurationSetType: "NetworkConfiguration",
983+ ConfigurationSetType: CONFIG_SET_NETWORK,
984 InputEndpoints: &[]InputEndpoint{
985 {
986 Name: "test-name",
987@@ -1345,7 +1345,7 @@
988 c.Check(inputEndpoint.Protocol, Equals, protocol)
989 c.Check(inputEndpoint.LoadBalancedEndpointSetName, Equals, bName)
990 c.Check(inputEndpoint.LocalPort, Equals, localPort)
991- c.Check(config.ConfigurationSetType, Equals, "NetworkConfiguration")
992+ c.Check(config.ConfigurationSetType, Equals, CONFIG_SET_NETWORK)
993 c.Check(*config.SubnetNames, DeepEquals, []string{"subnet-1", "subnet-2"})
994 }
995

Subscribers

People subscribed via source and target branches

to all changes: