Skip to content

Commit

Permalink
Fix: monitor-only not being set properly when false/no passed in conf…
Browse files Browse the repository at this point in the history
…ig (#285)

## Manual Testing
### Verify Bug
- Run new testcases provided (current branch) checking for when false
parameter is sent in using old watchdog file
- Test cases will fail as Manager daemon is not spawned
### Verify Fix
- Run same testcases as above using new watchdog
- See that Manager daemon is spawned in cases where false is sent as
parameter
  • Loading branch information
david-mclain authored Nov 27, 2024
1 parent 3018d6f commit 3e4d447
Show file tree
Hide file tree
Showing 4 changed files with 78 additions and 7 deletions.
25 changes: 23 additions & 2 deletions landscape/client/deployment.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,20 @@
import subprocess
import sys
import time
from argparse import SUPPRESS
from datetime import datetime
from datetime import timezone
from logging import debug
from argparse import SUPPRESS
from logging import info
from typing import Sequence

from twisted.logger import globalLogBeginner

from landscape import VERSION
from landscape.client import DEFAULT_CONFIG
from landscape.client import GROUP
from landscape.client import USER
from landscape.client import snap_http
from landscape.client import USER
from landscape.client.snap_utils import get_snap_info
from landscape.client.upgraders import UPGRADE_MANAGERS
from landscape.lib import logging
Expand Down Expand Up @@ -304,3 +305,23 @@ def generate_computer_title(auto_enroll_config):
title = hostname

return title


def convert_arg_to_bool(value: str) -> bool:
"""
Converts an argument provided that is in string format
to be a boolean value.
"""
TRUTHY_VALUES = {"true", "yes", "y", "1", "on"}
FALSY_VALUES = {"false", "no", "n", "0", "off"}

if value.lower() in TRUTHY_VALUES:
return True
elif value.lower() in FALSY_VALUES:
return False
else:
info(
"Error. Invalid boolean provided in config or parameters. "
+ "Defaulting to False.",
)
return False
30 changes: 29 additions & 1 deletion landscape/client/tests/test_deployment.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

from landscape.client.deployment import BaseConfiguration
from landscape.client.deployment import Configuration
from landscape.client.deployment import convert_arg_to_bool
from landscape.client.deployment import generate_computer_title
from landscape.client.deployment import get_versioned_persist
from landscape.client.deployment import init_logging
Expand Down Expand Up @@ -468,7 +469,7 @@ def test_generate_computer_title_wait_for_serial_no_serial_assertion(
)
self.assertIsNone(title)
mock_debug.assert_called_once_with(
"No serial assertion in snap info {}, waiting..."
"No serial assertion in snap info {}, waiting...",
)

@mock.patch("landscape.client.deployment.debug")
Expand Down Expand Up @@ -601,3 +602,30 @@ def test_generate_computer_title_with_date(
},
)
self.assertEqual(title, "2024-machine")


class ArgConversionTest(LandscapeTest):
"""Tests for `convert_arg_to_bool` function"""

def test_true_values(self):
TRUTHY_VALUES = {"true", "yes", "y", "1", "on", "TRUE", "Yes"}
for t in TRUTHY_VALUES:
val = convert_arg_to_bool(t)
self.assertTrue(val)

def test_false_values(self):
FALSY_VALUES = {"false", "no", "n", "0", "off", "FALSE", "No"}
for f in FALSY_VALUES:
val = convert_arg_to_bool(f)
self.assertFalse(val)

@mock.patch("landscape.client.deployment.info")
def test_invalid_values(self, logging):
INVALID_VALUES = {"invalid", "truthy", "2", "exit"}
for i in INVALID_VALUES:
val = convert_arg_to_bool(i)
logging.assert_called_with(
"Error. Invalid boolean provided in config or parameters. "
+ "Defaulting to False.",
)
self.assertFalse(val)
21 changes: 20 additions & 1 deletion landscape/client/tests/test_watchdog.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@
from twisted.internet.utils import getProcessOutput
from twisted.python.fakepwd import UserDatabase

from landscape.client import USER
import landscape.client.watchdog
from landscape.client import USER
from landscape.client.amp import ComponentConnector
from landscape.client.broker.amp import RemoteBrokerConnector
from landscape.client.reactor import LandscapeReactor
Expand Down Expand Up @@ -1147,13 +1147,32 @@ def test_monitor_only(self):
self.config.load(["--monitor-only"])
self.assertEqual(self.config.get_enabled_daemons(), [Broker, Monitor])

def test_monitor_only_false(self):
self.config.load(["--monitor-only", "false"])
self.assertEqual(
self.config.get_enabled_daemons(),
[Broker, Monitor, Manager],
)

def test_default_daemons(self):
self.config.load([])
self.assertEqual(
self.config.get_enabled_daemons(),
[Broker, Monitor, Manager],
)

@mock.patch("landscape.client.deployment.info")
def test_monitor_only_invalid_entry(self, logging):
self.config.load(["--monitor-only", "invalid-option"])
logging.assert_called_once_with(
"Error. Invalid boolean provided in config or parameters. "
+ "Defaulting to False.",
)
self.assertEqual(
self.config.get_enabled_daemons(),
[Broker, Monitor, Manager],
)


class WatchDogServiceTest(LandscapeTest):
def setUp(self):
Expand Down
9 changes: 6 additions & 3 deletions landscape/client/watchdog.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
The main C{landscape-client} program uses this watchdog.
"""

import errno
import os
import pwd
Expand All @@ -26,13 +25,14 @@
from twisted.internet.error import ProcessExitedAlready
from twisted.internet.protocol import ProcessProtocol

from landscape.client import IS_SNAP
from landscape.client import GROUP
from landscape.client import IS_SNAP
from landscape.client import USER
from landscape.client.broker.amp import RemoteBrokerConnector
from landscape.client.broker.amp import RemoteManagerConnector
from landscape.client.broker.amp import RemoteMonitorConnector
from landscape.client.deployment import Configuration
from landscape.client.deployment import convert_arg_to_bool
from landscape.client.deployment import init_logging
from landscape.client.reactor import LandscapeReactor
from landscape.lib.bootstrap import BootstrapDirectory
Expand Down Expand Up @@ -522,7 +522,10 @@ def make_parser(self):
)
parser.add_argument(
"--monitor-only",
action="store_true",
type=convert_arg_to_bool,
nargs="?",
const=True,
default=False,
help="Don't enable management features. This is "
"useful if you want to run the client as a non-root "
"user.",
Expand Down

0 comments on commit 3e4d447

Please sign in to comment.