Clean up orphaned chains

Signed-off-by: Geoff Franks <gfranks@vmware.com>

src/code.cloudfoundry.org/vxlan-policy-agent/converger/converger.go

@@ -19,7 +19,8 @@ type Planner interface {
 
 //go:generate counterfeiter -o fakes/rule_enforcer.go --fake-name RuleEnforcer . ruleEnforcer
 type ruleEnforcer interface {
-	EnforceRulesAndChain(enforcer.RulesWithChain) error
+	EnforceRulesAndChain(enforcer.RulesWithChain) (string, error)
+	DeleteChain(chainName string, parentChain string) error
 }
 
 //go:generate counterfeiter -o fakes/metrics_sender.go --fake-name MetricsSender . metricsSender
@@ -28,14 +29,15 @@ type metricsSender interface {
 }
 
 type SinglePollCycle struct {
-	planners       []Planner
-	enforcer       ruleEnforcer
-	metricsSender  metricsSender
-	logger         lager.Logger
-	policyRuleSets map[enforcer.Chain]enforcer.RulesWithChain
-	asgRuleSets    map[enforcer.Chain]enforcer.RulesWithChain
-	policyMutex    sync.Locker
-	asgMutex       sync.Locker
+	planners            []Planner
+	enforcer            ruleEnforcer
+	metricsSender       metricsSender
+	logger              lager.Logger
+	policyRuleSets      map[enforcer.Chain]enforcer.RulesWithChain
+	asgRuleSets         map[enforcer.Chain]enforcer.RulesWithChain
+	containerToASGChain map[enforcer.Chain]string
+	policyMutex         sync.Locker
+	asgMutex            sync.Locker
 }
 
 func NewSinglePollCycle(planners []Planner, re ruleEnforcer, ms metricsSender, logger lager.Logger) *SinglePollCycle {
@@ -81,7 +83,7 @@ func (m *SinglePollCycle) DoPolicyCycle() error {
 				"old rules":     oldRuleSet,
 				"new rules":     ruleSet,
 			})
-			err = m.enforcer.EnforceRulesAndChain(ruleSet)
+			_, err = m.enforcer.EnforceRulesAndChain(ruleSet)
 			if err != nil {
 				m.policyMutex.Unlock()
 				return fmt.Errorf("enforce: %s", err)
@@ -107,6 +109,9 @@ func (m *SinglePollCycle) DoASGCycle() error {
 	if m.asgRuleSets == nil {
 		m.asgRuleSets = make(map[enforcer.Chain]enforcer.RulesWithChain)
 	}
+	if m.containerToASGChain == nil {
+		m.containerToASGChain = make(map[enforcer.Chain]string)
+	}
 
 	pollStartTime := time.Now()
 	var enforceDuration time.Duration
@@ -129,14 +134,27 @@ func (m *SinglePollCycle) DoASGCycle() error {
 					"old rules":     oldRuleSet,
 					"new rules":     ruleset,
 				})
-				err = m.enforcer.EnforceRulesAndChain(ruleset)
+				chain, err := m.enforcer.EnforceRulesAndChain(ruleset)
+				m.containerToASGChain[ruleset.Chain] = chain
+
 				if err != nil {
 					m.asgMutex.Unlock()
 					return fmt.Errorf("enforce-asg: %s", err)
 				}
 				m.asgRuleSets[ruleset.Chain] = ruleset
 			}
 		}
+		for parentChain, _ := range m.containerToASGChain {
+			if _, ok := m.asgRuleSets[parentChain]; !ok {
+				err := m.enforcer.DeleteChain(parentChain.Table, m.containerToASGChain[parentChain])
+				if err != nil {
+					m.asgMutex.Unlock()
+					return fmt.Errorf("clean-up-orphaned-asg-chains: %s", err)
+				}
+				delete(m.containerToASGChain, parentChain)
+				delete(m.asgRuleSets, parentChain)
+			}
+		}
 
 		enforceDuration += time.Now().Sub(enforceStartTime)
 	}

src/code.cloudfoundry.org/vxlan-policy-agent/converger/converger_test.go

