Merge branch 'main' into ping-gmp-endpoint

.pylintrc

@@ -19,6 +19,19 @@ max-parents=8
 # "TODO(<username>):", "FIXME:", or anything else.
 notes=FIXME,XXX
 
+[TYPECHECK]
+# List of module names for which member attributes should not be checked
+# (useful for modules/projects where namespaces are manipulated during runtime
+# and thus existing member attributes cannot be deduced by static analysis. It
+# supports qualified module names, as well as Unix pattern matching.
+#
+# TODO(sergiitk): remove when astroid upgraded to astroid 2.15.5, see
+#   https://pylint.pycqa.org/projects/astroid/en/latest/changelog.html#what-s-new-in-astroid-2-15-5
+#   > Recognize stub pyi Python files.
+ignored-modules=
+  protos.grpc.testing.messages_pb2,
+  protos.grpc.testing.empty_pb2,
+
 [MESSAGES CONTROL]
 
 disable=

bin/pylint.sh

@@ -0,0 +1,59 @@
+#!/usr/bin/env bash
+# Copyright 2024 gRPC authors.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+set -eo pipefail
+
+SCRIPT_DIR="$( cd -- "$(dirname "$0")" >/dev/null 2>&1 ; pwd -P )"
+readonly SCRIPT_DIR
+XDS_K8S_DRIVER_DIR="$( cd -- "$(dirname "${SCRIPT_DIR}")" >/dev/null 2>&1 ; pwd -P )"
+readonly XDS_K8S_DRIVER_DIR
+
+# TODO(sergiitk): Once upgraded to be compatible with requirements-dev.txt, use the default venv.
+display_usage() {
+  cat <<EOF >/dev/stderr
+A helper to run black pylint.
+
+USAGE:
+  ./bin/pylint.sh
+
+ONE-TIME INSTALLATION:
+  1. python3.9 -m venv --upgrade-deps venv-pylint
+  2. source ./venv-pylint/bin/activate
+  3. pip install pylint==2.2.2 astroid==2.3.3 toml==0.10.2 "isort>=4.3.0,<5.0.0"
+  4. deactivate
+
+ENVIRONMENT:
+   XDS_K8S_DRIVER_PYLINT_VENV_DIR: the path to python virtual environment directory
+                                   Default: $XDS_K8S_DRIVER_DIR/venv-pylint
+EOF
+  exit 1
+}
+
+if [[ "$1" == "-h" || "$1" == "--help" ]]; then
+  display_usage
+fi
+
+cd "${XDS_K8S_DRIVER_DIR}"
+
+# Relative paths not yet supported by shellcheck.
+# shellcheck source=/dev/null
+XDS_K8S_DRIVER_VENV_DIR="${XDS_K8S_DRIVER_PYLINT_VENV_DIR:-$XDS_K8S_DRIVER_DIR/venv-pylint}" \
+  source "${XDS_K8S_DRIVER_DIR}/bin/ensure_venv.sh"
+
+EXIT=0
+set -x
+pylint -rn 'bin' 'framework' || EXIT=1
+pylint --rcfile=./tests/.pylintrc -rn 'tests' || EXIT=1
+exit "${EXIT}"

config/common.cfg

@@ -1,5 +1,5 @@
 --resource_prefix=psm-interop
---td_bootstrap_image=gcr.io/grpc-testing/td-grpc-bootstrap:7d8d90477792e2e1bfe3a3da20b3dc9ef01d326c
+--td_bootstrap_image=gcr.io/grpc-testing/td-grpc-bootstrap:2bf1b5ed00f852ffea8d24759c6fa673acc9ef10
 
 # The canonical implementation of the xDS test server.
 # Can be used in tests where language-specific xDS test server does not exist,

framework/test_cases/base_testcase.py

@@ -12,15 +12,17 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 """Base test case used for xds test suites."""
-
-from typing import Optional
+import inspect
+import traceback
+from typing import Optional, Union
 import unittest
 
 from absl import logging
 from absl.testing import absltest
 
 
 class BaseTestCase(absltest.TestCase):
+    # @override
     def run(self, result: Optional[unittest.TestResult] = None) -> None:
         super().run(result)
         # TODO(sergiitk): should this method be returning result? See
@@ -44,10 +46,8 @@ def run(self, result: Optional[unittest.TestResult] = None) -> None:
                 self.test_name,
                 f" | Errors count: {total_errors}" if total_errors > 1 else "",
             )
