fix: boolean arguments in fabric calls are interpreted correctly

[nstng]

Signed-off-by: Nils Semmelrock nils.semmelrock@tngtech.com

Summary

In #13984 lte/gateway/fabric.py was refactored to provide boolean arguments to a public function. A script call like fab my_function:my_bool_arg=False causes a call my_function("False"), i.e., a check if "False": would pass.

This is only relevant for public functions and not for (implicit) private function prefixed by _ (can not be called via fab).

Solution here: introduce a helper function that interprets such arguments.

Also here: use this solution for two existing functions

• run_integ_tests on federated_mode (default is False) - this is just a precaution as only fab run_integ_tests:federated_mode=False causes problems. But this is currently not used as False is the default.
• load_test on destroy_vm (default is True) - this does not work if fab load_test:destroy_vm=False is called. Currently not the case in workflows etc.

Test Plan

CI

Additional Information

• This change is backwards-breaking

[github-actions]

Thanks for opening a PR! 💯

A couple initial guidelines

• All commits must be signed off. This is enforced by DCO check.
• All PR titles must follow the semantic commits format. This is enforced by Semantic PR.

Howto

• Reviews. The "Reviewers" listed for this PR are the Magma maintainers who will shepherd it.
• Checks. All required CI checks must pass before merge.
• Merge. Once approved and passing CI checks, use the ready2merge label to indicate the maintainers can merge your PR.

More info

Please take a moment to read through the Magma project's

• Contributing Conventions for norms around contributed code

If this is your first Magma PR, also consider reading

• Developer Onboarding for onboarding as a new Magma developer
• Development Workflow for guidance on your first pull request
• CI Checks for points of contact for failing or flaky CI checks
• GitHub-to-Slack mappings for Magma maintainers for guidance on how to contact maintainers on Slack

[github-actions]

feg-workflow

    2 files  203 suites   40s ⏱️
374 tests 374 ✔️ 0 💤 0 ❌
388 runs  388 ✔️ 0 💤 0 ❌

Results for commit 43c1199.

[github-actions]

dp-workflow

14 tests   14 ✔️  2m 13s ⏱️
  1 suites    0 💤
  1 files      0 ❌

Results for commit 43c1199.

[github-actions]

agw-workflow

615 tests   611 ✔️  3m 45s ⏱️
    2 suites      4 💤
    2 files        0 ❌

Results for commit 43c1199.

[MoritzThomasHuebner]

We are already using the following pattern throughout the fabfile:

    destroy_vm = bool(strtobool(destroy_vm))
    provision_vm = bool(strtobool(provision_vm))

see e.g. line 494-495.

I think this would also do for the variables we are fixing in this PR. Otherwise, there does not seem to be a reason not to just use _to_boolean for every instance of a string representing a bool in this file.

[nstng]

This only works here because the default is already a string - e.g., destroy_vm='True'. This is not consistent in this fabfile and should be aligned. But I see this out of scope for a bug/fix PR.

[MoritzThomasHuebner]

Two other things:

• A similar function also exists in the cwf fabfile:

  magma/cwf/gateway/fabfile.py

  Line 280 in e736de2

  def _get_boolean_from_param(value):

  -> Perhaps there could be a more uniform solution for this issue.
• The same issue also clearly exists in the fabfile for the federated integ tests, e.g. the clear_orc8r variable is not correctly converted to a boolean:

  magma/lte/gateway/python/integ_tests/federated_tests/fabfile.py

  Line 66 in e736de2

  if clear_orc8r:

[nstng]

Two other things:

* A similar function also exists in the cwf fabfile: https://github.com/magma/magma/blob/e736de238665e0f3041de2400a121697b1fc0a6e/cwf/gateway/fabfile.py#L280
   -> Perhaps there could be a more uniform solution for this issue.

* The same issue also clearly exists in the fabfile for the federated integ tests, e.g. the `clear_orc8r` variable is not correctly converted to a boolean: https://github.com/magma/magma/blob/e736de238665e0f3041de2400a121697b1fc0a6e/lte/gateway/python/integ_tests/federated_tests/fabfile.py#L66

@MoritzThomasHuebner

• I agree this should be aligned among the fabfiles
• agree, this looks damaged

I can create an issue to refactor this - but I see it out of scope for this bug/fix.

[MoritzThomasHuebner]

Alright, let's keep the scope smaller for this PR and create a follow-up issue.

[nstng]

Alright, let's keep the scope smaller for this PR and create a follow-up issue.

see #14031