Merge remote-tracking branch 'origin/master' into nightly

CHANGELOG.md

@@ -18,8 +18,10 @@ Every user-facing change should have an entry in this changelog. Please respect
 
 ## Unreleased
 
+- [Bugfix] Log the shell commands that Tutor executes more accurately. (by @kdmccormick)
 - [Fix] `tutor dev quickstart` would fail under certain versions of docker-compose due to a bug in the logic that handled volume mounting. (by @kdmccormick)
 - [Bugfix] The `tutor k8s start` command will succeed even when `k8s-override` and `kustomization-patches-strategic-merge` are not specified. (by @edazzocaisser)
+- [Fix] `kubectl wait` checks deployments instead of pods as it could hang indefinitely if there are extra pods in a broken state. (by @keithgg)
 
 ## v14.0.3 (2022-07-09)
 

tests/test_utils.py

@@ -102,7 +102,23 @@ class UtilsTests(unittest.TestCase):
 
         result = utils.execute("echo", "")
         self.assertEqual(0, result)
-        self.assertEqual("echo \n", mock_stdout.getvalue())
+        self.assertEqual("echo ''\n", mock_stdout.getvalue())
+        self.assertEqual(1, process.wait.call_count)
+        process.kill.assert_not_called()
+
+    @patch("sys.stdout", new_callable=StringIO)
+    @patch("subprocess.Popen", autospec=True)
+    def test_execute_nested_command(
+        self, mock_popen: MagicMock, mock_stdout: StringIO
+    ) -> None:
+        process = mock_popen.return_value
+        mock_popen.return_value.__enter__.return_value = process
+        process.wait.return_value = 0
+        process.communicate.return_value = ("output", "error")
+
+        result = utils.execute("bash", "-c", "echo -n hi")
+        self.assertEqual(0, result)
+        self.assertEqual("bash -c 'echo -n hi'\n", mock_stdout.getvalue())
         self.assertEqual(1, process.wait.call_count)
         process.kill.assert_not_called()
 
@@ -117,7 +133,7 @@ class UtilsTests(unittest.TestCase):
         process.communicate.return_value = ("output", "error")
 
         self.assertRaises(exceptions.TutorError, utils.execute, "echo", "")
-        self.assertEqual("echo \n", mock_stdout.getvalue())
+        self.assertEqual("echo ''\n", mock_stdout.getvalue())
         self.assertEqual(1, process.wait.call_count)
         process.kill.assert_not_called()
 
@@ -131,7 +147,7 @@ class UtilsTests(unittest.TestCase):
         process.wait.side_effect = ZeroDivisionError("Exception occurred.")
 
         self.assertRaises(ZeroDivisionError, utils.execute, "echo", "")
-        self.assertEqual("echo \n", mock_stdout.getvalue())
+        self.assertEqual("echo ''\n", mock_stdout.getvalue())
         self.assertEqual(2, process.wait.call_count)
         process.kill.assert_called_once()
 

tutor/commands/k8s.py

@@ -332,10 +332,10 @@ def delete(context: K8sContext, yes: bool) -> None:
 def init(context: K8sContext, limit: Optional[str]) -> None:
     config = tutor_config.load(context.root)
     runner = context.job_runner(config)
-    wait_for_pod_ready(config, "caddy")
+    wait_for_deployment_ready(config, "caddy")
     for name in ["elasticsearch", "mysql", "mongodb"]:
         if tutor_config.is_service_activated(config, name):
-            wait_for_pod_ready(config, name)
+            wait_for_deployment_ready(config, name)
     jobs.initialise(runner, limit_to=limit)
 
 
@@ -458,7 +458,7 @@ def logs(
 @click.pass_obj
 def wait(context: K8sContext, name: str) -> None:
     config = tutor_config.load(context.root)
-    wait_for_pod_ready(config, name)
+    wait_for_deployment_ready(config, name)
 
 
 @click.command(
@@ -536,14 +536,14 @@ def kubectl_exec(config: Config, service: str, command: List[str]) -> int:
     )
 
 
-def wait_for_pod_ready(config: Config, service: str) -> None:
+def wait_for_deployment_ready(config: Config, service: str) -> None:
-    fmt.echo_info(f"Waiting for a {service} pod to be ready...")
+    fmt.echo_info(f"Waiting for a {service} deployment to be ready...")
     utils.kubectl(
         "wait",
         *resource_selector(config, f"app.kubernetes.io/name={service}"),
-        "--for=condition=ContainersReady",
+        "--for=condition=Available=True",
         "--timeout=600s",
-        "pod",
+        "deployment",
     )
 
 

tutor/commands/upgrade/k8s.py

@@ -120,7 +120,7 @@ def upgrade_from_maple(context: Context, config: Config) -> None:
         "--selector",
         "app.kubernetes.io/name=mysql",
     )
-    k8s.wait_for_pod_ready(config, "mysql")
+    k8s.wait_for_deployment_ready(config, "mysql")
 
     # lms upgrade
     k8s.kubectl_apply(
@@ -128,7 +128,7 @@ def upgrade_from_maple(context: Context, config: Config) -> None:
         "--selector",
         "app.kubernetes.io/name=lms",
     )
-    k8s.wait_for_pod_ready(config, "lms")
+    k8s.wait_for_deployment_ready(config, "lms")
 
     # Command backpopulate_user_tours
     k8s.kubectl_exec(
@@ -144,7 +144,7 @@ def upgrade_from_maple(context: Context, config: Config) -> None:
         "--selector",
         "app.kubernetes.io/name=cms",
     )
-    k8s.wait_for_pod_ready(config, "cms")
+    k8s.wait_for_deployment_ready(config, "cms")
 
     # Command backfill_course_tabs
     k8s.kubectl_exec(

tutor/utils.py

@@ -3,6 +3,7 @@ from functools import lru_cache
 import json
 import os
 import random
+import shlex
 import shutil
 import string
 import struct
@@ -201,7 +202,7 @@ def is_a_tty() -> bool:
 
 
 def execute(*command: str) -> int:
-    click.echo(fmt.command(" ".join(command)))
+    click.echo(fmt.command(_shlex_join(*command)))
     with subprocess.Popen(command) as p:
         try:
             result = p.wait(timeout=None)
@@ -220,6 +221,19 @@ def execute(*command: str) -> int:
     return result
 
 
+def _shlex_join(*split_command: str) -> str:
+    """
+    Return a shell-escaped string from *split_command.
+
+    TODO: REMOVE THIS FUNCTION AFTER 2023-06-27.
+      This function is a shim for the ``shlex.join`` standard library function,
+      which becomes available in Python 3.8  The end-of-life date for Python 3.7
+      is in Jan 2023 (https://endoflife.date/python). After that point, it
+      would be good to delete this function and just use Py3.8's ``shlex.join``.
+    """
+    return " ".join(shlex.quote(arg) for arg in split_command)
+
+
 def check_output(*command: str) -> bytes:
     literal_command = " ".join(command)
     click.echo(fmt.command(literal_command))