From 902e1834f75a9fffcbb2d568965996ea5f2530d3 Mon Sep 17 00:00:00 2001 From: Nageswara Nandigam <84482346+nagworld9@users.noreply.github.com> Date: Fri, 11 Feb 2022 13:26:47 -0800 Subject: [PATCH] ignore firewall packets reset error, check enable firewall config flag and extend cgroup extension monitoring expiry time (#2502) --- azurelinuxagent/common/conf.py | 6 +- azurelinuxagent/common/osutil/default.py | 5 +- azurelinuxagent/ga/update.py | 3 + tests/common/osutil/test_default.py | 7 ++ tests/ga/test_update.py | 109 ++++++++++++----------- tests/test_agent.py | 2 +- 6 files changed, 73 insertions(+), 59 deletions(-) diff --git a/azurelinuxagent/common/conf.py b/azurelinuxagent/common/conf.py index 65d0703094..ad6b939d54 100644 --- a/azurelinuxagent/common/conf.py +++ b/azurelinuxagent/common/conf.py @@ -160,7 +160,7 @@ def load_conf_from_file(conf_file_path, conf=__conf__): "ResourceDisk.MountOptions": None, "ResourceDisk.Filesystem": "ext3", "AutoUpdate.GAFamily": "Prod", - "Debug.CgroupMonitorExpiryTime": "2022-01-31", + "Debug.CgroupMonitorExpiryTime": "2022-03-31", "Debug.CgroupMonitorExtensionName": "Microsoft.Azure.Monitor.AzureMonitorLinuxAgent", } @@ -550,11 +550,11 @@ def get_agent_cpu_quota(conf=__conf__): def get_cgroup_monitor_expiry_time (conf=__conf__): """ - cgroups monitoring disabled after expiry time + cgroups monitoring for pilot extensions disabled after expiry time NOTE: This option is experimental and may be removed in later versions of the Agent. """ - return conf.get("Debug.CgroupMonitorExpiryTime", "2022-01-31") + return conf.get("Debug.CgroupMonitorExpiryTime", "2022-03-31") def get_cgroup_monitor_extension_name (conf=__conf__): """ diff --git a/azurelinuxagent/common/osutil/default.py b/azurelinuxagent/common/osutil/default.py index 1c6042165c..4c706a9e70 100644 --- a/azurelinuxagent/common/osutil/default.py +++ b/azurelinuxagent/common/osutil/default.py @@ -179,9 +179,10 @@ def get_firewall_dropped_packets(self, dst_ip=None): return int(m.group(1)) except Exception as e: - if isinstance(e, CommandError) and e.returncode == 3: # pylint: disable=E1101 - # Transient error that we ignore. This code fires every loop + if isinstance(e, CommandError) and (e.returncode == 3 or e.returncode == 4): # pylint: disable=E1101 + # Transient error that we ignore returncode 3. This code fires every loop # of the daemon (60m), so we will get the value eventually. + # ignore returncode 4 as temporary fix (RULE_REPLACE failed (Invalid argument)) return 0 logger.warn("Failed to get firewall packets: {0}", ustr(e)) return -1 diff --git a/azurelinuxagent/ga/update.py b/azurelinuxagent/ga/update.py index 293dd2313a..cfac6f48c1 100644 --- a/azurelinuxagent/ga/update.py +++ b/azurelinuxagent/ga/update.py @@ -1075,6 +1075,9 @@ def _ensure_firewall_rules_persisted(dst_ip): def _add_accept_tcp_firewall_rule_if_not_enabled(self, dst_ip): + if not conf.enable_firewall(): + return + def _execute_run_command(command): # Helper to execute a run command, returns True if no exception # Here we primarily check if an iptable rule exist. True if it exits , false if not diff --git a/tests/common/osutil/test_default.py b/tests/common/osutil/test_default.py index e34e3d4f47..5e61034a00 100644 --- a/tests/common/osutil/test_default.py +++ b/tests/common/osutil/test_default.py @@ -715,6 +715,13 @@ def test_get_firewall_dropped_packets_should_ignore_transient_errors(self): mock_iptables.set_command(osutil.get_firewall_packets_command(mock_iptables.wait), exit_code=3, output="can't initialize iptables table `security': iptables who? (do you need to insmod?)") self.assertEqual(0, osutil.DefaultOSUtil().get_firewall_dropped_packets()) + def test_get_firewall_dropped_packets_should_ignore_returncode_4(self): + + with TestOSUtil._mock_iptables() as mock_iptables: + with patch.object(osutil, '_enable_firewall', True): + mock_iptables.set_command(osutil.get_firewall_packets_command(mock_iptables.wait), exit_code=4, output="iptables v1.8.2 (nf_tables): RULE_REPLACE failed (Invalid argument): rule in chain OUTPUT") + self.assertEqual(0, osutil.DefaultOSUtil().get_firewall_dropped_packets()) + def test_get_firewall_dropped_packets(self): destination = '168.63.129.16' diff --git a/tests/ga/test_update.py b/tests/ga/test_update.py index 6213957b4a..2bc627ddae 100644 --- a/tests/ga/test_update.py +++ b/tests/ga/test_update.py @@ -1699,73 +1699,76 @@ def test_it_should_set_dns_tcp_iptable_if_drop_available_accept_unavailable(self with TestOSUtil._mock_iptables() as mock_iptables: with _get_update_handler(test_data=DATA_FILE) as (update_handler, _): - with patch.object(osutil, '_enable_firewall', True): - # drop rule is present - mock_iptables.set_command(osutil.get_firewall_drop_command(mock_iptables.wait, AddFirewallRules.CHECK_COMMAND, mock_iptables.destination), exit_code=0) - # non root tcp iptable rule is absent - mock_iptables.set_command(osutil.get_accept_tcp_rule(mock_iptables.wait, AddFirewallRules.CHECK_COMMAND, mock_iptables.destination), exit_code=1) - update_handler.run(debug=True) - - drop_check_command = TestOSUtil._command_to_string(osutil.get_firewall_drop_command(mock_iptables.wait, AddFirewallRules.CHECK_COMMAND, mock_iptables.destination)) - accept_tcp_check_rule = TestOSUtil._command_to_string(osutil.get_accept_tcp_rule(mock_iptables.wait, AddFirewallRules.CHECK_COMMAND, mock_iptables.destination)) - accept_tcp_insert_rule = TestOSUtil._command_to_string(osutil.get_accept_tcp_rule(mock_iptables.wait, AddFirewallRules.INSERT_COMMAND, mock_iptables.destination)) - - # Filtering the mock iptable command calls with only the once related to this test. - filtered_mock_iptable_calls = [cmd for cmd in mock_iptables.command_calls if cmd in [drop_check_command, accept_tcp_check_rule, accept_tcp_insert_rule]] - - self.assertEqual(len(filtered_mock_iptable_calls), 3, "Incorrect number of calls to iptables: [{0}]".format(mock_iptables.command_calls)) - self.assertEqual(filtered_mock_iptable_calls[0], drop_check_command, - "The first command should check the drop rule") - self.assertEqual(filtered_mock_iptable_calls[1], accept_tcp_check_rule, - "The second command should check the accept rule") - self.assertEqual(filtered_mock_iptable_calls[2], accept_tcp_insert_rule, - "The third command should add the accept rule") + with patch('azurelinuxagent.common.conf.enable_firewall', return_value=True): + with patch.object(osutil, '_enable_firewall', True): + # drop rule is present + mock_iptables.set_command(osutil.get_firewall_drop_command(mock_iptables.wait, AddFirewallRules.CHECK_COMMAND, mock_iptables.destination), exit_code=0) + # non root tcp iptable rule is absent + mock_iptables.set_command(osutil.get_accept_tcp_rule(mock_iptables.wait, AddFirewallRules.CHECK_COMMAND, mock_iptables.destination), exit_code=1) + update_handler.run(debug=True) + + drop_check_command = TestOSUtil._command_to_string(osutil.get_firewall_drop_command(mock_iptables.wait, AddFirewallRules.CHECK_COMMAND, mock_iptables.destination)) + accept_tcp_check_rule = TestOSUtil._command_to_string(osutil.get_accept_tcp_rule(mock_iptables.wait, AddFirewallRules.CHECK_COMMAND, mock_iptables.destination)) + accept_tcp_insert_rule = TestOSUtil._command_to_string(osutil.get_accept_tcp_rule(mock_iptables.wait, AddFirewallRules.INSERT_COMMAND, mock_iptables.destination)) + + # Filtering the mock iptable command calls with only the once related to this test. + filtered_mock_iptable_calls = [cmd for cmd in mock_iptables.command_calls if cmd in [drop_check_command, accept_tcp_check_rule, accept_tcp_insert_rule]] + + self.assertEqual(len(filtered_mock_iptable_calls), 3, "Incorrect number of calls to iptables: [{0}]".format(mock_iptables.command_calls)) + self.assertEqual(filtered_mock_iptable_calls[0], drop_check_command, + "The first command should check the drop rule") + self.assertEqual(filtered_mock_iptable_calls[1], accept_tcp_check_rule, + "The second command should check the accept rule") + self.assertEqual(filtered_mock_iptable_calls[2], accept_tcp_insert_rule, + "The third command should add the accept rule") def test_it_should_not_set_dns_tcp_iptable_if_drop_unavailable(self): with TestOSUtil._mock_iptables() as mock_iptables: with _get_update_handler(test_data=DATA_FILE) as (update_handler, _): - with patch.object(osutil, '_enable_firewall', True): - # drop rule is not available - mock_iptables.set_command(osutil.get_firewall_drop_command(mock_iptables.wait, AddFirewallRules.CHECK_COMMAND, mock_iptables.destination), exit_code=1) + with patch('azurelinuxagent.common.conf.enable_firewall', return_value=True): + with patch.object(osutil, '_enable_firewall', True): + # drop rule is not available + mock_iptables.set_command(osutil.get_firewall_drop_command(mock_iptables.wait, AddFirewallRules.CHECK_COMMAND, mock_iptables.destination), exit_code=1) - update_handler.run(debug=True) + update_handler.run(debug=True) - drop_check_command = TestOSUtil._command_to_string(osutil.get_firewall_drop_command(mock_iptables.wait, AddFirewallRules.CHECK_COMMAND, mock_iptables.destination)) - accept_tcp_check_rule = TestOSUtil._command_to_string(osutil.get_accept_tcp_rule(mock_iptables.wait, AddFirewallRules.CHECK_COMMAND, mock_iptables.destination)) - accept_tcp_insert_rule = TestOSUtil._command_to_string(osutil.get_accept_tcp_rule(mock_iptables.wait, AddFirewallRules.INSERT_COMMAND, mock_iptables.destination)) + drop_check_command = TestOSUtil._command_to_string(osutil.get_firewall_drop_command(mock_iptables.wait, AddFirewallRules.CHECK_COMMAND, mock_iptables.destination)) + accept_tcp_check_rule = TestOSUtil._command_to_string(osutil.get_accept_tcp_rule(mock_iptables.wait, AddFirewallRules.CHECK_COMMAND, mock_iptables.destination)) + accept_tcp_insert_rule = TestOSUtil._command_to_string(osutil.get_accept_tcp_rule(mock_iptables.wait, AddFirewallRules.INSERT_COMMAND, mock_iptables.destination)) - # Filtering the mock iptable command calls with only the once related to this test. - filtered_mock_iptable_calls = [cmd for cmd in mock_iptables.command_calls if cmd in [drop_check_command, accept_tcp_check_rule, accept_tcp_insert_rule]] + # Filtering the mock iptable command calls with only the once related to this test. + filtered_mock_iptable_calls = [cmd for cmd in mock_iptables.command_calls if cmd in [drop_check_command, accept_tcp_check_rule, accept_tcp_insert_rule]] - self.assertEqual(len(filtered_mock_iptable_calls), 1, "Incorrect number of calls to iptables: [{0}]".format(mock_iptables.command_calls)) - self.assertEqual(filtered_mock_iptable_calls[0], drop_check_command, - "The first command should check the drop rule") + self.assertEqual(len(filtered_mock_iptable_calls), 1, "Incorrect number of calls to iptables: [{0}]".format(mock_iptables.command_calls)) + self.assertEqual(filtered_mock_iptable_calls[0], drop_check_command, + "The first command should check the drop rule") def test_it_should_not_set_dns_tcp_iptable_if_drop_and_accept_available(self): with TestOSUtil._mock_iptables() as mock_iptables: with _get_update_handler(test_data=DATA_FILE) as (update_handler, _): - with patch.object(osutil, '_enable_firewall', True): - # drop rule is available - mock_iptables.set_command(osutil.get_firewall_drop_command(mock_iptables.wait, AddFirewallRules.CHECK_COMMAND, mock_iptables.destination), exit_code=0) - # non root tcp iptable rule is available - mock_iptables.set_command(osutil.get_accept_tcp_rule(mock_iptables.wait, AddFirewallRules.CHECK_COMMAND, mock_iptables.destination), exit_code=0) - - update_handler.run(debug=True) - - drop_check_command = TestOSUtil._command_to_string(osutil.get_firewall_drop_command(mock_iptables.wait, AddFirewallRules.CHECK_COMMAND, mock_iptables.destination)) - accept_tcp_check_rule = TestOSUtil._command_to_string(osutil.get_accept_tcp_rule(mock_iptables.wait, AddFirewallRules.CHECK_COMMAND, mock_iptables.destination)) - accept_tcp_insert_rule = TestOSUtil._command_to_string(osutil.get_accept_tcp_rule(mock_iptables.wait, AddFirewallRules.INSERT_COMMAND, mock_iptables.destination)) - - # Filtering the mock iptable command calls with only the once related to this test. - filtered_mock_iptable_calls = [cmd for cmd in mock_iptables.command_calls if cmd in [drop_check_command, accept_tcp_check_rule, accept_tcp_insert_rule]] - - self.assertEqual(len(filtered_mock_iptable_calls), 2, "Incorrect number of calls to iptables: [{0}]".format(mock_iptables.command_calls)) - self.assertEqual(filtered_mock_iptable_calls[0], drop_check_command, - "The first command should check the drop rule") - self.assertEqual(filtered_mock_iptable_calls[1], accept_tcp_check_rule, - "The second command should check the accept rule") + with patch('azurelinuxagent.common.conf.enable_firewall', return_value=True): + with patch.object(osutil, '_enable_firewall', True): + # drop rule is available + mock_iptables.set_command(osutil.get_firewall_drop_command(mock_iptables.wait, AddFirewallRules.CHECK_COMMAND, mock_iptables.destination), exit_code=0) + # non root tcp iptable rule is available + mock_iptables.set_command(osutil.get_accept_tcp_rule(mock_iptables.wait, AddFirewallRules.CHECK_COMMAND, mock_iptables.destination), exit_code=0) + + update_handler.run(debug=True) + + drop_check_command = TestOSUtil._command_to_string(osutil.get_firewall_drop_command(mock_iptables.wait, AddFirewallRules.CHECK_COMMAND, mock_iptables.destination)) + accept_tcp_check_rule = TestOSUtil._command_to_string(osutil.get_accept_tcp_rule(mock_iptables.wait, AddFirewallRules.CHECK_COMMAND, mock_iptables.destination)) + accept_tcp_insert_rule = TestOSUtil._command_to_string(osutil.get_accept_tcp_rule(mock_iptables.wait, AddFirewallRules.INSERT_COMMAND, mock_iptables.destination)) + + # Filtering the mock iptable command calls with only the once related to this test. + filtered_mock_iptable_calls = [cmd for cmd in mock_iptables.command_calls if cmd in [drop_check_command, accept_tcp_check_rule, accept_tcp_insert_rule]] + + self.assertEqual(len(filtered_mock_iptable_calls), 2, "Incorrect number of calls to iptables: [{0}]".format(mock_iptables.command_calls)) + self.assertEqual(filtered_mock_iptable_calls[0], drop_check_command, + "The first command should check the drop rule") + self.assertEqual(filtered_mock_iptable_calls[1], accept_tcp_check_rule, + "The second command should check the accept rule") @contextlib.contextmanager def _setup_test_for_ext_event_dirs_retention(self): diff --git a/tests/test_agent.py b/tests/test_agent.py index 1ce321290c..6cf0632b32 100644 --- a/tests/test_agent.py +++ b/tests/test_agent.py @@ -36,7 +36,7 @@ Debug.CgroupDisableOnProcessCheckFailure = True Debug.CgroupDisableOnQuotaCheckFailure = True Debug.CgroupLogMetrics = False -Debug.CgroupMonitorExpiryTime = 2022-01-31 +Debug.CgroupMonitorExpiryTime = 2022-03-31 Debug.CgroupMonitorExtensionName = Microsoft.Azure.Monitor.AzureMonitorLinuxAgent Debug.EnableFastTrack = True Debug.EnableGAVersioning = False