diff --git a/controllers/gce/firewalls/firewalls.go b/controllers/gce/firewalls/firewalls.go index 3aa07d80d..211595aad 100644 --- a/controllers/gce/firewalls/firewalls.go +++ b/controllers/gce/firewalls/firewalls.go @@ -80,18 +80,19 @@ func (fr *FirewallRules) Sync(nodePorts []int64, nodeNames []string) error { existingCIDRs := sets.NewString(rule.SourceRanges...) // Do not update if ports and source cidrs are not outdated. - // NOTE: We are not checking if nodeNames matches the firwall targetTags + // NOTE: We are not checking if nodeNames matches the firewall targetTags if requiredPorts.Equal(existingPorts) && requiredCIDRs.Equal(existingCIDRs) { + glog.V(4).Info("Firewall does not need update of ports or source ranges") return nil } - glog.V(3).Infof("Firewall rule %v already exists, updating nodeports %v", name, nodePorts) - return fr.cloud.UpdateFirewall(suffix, "GCE L7 firewall rule", fr.srcRanges, nodePorts, nodeNames) + glog.V(3).Infof("Firewall %v already exists, updating nodeports %v", name, nodePorts) + return fr.cloud.UpdateFirewall(suffix, "GCE L7 firewall", fr.srcRanges, nodePorts, nodeNames) } // Shutdown shuts down this firewall rules manager. func (fr *FirewallRules) Shutdown() error { - glog.Infof("Deleting firewall rule with suffix %v", fr.namer.FrSuffix()) + glog.Infof("Deleting firewall with suffix %v", fr.namer.FrSuffix()) err := fr.cloud.DeleteFirewall(fr.namer.FrSuffix()) if err != nil && utils.IsHTTPErrorCode(err, 404) { glog.Infof("Firewall with suffix %v didn't exist at Shutdown", fr.namer.FrSuffix()) diff --git a/controllers/gce/firewalls/firewalls_test.go b/controllers/gce/firewalls/firewalls_test.go index fb3708f39..ccdf1c9a3 100644 --- a/controllers/gce/firewalls/firewalls_test.go +++ b/controllers/gce/firewalls/firewalls_test.go @@ -1,3 +1,19 @@ +/* +Copyright 2015 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + package firewalls import ( @@ -9,6 +25,8 @@ import ( netset "k8s.io/kubernetes/pkg/util/net/sets" ) +const allCIDR = "0.0.0.0/0" + func TestSyncFirewallPool(t *testing.T) { namer := utils.NewNamer("ABC", "XYZ") fwp := NewFakeFirewallsProvider(namer) @@ -32,13 +50,12 @@ func TestSyncFirewallPool(t *testing.T) { } verifyFirewallRule(fwp, ruleName, nodePorts, nodes, l7SrcRanges, t) - all := "0.0.0.0/0" - srcRanges, _ := netset.ParseIPNets(all) + srcRanges, _ := netset.ParseIPNets(allCIDR) err = fwp.UpdateFirewall(namer.FrSuffix(), "", srcRanges, nodePorts, nodes) if err != nil { t.Errorf("failed to update firewall rule, err: %v", err) } - verifyFirewallRule(fwp, ruleName, nodePorts, nodes, []string{all}, t) + verifyFirewallRule(fwp, ruleName, nodePorts, nodes, []string{allCIDR}, t) // Run Sync and expect l7 src ranges to be returned err = fp.Sync(nodePorts, nodes) @@ -47,7 +64,7 @@ func TestSyncFirewallPool(t *testing.T) { } verifyFirewallRule(fwp, ruleName, nodePorts, nodes, l7SrcRanges, t) - // Add node and expect firwall to remain the same + // Add node and expect firewall to remain the same // NOTE: See computeHostTag(..) in gce cloudprovider nodes = []string{"node-a", "node-b", "node-c", "node-d"} err = fp.Sync(nodePorts, nodes) @@ -81,12 +98,12 @@ func verifyFirewallRule(fwp *fakeFirewallsProvider, ruleName string, expectedPor t.Errorf("could not retrieve firewall via cloud api, err %v", err) } - // Verify firwall rule has correct ports + // Verify firewall rule has correct ports if !sets.NewString(f.Allowed[0].Ports...).Equal(sets.NewString(strPorts...)) { t.Errorf("allowed ports doesn't equal expected ports, Actual: %v, Expected: %v", f.Allowed[0].Ports, strPorts) } - // Verify firwall rule has correct CIDRs + // Verify firewall rule has correct CIDRs if !sets.NewString(f.SourceRanges...).Equal(sets.NewString(expectedCIDRs...)) { t.Errorf("source CIDRs doesn't equal expected CIDRs. Actual: %v, Expected: %v", f.SourceRanges, expectedCIDRs) }