Skip to content

Commit

Permalink
Merge pull request #16 from vapor-ware/mhink-device-fail
Browse files Browse the repository at this point in the history
Do not fail synse startup on device registration failure.
  • Loading branch information
MatthewHink committed Oct 23, 2017
2 parents aa43a66 + a57e63b commit 9561a01
Show file tree
Hide file tree
Showing 6 changed files with 57 additions and 66 deletions.
9 changes: 0 additions & 9 deletions synse/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -163,8 +163,6 @@ def register_app_devices(application):

app_cache = (_devices, _single_board_devices)

_failed_registration = False

for device_interface, device_config in DEVICES.iteritems():
device_interface = device_interface.lower()

Expand All @@ -182,13 +180,6 @@ def register_app_devices(application):
logger.error('Failed to register {} device: {}'.format(
device_interface, device_config))
logger.exception(e)
_failed_registration = True

if _failed_registration:
raise ValueError(
'Failed to register all configured devices -- check that the device configuration '
'files are correct.'
)

application.config['DEVICES'] = _devices
application.config['SINGLE_BOARD_DEVICES'] = _single_board_devices
Expand Down
5 changes: 3 additions & 2 deletions synse/devicebus/devices/i2c/i2c_device.py
Original file line number Diff line number Diff line change
Expand Up @@ -133,9 +133,10 @@ def register(cls, devicebus_config, app_config, app_cache):
try:
device_instance = device_model(**i2c_device)
except Exception as e:
logger.error('Error initializing device: ')
logger.error('Error initializing device: {}'.format(i2c_device))
logger.exception(e)
raise
# A raise here would cause subsequently configured devices to fail to register.
# We want to try to register those devices.

# cache reference to device
device_cache[device_instance.device_uuid] = device_instance
Expand Down
3 changes: 1 addition & 2 deletions synse/devicebus/devices/ipmi/ipmi_device.py
Original file line number Diff line number Diff line change
Expand Up @@ -360,9 +360,8 @@ def register(cls, devicebus_config, app_config, app_cache):
thread_pool.wait_for_task_completion()

# check for device initialization failures
if device_init_failure:
if len(device_init_failure) != 0:
logger.error('Failed to initialize IPMI devices: {}'.format(device_init_failure))
raise SynseException('Failed to initialize IPMI devices.')