-            if test_errors:
-                self._print_error_list(test_errors, is_unexpected_error=True)
-            if test_failures:
-                self._print_error_list(test_failures)
+            self._log_test_errors(test_errors, is_unexpected=True)
+            self._log_test_errors(test_failures)
         elif test_unexpected_successes:
             logging.error(
                 "----- PSM Test Case UNEXPECTEDLY SUCCEEDED: %s -----\n",
@@ -85,22 +85,75 @@ def test_name(self) -> str:
         """
         return self.id().removeprefix("__main__.").split(" ", 1)[0]
 
-    def _print_error_list(
-        self, errors: list[str], is_unexpected_error: bool = False
+    def _log_test_errors(self, errors: list[str], is_unexpected: bool = False):
+        for err in errors:
+            self._log_framed_test_failure(self.test_name, err, is_unexpected)
+
+    @classmethod
+    def _log_class_hook_failure(cls, error: Exception):
+        """
+        Log error helper for failed unittest hooks, e.g. setUpClass.
+
+        Normally we don't want to make external calls in setUpClass.
+        But when we do, we want to wrap them into try/except, and call
+        _log_class_hook_failure, so the error is logged in our standard format.
+        Don't forget to re-raise!
+
+        Example:
+            @classmethod
+            def setUpClass(cls):
+                try:
+                    # Making bad external calls that end up raising
+                    raise OSError("Network bad!")
+                except Exception as error:  # noqa pylint: disable=broad-except
+                    cls._log_class_hook_failure(error)
+                    raise
+        """
+        caller: str
+        try:
+            caller_info: inspect.FrameInfo = inspect.stack()[1]
+            caller: str = caller_info.function
+        except (IndexError, AttributeError):
+            caller = "undefined_hook"
+
+        fake_test_id = f"{cls.__name__}.{caller}"
+        # The same test name transformation as in self.test_name().
+        # TODO(sergiitk): move the transformation to a classmethod.
+        test_name = fake_test_id.removeprefix("__main__.").split(" ", 1)[0]
+        logging.error("----- PSM Test Case FAILED: %s -----", test_name)
+        cls._log_framed_test_failure(test_name, error, is_unexpected=True)
+
+    @classmethod
+    def _log_framed_test_failure(
+        cls,
+        test_name: str,
+        error: Union[str, Exception],
+        is_unexpected: bool = False,
     ) -> None:
+        trace: str
+        if isinstance(error, Exception):
+            trace = cls._format_error_with_trace(error)
+        else:
+            trace = error
+
         # FAILURE is an errors explicitly signalled using one of the
         # TestCase.assert*() methods, while ERROR means an unexpected exception.
-        fail_type: str = "ERROR" if is_unexpected_error else "FAILURE"
-        for err in errors:
-            logging.error(
-                "(%(fail_type)s) PSM Interop Test Failed: %(test_id)s"
-                "\n^^^^^"
-                "\n[%(test_id)s] PSM Failed Test Traceback BEGIN"
-                "\n%(error)s"
-                "[%(test_id)s] PSM Failed Test Traceback END\n",
-                {
-                    "test_id": self.test_name,
-                    "fail_type": fail_type,
-                    "error": err,
-                },
-            )
+        fail_type: str = "ERROR" if is_unexpected else "FAILURE"
+        logging.error(
+            "(%(fail_type)s) PSM Interop Test Failed: %(test_id)s"
+            "\n^^^^^"
+            "\n[%(test_id)s] PSM Failed Test Traceback BEGIN"
+            "\n%(error)s"
+            "[%(test_id)s] PSM Failed Test Traceback END\n",
+            {
+                "test_id": test_name,
+                "fail_type": fail_type,
+                "error": trace,
+            },
+        )
+
+    @classmethod
+    def _format_error_with_trace(cls, error: Exception) -> str:
+        return "".join(
+            traceback.TracebackException.from_exception(error).format()
+        )

framework/xds_url_map_testcase.py

@@ -380,27 +380,33 @@ def setUpClass(cls):
         # whether setUpClass failed.
         cls.addClassCleanup(cls.cleanupAfterTests)
 
-        if not cls.started_test_cases:
-            # Create the GCP resource once before the first test start
-            GcpResourceManager().setup(cls.test_case_classes)
-        cls.started_test_cases.add(cls.__name__)
-
-        # Create the test case's own client runner with it's own namespace,
-        # enables concurrent running with other test cases.
-        cls.test_client_runner = (
-            GcpResourceManager().create_test_client_runner()
-        )
-        # Start the client, and allow the test to override the initial RPC config.
-        rpc, metadata = cls.client_init_config(
-            rpc="UnaryCall,EmptyCall", metadata=""
-        )
-        cls.test_client = cls.test_client_runner.run(
-            server_target=f"xds:///{cls.hostname()}",
-            rpc=rpc,
-            metadata=metadata,
-            qps=QPS.value,
-            print_response=True,
-        )
+        # Normally we don't want to make external calls in setUpClass.
+        try:
+            if not cls.started_test_cases:
+                # Create the GCP resource once before the first test start
+                GcpResourceManager().setup(cls.test_case_classes)
+            cls.started_test_cases.add(cls.__name__)
+
+            # Create the test case's own client runner with it's own namespace,
+            # enables concurrent running with other test cases.
+            cls.test_client_runner = (
+                GcpResourceManager().create_test_client_runner()
+            )
+            # Start the client, and allow the test to override the initial
+            # RPC config.
+            rpc, metadata = cls.client_init_config(
+                rpc="UnaryCall,EmptyCall", metadata=""
+            )
+            cls.test_client = cls.test_client_runner.run(
+                server_target=f"xds:///{cls.hostname()}",
+                rpc=rpc,
+                metadata=metadata,
+                qps=QPS.value,
+                print_response=True,
+            )
+        except Exception as error:  # noqa pylint: disable=broad-except
+            cls._log_class_hook_failure(error)
+            raise
 
     @classmethod
     def cleanupAfterTests(cls):

requirements.lock

@@ -4,10 +4,10 @@ absl-py==0.15.0
 google-api-python-client==1.12.11
 google-cloud-secret-manager==2.15.1
 google-cloud-monitoring==2.18.0
-grpcio==1.57.0
-grpcio-health-checking==1.57.0
-grpcio-tools==1.57.0
-grpcio-channelz==1.57.0
+grpcio==1.60.1
+grpcio-health-checking==1.60.1
+grpcio-tools==1.60.1
+grpcio-channelz==1.60.1
 kubernetes==27.2.0
 six==1.16.0
 tenacity==6.3.1
@@ -25,7 +25,7 @@ google-auth==2.22.0
 google-auth-httplib2==0.1.0
 googleapis-common-protos==1.60.0
 grpc-google-iam-v1==0.12.6
-grpcio-status==1.57.0
+grpcio-status==1.60.1
 httplib2==0.22.0
 idna==3.4
 MarkupSafe==2.1.3

requirements.txt

@@ -4,10 +4,10 @@ absl-py~=0.11
 google-api-python-client~=1.12
 google-cloud-secret-manager~=2.1
 google-cloud-monitoring~=2.18
-grpcio~=1.57
-grpcio-health-checking~=1.57
-grpcio-tools~=1.57
-grpcio-channelz~=1.57
+grpcio~=1.60
+grpcio-health-checking~=1.60
+grpcio-tools~=1.60
+grpcio-channelz~=1.60
 kubernetes~=27.2
 six~=1.13
 tenacity~=6.2

tests/bootstrap_generator_test.py

@@ -46,6 +46,14 @@
 # after the release is published.
 def bootstrap_version_testcases() -> List:
     return (
+        dict(
+            version="v0.16.0",
+            image="gcr.io/grpc-testing/td-grpc-bootstrap:2bf1b5ed00f852ffea8d24759c6fa673acc9ef10",
+        ),
+        dict(
+            version="v0.15.0",
+            image="gcr.io/grpc-testing/td-grpc-bootstrap:7d8d90477792e2e1bfe3a3da20b3dc9ef01d326c",
+        ),
         dict(
             version="v0.14.0",
             image="gcr.io/grpc-testing/td-grpc-bootstrap:d6baaf7b0e0c63054ac4d9bedc09021ff261d599",

tests/fake_test.py

@@ -80,5 +80,24 @@ def test_even(self):
                     self.fail(f"Integer {num} is odd")
 
 
+class FakeSetupClassTest(xds_k8s_testcase.XdsKubernetesBaseTestCase):
+    """A fake class to debug BaseTestCase logs produced by setupClassError.
+
+    See FakeTest for notes on provisioning.
+    """
+
+    @classmethod
+    def setUpClass(cls):
+        try:
+            # Making bad external calls that end up raising
+            raise OSError("Network bad!")
+        except Exception as error:  # noqa pylint: disable=broad-except
+            cls._log_class_hook_failure(error)
+            raise
+
+    def test_should_never_run(self):
+        self.fail("IF YOU SEE ME, SOMETHING IS WRONG!")
+
+
 if __name__ == "__main__":
     absltest.main()