Merge pull request #3958 from Yelp/jfong/PAASTA-18194-all-namespaces-…

…option

PAASTA-18194: Stop querying all managed namespaces by default for paasta status

paasta_tools/api/api_docs/oapi.yaml

@@ -1480,6 +1480,12 @@ paths:
         required: false
         schema:
           type: boolean
+      - description: Search all namespaces for running copies
+        in: query
+        name: all_namespaces
+        required: false
+        schema:
+          type: boolean
       responses:
         "200":
           content:

paasta_tools/api/api_docs/swagger.json

@@ -540,6 +540,13 @@
                         "name": "new",
                         "required": false,
                         "type": "boolean"
+                    },
+                    {
+                        "in": "query",
+                        "description": "Search all namespaces for running copies",
+                        "name": "all_namespaces",
+                        "required": false,
+                        "type": "boolean"
                     }
                 ]
             }

paasta_tools/api/views/instance.py

@@ -136,6 +136,7 @@ def instance_status(request):
     instance = request.swagger_data.get("instance")
     verbose = request.swagger_data.get("verbose") or 0
     use_new = request.swagger_data.get("new") or False
+    all_namespaces = request.swagger_data.get("all_namespaces") or False
     include_envoy = request.swagger_data.get("include_envoy")
     if include_envoy is None:
         include_envoy = True
@@ -199,6 +200,7 @@ def instance_status(request):
                     use_new=use_new,
                     instance_type=instance_type,
                     settings=settings,
+                    all_namespaces=all_namespaces,
                 )
             )
         elif instance_type == "tron":

paasta_tools/cli/cmds/status.py

@@ -170,6 +170,14 @@ def add_subparser(
         default=DEFAULT_SOA_DIR,
         help="define a different soa config directory",
     )
+    status_parser.add_argument(
+        "-A",
+        "--all-namespaces",
+        dest="all_namespaces",
+        action="store_true",
+        default=False,
+        help="Search all PaaSTA-managed namespaces for possible running versions (Will search only your currently-configured namespace by default). Useful if you are moving your instance(s) to a new namespace",
+    )
 
     version = status_parser.add_mutually_exclusive_group()
 
@@ -292,6 +300,7 @@ def paasta_status_on_api_endpoint(
     verbose: int,
     new: bool = False,
     is_eks: bool = False,
+    all_namespaces: bool = False,
 ) -> int:
     output = [
         "",
@@ -310,6 +319,7 @@ def paasta_status_on_api_endpoint(
             instance=instance,
             verbose=verbose,
             new=new,
+            all_namespaces=all_namespaces,
         )
     except client.api_error as exc:
         output.append(PaastaColors.red(exc.reason))
@@ -2140,6 +2150,7 @@ def report_status_for_cluster(
     lock: Lock,
     verbose: int = 0,
     new: bool = False,
+    all_namespaces: bool = False,
 ) -> Tuple[int, Sequence[str]]:
     """With a given service and cluster, prints the status of the instances
     in that cluster"""
@@ -2193,6 +2204,7 @@ def report_status_for_cluster(
                 lock=lock,
                 verbose=verbose,
                 new=new,
+                all_namespaces=all_namespaces,
                 is_eks=(instance_config_class in EKS_DEPLOYMENT_CONFIGS),
             )
         )
@@ -2416,6 +2428,7 @@ def paasta_status(args) -> int:
                             lock=lock,
                             verbose=args.verbose,
                             new=new,
+                            all_namespaces=args.all_namespaces,
                         ),
                     )
                 )

paasta_tools/instance/kubernetes.py

@@ -606,6 +606,7 @@ async def kubernetes_status_v2(
     include_envoy: bool,
     instance_type: str,
     settings: Any,
+    all_namespaces: bool = False,
 ) -> Dict[str, Any]:
     status: Dict[str, Any] = {}
     config_loader = LONG_RUNNING_INSTANCE_TYPE_HANDLERS[instance_type].loader
