Merge ~hemanth-n/maas:lp1906212 into maas:master

[Hemanth Nakkina]

improper conversion of timeout to seconds

In commissiong/testing scripts, timeout_seconds is derived from the timeout datetime object. The conversion gathered just the seconds and ignored the days.

Using the function timeout_seconds() of the datetime object resolves the problem.

[MAAS Lander (maas-lander)]

UNIT TESTS
-b lp1906212 lp:~hemanth-n/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci.internal:8080/job/maas/job/branch-tester/8783/console
COMMIT: 0e4a5512852ed1cbf956643b7fd37d1897f96927

review: Needs Fixing

[Adam Collard (adam-collard)]

jenkins: !test

[MAAS Lander (maas-lander)]

UNIT TESTS
-b lp1906212 lp:~hemanth-n/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: 0e4a5512852ed1cbf956643b7fd37d1897f96927

review: Approve

[MAAS Lander (maas-lander)]

UNIT TESTS
-b lp1906212 lp:~hemanth-n/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: b4630a193b89faf4baf7c81edaa038e7063a75ef

review: Approve

[Adam Collard (adam-collard)]

Can you please add a test for this? The change looks good (thank you!) just needs an example in the tests where timeout > 24h

[Dougal Matthews (d0ugal)]

Agreed, a test for this would be good.

review: Needs Fixing

[Hemanth Nakkina (hemanth-n)]

Added test cases. Modified commissioning script under src/metadataserver/builtin_scripts/ with an assumption the scripts are only used for unit/functional tests. Correct me if i am wrong.

review: Needs Resubmitting

[MAAS Lander (maas-lander)]

UNIT TESTS
-b lp1906212 lp:~hemanth-n/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci.internal:8080/job/maas/job/branch-tester/8892/console
COMMIT: 464a7436c2320731f21807a7b16644152dbbd9b6

review: Needs Fixing

[Hemanth Nakkina (hemanth-n)]

Reverted back previous patch since the builtin scripts should not be changed for unit/functional tests

[MAAS Lander (maas-lander)]

UNIT TESTS
-b lp1906212 lp:~hemanth-n/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: e00de0d52a067bae08b3eb68a9fb3855a83669a7

review: Approve

[Hemanth Nakkina (hemanth-n)]

Test case added, please review

review: Needs Resubmitting

[MAAS Lander (maas-lander)]

UNIT TESTS
-b lp1906212 lp:~hemanth-n/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: 229679bc3c599cd0be1d0e93c514072aff7e0789

review: Approve

[Lee Trager (ltrager)]

Thanks for the fix!

review: Approve

diff --git a/src/metadataserver/api.py b/src/metadataserver/api.py
index f3e82c9..f59f973 100644
--- a/src/metadataserver/api.py
+++ b/src/metadataserver/api.py
@@ -1197,7 +1197,7 @@ class AnonMAASScriptsHandler(AnonymousOperationsHandler):
                             "name": script.name,
                             "path": path,
                             "script_version_id": script.script.id,
-                            "timeout_seconds": script.timeout.seconds,
+                            "timeout_seconds": script.timeout.total_seconds(),
                             "parallel": script.parallel,
                             "hardware_type": script.hardware_type,
                             "parameters": parameters,
@@ -1248,7 +1248,9 @@ class MAASScriptsHandler(OperationsHandler):
                     "path": path,
                     "script_result_id": script_result.id,
                     "script_version_id": script_result.script.script.id,
-                    "timeout_seconds": script_result.script.timeout.seconds,
+                    "timeout_seconds": (
+                        script_result.script.timeout.total_seconds()
+                    ),
                     "parallel": script_result.script.parallel,
                     "hardware_type": script_result.script.hardware_type,
                     "parameters": script_result.parameters,
diff --git a/src/metadataserver/tests/test_api.py b/src/metadataserver/tests/test_api.py
index 85bc9fa..62cd333 100644
--- a/src/metadataserver/tests/test_api.py
+++ b/src/metadataserver/tests/test_api.py
@@ -6,7 +6,7 @@
 
 import base64
 from collections import namedtuple
-from datetime import datetime
+from datetime import datetime, timedelta
 import http.client
 from io import BytesIO
 import json
