sts-sponsors team mailing list archive
-
sts-sponsors team
-
Mailing list archive
-
Message #05252
[Merge] ~ack/maas:drop-unused-rack-commands into maas:master
Alberto Donato has proposed merging ~ack/maas:drop-unused-rack-commands into maas:master.
Commit message:
drop unused maas-rack subcommands
this drops
- support-dump
- check-for-shared-secret
- drop unused "observe-dhcp
Requested reviews:
MAAS Maintainers (maas-maintainers)
For more details, see:
https://code.launchpad.net/~ack/maas/+git/maas/+merge/437565
--
Your team MAAS Maintainers is requested to review the proposed merge of ~ack/maas:drop-unused-rack-commands into maas:master.
diff --git a/src/provisioningserver/__main__.py b/src/provisioningserver/__main__.py
index 29dc911..63c3d0f 100644
--- a/src/provisioningserver/__main__.py
+++ b/src/provisioningserver/__main__.py
@@ -16,7 +16,6 @@ import provisioningserver.upgrade_cluster
import provisioningserver.utils.arp
import provisioningserver.utils.avahi
import provisioningserver.utils.beaconing
-import provisioningserver.utils.dhcp
import provisioningserver.utils.scan_network
from provisioningserver.utils.script import MainScript
@@ -24,7 +23,6 @@ COMMON_COMMANDS = {
"observe-arp": provisioningserver.utils.arp,
"observe-beacons": provisioningserver.utils.beaconing,
"observe-mdns": provisioningserver.utils.avahi,
- "observe-dhcp": provisioningserver.utils.dhcp,
"scan-network": provisioningserver.utils.scan_network,
"setup-dns": provisioningserver.dns.commands.setup_dns,
"get-named-conf": provisioningserver.dns.commands.get_named_conf,
@@ -32,11 +30,9 @@ COMMON_COMMANDS = {
}
RACK_ONLY_COMMANDS = {
- "check-for-shared-secret": security.CheckForSharedSecretScript,
"config": provisioningserver.cluster_config_command,
"install-shared-secret": security.InstallSharedSecretScript,
"register": provisioningserver.register_command,
- "support-dump": provisioningserver.support_dump,
"upgrade-cluster": provisioningserver.upgrade_cluster,
}
diff --git a/src/provisioningserver/dhcp/detect.py b/src/provisioningserver/dhcp/detect.py
index feef033..78d1907 100644
--- a/src/provisioningserver/dhcp/detect.py
+++ b/src/provisioningserver/dhcp/detect.py
@@ -352,7 +352,7 @@ class DHCPRequestMonitor:
continue
else:
offer = DHCP(data)
- if not offer.is_valid():
+ if not offer.valid:
log.info(
"Invalid DHCP response received from {address} "
"on '{ifname}': {reason}",
diff --git a/src/provisioningserver/security.py b/src/provisioningserver/security.py
index 00caaf6..612916c 100644
--- a/src/provisioningserver/security.py
+++ b/src/provisioningserver/security.py
@@ -200,31 +200,3 @@ class InstallSharedSecretScript:
MAAS_SHARED_SECRET.set(secret_hex)
print(f"Secret installed to {MAAS_SHARED_SECRET.path}.")
raise SystemExit(0)
-
-
-class CheckForSharedSecretScript:
- """Check for the presence of a shared-secret on a cluster.
-
- This class conforms to the contract that :py:func:`MainScript.register`
- requires.
- """
-
- @staticmethod
- def add_arguments(parser):
- """Initialise options for checking the presence of a shared-secret.
-
- :param parser: An instance of :class:`ArgumentParser`.
- """
-
- @staticmethod
- def run(args):
- """Check for the presence of a shared-secret on this cluster.
-
- Exits 0 (zero) if a shared-secret has been installed.
- """
- if MAAS_SHARED_SECRET.get() is None:
- print("Shared-secret is NOT installed.")
- raise SystemExit(1)
- else:
- print("Shared-secret is installed.")
- raise SystemExit(0)
diff --git a/src/provisioningserver/support_dump.py b/src/provisioningserver/support_dump.py
deleted file mode 100644
index e9c6233..0000000
--- a/src/provisioningserver/support_dump.py
+++ /dev/null
@@ -1,124 +0,0 @@
-# Copyright 2016 Canonical Ltd. This software is licensed under the
-# GNU Affero General Public License version 3 (see the file LICENSE).
-
-"""MAAS rack controller support dump commands."""
-
-
-from functools import lru_cache
-import json
-import os
-from pprint import pprint
-import traceback
-
-from provisioningserver.boot.tftppath import (
- get_image_metadata,
- list_boot_images,
-)
-from provisioningserver.config import ClusterConfiguration
-from provisioningserver.utils.ipaddr import get_ip_addr
-from provisioningserver.utils.iproute import get_ip_route
-from provisioningserver.utils.network import get_all_interfaces_definition
-
-all_arguments = ("--networking", "--config", "--images")
-
-
-def add_arguments(parser):
- """Add this command's options to the `ArgumentParser`.
-
- Specified by the `ActionScript` interface.
- """
- parser.description = run.__doc__
- parser.add_argument(
- "--networking",
- action="store_true",
- required=False,
- help="Dump networking information.",
- )
- parser.add_argument(
- "--config",
- action="store_true",
- required=False,
- help="Dump configuration information.",
- )
- parser.add_argument(
- "--images",
- action="store_true",
- required=False,
- help="Dump boot image information.",
- )
-
-
-@lru_cache(1)
-def get_cluster_configuration():
- config_dict = {}
- try:
- with ClusterConfiguration.open() as configuration:
- for var in vars(ClusterConfiguration):
- if not var.startswith("_"):
- config_dict[var] = getattr(configuration, var)
- except Exception:
- print("Warning: Could not load cluster configuration.")
- print("(some data may not be accurate)")
- print()
- return config_dict
-
-
-# The following lists define the functions and/or commands that will be run
-# during each phase of the support dump.
-NETWORKING_DUMP = [
- {"function": get_ip_addr},
- {"function": get_ip_route},
- {"function": get_all_interfaces_definition},
-]
-
-CONFIG_DUMP = [{"function": get_cluster_configuration}]
-
-IMAGES_DUMP = [
- {"function": list_boot_images},
- {
- "title": "get_image_metadata()",
- "function": lambda *args: json.loads(get_image_metadata(*args)),
- },
-]
-
-
-def _dump(item, *args, **kwargs):
- if "function" in item:
- function = item["function"]
- title = item["title"] if "title" in item else function.__name__ + "()"
- print("### %s ###" % title)
- try:
- pprint(function(*args, **kwargs))
- except Exception:
- print(traceback.format_exc())
- print()
- if "command" in item:
- print("### %s ###" % item["command"])
- os.system("%s 2>&1" % item["command"])
- print()
-
-
-def run(args):
- """Dump support information. By default, dumps everything available."""
- params = vars(args).copy()
- networking = params.pop("networking", False)
- config = params.pop("config", False)
- images = params.pop("images", False)
-
- config_dict = get_cluster_configuration()
-
- complete = False
- if not networking and not images and not config:
- complete = True
-
- if complete or networking:
- for item in NETWORKING_DUMP:
- _dump(item)
-
- if complete or config:
- for item in CONFIG_DUMP:
- _dump(item)
-
- if complete or images:
- for item in IMAGES_DUMP:
- _dump(item, config_dict["tftp_root"])
diff --git a/src/provisioningserver/tests/test_security.py b/src/provisioningserver/tests/test_security.py
index 77d274b..4497686 100644
--- a/src/provisioningserver/tests/test_security.py
+++ b/src/provisioningserver/tests/test_security.py
@@ -164,40 +164,6 @@ class TestInstallSharedSecretScript(MAASTestCase):
)
-class TestCheckForSharedSecretScript(MAASTestCase):
- def setUp(self):
- super().setUp()
- tempdir = Path(self.make_dir())
- utils_env.MAAS_SHARED_SECRET.clear_cached()
- self.patch(
- utils_env.MAAS_SHARED_SECRET, "_path", lambda: tempdir / "secret"
- )
- self._mock_print = self.patch(security, "print")
-
- def test_has_add_arguments(self):
- # It doesn't do anything, but it's there to fulfil the contract with
- # ActionScript/MainScript.
- security.CheckForSharedSecretScript.add_arguments(sentinel.parser)
- self.assertIsNotNone("Obligatory assertion.")
-
- def test_exits_non_zero_if_secret_does_not_exist(self):
- error = self.assertRaises(
- SystemExit, security.CheckForSharedSecretScript.run, sentinel.args
- )
- self.assertEqual(1, error.code)
- self._mock_print.assert_called_once_with(
- "Shared-secret is NOT installed."
- )
-
- def test_exits_zero_if_secret_exists(self):
- utils_env.MAAS_SHARED_SECRET.set(security.to_hex(factory.make_bytes()))
- error = self.assertRaises(
- SystemExit, security.CheckForSharedSecretScript.run, sentinel.args
- )
- self.assertEqual(0, error.code)
- self._mock_print.assert_called_once_with("Shared-secret is installed.")
-
-
class TestFernetEncryption(SharedSecretTestCase):
def setUp(self):
super().setUp()
diff --git a/src/provisioningserver/tests/test_support_dump.py b/src/provisioningserver/tests/test_support_dump.py
deleted file mode 100644
index 5e62ec8..0000000
--- a/src/provisioningserver/tests/test_support_dump.py
+++ /dev/null
@@ -1,135 +0,0 @@
-# Copyright 2016 Canonical Ltd. This software is licensed under the
-# GNU Affero General Public License version 3 (see the file LICENSE).
-
-"""Tests for configuration update code."""
-
-
-from unittest.mock import Mock
-
-from testtools.matchers import Equals, HasLength
-
-from maastesting.testcase import MAASTestCase
-from provisioningserver import support_dump
-
-
-class TestUpdateMaasClusterConf(MAASTestCase):
- # Test to ensure that if exceptions are thrown during the dump, the dump
- # continues through all the items to dump anyway.
- scenarios = (
- ("without_exception_mock", {"exceptions": False}),
- ("with_exception_mock", {"exceptions": True}),
- )
-
- def setUp(self):
- super().setUp()
- self.mock_calls = 0
- self.last_args = None
- self.last_kwargs = None
- self.expected_networking = len(support_dump.NETWORKING_DUMP)
- self.expected_config = len(support_dump.CONFIG_DUMP)
- self.expected_images = len(support_dump.IMAGES_DUMP)
- for item in support_dump.NETWORKING_DUMP:
- self.substitute_mock_call(item, self.exceptions)
- for item in support_dump.CONFIG_DUMP:
- self.substitute_mock_call(item, self.exceptions)
- for item in support_dump.IMAGES_DUMP:
- self.substitute_mock_call(item, self.exceptions)
-
- def substitute_mock_call(self, item, with_exception):
- if with_exception:
- item["function"] = self.mock_dump_with_raise
- else:
- item["function"] = self.mock_dump
-
- if "command" in item:
- item["command"] = "/bin/true"
-
- def make_args(self, **kwargs):
- args = Mock()
- args.__dict__.update(kwargs)
- return args
-
- def mock_dump(self, *args, **kwargs):
- self.mock_calls += 1
- self.last_args = args
- self.last_kwargs = kwargs
-
- def mock_dump_with_raise(self, *args, **kwargs):
- self.mock_calls += 1
- self.last_args = args
- self.last_kwargs = kwargs
- raise Exception("Fatality.")
-
- def test_dump_with_no_args_dumps_everything(self):
- support_dump.run(self.make_args())
- self.assertThat(
- self.mock_calls,
- Equals(
- self.expected_config
- + self.expected_images
- + self.expected_networking
- ),
- )
-
- def test_dump_with_networking_arg_dumps_expected(self):
- support_dump.run(self.make_args(networking=True))
- self.assertEqual(self.expected_networking, self.mock_calls)
-
- def test_dump_with_config_arg_dumps_expected_functions(self):
- support_dump.run(self.make_args(config=True))
- self.assertEqual(self.expected_config, self.mock_calls)
-
- def test_dump_with_images_arg_dumps_expected_functions(self):
- support_dump.run(self.make_args(images=True))
- self.assertEqual(self.expected_images, self.mock_calls)
-
- def test_dump_with_images_preserves_args(self):
- support_dump.run(self.make_args(images=True))
- self.assertThat(self.last_args, HasLength(1))
-
- def test_dump_with_images_and_config_args_dumps_expected_functions(self):
- support_dump.run(self.make_args(images=True, config=True))
- self.assertEqual(
- self.expected_images + self.expected_config,
- self.mock_calls,
- )
-
- def test_dump_with_images_and_networking_args_dumps_expected_functions(
- self,
- ):
- support_dump.run(self.make_args(images=True, networking=True))
- self.assertEqual(
- self.expected_images + self.expected_networking,
- self.mock_calls,
- )
-
- def test_dump_with_networking_and_config_args_dumps_expected_functions(
- self,
- ):
- support_dump.run(self.make_args(networking=True, config=True))
- self.assertEqual(
- self.expected_networking + self.expected_config,
- self.mock_calls,
- )
-
- def test_dump_with_networking_and_images_args_dumps_expected_functions(
- self,
- ):
- support_dump.run(self.make_args(networking=True, images=True))
- self.assertEqual(
- self.expected_networking + self.expected_images,
- self.mock_calls,
- )
-
- def test_dump_with_all_args_dumps_all_functions(self):
- support_dump.run(
- self.make_args(networking=True, images=True, config=True)
- )
- self.assertThat(
- self.mock_calls,
- Equals(
- self.expected_config
- + self.expected_images
- + self.expected_networking
- ),
- )
diff --git a/src/provisioningserver/utils/dhcp.py b/src/provisioningserver/utils/dhcp.py
index aafbe4c..f9d1867 100644
--- a/src/provisioningserver/utils/dhcp.py
+++ b/src/provisioningserver/utils/dhcp.py
@@ -5,34 +5,13 @@
from collections import namedtuple
-from datetime import datetime
from io import BytesIO
-import os
-from pprint import pformat
-import stat
import struct
-import subprocess
-import sys
-from textwrap import dedent
+from typing import Optional
from netaddr import IPAddress
-from provisioningserver.path import get_path
-from provisioningserver.utils import sudo
-from provisioningserver.utils.network import bytes_to_ipaddress, format_eui
-from provisioningserver.utils.pcap import PCAP, PCAPError
-from provisioningserver.utils.script import ActionScriptError
-from provisioningserver.utils.tcpip import (
- decode_ethernet_udp_packet,
- PacketProcessingError,
-)
-
-# The SEEN_AGAIN_THRESHOLD is a time (in seconds) that determines how often
-# to report (IP, MAC) bindings that have been seen again (or "REFRESHED").
-# While it is important for MAAS to know about "NEW" and "MOVED" bindings
-# immediately, "REFRESHED" bindings occur too often to be useful, and
-# are thus throttled by this value.
-SEEN_AGAIN_THRESHOLD = 600
+from provisioningserver.utils.network import bytes_to_ipaddress
# Definitions for DHCP packet used with `struct`.
# See https://tools.ietf.org/html/rfc2131#section-2 for packet format.
@@ -147,11 +126,8 @@ class DHCP:
raise InvalidDHCPPacket("Truncated DHCP option value.")
yield option_code, option_value
- def is_valid(self):
- return self.valid
-
@property
- def server_identifier(self) -> IPAddress:
+ def server_identifier(self) -> Optional[IPAddress]:
"""Returns the DHCP server identifier option.
This returns the IP address of the DHCP server.
@@ -162,128 +138,3 @@ class DHCP:
if server_identifier_bytes is not None:
return bytes_to_ipaddress(server_identifier_bytes)
return None
-
- def write(self, out=sys.stdout):
- """Output text-based details about this DHCP packet to the specified
- file or stream.
-
- :param out: An object with `write(str)` and `flush()` methods.
- """
- packet = pformat(self.packet)
- out.write(packet)
- out.write("\n")
- options = pformat(self.options)
- out.write(options)
- out.write("\nServer identifier: %s\n\n" % self.server_identifier)
- out.flush()
-
-
-def observe_dhcp_packets(input=sys.stdin.buffer, out=sys.stdout):
- """Read stdin and look for tcpdump binary DHCP output.
-
- :param input: Stream to read PCAP data from.
- :type input: a file or stream supporting `read(int)`
- :param out: Stream to write to.
- :type input: a file or stream supporting `write(str)` and `flush()`.
- """
- try:
- pcap = PCAP(input)
- if pcap.global_header.data_link_type != 1:
- # Not an Ethernet interface. Need to exit here, because our
- # assumptions about the link layer header won't be correct.
- return 4
- for pcap_header, packet_bytes in pcap:
- out.write(str(datetime.now()))
- out.write("\n")
- try:
- packet = decode_ethernet_udp_packet(packet_bytes, pcap_header)
- dhcp = DHCP(packet.payload)
- if not dhcp.is_valid():
- out.write(dhcp.invalid_reason)
- out.write(
- " Source MAC address: %s\n"
- % format_eui(packet.l2.src_eui)
- )
- out.write(
- "Destination MAC address: %s\n"
- % format_eui(packet.l2.dst_eui)
- )
- if packet.l2.vid is not None:
- out.write(" Seen on 802.1Q VID: %s\n" % packet.l2.vid)
- out.write(" Source IP address: %s\n" % packet.l3.src_ip)
- out.write(" Destination IP address: %s\n" % packet.l3.dst_ip)
- dhcp.write(out=out)
- out.flush()
- except PacketProcessingError as e:
- out.write(e.error)
- out.write("\n\n")
- out.flush()
- except EOFError:
- # Capture aborted before it could even begin. Note that this does not
- # occur if the end-of-stream occurs normally. (In that case, the
- # program will just exit.)
- return 3
- except PCAPError:
- # Capture aborted due to an I/O error.
- return 2
- return None
-
-
-def add_arguments(parser):
- """Add this command's options to the `ArgumentParser`.
-
- Specified by the `ActionScript` interface.
- """
- parser.description = dedent(
- """\
- Observes DHCP traffic specified interface.
- """
- )
- parser.add_argument(
- "interface",
- type=str,
- nargs="?",
- help="Ethernet interface from which to capture traffic. Optional if "
- "an input file is specified.",
- )
- parser.add_argument(
- "-i",
- "--input-file",
- type=str,
- required=False,
- help="File to read PCAP output from. Use - for stdin. Default is to "
- "call `sudo /usr/lib/maas/dhcp-monitor` to get input.",
- )
-
-
-def run(
- args, output=sys.stdout, stdin=sys.stdin, stdin_buffer=sys.stdin.buffer
-):
- """Observe an Ethernet interface and print DHCP packets."""
- network_monitor = None
- if args.input_file is None:
- if args.interface is None:
- raise ActionScriptError("Required argument: interface")
- cmd = sudo([get_path("/usr/lib/maas/dhcp-monitor"), args.interface])
- network_monitor = subprocess.Popen(
- cmd,
- stdin=subprocess.DEVNULL,
- stdout=subprocess.PIPE,
- stderr=subprocess.DEVNULL,
- )
- infile = network_monitor.stdout
- else:
- if args.input_file == "-":
- mode = os.fstat(stdin.fileno()).st_mode
- if not stat.S_ISFIFO(mode):
- raise ActionScriptError("Expected stdin to be a pipe.")
- infile = stdin_buffer
- else:
- infile = open(args.input_file, "rb")
- return_code = observe_dhcp_packets(input=infile, out=output)
- if return_code is not None:
- raise SystemExit(return_code)
- if network_monitor is not None:
- return_code = network_monitor.poll()
- if return_code is not None:
- raise SystemExit(return_code)
diff --git a/src/provisioningserver/utils/tests/test_dhcp.py b/src/provisioningserver/utils/tests/test_dhcp.py
index 6fb8d50..6fd93d2 100644
--- a/src/provisioningserver/utils/tests/test_dhcp.py
+++ b/src/provisioningserver/utils/tests/test_dhcp.py
@@ -1,8 +1,6 @@
# Copyright 2016 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
-"""Tests for ``provisioningserver.utils.dhcp``."""
-
from netaddr import IPAddress
@@ -13,43 +11,43 @@ from provisioningserver.utils.dhcp import DHCP
class TestDHCP(MAASTestCase):
- def test_is_valid_returns_false_for_truncated_packet(self):
+ def test_valid_returns_false_for_truncated_packet(self):
packet = factory.make_dhcp_packet(truncated=True)
dhcp = DHCP(packet)
- self.assertFalse(dhcp.is_valid())
+ self.assertFalse(dhcp.valid)
self.assertThat(
dhcp.invalid_reason, DocTestMatches("Truncated DHCP packet.")
)
- def test_is_valid_returns_false_for_invalid_cookie(self):
+ def test_valid_returns_false_for_invalid_cookie(self):
packet = factory.make_dhcp_packet(bad_cookie=True)
dhcp = DHCP(packet)
- self.assertFalse(dhcp.is_valid())
+ self.assertFalse(dhcp.valid)
self.assertThat(
dhcp.invalid_reason, DocTestMatches("Invalid DHCP cookie.")
)
- def test_is_valid_returns_false_for_truncated_option_length(self):
+ def test_valid_returns_false_for_truncated_option_length(self):
packet = factory.make_dhcp_packet(truncated_option_length=True)
dhcp = DHCP(packet)
- self.assertFalse(dhcp.is_valid())
+ self.assertFalse(dhcp.valid)
self.assertThat(
dhcp.invalid_reason,
DocTestMatches("Truncated length field in DHCP option."),
)
- def test_is_valid_returns_false_for_truncated_option_value(self):
+ def test_valid_returns_false_for_truncated_option_value(self):
packet = factory.make_dhcp_packet(truncated_option_value=True)
dhcp = DHCP(packet)
- self.assertFalse(dhcp.is_valid())
+ self.assertFalse(dhcp.valid)
self.assertThat(
dhcp.invalid_reason, DocTestMatches("Truncated DHCP option value.")
)
- def test_is_valid_return_true_for_valid_packet(self):
+ def test_valid_return_true_for_valid_packet(self):
packet = factory.make_dhcp_packet()
dhcp = DHCP(packet)
- self.assertTrue(dhcp.is_valid())
+ self.assertTrue(dhcp.valid)
def test_returns_server_identifier_if_included(self):
server_ip = factory.make_ip_address(ipv6=False)
@@ -57,11 +55,11 @@ class TestDHCP(MAASTestCase):
include_server_identifier=True, server_ip=server_ip
)
dhcp = DHCP(packet)
- self.assertTrue(dhcp.is_valid())
+ self.assertTrue(dhcp.valid)
self.assertEqual(IPAddress(server_ip), dhcp.server_identifier)
def test_server_identifier_none_if_not_included(self):
packet = factory.make_dhcp_packet(include_server_identifier=False)
dhcp = DHCP(packet)
- self.assertTrue(dhcp.is_valid())
+ self.assertTrue(dhcp.valid)
self.assertIsNone(dhcp.server_identifier)
Follow ups
