Old Amarisoft versions emit data that is different compared to what 640c0130 expected:

- there can be trailing spaces after /dev/sdrX@Y
  fix: adjust /dev/sdr regex
- there is empty line coming in the end that fas failing in l.split(':', 1)
  fix: ignore empty lines and make detection of : in the line more
       robust with reporting which line is not valid instead of just
       "ERROR not enough values to unpack (expected 2, got 1)"
- there is CPRI key instead of CPRI_option, and the data in both are different.
  fix: detect automatically whether it is CPRI_option or CPRI option that is present.

/Co-authored-by: lu.xu <lu.xu@nexedi.com>

Also improve tests to minimize assertions and make them more readable,
including separating specific tests from global ones, which will make
it much easier to improve the coverage.

When testing is True we do not have hardware around and only check
generated configuration files. Most lopcomm promises were already
skipping themselves under testing=True environment, but
check_lopcomm_config_log and check_lopcomm_stats_log were missing
testing checks. This leads to test failures when trying to test
ors-amarisoft with Lopcomm driver in my upcoming work via [1] e.g. as

    ================================================================================
    Error with promises for the following partitions:
      TestENB_Lopcomm4-0[enb]: Promise 'RU1-stats-log.py' failed with output: Not subscribed

-> Fix it by making sure `testing==True` checks are present in all
   promises related to lopcomm.

P.S. in current ors-amarisoft Lopcomm driver is not tested at all: even
though we have testFDD-LOPCOMM.py there[2] `python unittest discover ...`
does not pick up test files with dash in their name. This is also
true for all other dashed test files. Currently only testTDD.py and
testFDD.py are run during the tests there.

[1] https://lab.nexedi.com/kirr/slapos/blob/5b8ec2b3/software/ors-amarisoft/test/test.py
[2] https://lab.nexedi.com/nexedi/slapos/tree/15871bbf/software/ors-amarisoft/test

/cc @lu.xu, @tomo, @xavier_thompson, @Daetalus
/reviewed-by @jhuge
/reviewed-on !129

    slapos.toolbox$ rst2html.py CHANGES.txt CHANGES.html
    CHANGES.txt:33: (WARNING/2) Bullet list ends without a blank line; unexpected unindent.

-> Fix it.

Currently this promise is implemented by grepping whole rf_info output
for "HW" and "SW" strings. But this won't work ok in the presence of
multiple CPRI devices. Imagine, for example if one device has CPRI lock,
while the other does not:

    PCIe CPRI /dev/sdr2@1:
      Hardware ID: 0x4b12
      DNA: [0x0048248a334a7054]
      Serial: ''
      FPGA revision: 2023-06-23  10:05:24
      FPGA vccint: 0.98 V
      FPGA vccaux: 1.76 V
      FPGA vccbram: 0.98 V
      FPGA temperature: 71.9 °C
      Clock tune: 0.0 ppm
      NUMA: 0
      CPRI_option: '5' (x8) lock=no                     <-- NOTE
      DMA0: TX fifo: 66.67us  Usage=16/32768 (0%)
      DMA0: RX fifo: 66.67us  Usage=16/32768 (0%)
      DMA0 Underflows: 0
      DMA0 Overflows: 0
    PCIe CPRI /dev/sdr3@1:
      Hardware ID: 0x4b12
      DNA: [0x0048248a334a7054]
      Serial: ''
      FPGA revision: 2023-06-23  10:05:24
      FPGA vccint: 0.98 V
      FPGA vccaux: 1.77 V
      FPGA vccbram: 0.98 V
      FPGA temperature: 71.7 °C
      Clock tune: 0.0 ppm
      NUMA: 0
      CPRI_option: '5' (x8) lock=HW+SW rx/tx=46.606us   <-- NOTE
        Port #0: T14=46.606us
      DMA0: TX fifo: 66.67us  Usage=16/32768 (0%)
      DMA0: RX fifo: 66.67us  Usage=16/32768 (0%)
      DMA0 Underflows: 0
      DMA0 Overflows: 0

