Skip to content
Open
2 changes: 1 addition & 1 deletion src/middlewared/middlewared/plugins/failover_/event.py
Original file line number Diff line number Diff line change
Expand Up @@ -755,7 +755,7 @@ def vrrp_master(self, job, fobj, ifname, event):
logger.info('Done starting truecommand service (if necessary)')

logger.info('Configuring TrueNAS Connect Service (if necessary)')
self.run_call('tn_connect.state.check')
self.middleware.create_task(self.middleware.call('tn_connect.state.check', True))
logger.info('Done configuring TrueNAS Connect Service (if necessary)')

logger.info('Configuring TrueSearch (if necessary)')
Expand Down
9 changes: 6 additions & 3 deletions src/middlewared/middlewared/plugins/truenas_connect/acme.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,9 @@ async def update_ui(self, start_heartbeat=True):
else:
logger.debug('TNC cert configured successfully')
await self.middleware.call('tn_connect.set_status', Status.CONFIGURED.name)
logger.debug('Initiating TNC heartbeat')
self.middleware.create_task(self.middleware.call('tn_connect.heartbeat.start'))
if start_heartbeat:
logger.debug('Initiating TNC heartbeat')
self.middleware.create_task(self.middleware.call('tn_connect.heartbeat.start'))
# Let's restart UI now
# TODO: Hash this out with everyone
await self.middleware.call('system.general.ui_restart', 2)
Expand Down Expand Up @@ -230,7 +231,9 @@ async def check_status(middleware):


async def _event_system_ready(middleware, event_type, args):
await check_status(middleware)
if not await middleware.call('system.is_ha_capable'):
# For HA systems, failover logic will handle this
await check_status(middleware)


async def setup(middleware):
Expand Down
16 changes: 0 additions & 16 deletions src/middlewared/middlewared/plugins/truenas_connect/heartbeat.py
Original file line number Diff line number Diff line change
Expand Up @@ -133,19 +133,3 @@ async def payload(self, disk_mapping=None):
'alerts': await self.middleware.call('alert.list'),
'stats': stats,
}


async def check_status(middleware):
tnc_config = await middleware.call('tn_connect.config')
if tnc_config['status'] in CONFIGURED_TNC_STATES:
middleware.create_task(middleware.call('tn_connect.heartbeat.start'))


async def _event_system_ready(middleware, event_type, args):
await check_status(middleware)


async def setup(middleware):
middleware.event_subscribe('system.ready', _event_system_ready)
if await middleware.call('system.ready'):
await check_status(middleware)
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,11 @@ async def register_update_ips(self, ips=None, create_wildcard=False):
# If no IPs provided, use combined IPs from config (direct IPs + interface IPs)
if ips is None:
ips = tnc_config['ips'] + tnc_config.get('interfaces_ips', [])

if await self.middleware.call('system.is_ha_capable'):
# For HA based systems, we want to ensure that VIP(s) always get added
ips = (await self.middleware.call('tn_connect.ha_vips')) + ips