@@ -224,7 +224,7 @@ var _ = Describe("Single Poll Cycle", func() {
 
 		Context("when policy enforcer errors", func() {
 			BeforeEach(func() {
-				fakeEnforcer.EnforceRulesAndChainReturns(errors.New("eggplant"))
+				fakeEnforcer.EnforceRulesAndChainReturns("professional tests", errors.New("eggplant"))
 			})
 
 			It("logs the error and returns", func() {
@@ -372,6 +372,43 @@ var _ = Describe("Single Poll Cycle", func() {
 			})
 		})
 
+		Context("when an ASG ruleset is  present when there are no contianers associated with it", func() {
+			BeforeEach(func() {
+				//				create some fake ASG iptables
+				var orphanRulesWithChain []enforcer.RulesWithChain
+				orphanRulesWithChain = []enforcer.RulesWithChain{
+					{
+						Rules: []rules.IPTablesRule{[]string{"asg-rule"}},
+						Chain: enforcer.Chain{
+							Table:       "asg-table-orphan",
+							ParentChain: "INPUT",
+							Prefix:      "some-prefix-orphan",
+						},
+					},
+				}
+				fakeLocalPlanner.GetASGRulesAndChainsReturns(orphanRulesWithChain, nil)
+				err := p.DoASGCycle()
+				Expect(err).NotTo(HaveOccurred())
+				Expect(fakeEnforcer.EnforceRulesAndChainCallCount()).To(Equal(3))
+			})
+
+			It("returns a error when attempting to delete extra ASG Rules ", func() {
+				err := p.DoASGCycle()
+				Expect(err).NotTo(HaveOccurred())
+				Expect(fakeEnforcer.DeleteChainCallCount()).To(Equal(1))
+				Expect(fakeEnforcer.DeleteChainArgsForCall(0)).To(Equal("raises for everybody!"))
+			})
+
+			It("removes the fake ASG iptables/rules", func() {
+
+			})
+
+			It("Does not removoe the valid ASG iptables/rules", func() {
+
+			})
+
+		})
+
 		Context("when a ruleset has all rules removed since the last poll cycle", func() {
 			BeforeEach(func() {
 				err := p.DoASGCycle()
@@ -451,7 +488,7 @@ var _ = Describe("Single Poll Cycle", func() {
 
 		Context("when ASG enforcer errors", func() {
 			BeforeEach(func() {
-				fakeEnforcer.EnforceRulesAndChainReturns(errors.New("eggplant"))
+				fakeEnforcer.EnforceRulesAndChainReturns("word string", errors.New("eggplant"))
 			})
 
 			It("logs the error and returns", func() {

src/code.cloudfoundry.org/vxlan-policy-agent/enforcer/enforcer.go

@@ -79,11 +79,11 @@ func (r *RulesWithChain) Equals(other RulesWithChain) bool {
 	return true
 }
 
-func (e *Enforcer) EnforceRulesAndChain(rulesAndChain RulesWithChain) error {
+func (e *Enforcer) EnforceRulesAndChain(rulesAndChain RulesWithChain) (string, error) {
 	return e.EnforceOnChain(rulesAndChain.Chain, rulesAndChain.Rules)
 }
 
-func (e *Enforcer) EnforceOnChain(c Chain, rules []rules.IPTablesRule) error {
+func (e *Enforcer) EnforceOnChain(c Chain, rules []rules.IPTablesRule) (string, error) {
 	var managedChainsRegex string
 	if c.ManagedChainsRegex != "" {
 		managedChainsRegex = c.ManagedChainsRegex
@@ -93,14 +93,14 @@ func (e *Enforcer) EnforceOnChain(c Chain, rules []rules.IPTablesRule) error {
 	return e.Enforce(c.Table, c.ParentChain, c.Prefix, managedChainsRegex, c.CleanUpParentChain, rules...)
 }
 
-func (e *Enforcer) Enforce(table, parentChain, chainPrefix, managedChainsRegex string, cleanupParentChain bool, rulespec ...rules.IPTablesRule) error {
+func (e *Enforcer) Enforce(table, parentChain, chainPrefix, managedChainsRegex string, cleanupParentChain bool, rulespec ...rules.IPTablesRule) (string, error) {
 	newTime := e.timestamper.CurrentTime()
 	chain := fmt.Sprintf("%s%d", chainPrefix, newTime)
 
 	err := e.iptables.NewChain(table, chain)
 	if err != nil {
 		e.Logger.Error("create-chain", err)
-		return fmt.Errorf("creating chain: %s", err)
+		return "", fmt.Errorf("creating chain: %s", err)
 	}
 
 	if e.conf.DisableContainerNetworkPolicy {
@@ -110,21 +110,21 @@ func (e *Enforcer) Enforce(table, parentChain, chainPrefix, managedChainsRegex s
 	err = e.iptables.BulkInsert(table, parentChain, 1, rules.IPTablesRule{"-j", chain})
 	if err != nil {
 		e.Logger.Error("insert-chain", err)
-		return fmt.Errorf("inserting chain: %s", err)
+		return "", fmt.Errorf("inserting chain: %s", err)
 	}
 
 	err = e.iptables.BulkAppend(table, chain, rulespec...)
 	if err != nil {
-		return fmt.Errorf("bulk appending: %s", err)
+		return "", fmt.Errorf("bulk appending: %s", err)
 	}
 
 	err = e.cleanupOldRules(table, parentChain, managedChainsRegex, cleanupParentChain, newTime)
 	if err != nil {
 		e.Logger.Error("cleanup-rules", err)
-		return err
+		return "", err
 	}
 
-	return nil
+	return chain, nil
 }
 
 func (e *Enforcer) cleanupOldRules(table, parentChain, managedChainsRegex string, cleanupParentChain bool, newTime int64) error {
@@ -174,12 +174,19 @@ func (e *Enforcer) cleanupOldChain(table, parentChain, timeStampedChain string)
 		return fmt.Errorf("cleanup old chain: %s", err)
 	}
 
-	err = e.iptables.ClearChain(table, timeStampedChain)
+	err = e.DeleteChain(table, timeStampedChain)
+
+	return err
+}
+
+func (e *Enforcer) DeleteChain(table string, chain string) error {
+
+	err := e.iptables.ClearChain(table, chain)
 	if err != nil {
 		return fmt.Errorf("cleanup old chain: %s", err)
 	}
 
-	err = e.iptables.DeleteChain(table, timeStampedChain)
+	err = e.iptables.DeleteChain(table, chain)
 	if err != nil {
 		return fmt.Errorf("cleanup old chain: %s", err)
 	}