the old code would still report "CPRI locked all ok" and also globally
without indicating which CPRI channel is locked.

-> Fix it by adjusting check_cpri_lock to parse rf_info text more
precisely, detect devices there and to understand which device has CPRI
lock and which does not.

For now this change is accompanied by the following change in
ors-amarisoft SR to keep it working:

    --- a/software/ors-amarisoft/instance-enb.jinja2.cfg
    +++ b/software/ors-amarisoft/instance-enb.jinja2.cfg
    @@ -35,7 +35,6 @@ parts =
       check-lopcomm-sync.py
       check-lopcomm-config-log.py
       check-lopcomm-stats-log.py
    -  check-cpri-lock.py
     {% endif %}
     {% if slapparameter_dict.get("dnsmasq", None) %}
       dnsmasq-service
    @@ -48,6 +47,7 @@ parts =
     {% endif %}
       monitor-base
       publish-connection-information
    +{% set extra_part_list = [] %}

     extends = {{ monitor_template }}

    @@ -688,12 +688,21 @@ config-testing = {{ slapparameter_dict.get("testing", False) }}
     config-config-log = ${lopcomm-rrh-config-template:log-output}
     config-stats-period = {{ slapparameter_dict.get("enb_stats_fetch_period", 60) }}

    -[check-cpri-lock.py]
    +{%  if ru == "lopcomm" %}
    +{%-   set cell_list = slapparameter_dict.get('cell_list', {'default': {}}) %}
    +{%-   for i, k in enumerate(cell_list) %}
    +{%-     set sfp_port = cell_list[k].get('cpri_port_number', i) %}
    +{%-     do extra_part_list.append('SFP{{sfp_port}}-cpri-lock.py') %}
    +[SFP{{sfp_port}}-cpri-lock.py]
     <= macro.promise
     promise = check_cpri_lock
     config-testing = {{ slapparameter_dict.get("testing", False) }}
    +config-sdr_dev  = {{ slapparameter_dict.get('sdr_number', 0) }}
    +config-sfp_port = {{ sfp_port }}
     config-amarisoft-rf-info-log = ${amarisoft-rf-info-template:log-output}
     config-stats-period = {{ slapparameter_dict.get("enb_stats_fetch_period", 60) }}
    +{%-  endfor %}
    +{% endif %}

     [check-rx-saturated.py]
     <= macro.promise
    @@ -702,3 +711,9 @@ config-testing = {{ slapparameter_dict.get("testing", False) }}
     config-amarisoft-stats-log = ${amarisoft-stats-template:log-output}
     config-stats-period = {{ slapparameter_dict.get("enb_stats_fetch_period", 60) }}
     config-max-rx-sample-db = {{ slapparameter_dict.get("max_rx_sample_db", 0) }}
    +
    +[buildout]
    +parts +=
    +{%- for part in extra_part_list %}
    +    {{ part }}
    +{%- endfor %}

(posted in slapos!1461)

The way rf_info text is parsed could be also useful in the future to
e.g. detect FPGA revision of the boards and report their recency status
via promise.

/cc @jhuge, @tomo, @xavier_thompson, @Daetalus
/reviewed-by @lu.xu
/reviewed-on !127

This promise plugin was using tail_file(self.amarisoft_rf_info_log) to
get the most recent log entry to check. But tail_file simply return last
line of the log file without checking timestamp of returned entry. So it
could be the case that last line is ages ago, reports "CPRI locked ok"
and then, if there are no more rf_info entries for one reason or
another, the CPRI-locked promise will be holding green despite the fact
that data became stale.

-> Fix it by explicitly checking timestamp of last log entry to be in
expected recent range.

/cc @jhuge, @tomo, @xavier_thompson, @Daetalus
/reviewed-by @lu.xu
/reviewed-on !127

promise/plugin/check_cpri_lock: test: Factor common code to write to log and promise into common place

We are going to add more tests. Keeping the code to initialize test
environment duplicated over and over will be not convenient.