@staticmethod
def _process_bmc(bmc, app_config, rack_id, board_range, device_init_failure, mutate_lock,
Expand Down
3 changes: 1 addition & 2 deletions synse/devicebus/devices/redfish/redfish_device.py
Original file line number Diff line number Diff line change
Expand Up @@ -275,9 +275,8 @@ def register(cls, devicebus_config, app_config, app_cache):
thread_pool.wait_for_task_completion()

# check for device initialization failures
if device_init_failure:
if len(device_init_failure) != 0:
logger.error('Failed to initialize Redfish devices: {}'.format(device_init_failure))
raise SynseException('Failed to initialize Redfish devices.')

@staticmethod
def _process_server(server, app_config, rack_id, board_range, device_init_failure, mutate_lock,
Expand Down
5 changes: 3 additions & 2 deletions synse/devicebus/devices/rs485/rs485_device.py
Original file line number Diff line number Diff line change
Expand Up @@ -134,9 +134,10 @@ def register(cls, devicebus_config, app_config, app_cache):
try:
device_instance = device_model(**rs485_device)
except Exception as e:
logger.error('Error initializing device: ')
logger.error('Error initializing device: {}'.format(rs485_device))
logger.exception(e)
raise
# A raise here would cause subsequently configured devices to fail to register.
# We want to try to register those devices.

# cache reference to device
device_cache[device_instance.device_uuid] = device_instance
Expand Down
98 changes: 49 additions & 49 deletions synse/tests/endpoint_utilities/test_endpoint_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -1280,7 +1280,8 @@ def test_016_register_ipmi(self):

def test_017_register_ipmi(self):
""" Test registering a single IPMI device when a device value is missing
from the given config.
from the given config. This test should not raise because Synse still
needs to start up.
"""
# define the IPMI config
ipmi_devices = {
Expand Down Expand Up @@ -1314,16 +1315,15 @@ def test_017_register_ipmi(self):
self.assertEqual(len(app.config['DEVICES']), 0)
self.assertEqual(len(app.config['SINGLE_BOARD_DEVICES']), 0)

with self.assertRaises(SynseException):
# register the ipmi device(s)
IPMIDevice.register(
devicebus_config=ipmi_devices,
app_config=app.config,
app_cache=(
app.config['DEVICES'],
app.config['SINGLE_BOARD_DEVICES']
)
# register the ipmi device(s)
IPMIDevice.register(
devicebus_config=ipmi_devices,
app_config=app.config,
app_cache=(
app.config['DEVICES'],
app.config['SINGLE_BOARD_DEVICES']
)
)

# verify that the failure did not mutate any state
self.assertEqual(len(app.config['DEVICES']), 0)
Expand All @@ -1332,6 +1332,7 @@ def test_017_register_ipmi(self):
def test_018_register_ipmi(self):
""" Test registering multiple IPMI devices when a device value is missing
from the given config. In this case, the first record is malformed.
This test should not raise because Synse still needs to start up.
"""
# define the IPMI config
ipmi_devices = {
Expand Down Expand Up @@ -1381,16 +1382,15 @@ def test_018_register_ipmi(self):
self.assertEqual(len(app.config['DEVICES']), 0)
self.assertEqual(len(app.config['SINGLE_BOARD_DEVICES']), 0)

with self.assertRaises(SynseException):
# register the ipmi device(s)
IPMIDevice.register(
devicebus_config=ipmi_devices,
app_config=app.config,
app_cache=(
app.config['DEVICES'],
app.config['SINGLE_BOARD_DEVICES']
)
# register the ipmi device(s)
IPMIDevice.register(
devicebus_config=ipmi_devices,
app_config=app.config,
app_cache=(
app.config['DEVICES'],
app.config['SINGLE_BOARD_DEVICES']
)
)

# because devices are being initialized in threads, we should expect to see
# state mutated despite the bad config
Expand All @@ -1399,7 +1399,8 @@ def test_018_register_ipmi(self):

def test_019_register_ipmi(self):
""" Test registering multiple IPMI devices when a device value is missing
from the given config. In this case, the last record is malformed.
from the given config. In this case, the last record is malformed. This
test should not raise because Synse still needs to start up.
"""
# define the IPMI config
ipmi_devices = {
Expand Down Expand Up @@ -1449,16 +1450,15 @@ def test_019_register_ipmi(self):
self.assertEqual(len(app.config['DEVICES']), 0)
self.assertEqual(len(app.config['SINGLE_BOARD_DEVICES']), 0)

with self.assertRaises(SynseException):
# register the ipmi device(s)
IPMIDevice.register(
devicebus_config=ipmi_devices,
app_config=app.config,
app_cache=(
app.config['DEVICES'],
app.config['SINGLE_BOARD_DEVICES']
)
# register the ipmi device(s)
IPMIDevice.register(
devicebus_config=ipmi_devices,
app_config=app.config,
app_cache=(
app.config['DEVICES'],
app.config['SINGLE_BOARD_DEVICES']
)
)

# here, we will get mutated state since valid configs were found
# before the invalid config. this is okay though because it raises
Expand Down Expand Up @@ -1945,7 +1945,8 @@ def test_025_register_ipmi(self):
""" Test registering an IPMI device where an invalid hostname is given as the BMC
IP. Unlike the case where an IP can be specified and it need not connect (e.g. can
timeout without failing registration), this will fail registration under the guise
of 'misconfiguration', since the provided hostname is not defined.
of 'misconfiguration', since the provided hostname is not defined. This
test should not raise because Synse still needs to start up.
"""
# define the IPMI config
ipmi_devices = {
Expand Down Expand Up @@ -1980,16 +1981,15 @@ def test_025_register_ipmi(self):
self.assertEqual(len(app.config['DEVICES']), 0)
self.assertEqual(len(app.config['SINGLE_BOARD_DEVICES']), 0)

with self.assertRaises(SynseException):
# register the ipmi device(s)
IPMIDevice.register(
devicebus_config=ipmi_devices,
app_config=app.config,
app_cache=(
app.config['DEVICES'],
app.config['SINGLE_BOARD_DEVICES']
)
# register the ipmi device(s)
IPMIDevice.register(
devicebus_config=ipmi_devices,
app_config=app.config,
app_cache=(
app.config['DEVICES'],
app.config['SINGLE_BOARD_DEVICES']
)
)

# since the device failed to register, we should not see any mutations to
# the state.
Expand All @@ -1998,7 +1998,8 @@ def test_025_register_ipmi(self):

def test_026_register_ipmi(self):
""" Test registering multiple IPMI devices when a device value is missing
from the given config. In this case, the last record is malformed.
from the given config. In this case, the last record is malformed. This
test should not raise because Synse still needs to start up.
"""
# define the IPMI config
ipmi_devices = {
Expand Down Expand Up @@ -2048,16 +2049,15 @@ def test_026_register_ipmi(self):
self.assertEqual(len(app.config['DEVICES']), 0)
self.assertEqual(len(app.config['SINGLE_BOARD_DEVICES']), 0)

with self.assertRaises(SynseException):
# register the ipmi device(s)
IPMIDevice.register(
devicebus_config=ipmi_devices,
app_config=app.config,
app_cache=(
app.config['DEVICES'],
app.config['SINGLE_BOARD_DEVICES']
)
# register the ipmi device(s)
IPMIDevice.register(
devicebus_config=ipmi_devices,
app_config=app.config,
app_cache=(
app.config['DEVICES'],
app.config['SINGLE_BOARD_DEVICES']
)
)

# here, we will get mutated state since valid configs were found
# before the invalid config. this is okay though because it raises
Expand Down

0 comments on commit 9561a01

Please sign in to comment.