@@ -1446,7 +1446,7 @@ class TestMAASScripts(MAASServerTestCase):
             if script_result.script is None:
                 script = NODE_INFO_SCRIPTS[script_result.name]
                 content = script["content"]
-                md_item["timeout_seconds"] = script["timeout"].seconds
+                md_item["timeout_seconds"] = script["timeout"].total_seconds()
                 md_item["parallel"] = script.get(
                     "parallel", SCRIPT_PARALLEL.DISABLED
                 )
@@ -1463,7 +1463,7 @@ class TestMAASScripts(MAASServerTestCase):
                 md_item["script_version_id"] = script_result.script.script.id
                 md_item[
                     "timeout_seconds"
-                ] = script_result.script.timeout.seconds
+                ] = script_result.script.timeout.total_seconds()
                 md_item["parallel"] = script_result.script.parallel
                 md_item["hardware_type"] = script_result.script.hardware_type
                 md_item["parameters"] = script_result.parameters
@@ -1581,7 +1581,7 @@ class TestMAASScripts(MAASServerTestCase):
                         "name": script.name,
                         "path": path,
                         "script_version_id": script.script.id,
-                        "timeout_seconds": script.timeout.seconds,
+                        "timeout_seconds": script.timeout.total_seconds(),
                         "parallel": script.parallel,
                         "hardware_type": script.hardware_type,
                         "parameters": parameters,
@@ -1635,7 +1635,7 @@ class TestMAASScripts(MAASServerTestCase):
                         "name": script.name,
                         "path": path,
                         "script_version_id": script.script.id,
-                        "timeout_seconds": script.timeout.seconds,
+                        "timeout_seconds": script.timeout.total_seconds(),
                         "parallel": script.parallel,
                         "hardware_type": script.hardware_type,
                         "parameters": parameters,
@@ -1707,7 +1707,7 @@ class TestMAASScripts(MAASServerTestCase):
                         "name": script.name,
                         "path": path,
                         "script_version_id": script.script.id,
-                        "timeout_seconds": script.timeout.seconds,
+                        "timeout_seconds": script.timeout.total_seconds(),
                         "parallel": script.parallel,
                         "hardware_type": script.hardware_type,
                         "parameters": parameters,
@@ -2049,7 +2049,9 @@ class TestMAASScripts(MAASServerTestCase):
                             ),
                             "script_result_id": script_result.id,
                             "script_version_id": script_result.script.script.id,
-                            "timeout_seconds": script_result.script.timeout.seconds,
+                            "timeout_seconds": (
+                                script_result.script.timeout.total_seconds()
+                            ),
                             "parallel": script_result.script.parallel,
                             "hardware_type": script_result.script.hardware_type,
                             "parameters": script_result.parameters,
@@ -2262,6 +2264,51 @@ class TestMAASScripts(MAASServerTestCase):
             % (response.status_code, response.content),
         )
 
+    def test_scripts_with_timeout_greater_than_24_hours(self):
+        start_time = floor(time.time())
+        node = factory.make_Node(
+            status=NODE_STATUS.TESTING, with_empty_script_sets=True
+        )
+        # Make script timeout value to 24:05:00
+        timeout = timedelta(days=1, seconds=600)
+        script = factory.make_Script(
+            script_type=SCRIPT_TYPE.TESTING, timeout=timeout
+        )
+        factory.make_ScriptResult(
+            script_set=node.current_testing_script_set,
+            script=script,
+            status=SCRIPT_STATUS.PENDING,
+        )
+        response = make_node_client(node=node).get(
+            reverse("maas-scripts", args=["latest"])
+        )
+
+        tar = tarfile.open(mode="r", fileobj=BytesIO(response.content))
+        end_time = ceil(time.time())
+
+        testing_meta_data = self.validate_scripts(
+            node.current_testing_script_set,
+            "testing",
+            tar,
+            start_time,
+            end_time,
+        )
+        meta_data = json.loads(
+            tar.extractfile("index.json").read().decode("utf-8")
+        )
+        # Assertion should fail if timeout conversions are improper
+        self.assertDictEqual(
+            {
+                "1.0": {
+                    "testing_scripts": sorted(
+                        testing_meta_data,
+                        key=itemgetter("name", "script_result_id"),
+                    ),
+                }
+            },
+            meta_data,
+        )
+
 
 class TestCommissioningAPI(MAASServerTestCase):
     def setUp(self):