Fix a delay in the internet connectivity check

framework/python/src/common/tasks.py

@@ -25,6 +25,7 @@
 # Check adapters period seconds
 # Check adapters period seconds
 CHECK_NETWORK_ADAPTERS_PERIOD = 5
+CHECK_INTERNET_PERIOD = 2
 INTERNET_CONNECTION_TOPIC = 'events/internet'
 NETWORK_ADAPTERS_TOPIC = 'events/adapter'
 
@@ -44,15 +45,7 @@ def __init__(
     # Prevent scheduler warnings
     self._scheduler._logger.setLevel(logging.ERROR)
 
-  @asynccontextmanager
-  async def start(self, app: FastAPI):  # pylint: disable=unused-argument
-    """Start background tasks
-
-    Args:
-        app (FastAPI): app instance
-    """
-    # Job that checks for changes in network adapters
-    self._scheduler.add_job(
+    self.adapters_checker_job = self._scheduler.add_job(
         func=self._testrun.get_net_orc().network_adapters_checker,
         kwargs={
                 'mqtt_client': self._mqtt_client,
@@ -61,14 +54,25 @@ async def start(self, app: FastAPI):  # pylint: disable=unused-argument
         trigger='interval',
         seconds=CHECK_NETWORK_ADAPTERS_PERIOD,
     )
-    self._scheduler.add_job(
-        func=self._testrun.get_net_orc().internet_conn_checker,
-        kwargs={
-                'mqtt_client': self._mqtt_client,
-                'topic': INTERNET_CONNECTION_TOPIC
-                },
-        trigger='interval',
-        seconds=CHECK_NETWORK_ADAPTERS_PERIOD,
-    )
+    # add internet connection cheking job only in single-intf mode
+    if 'single_intf' not in self._testrun.get_session().get_runtime_params():
+      self.internet_shecker = self._scheduler.add_job(
+          func=self._testrun.get_net_orc().internet_conn_checker,
+          kwargs={
+                  'mqtt_client': self._mqtt_client,
+                  'topic': INTERNET_CONNECTION_TOPIC
+                  },
+          trigger='interval',
+          seconds=CHECK_INTERNET_PERIOD,
+      )
+
+  @asynccontextmanager
+  async def start(self, app: FastAPI):  # pylint: disable=unused-argument
+    """Start background tasks
+
+    Args:
+        app (FastAPI): app instance
+    """
+    # Job that checks for changes in network adapters
     self._scheduler.start()
     yield

framework/python/src/common/util.py

@@ -24,7 +24,7 @@
 LOGGER = logger.get_logger('util')
 
 
-def run_command(cmd, output=True):
+def run_command(cmd, output=True, timeout=None):
   """Runs a process at the os level
   By default, returns the standard output and error output
   If the caller sets optional output parameter to False,
@@ -36,7 +36,7 @@ def run_command(cmd, output=True):
   with subprocess.Popen(shlex.split(cmd),
                              stdout=subprocess.PIPE,
                              stderr=subprocess.PIPE) as process:
-    stdout, stderr = process.communicate()
+    stdout, stderr = process.communicate(timeout)
 
     if process.returncode != 0 and output:
       err_msg = f'{stderr.strip()}. Code: {process.returncode}'

framework/python/src/net_orc/ip_control.py

@@ -239,7 +239,7 @@ def configure_container_interface(self,
 
   def ping_via_gateway(self, host):
     """Ping the host trough the gateway container"""
-    command = f'docker exec tr-ct-gateway ping -W 1 -c 1 {host}'
+    command = f'timeout 3 docker exec tr-ct-gateway ping -W 1 -c 1 {host}'
     output = util.run_command(command)
     if '0% packet loss' in output[0]:
       return True

framework/python/src/net_orc/network_orchestrator.py

@@ -805,21 +805,18 @@ def is_device_connected(self):
   def internet_conn_checker(self, mqtt_client: mqtt.MQTT, topic: str):
     """Checks internet connection and sends a status to frontend"""
 
-    # Default message
-    message = {'connection': False}
-
-    # Only check if Testrun is running
-    if self.get_session().get_status() not in [
-      'Waiting for Device', 'Monitoring', 'In Progress'
-    ]:
-      message['connection'] = None
-
-    # Only run if single intf mode not used
-    elif 'single_intf' not in self._session.get_runtime_params():
+    # Only check if Testrun is running not in single-intf mode
+    if (self.get_session().get_status() in [
+                                          'Waiting for Device',
+                                          'Monitoring',
+                                          'In Progress'
+                                          ]):
+      # Default message
+      message = {'connection': False}
       iface = self._session.get_internet_interface()
 
       # Check that an internet intf has been selected
-      if iface and iface in self._session.get_ifaces():
+      if iface and iface in self._ip_ctrl.get_sys_interfaces():
 
         # Ping google.com from gateway container
         internet_connection = self._ip_ctrl.ping_via_gateway(
@@ -828,8 +825,8 @@ def internet_conn_checker(self, mqtt_client: mqtt.MQTT, topic: str):
         if internet_connection:
           message['connection'] = True
 
-    # Broadcast via MQTT client
-    mqtt_client.send_message(topic, message)
+      # Broadcast via MQTT client
+      mqtt_client.send_message(topic, message)
 
 class NetworkModule:
   """Define all the properties of a Network Module"""

modules/ui/src/app/app.component.spec.ts

@@ -56,6 +56,7 @@ import {
   selectHasDevices,
   selectHasRiskProfiles,
   selectInterfaces,
+  selectInternetConnection,
   selectIsOpenStartTestrun,
   selectIsOpenWaitSnackBar,
   selectMenuOpened,
@@ -124,10 +125,7 @@ describe('AppComponent', () => {
       'focusFirstElementInContainer',
     ]);
     mockLiveAnnouncer = jasmine.createSpyObj('mockLiveAnnouncer', ['announce']);
-    mockMqttService = jasmine.createSpyObj([
-      'getNetworkAdapters',
-      'getInternetConnection',
-    ]);
+    mockMqttService = jasmine.createSpyObj(['getNetworkAdapters']);
 
     TestBed.configureTestingModule({
       imports: [
@@ -166,6 +164,7 @@ describe('AppComponent', () => {
           selectors: [
             { selector: selectInterfaces, value: {} },
             { selector: selectHasConnectionSettings, value: true },
+            { selector: selectInternetConnection, value: true },
             { selector: selectError, value: null },
             { selector: selectMenuOpened, value: false },
             { selector: selectHasDevices, value: false },

modules/ui/src/app/app.component.ts

@@ -83,7 +83,6 @@ export class AppComponent {
     this.appStore.getReports();
     this.appStore.getTestModules();
     this.appStore.getNetworkAdapters();
-    this.appStore.getInternetConnection();
     this.matIconRegistry.addSvgIcon(
       'devices',
       this.domSanitizer.bypassSecurityTrustResourceUrl(DEVICES_LOGO_URL)

modules/ui/src/app/app.store.spec.ts

@@ -24,6 +24,7 @@ import {
   selectHasDevices,
   selectHasRiskProfiles,
   selectInterfaces,
+  selectInternetConnection,
   selectIsOpenWaitSnackBar,
   selectMenuOpened,
   selectReports,
@@ -85,10 +86,7 @@ describe('AppStore', () => {
     mockFocusManagerService = jasmine.createSpyObj([
       'focusFirstElementInContainer',
     ]);
-    mockMqttService = jasmine.createSpyObj([
-      'getNetworkAdapters',
-      'getInternetConnection',
-    ]);
+    mockMqttService = jasmine.createSpyObj(['getNetworkAdapters']);
 
     TestBed.configureTestingModule({
       providers: [
@@ -98,6 +96,7 @@ describe('AppStore', () => {
             { selector: selectStatus, value: null },
             { selector: selectIsOpenWaitSnackBar, value: false },
             { selector: selectTestModules, value: MOCK_TEST_MODULES },
+            { selector: selectInternetConnection, value: false },
           ],
         }),
         { provide: TestRunService, useValue: mockService },
@@ -165,7 +164,7 @@ describe('AppStore', () => {
           isMenuOpen: true,
           interfaces: {},
           settingMissedError: null,
-          hasInternetConnection: null,
+          hasInternetConnection: false,
         });
         done();
       });
@@ -307,19 +306,5 @@ describe('AppStore', () => {
         );
       });
     });
-
-    describe('getInternetConnection', () => {
-      it('should update store', done => {
-        mockMqttService.getInternetConnection.and.returnValue(
-          of({ connection: false })
-        );
-        appStore.getInternetConnection();
-
-        appStore.viewModel$.pipe(take(1)).subscribe(store => {
-          expect(store.hasInternetConnection).toEqual(false);
-          done();
-        });
-      });
-    });
   });
 });

modules/ui/src/app/app.store.ts

@@ -16,13 +16,14 @@
 
 import { Injectable } from '@angular/core';
 import { ComponentStore } from '@ngrx/component-store';
-import { tap, withLatestFrom } from 'rxjs/operators';
+import { tap } from 'rxjs/operators';
 import {
   selectError,
   selectHasConnectionSettings,
   selectHasDevices,
   selectHasRiskProfiles,
   selectInterfaces,
+  selectInternetConnection,
   selectMenuOpened,
   selectReports,
   selectStatus,
@@ -55,16 +56,13 @@ export const CONSENT_SHOWN_KEY = 'CONSENT_SHOWN';
 export interface AppComponentState {
   consentShown: boolean;
   isStatusLoaded: boolean;
-  hasInternetConnection: boolean | null;
   systemStatus: TestrunStatus | null;
 }
 @Injectable()
 export class AppStore extends ComponentStore<AppComponentState> {
   private consentShown$ = this.select(state => state.consentShown);
   private isStatusLoaded$ = this.select(state => state.isStatusLoaded);
-  private hasInternetConnection$ = this.select(
-    state => state.hasInternetConnection
-  );
+  private hasInternetConnection$ = this.store.select(selectInternetConnection);
   private hasDevices$ = this.store.select(selectHasDevices);
   private hasRiskProfiles$ = this.store.select(selectHasRiskProfiles);
   private reports$ = this.store.select(selectReports);
@@ -102,13 +100,6 @@ export class AppStore extends ComponentStore<AppComponentState> {
     isStatusLoaded,
   }));
 
-  updateHasInternetConnection = this.updater(
-    (state, hasInternetConnection: boolean | null) => ({
-      ...state,
-      hasInternetConnection,
-    })
-  );
-
   setContent = this.effect<void>(trigger$ => {
     return trigger$.pipe(
       tap(() => {
@@ -170,19 +161,6 @@ export class AppStore extends ComponentStore<AppComponentState> {
     );
   });
 
-  getInternetConnection = this.effect(trigger$ => {
-    return trigger$.pipe(
-      exhaustMap(() => {
-        return this.testRunMqttService.getInternetConnection().pipe(
-          withLatestFrom(this.hasInternetConnection$),
-          tap(([{ connection }]) => {
-            this.updateHasInternetConnection(connection);
-          })
-        );
-      })
-    );
-  });
-
   private notifyAboutTheAdapters(adapters: SystemInterfaces) {
     this.notificationService.notify(
       `New network adapter(s) ${Object.keys(adapters).join(', ')} has been detected. You can switch to using it in the System settings menu`
@@ -250,7 +228,6 @@ export class AppStore extends ComponentStore<AppComponentState> {
       consentShown: sessionStorage.getItem(CONSENT_SHOWN_KEY) !== null,
       isStatusLoaded: false,
       systemStatus: null,
-      hasInternetConnection: null,
     });
   }
 }

modules/ui/src/app/store/actions.ts

@@ -142,3 +142,8 @@ export const setTestModules = createAction(
   '[Shared] Set Test Modules',
   props<{ testModules: TestModule[] }>()
 );
+
+export const updateInternetConnection = createAction(
+  '[Shared] Fetch internet connection',
+  props<{ internetConnection: boolean | null }>()
+);

modules/ui/src/app/store/effects.spec.ts

@@ -67,7 +67,10 @@ describe('Effects', () => {
       'openSnackBar',
     ]);
   const mockMqttService: jasmine.SpyObj<TestRunMqttService> =
-    jasmine.createSpyObj('mockMqttService', ['getStatus']);
+    jasmine.createSpyObj('mockMqttService', [
+      'getStatus',
+      'getInternetConnection',
+    ]);
 
   beforeEach(() => {
     testRunServiceMock = jasmine.createSpyObj('testRunServiceMock', [
@@ -88,6 +91,9 @@ describe('Effects', () => {
     );
     testRunServiceMock.fetchProfiles.and.returnValue(of([]));
     testRunServiceMock.getHistory.and.returnValue(of([]));
+    mockMqttService.getInternetConnection.and.returnValue(
+      of({ connection: false })
+    );
 
     mockMqttService.getStatus.and.returnValue(
       of(MOCK_PROGRESS_DATA_IN_PROGRESS)
@@ -445,6 +451,12 @@ describe('Effects', () => {
           done();
         });
       });
+
+      it('should call fetchInternetConnection for status "in progress"', () => {
+        effects.onFetchSystemStatusSuccess$.subscribe(() => {
+          expect(mockMqttService.getInternetConnection).toHaveBeenCalled();
+        });
+      });
     });
 
     describe('with status "waiting for device"', () => {