/cc @jhuge, @tomo, @xavier_thompson, @Daetalus
/reviewed-by @lu.xu
/reviewed-on !127

When files are opened with 'w' mode - they are truncated to empty state.

/cc @jhuge, @tomo, @xavier_thompson, @Daetalus
/reviewed-by @lu.xu
/reviewed-on !127

Currently check_rx_saturated checks saturation on all rx antennas. This
works well for ORS, which has only one SDR board, but won't work for
multiRU case where we will need to check RX for saturation for each
Radio Unit separately.

-> As a preparatory step adjust check_rx_saturated to take list of which
RX antennas to check for saturation.

For now this change is accompanied by the following change in
ors-amarisoft SR to keep it working as before:

    --- a/software/ors-amarisoft/instance-enb.jinja2.cfg
    +++ b/software/ors-amarisoft/instance-enb.jinja2.cfg
    @@ -699,6 +699,7 @@ config-stats-period = {{ slapparameter_dict.get("enb_stats_fetch_period", 60) }}
     <= macro.promise
     promise = check_rx_saturated
     config-testing = {{ slapparameter_dict.get("testing", False) }}
    +config-rf-rx-chan-list = {{ list(range(0, int(slapparameter_dict.get('n_antenna_ul', default_n_antenna_ul)))) }}
     config-amarisoft-stats-log = ${amarisoft-stats-template:log-output}
     config-stats-period = {{ slapparameter_dict.get("enb_stats_fetch_period", 60) }}
     config-max-rx-sample-db = {{ slapparameter_dict.get("max_rx_sample_db", 0) }}

(posted in slapos!1459)

/cc @lu.xu, @xavier_thompson, @Daetalus
/reviewed-by @jhuge
/partly-reviewed-by @tomo
/reviewed-on !126

Currently check_sdr_busy always checks /dev/sdr0 (via `-c 0`) and
also implicitly DMA channel 0 as sdr_util help explains:

    ./sdr_util  -h
    ...
    -c device_num              select the device (default = all)
    -d channel_num             select the channel (default = 0)

This works well for ORS, which always has only single SDR board.

However for cases when there are either a) multiple SDR boards, or b)
single CPRI board, the assumption is incorrect:

- for multiple SDR boards we need to be able to specify /dev/sdrX
  instead of sdr0.

- for the case of one CPRI boards, links to different Radio Units
  attached to different SFP ports are associated with different DMA
  channels, so to test e.g. whether RU1 is being used it is necessary to
  check e.g. sdr0@0, while for RU2 it is necessary to check sdr0@1.

  I explicitly verified that for CPRI case

  a) Amarisoft kernel driver allows /dev/sdrX associated with CPRI card
     to be opened multiple times simultaneously - up to the amount of SFP
     ports = # of DMA channels, and

  b) that only DMA channels / SFP ports actually being in use are
     reported as busy by sdr_util, while the other DMA channels / SFP
     ports are not reported as busy.

  I also verified that for regular SDR board (not CPRI) using -d 0 does
  not change any verification semantic as such SDR boards have only one
  DMA channel.

-> As a preparatory step for multiRU work adjust check_sdr_busy promise
to take SDR device number and DMA channel number as arguments instead of
hardcoding sdr0@0.

For now this change is accompanied by the following change in
ors-amarisoft SR to keep it working as before:

    --- a/software/ors-amarisoft/instance-enb.jinja2.cfg
    +++ b/software/ors-amarisoft/instance-enb.jinja2.cfg
    @@ -615,6 +615,8 @@ name = ${:_buildout_section_name_}
     promise = check_sdr_busy
     config-testing = {{ slapparameter_dict.get("testing", False) }}
     config-sdr = {{ sdr }}
    +config-sdr_dev  = 0
    +config-dma_chan = 0

(posted in slapos!1458)

/cc @xavier_thompson, @Daetalus
/reviewed-by @jhuge, @lu.xu
/partly-reviewed-by @tomo
/reviewed-on !125