@@ -620,9 +621,12 @@ async def kubernetes_status_v2(
     if kube_client is None:
         return status
 
-    relevant_namespaces = await a_sync.to_async(find_all_relevant_namespaces)(
-        service, instance, kube_client, job_config
-    )
+    if all_namespaces:
+        relevant_namespaces = await a_sync.to_async(find_all_relevant_namespaces)(
+            service, instance, kube_client, job_config
+        )
+    else:
+        relevant_namespaces = {job_config.get_kubernetes_namespace()}
 
     tasks: List["asyncio.Future[Dict[str, Any]]"] = []
 
@@ -1240,6 +1244,7 @@ def instance_status(
     use_new: bool,
     instance_type: str,
     settings: Any,
+    all_namespaces: bool,
 ) -> Mapping[str, Any]:
     status = {}
 
@@ -1267,6 +1272,7 @@ def instance_status(
                 verbose=verbose,
                 include_envoy=include_envoy,
                 settings=settings,
+                all_namespaces=all_namespaces,
             )
         else:
             status["kubernetes"] = kubernetes_status(

paasta_tools/paastaapi/api/service_api.py

@@ -1322,6 +1322,7 @@ def __status_instance(
                 include_envoy (bool): Include Envoy information. [optional]
                 include_mesos (bool): Include Mesos information. [optional]
                 new (bool): Use new version of paasta status for services. [optional]
+                all_namespaces (bool): Search all namespaces for running copies. [optional]
                 _return_http_data_only (bool): response data without head status
                     code and headers. Default is True.
                 _preload_content (bool): if False, the urllib3.HTTPResponse object
@@ -1389,6 +1390,7 @@ def __status_instance(
                     'include_envoy',
                     'include_mesos',
                     'new',
+                    'all_namespaces',
                 ],
                 'required': [
                     'service',
@@ -1419,6 +1421,8 @@ def __status_instance(
                         (bool,),
                     'new':
                         (bool,),
+                    'all_namespaces':
+                        (bool,),
                 },
                 'attribute_map': {
                     'service': 'service',
@@ -1427,6 +1431,7 @@ def __status_instance(
                     'include_envoy': 'include_envoy',
                     'include_mesos': 'include_mesos',
                     'new': 'new',
+                    'all_namespaces': 'all_namespaces',
                 },
                 'location_map': {
                     'service': 'path',
@@ -1435,6 +1440,7 @@ def __status_instance(
                     'include_envoy': 'query',
                     'include_mesos': 'query',
                     'new': 'query',
+                    'all_namespaces': 'query',
                 },
                 'collection_format_map': {
                 }

tests/cli/test_cmds_status.py

@@ -340,6 +340,7 @@ def test_status_calls_sergeants(
     args.registration = None
     args.service_instance = None
     args.new = False
+    args.all_namespaces = False
     return_value = paasta_status(args)
 
     assert return_value == 1776
@@ -355,6 +356,7 @@ def test_status_calls_sergeants(
         lock=mock.ANY,
         verbose=False,
         new=False,
+        all_namespaces=False,
     )
 
 
@@ -390,6 +392,7 @@ def __init__(
         service_instance=None,
         new=False,
         old=False,
+        all_namespaces=False,
     ):
         self.service = service
         self.soa_dir = soa_dir
@@ -402,6 +405,7 @@ def __init__(
         self.service_instance = service_instance
         self.new = new
         self.old = old
+        self.all_namespaces = all_namespaces
 
 
 @patch("paasta_tools.cli.cmds.status.get_instance_configs_for_service", autospec=True)
@@ -890,6 +894,7 @@ def test_status_with_registration(
         verbose=False,
         service_instance=None,
         new=False,
+        all_namespaces=True,  # Bonus all_namespaces test
     )
     return_value = paasta_status(args)
 
@@ -908,6 +913,7 @@ def test_status_with_registration(
         lock=mock.ANY,
         verbose=args.verbose,
         new=False,
+        all_namespaces=True,
     )
 
 

tests/instance/test_kubernetes.py

@@ -120,6 +120,7 @@ def instance_status_kwargs():
         include_envoy=False,
         settings=mock.Mock(),
         use_new=False,
+        all_namespaces=False,
     )
 
 
@@ -603,6 +604,78 @@ def test_pod_timeout(
         assert status
         assert "Could not fetch instance data" in status["error_message"]
 
+    def test_all_namespaces(
+        self,
+        mock_replicasets_for_service_instance,
+        mock_LONG_RUNNING_INSTANCE_TYPE_HANDLERS,
+        mock_load_service_namespace_config,
+        mock_pods_for_service_instance,
+        mock_mesh_status,
+        mock_get_pod_event_messages,
+        mock_pod,
+        mock_find_all_relevant_namespaces,
+    ):
+        mock_find_all_relevant_namespaces.return_value = ["paasta"]
+        mock_job_config = mock.Mock(
+            get_persistent_volumes=mock.Mock(return_value=[]),
+            get_kubernetes_namespace=mock.Mock(return_value="paastasvc-service"),
+        )
+        mock_LONG_RUNNING_INSTANCE_TYPE_HANDLERS[
+            "kubernetes"
+        ].loader.return_value = mock_job_config
+        mock_replicasets_for_service_instance.return_value = [
+            Struct(
+                spec=Struct(replicas=1),
+                metadata=Struct(
+                    name="replicaset_1",
+                    creation_timestamp=datetime.datetime(2021, 3, 5),
+                    deletion_timestamp=None,
+                    labels={
+                        "paasta.yelp.com/git_sha": "aaa000",
+                        "paasta.yelp.com/config_sha": "config000",
+                    },
+                ),
+            ),
+        ]
+        mock_load_service_namespace_config.return_value = {}
+        mock_job_config.get_registrations.return_value = ["service.instance"]
+        mock_get_pod_event_messages.return_value = []
+
+        with asynctest.patch(
+            "paasta_tools.instance.kubernetes.get_versions_for_replicasets",
+            autospec=True,
+        ) as mock_get_versions_for_replicasets:
+            pik.kubernetes_status_v2(
+                service="service",
+                instance="instance",
+                verbose=0,
+                include_envoy=False,
+                instance_type="kubernetes",
+                settings=mock.Mock(),
+            )
+
+        # We are only testing that we
+        assert not mock_find_all_relevant_namespaces.called
+        _, _, get_rs_kwargs = mock_get_versions_for_replicasets.mock_calls[0]
+        assert get_rs_kwargs["namespaces"] == {"paastasvc-service"}
+
+        with asynctest.patch(
+            "paasta_tools.instance.kubernetes.get_versions_for_replicasets",
+            autospec=True,
+        ) as mock_get_versions_for_replicasets:
+            pik.kubernetes_status_v2(
+                service="service",
+                instance="instance",
+                verbose=0,
+                include_envoy=False,
+                instance_type="kubernetes",
+                settings=mock.Mock(),
+                all_namespaces=True,
+            )
+        assert mock_find_all_relevant_namespaces.called
+        _, _, get_rs_kwargs = mock_get_versions_for_replicasets.mock_calls[0]
+        assert get_rs_kwargs["namespaces"] == ["paasta"]
+
 
 @mock.patch("paasta_tools.kubernetes_tools.get_kubernetes_app_by_name", autospec=True)
 def test_job_status_include_replicaset_non_verbose(mock_get_kubernetes_app_by_name):

tox.ini

@@ -24,7 +24,7 @@ commands =
 
 [testenv:dev-api]
 envdir = .tox/py38-linux/
-passenv = PAASTA_TEST_CLUSTER KUBECONFIG PAASTA_SYSTEM_CONFIG_DIR
+passenv = PAASTA_TEST_CLUSTER KUBECONFIG PAASTA_SYSTEM_CONFIG_DIR KUBECONTEXT AWS_PROFILE
 deps =
     --only-binary=grpcio
     --requirement={toxinidir}/requirements.txt