Code review comment for lp:~gz/goose/novaservice_preserve_negative_ports

Reviewers: mp+159519_code.launchpad.net,

Message:
Please take a look.

Description:
Preserve negative ports on novaservice

Creating a security group rule with the value -1 for the ports has a
special
meaning in some cases, the test service needs to preserve these values
rather
than treating it as the ports not having been set.

https://code.launchpad.net/~gz/goose/novaservice_preserve_negative_ports/+merge/159519

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/8796045/

Affected files:
   A [revision details]
   M testservices/novaservice/service.go
   M testservices/novaservice/service_test.go

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: tarmac-20130328125516-y1zpoibtfvl873gg
+New revision: <email address hidden>

Index: testservices/novaservice/service.go
=== modified file 'testservices/novaservice/service.go'
--- testservices/novaservice/service.go 2013-02-25 07:16:32 +0000
+++ testservices/novaservice/service.go 2013-04-17 22:27:24 +0000
@@ -458,10 +458,10 @@
     Name: sourceGroup.Name,
    }
   }
- if rule.FromPort > 0 {
+ if rule.FromPort != 0 {
    newrule.FromPort = &rule.FromPort
   }
- if rule.ToPort > 0 {
+ if rule.ToPort != 0 {
    newrule.ToPort = &rule.ToPort
   }
   if rule.IPProtocol != "" {

Index: testservices/novaservice/service_test.go
=== modified file 'testservices/novaservice/service_test.go'
--- testservices/novaservice/service_test.go 2013-02-08 17:21:44 +0000
+++ testservices/novaservice/service_test.go 2013-04-17 22:27:24 +0000
@@ -789,6 +789,38 @@
   c.Assert(*gr, DeepEquals, group)
  }

+func (s *NovaSuite) TestAddSecurityGroupRuleKeepsNegativePort(c *C) {
+ group := nova.SecurityGroup{
+ Id: 1,
+ Name: "test",
+ TenantId: s.service.TenantId,
+ }
+ s.createGroup(c, group)
+ defer s.deleteGroup(c, group)
+ ri := nova.RuleInfo{
+ IPProtocol: "icmp",
+ FromPort: -1,
+ ToPort: -1,
+ Cidr: "0.0.0.0/0",
+ ParentGroupId: group.Id,
+ }
+ rule := nova.SecurityGroupRule{
+ Id: 10,
+ ParentGroupId: group.Id,
+ FromPort: &ri.FromPort,
+ ToPort: &ri.ToPort,
+ IPProtocol: &ri.IPProtocol,
+ IPRange: map[string]string{"cidr": "0.0.0.0/0"},
+ }
+ s.ensureNoRule(c, rule)
+ err := s.service.addSecurityGroupRule(rule.Id, ri)
+ c.Assert(err, IsNil)
+ defer s.deleteRule(c, rule)
+ returnedGroup, err := s.service.securityGroup(group.Id)
+ c.Assert(err, IsNil)
+ c.Assert(returnedGroup.Rules, DeepEquals, []nova.SecurityGroupRule{rule})
+}
+
  func (s *NovaSuite) TestRemoveSecurityGroupRuleTwiceFails(c *C) {
   group := nova.SecurityGroup{Id: 1}
   s.createGroup(c, group)