try:
return await register_update_ips(tnc_config, ips, create_wildcard)
except TNCCallError as e:
Expand Down Expand Up @@ -92,6 +97,10 @@ async def handle_update_ips(self, event_type, args):
Handle IP address changes for TrueNAS Connect.
This method is called when an IP address change event occurs.
"""
if not await self.middleware.call('failover.is_single_master_node'):
# We only want this to happen on master/single nodes
return

tnc_config = await self.middleware.call('tn_connect.config')

# Skip if interface is None (can happen in some edge cases)
Expand Down
11 changes: 10 additions & 1 deletion src/middlewared/middlewared/plugins/truenas_connect/state.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

from middlewared.service import Service

from .utils import CONFIGURED_TNC_STATES


logger = logging.getLogger('truenas_connect')

Expand All @@ -14,7 +16,7 @@ class Config:
namespace = 'tn_connect.state'
private = True

async def check(self):
async def check(self, restart_ui=False):
tnc_config = await self.middleware.call('tn_connect.config')
if tnc_config['status'] == Status.REGISTRATION_FINALIZATION_WAITING.name:
logger.debug(
Expand All @@ -35,3 +37,10 @@ async def check(self):
elif tnc_config['status'] == Status.CERT_RENEWAL_IN_PROGRESS.name:
logger.debug('Middleware started and cert renewal is in progress, initiating process')
self.middleware.create_task(self.middleware.call('tn_connect.acme.renew_cert'))

if tnc_config['status'] in CONFIGURED_TNC_STATES:
logger.debug('Triggering heartbeat start')
self.middleware.create_task(self.middleware.call('tn_connect.heartbeat.start'))
if restart_ui:
logger.debug('Restarting UI')
await self.middleware.call('system.general.ui_restart', 2)
38 changes: 30 additions & 8 deletions src/middlewared/middlewared/plugins/truenas_connect/update.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,17 +79,39 @@ async def config_extend(self, config):
config['certificate'] = config['certificate']['id']
return config

@private
async def ha_vips(self):
vips = []
for interface in await self.middleware.call('interface.query'):
for vip_entry in interface.get('failover_virtual_aliases', []):
vips.append(vip_entry['address'])
return vips

@private
async def validate_data(self, old_config, data):
verrors = ValidationErrors()

# Ensure at least one interface or at least one IP is provided (unless use_all_interfaces is True)
if data['enabled'] and not data['ips'] and not data['interfaces'] and not data['use_all_interfaces']:
verrors.add(
'tn_connect_update',
'At least one IP or interface must be provided when TrueNAS Connect is enabled '
'(or use_all_interfaces must be set to true)'
)
if data['enabled']:
if await self.middleware.call('system.is_ha_capable'):
# In case of HA, we want to ensure following:
# 1) We have VIP available
# 2) User has not specified interfaces/use all interfaces
if not await self.ha_vips():
verrors.add(
'tn_connect_update.enabled',
'HA systems must be in a healthy state to enable TNC ensuring we have VIP available'
)
for k in filter(lambda k: data[k], ('interfaces', 'use_all_interfaces')):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will we run into any issue with use_all_interfaces as on by default?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HA systems are meant to be used using VIP, @yocalebo do you think we should allow specific or all interfaces ?
Basically these attrs control what IPs we send to TNC based on what is configured on selected/all interfaces.

For public or special ip cases, we are allowing that right now separately

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with disabling that setting for HA, just don't know if there also needs to be a change on registration since it does default to true.

verrors.add(
f'tn_connect_update.{k}',
'Must be unset on HA systems'
)
elif not data['ips'] and not data['interfaces'] and not data['use_all_interfaces']:
# Ensure at least one interface or at least one IP is provided (unless use_all_interfaces is True)
verrors.add(
'tn_connect_update',
'At least one IP or interface must be provided when TrueNAS Connect is enabled '
'(or use_all_interfaces must be set to true)'
)

data['ips'] = [str(ip) for ip in data['ips']]

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,9 @@ class TestTNCInterfacesValidation:
@pytest.mark.asyncio
async def test_validate_data_requires_ip_or_interface(self, tnc_service):
"""Test that at least one IP or interface is required when enabled."""
# Mock system.is_ha_capable to return False for non-HA systems
tnc_service.middleware.call = AsyncMock(return_value=False)

old_config = {
'enabled': False, 'ips': [], 'interfaces': [], 'status': Status.DISABLED.name,
'use_all_interfaces': True
Expand All @@ -70,7 +73,9 @@ async def test_validate_data_interface_exists(self, tnc_service, mock_interfaces

# Create a function that handles the calls appropriately
def mock_middleware_call(method, *args, **kwargs):
if method == 'interface.query':
if method == 'system.is_ha_capable':
return False
elif method == 'interface.query':
return mock_interface_names
elif method == 'interface.ip_in_use':
# Return empty list since we're testing interface validation error
Expand Down Expand Up @@ -98,6 +103,8 @@ async def test_validate_data_interface_has_ip(self, tnc_service, mock_interfaces
mock_interface_names = [{'name': 'ens3'}, {'name': 'ens4'}, {'name': 'ens5'}]
tnc_service.middleware.call = AsyncMock()
tnc_service.middleware.call.side_effect = [
# system.is_ha_capable() call - return False for non-HA
False,
# interface.query() call with retrieve_names_only=True
mock_interface_names,
# interface.ip_in_use() call - returns empty list for ens5
Expand All @@ -120,7 +127,15 @@ async def test_validate_data_interface_with_direct_ips(self, tnc_service, mock_i
"""Test that interface without IPs is allowed if direct IPs are provided."""
# When retrieve_names_only=True, interface.query returns only names
mock_interface_names = [{'name': 'ens3'}, {'name': 'ens4'}, {'name': 'ens5'}]
tnc_service.middleware.call = AsyncMock(return_value=mock_interface_names)

def mock_middleware_call(method, *args, **kwargs):
if method == 'system.is_ha_capable':
return False
elif method == 'interface.query':
return mock_interface_names
return None

tnc_service.middleware.call = AsyncMock(side_effect=mock_middleware_call)

old_config = {
'enabled': False, 'ips': [], 'interfaces': [], 'status': Status.DISABLED.name,
Expand Down Expand Up @@ -175,7 +190,8 @@ class TestTNCUseAllInterfaces:
@pytest.mark.asyncio
async def test_validate_data_use_all_interfaces_no_ips_or_interfaces(self, tnc_service, mock_interfaces):
"""Test that use_all_interfaces=True allows enabling without specific IPs or interfaces."""
tnc_service.middleware.call = AsyncMock()
# Mock system.is_ha_capable to return False for non-HA systems
tnc_service.middleware.call = AsyncMock(return_value=False)

old_config = {
'enabled': False, 'ips': [], 'interfaces': [], 'status': Status.DISABLED.name,
Expand All @@ -189,7 +205,8 @@ async def test_validate_data_use_all_interfaces_no_ips_or_interfaces(self, tnc_s
@pytest.mark.asyncio
async def test_validate_data_use_all_interfaces_false_requires_ips_or_interfaces(self, tnc_service):
"""Test that use_all_interfaces=False requires at least one IP or interface."""
tnc_service.middleware.call = AsyncMock()
# Mock system.is_ha_capable to return False for non-HA systems
tnc_service.middleware.call = AsyncMock(return_value=False)

old_config = {
'enabled': False, 'ips': [], 'interfaces': [], 'status': Status.DISABLED.name,
Expand Down Expand Up @@ -327,7 +344,9 @@ async def test_do_update_extracts_interface_ips(self, tnc_service, mock_interfac

# Create a function that handles the calls appropriately
def mock_middleware_call(method, *args, **kwargs):
if method == 'interface.query':
if method == 'system.is_ha_capable':
return False
elif method == 'interface.query':
return mock_interface_names
elif method == 'interface.ip_in_use':
# Return IPs for the requested interfaces
Expand Down Expand Up @@ -436,7 +455,9 @@ async def test_do_update_filters_link_local_ipv6(self, tnc_service):

# Create a function that handles the calls appropriately
def mock_middleware_call(method, *args, **kwargs):
if method == 'interface.query':
if method == 'system.is_ha_capable':
return False
elif method == 'interface.query':
return mock_interface_names
elif method == 'interface.ip_in_use':
# Return IPs for the requested interfaces (already filters link-local)
Expand Down Expand Up @@ -543,6 +564,8 @@ async def test_do_update_combined_ips_registration(self, tnc_service, mock_inter
mock_interface_names = [{'name': 'ens3'}, {'name': 'ens4'}, {'name': 'ens5'}]

tnc_service.middleware.call.side_effect = [
# system.is_ha_capable() call - return False for non-HA
False,
# interface.query() call in validate_data with retrieve_names_only=True
mock_interface_names,
# interface.ip_in_use() call in do_update (for ens4)
Expand All @@ -569,7 +592,7 @@ async def test_do_update_combined_ips_registration(self, tnc_service, mock_inter
await tnc_service.do_update(data)

# Verify hostname.register_update_ips was called with combined IPs
register_call = tnc_service.middleware.call.call_args_list[3] # Updated index after cache.pop
register_call = tnc_service.middleware.call.call_args_list[4] # Updated index after system.is_ha_capable
assert register_call[0][0] == 'tn_connect.hostname.register_update_ips'
combined_ips = register_call[0][1]
assert set(combined_ips) == {'192.168.1.100', '10.0.0.10'}
Expand Down Expand Up @@ -677,11 +700,19 @@ async def test_register_update_ips_uses_combined_ips(self):
from middlewared.plugins.truenas_connect.hostname import TNCHostnameService

service = TNCHostnameService(MagicMock())
service.middleware.call = AsyncMock(return_value={
'ips': ['192.168.1.10'],
'interfaces_ips': ['10.0.0.10', '172.16.0.10'],
'jwt_token': 'test_token'
})

def mock_middleware_call(method, *args, **kwargs):
if method == 'tn_connect.config_internal':
return {
'ips': ['192.168.1.10'],
'interfaces_ips': ['10.0.0.10', '172.16.0.10'],
'jwt_token': 'test_token'
}
elif method == 'system.is_ha_capable':
return False
return None

service.middleware.call = AsyncMock(side_effect=mock_middleware_call)

with patch('middlewared.plugins.truenas_connect.hostname.register_update_ips',
new_callable=AsyncMock) as mock_register:
Expand All @@ -700,11 +731,19 @@ async def test_register_update_ips_with_explicit_ips(self):
from middlewared.plugins.truenas_connect.hostname import TNCHostnameService

service = TNCHostnameService(MagicMock())
service.middleware.call = AsyncMock(return_value={
'ips': ['192.168.1.10'],
'interfaces_ips': ['10.0.0.10'],
'jwt_token': 'test_token'
})

def mock_middleware_call(method, *args, **kwargs):
if method == 'tn_connect.config_internal':
return {
'ips': ['192.168.1.10'],
'interfaces_ips': ['10.0.0.10'],
'jwt_token': 'test_token'
}
elif method == 'system.is_ha_capable':
return False
return None

service.middleware.call = AsyncMock(side_effect=mock_middleware_call)

with patch('middlewared.plugins.truenas_connect.hostname.register_update_ips',
new_callable=AsyncMock) as mock_register:
Expand Down