From 1d40271a76b4b8e65a996697d896d491c774f7f9 Mon Sep 17 00:00:00 2001 From: Matt Date: Wed, 14 Sep 2016 10:37:42 -0700 Subject: [PATCH 1/5] Start adding config option to disable IPv6 --- calico/felix/config.py | 21 ++++++++++++++++++--- calico/felix/felix.py | 9 ++++++++- calico/felix/futils.py | 2 +- 3 files changed, 27 insertions(+), 5 deletions(-) diff --git a/calico/felix/config.py b/calico/felix/config.py index 05cf9c05eb3..35e1dc81faf 100644 --- a/calico/felix/config.py +++ b/calico/felix/config.py @@ -320,13 +320,21 @@ def __init__(self, config_path): "default value opens the ssh port, 22.", [22], value_is_int_list=True) self.add_parameter("FailsafeOutboundHostPorts", - "Comma-separated list of numeric TCP ports to allow" - "traffic to from all host endpoints. Useful to " - "avoid accidentally cutting off, for example, " + "Comma-separated list of numeric TCP ports to " + "allow traffic to from all host endpoints. Useful " + "to avoid accidentally cutting off, for example, " "access to etcd. The default value allows " "connectivity to etcd's default ports " "2379,2380,4001 and 7001.", [2379,2380,4001,7001], value_is_int_list=True) + self.add_parameter("Ipv6Support", + "Whether IPv6 support is enabled. If 'on', Felix " + "will program ip6tables rules and any IPv6 routes; " + "if 'off', Felix will not provide any IPv6 " + "function. If set to 'auto', Felix will attempt " + "to detect whether the system supports IPv6 and " + "use if it it does.", + "auto") # The following setting determines which flavour of Iptables Generator # plugin is loaded. Note: this plugin support is currently highly @@ -438,6 +446,7 @@ def _finish_update(self, final=False): self.IGNORE_LOOSE_RPF = self.parameters["IgnoreLooseRPF"].value self.CLUSTER_GUID = self.parameters["ClusterGUID"].value self.USAGE_REPORT = self.parameters["UsageReportingEnabled"].value + self.IPV6_SUPPORT = self.parameters["Ipv6Support"].value.lower() self._validate_cfg(final=final) @@ -773,6 +782,12 @@ def _validate_cfg(self, final=True): raise ConfigException("Out-of-range port %s" % p, self.parameters[name]) + if self.IPV6_SUPPORT not in ("on", "off", "auto"): + raise ConfigException( + "Invalid field value", + self.parameters["Ipv6Support"] + ) + if not final: # Do not check that unset parameters are defaulted; we have more # config to read. diff --git a/calico/felix/felix.py b/calico/felix/felix.py index 8b132dba99f..1da32c06307 100755 --- a/calico/felix/felix.py +++ b/calico/felix/felix.py @@ -136,7 +136,14 @@ def _main_greenlet(config): v4_fip_manager, ] - v6_enabled, ipv6_reason = futils.ipv6_supported() + v6_enabled = (config.IPV6_SUPPORT == ") + if config.IPV6_SUPPORT == "on": + v6_enabled = True + elif config.IPV6_SUPPORT == "auto": + v6_enabled, ipv6_reason = futils.detect_ipv6_supported() + else: + v6_enabled = False + ipv6_reason = if v6_enabled: v6_raw_updater = IptablesUpdater("raw", ip_version=6, config=config) v6_filter_updater = IptablesUpdater("filter", ip_version=6, diff --git a/calico/felix/futils.py b/calico/felix/futils.py index 84032ea1038..1d10ff7d5f6 100644 --- a/calico/felix/futils.py +++ b/calico/felix/futils.py @@ -528,7 +528,7 @@ def find_set_bits(mask): mask = next_mask -def ipv6_supported(): +def detect_ipv6_supported(): """Checks whether we can support IPv6 on this host. :returns tuple[bool,str]: supported, reason for lack of support or None. From b629d5968c7d645d0b7b1c6232a3ed65d6cdb1f2 Mon Sep 17 00:00:00 2001 From: Casey Davenport Date: Wed, 14 Sep 2016 10:48:43 -0700 Subject: [PATCH 2/5] Continue making Ipv6 support a felix option --- calico/felix/config.py | 6 +++--- calico/felix/felix.py | 8 +++++--- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/calico/felix/config.py b/calico/felix/config.py index 35e1dc81faf..8b64da63ae5 100644 --- a/calico/felix/config.py +++ b/calico/felix/config.py @@ -328,9 +328,9 @@ def __init__(self, config_path): "2379,2380,4001 and 7001.", [2379,2380,4001,7001], value_is_int_list=True) self.add_parameter("Ipv6Support", - "Whether IPv6 support is enabled. If 'on', Felix " + "Whether IPv6 support is enabled. If 'true', Felix " "will program ip6tables rules and any IPv6 routes; " - "if 'off', Felix will not provide any IPv6 " + "if 'false', Felix will not provide any IPv6 " "function. If set to 'auto', Felix will attempt " "to detect whether the system supports IPv6 and " "use if it it does.", @@ -782,7 +782,7 @@ def _validate_cfg(self, final=True): raise ConfigException("Out-of-range port %s" % p, self.parameters[name]) - if self.IPV6_SUPPORT not in ("on", "off", "auto"): + if self.IPV6_SUPPORT not in ("true", "false", "auto"): raise ConfigException( "Invalid field value", self.parameters["Ipv6Support"] diff --git a/calico/felix/felix.py b/calico/felix/felix.py index 1da32c06307..6334041af0d 100755 --- a/calico/felix/felix.py +++ b/calico/felix/felix.py @@ -136,14 +136,16 @@ def _main_greenlet(config): v4_fip_manager, ] - v6_enabled = (config.IPV6_SUPPORT == ") - if config.IPV6_SUPPORT == "on": + # Determine if ipv6 is enabled using the config option. + if config.IPV6_SUPPORT == "true": v6_enabled = True + ipv6_reason = None elif config.IPV6_SUPPORT == "auto": v6_enabled, ipv6_reason = futils.detect_ipv6_supported() else: v6_enabled = False - ipv6_reason = + ipv6_reason = "Ipv6Support is 'false'" + if v6_enabled: v6_raw_updater = IptablesUpdater("raw", ip_version=6, config=config) v6_filter_updater = IptablesUpdater("filter", ip_version=6, From d6a67344b6b31ace93a5f03d75914d9c5eff5e55 Mon Sep 17 00:00:00 2001 From: Casey Davenport Date: Wed, 14 Sep 2016 13:00:41 -0700 Subject: [PATCH 3/5] Add additional check to "auto" --- calico/felix/futils.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/calico/felix/futils.py b/calico/felix/futils.py index 1d10ff7d5f6..ad64c79dc54 100644 --- a/calico/felix/futils.py +++ b/calico/felix/futils.py @@ -541,6 +541,13 @@ def detect_ipv6_supported(): return False, ("ip6tables not installed; Calico IPv6 support requires " "Linux kernel v3.3 or above and ip6tables v1.4.14 or " "above.") + + # Check for the existence of the IPv6 NAT table. + try: + check_call(["ip6tables-save", "--table", "nat"]) + except FailedSystemCall: + return False, "Failed to load IPv6 NAT table" + try: # Use -C, which checks for a particular rule. We don't expect the rule # to exist but iptables will give us a distinctive error if the From 8990e4ae1adc582a994bf5373c423a1c2cd911cb Mon Sep 17 00:00:00 2001 From: Casey Davenport Date: Wed, 14 Sep 2016 14:05:09 -0700 Subject: [PATCH 4/5] UTs and Docs --- calico/felix/test/test_futils.py | 14 +++++++------- docs/source/configuration.rst | 5 +++++ 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/calico/felix/test/test_futils.py b/calico/felix/test/test_futils.py index a421aea1434..c56d434999e 100644 --- a/calico/felix/test/test_futils.py +++ b/calico/felix/test/test_futils.py @@ -137,7 +137,7 @@ def assert_safe_truncate(self, s, length, expected): self.assertEqual(result, expected, "Expected %r to be truncated as %r but got %r" % (s, expected, result)) - + def test_longest_prefix(self): self.assertEqual(futils.find_longest_prefix([]), None) self.assertEqual(futils.find_longest_prefix(["a"]), "a") @@ -152,10 +152,10 @@ def test_longest_prefix(self): @mock.patch("os.path.exists", autospec=True) @mock.patch("calico.felix.futils.check_call", autospec=True) @mock.patch("calico.felix.futils.Popen", autospec=True) - def test_ipv6_supported(self, m_popen, m_check_call, m_exists): + def test_detect_ipv6_supported(self, m_popen, m_check_call, m_exists): m_popen.return_value.communicate.return_value = "", "" m_exists.return_value = True - self.assertEqual(futils.ipv6_supported(), (True, None)) + self.assertEqual(futils.detect_ipv6_supported(), (True, None)) @mock.patch("os.path.exists", autospec=True) @mock.patch("calico.felix.futils.check_call", autospec=True) @@ -163,7 +163,7 @@ def test_ipv6_supported(self, m_popen, m_check_call, m_exists): def test_ipv6_compiled_out(self, m_popen, m_check_call, m_exists): m_popen.return_value.communicate.return_value = "", "" m_exists.return_value = False - self.assertEqual(futils.ipv6_supported(), (False, mock.ANY)) + self.assertEqual(futils.detect_ipv6_supported(), (False, mock.ANY)) @mock.patch("os.path.exists", autospec=True) @mock.patch("calico.felix.futils.check_call", autospec=True) @@ -172,7 +172,7 @@ def test_ipv6_missing_ip6tables(self, m_popen, m_check_call, m_exists): m_popen.return_value.communicate.return_value = "", "" m_exists.return_value = True m_check_call.side_effect = futils.FailedSystemCall() - self.assertEqual(futils.ipv6_supported(), (False, mock.ANY)) + self.assertEqual(futils.detect_ipv6_supported(), (False, mock.ANY)) @mock.patch("os.path.exists", autospec=True) @mock.patch("calico.felix.futils.check_call", autospec=True) @@ -184,7 +184,7 @@ def test_ipv6_missing_rpfilter(self, m_popen, m_check_call, m_exists): "ip6tables vA.B.C: Couldn't load match `rpfilter':No such file or " "directory" ) - self.assertEqual(futils.ipv6_supported(), (False, mock.ANY)) + self.assertEqual(futils.detect_ipv6_supported(), (False, mock.ANY)) @mock.patch("os.path.exists", autospec=True) @mock.patch("calico.felix.futils.check_call", autospec=True) @@ -192,7 +192,7 @@ def test_ipv6_missing_rpfilter(self, m_popen, m_check_call, m_exists): def test_ipv6_missing_rpfilter_error(self, m_popen, m_check_call, m_exists): m_exists.return_value = True m_popen.side_effect = OSError() - self.assertEqual(futils.ipv6_supported(), (False, mock.ANY)) + self.assertEqual(futils.detect_ipv6_supported(), (False, mock.ANY)) @mock.patch("calico.felix.futils.urllib3.disable_warnings", autospec=True) @mock.patch("calico.felix.futils.urllib3.util.retry.Retry", autospec=True) diff --git a/docs/source/configuration.rst b/docs/source/configuration.rst index e7a6d58d3c6..e1376f90f8e 100644 --- a/docs/source/configuration.rst +++ b/docs/source/configuration.rst @@ -175,6 +175,11 @@ The full list of parameters which can be set is as follows. | | | cutting off a host with incorrect configuration. The default value opens etcd's standard | | | | ports to ensure that Felix does not get cut off from etcd. | +----------------------------------+---------------------------------------+-------------------------------------------------------------------------------------------+ +| Ipv6Support | auto | Whether IPv6 support is enabled. If 'true', Felix will program ip6tables rules | +| | | and any IPv6 routes; if 'false', Felix will not provide any IPv6 function. If set | +| | | to 'auto', Felix will attempt to detect whether the system supports | +| | | IPv6 and use if it it does. | ++----------------------------------+---------------------------------------+-------------------------------------------------------------------------------------------+ Environment variables From 1e0b696824210ee03aa1bb1f3ab18871c1658cc9 Mon Sep 17 00:00:00 2001 From: Casey Davenport Date: Thu, 15 Sep 2016 10:24:55 -0700 Subject: [PATCH 5/5] Fixup review comments --- calico/felix/config.py | 9 ++++----- calico/felix/test/test_config.py | 6 ++++++ calico/felix/test/test_futils.py | 7 +++++++ docs/source/configuration.rst | 2 +- 4 files changed, 18 insertions(+), 6 deletions(-) diff --git a/calico/felix/config.py b/calico/felix/config.py index 8b64da63ae5..174dbc618b7 100644 --- a/calico/felix/config.py +++ b/calico/felix/config.py @@ -333,7 +333,7 @@ def __init__(self, config_path): "if 'false', Felix will not provide any IPv6 " "function. If set to 'auto', Felix will attempt " "to detect whether the system supports IPv6 and " - "use if it it does.", + "use it if it does.", "auto") # The following setting determines which flavour of Iptables Generator @@ -783,10 +783,9 @@ def _validate_cfg(self, final=True): self.parameters[name]) if self.IPV6_SUPPORT not in ("true", "false", "auto"): - raise ConfigException( - "Invalid field value", - self.parameters["Ipv6Support"] - ) + log.warning("Unrecognized value for Ipv6Support (%s), " + "defaulting to 'auto'", self.IPV6_SUPPORT) + self.IPV6_SUPPORT = "auto" if not final: # Do not check that unset parameters are defaulted; we have more diff --git a/calico/felix/test/test_config.py b/calico/felix/test/test_config.py index 9f1363ee2cf..7847a9d5328 100644 --- a/calico/felix/test/test_config.py +++ b/calico/felix/test/test_config.py @@ -96,6 +96,7 @@ def test_default_config(self): self.assertEqual(config.REPORTING_TTL_SECS, 90) self.assertEqual(config.IPTABLES_MARK_MASK, 0xff000000) self.assertEqual(config.IPTABLES_MARK_ACCEPT, "0x1000000") + self.assertEqual(config.IPV6_SUPPORT, "auto") def test_bad_plugin_name(self): env_dict = {"FELIX_IPTABLESGENERATORPLUGIN": "unknown"} @@ -105,6 +106,11 @@ def test_bad_plugin_name(self): '"calico.felix.iptables_generator".'): config = load_config("felix_default.cfg", env_dict=env_dict) + def test_bad_ipv6_support_value(self): + env_dict = {"FELIX_IPV6SUPPORT": "badvalue"} + config = load_config("felix_default.cfg", env_dict=env_dict) + self.assertEqual(config.IPV6_SUPPORT, "auto") + def test_invalid_port(self): data = { "felix_invalid_port.cfg": "Invalid port in field", "felix_invalid_addr.cfg": "Invalid or unresolvable", diff --git a/calico/felix/test/test_futils.py b/calico/felix/test/test_futils.py index c56d434999e..7bb77d4bf61 100644 --- a/calico/felix/test/test_futils.py +++ b/calico/felix/test/test_futils.py @@ -194,6 +194,13 @@ def test_ipv6_missing_rpfilter_error(self, m_popen, m_check_call, m_exists): m_popen.side_effect = OSError() self.assertEqual(futils.detect_ipv6_supported(), (False, mock.ANY)) + @mock.patch("os.path.exists", autospec=True) + @mock.patch("calico.felix.futils.check_call", autospec=True) + def test_ipv6_missing_nat_table(self, m_check_call, m_exists): + m_exists.return_value = True + m_check_call.side_effect = iter([None, futils.FailedSystemCall()]) + self.assertEqual(futils.detect_ipv6_supported(), (False, mock.ANY)) + @mock.patch("calico.felix.futils.urllib3.disable_warnings", autospec=True) @mock.patch("calico.felix.futils.urllib3.util.retry.Retry", autospec=True) @mock.patch("calico.felix.futils.urllib3.PoolManager", autospec=True) diff --git a/docs/source/configuration.rst b/docs/source/configuration.rst index e1376f90f8e..c61f8ac3191 100644 --- a/docs/source/configuration.rst +++ b/docs/source/configuration.rst @@ -178,7 +178,7 @@ The full list of parameters which can be set is as follows. | Ipv6Support | auto | Whether IPv6 support is enabled. If 'true', Felix will program ip6tables rules | | | | and any IPv6 routes; if 'false', Felix will not provide any IPv6 function. If set | | | | to 'auto', Felix will attempt to detect whether the system supports | -| | | IPv6 and use if it it does. | +| | | IPv6 and use it if it does. | +----------------------------------+---------------------------------------+-------------------------------------------------------------------------------------------+