Running tests with recent numpy (1.26.0) yields the following:

    slapos.toolbox$ python -m unittest discover -v   -s slapos/test/promise/plugin/ -t . -k test_display_prediction
    ...
    Enable to display disk space predictions: True
    module 'numpy' has no attribute 'float'.
    `np.float` was a deprecated alias for the builtin `float`. To avoid this error in existing code, use `float` by itself. Doing this will not modify any behavior and is safe. If you specifically wanted the numpy scalar type, use `np.float64` here.
    The aliases was originally deprecated in NumPy 1.20; for more details and guidance see the original release note at:
        https://numpy.org/devdocs/release/1.20.0-notes.html#deprecations
    Traceback (most recent call last):
      File "/home/kirr/src/wendelin/slapos/slapos.core/slapos/grid/promise/generic.py", line 500, in run
        self.sense()
      File "/home/kirr/src/wendelin/slapos/slapos.toolbox/slapos/promise/plugin/check_free_disk_space.py", line 327, in sense
        disk_space_prediction_tuple = self.diskSpacePrediction(
                                      ^^^^^^^^^^^^^^^^^^^^^^^^^
      File "/home/kirr/src/wendelin/slapos/slapos.toolbox/slapos/promise/plugin/check_free_disk_space.py", line 182, in diskSpacePrediction
        df = df.astype({'free': np.float})
                                ^^^^^^^^
      File "/home/kirr/src/wendelin/venv/py3.venv/lib/python3.11/site-packages/numpy/__init__.py", line 324, in __getattr__
        raise AttributeError(__former_attrs__[attr])
    AttributeError: module 'numpy' has no attribute 'float'.
    `np.float` was a deprecated alias for the builtin `float`. To avoid this error in existing code, use `float` by itself. Doing this will not modify any behavior and is safe. If you specifically wanted the numpy scalar type, use `np.float64` here.
    The aliases was originally deprecated in NumPy 1.20; for more details and guidance see the original release note at:
        https://numpy.org/devdocs/release/1.20.0-notes.html#deprecations
    ERROR

-> Replace np.float with float as the error says.

/cc @Just1, @lu.xu, @jhuge, @tomo
/reviewed-on !123

folder_disk_test.sql was opened to be read but was not closed:

    slapos.toolbox$ python -m unittest discover -v   -s slapos/test/promise/plugin/ -t . -k test_display_prediction
    test_display_prediction (slapos.test.promise.plugin.test_check_free_disk_space.testcheckfreediskspace.test_display_prediction) ...
    /usr/lib/python3.11/unittest/case.py:576: resourcewarning: unclosed file <_io.textiowrapper name='/home/kirr/src/wendelin/slapos/slapos.toolbox/slapos/test/promise/data/disktest.sql' mode='r' encoding='utf-8'>
      self.setup()

/cc @Just1, @lu.xu, @jhuge, @tomo
/reviewed-on !124

Same logic as in previous patch. Suggested by @jeorme in
!122 (comment 193062) .

/suggested-and-reviewed-by @jerome
/reviewed-n !122

In python `is` checks object identity, not content:

    In [1]: a = 'hello'

    In [2]: a += ' world'

    In [3]: b = 'hello world'

    In [4]: id(a)
    Out[4]: 139735676540976

    In [5]: id(b)
    Out[5]: 139735675829040

    In [6]: a is b              <-- NOTE
    Out[6]: False

    In [7]: a == b              <-- NOTE
    Out[7]: True

So comparing strings by is is generally incorrect.

-> Fix strings comparision to use == / != everywhere (at least in found
places where string is compared wrt string literal)

We already had similar fix in a8526f4e, but seems the story continues
again.

/cc @alain.takoudjou, @Just1, @lu.xu, @jhuge, @tomo
/reviewed-by @jerome
/reviewed-on !122

Hello up there,

This merge-requests primarily adjusts promises code to treat `data` field in `*.json.log` as real JSON. It is counterpart to slapos!1447 .

- patch 1 syncs tests to current state of ORS SR;
- patch 2 removes replacement of `'` with `"` in input data;
- patch 3 removes UTF-8 decoding when input is fed to json.loads.

Please see individual patches for details.

Kirill

/cc @jhuge, @lu.xu, @tomo, @xavier_thompson, @Daetalus, @jerome
/reviewed-on !120 and slapos!1447

Starting from py3.6 json.loads accepts all unicode, bytes and bytearray:

    https://docs.python.org/3/library/json.html#:~:text=json.loads(s
    https://github.com/python/cpython/commit/b161562f72a2
    https://bugs.python.org/issue17909

Adjust promises correspondingly to slapos!1447
which changed ors-amarisoft software-release to emit log payload via
json.dumps instead of py %s.

Do this for both promises code and JSON-loading utilities in slapos/promise/plugin/util.py also used inside ORS promises code.
We can adjust the utilities because there are two of them (get_json_log_data_interval and get_json_log_latest_timestamp) with
get_json_log_latest_timestamp being used only in ORS-promises:

    slapos.toolbox$ git grep get_json_log_latest_timestamp
    slapos/promise/plugin/check_amarisoft_stats_log.py:from .util import get_json_log_latest_timestamp
    slapos/promise/plugin/check_amarisoft_stats_log.py:    latest_timestamp = get_json_log_latest_timestamp(self.amarisoft_stats_log)
    slapos/promise/plugin/util.py:def get_json_log_latest_timestamp(json_log_file):

and get_json_log_data_interval being used in ORS promises and
check_cpu_temperature, check_network_transit and check_ram_usage:

    slapos.toolbox$ git grep get_json_log_data_interval
    slapos/promise/plugin/check_baseband_latency.py:from .util import get_json_log_data_interval
    slapos/promise/plugin/check_baseband_latency.py:    data_list = get_json_log_data_interval(self.amarisoft_stats_log, self.stats_period * 5)
    slapos/promise/plugin/check_cpu_temperature.py:      temp_list = self.get_json_log_data_interval(self.avg_temp_duration)
    slapos/promise/plugin/check_network_transit.py:      temp_list = self.get_json_log_data_interval(self.transit_period)
    slapos/promise/plugin/check_ram_usage.py:      temp_list = self.get_json_log_data_interval(self.avg_ram_period)
    slapos/promise/plugin/check_rx_saturated.py:from .util import get_json_log_data_interval
    slapos/promise/plugin/check_rx_saturated.py:    data_list = get_json_log_data_interval(self.amarisoft_stats_log, self.stats_period * 2)
    slapos/promise/plugin/util.py:def get_json_log_data_interval(json_log_file, interval):
    slapos/promise/plugin/util.py:  def get_json_log_data_interval(self, interval):
    slapos/promise/plugin/util.py:    return get_json_log_data_interval(self.__json_log_file, interval)

However all check_cpu_temperature, check_network_transit and
check_ram_usage produce their *.json.log files themselves and already
emit `data` via json.dumps:

https://lab.nexedi.com/nexedi/slapos.toolbox/blob/453dce5f/slapos/promise/plugin/check_cpu_temperature.py#L41-54
https://lab.nexedi.com/nexedi/slapos.toolbox/blob/453dce5f/slapos/promise/plugin/check_network_transit.py#L29-42
https://lab.nexedi.com/nexedi/slapos.toolbox/blob/453dce5f/slapos/promise/plugin/check_ram_usage.py#L34-46

So it is safe to adjust the utilities.

tests/promise/plugin: Adjust Amarisoft-related tests to match amarisoft_stats.json.log emitted by SR

In slapos@b32b4a8e `data` field in
that log switched from emitting objects via py repr to emitting them via
JSON. As the result, in particular, strings are now emitted quoted with
" instead of with ' .

-> Adjust all tests that simulate amarisoft_stats.json.log to follow
actual software-release behaviour.

The tests continue to pass.

See merge request !119

See https://pypi.org/help/#description-content-type

In some deployments it is important to being able to select only specific
senses on which the promise shall react.

See also nexedi/slapos@4230cd1b