Merge pull request emissary-ingress#4308 from emissary-ingress/lukesh…

…u/test-cleanup

Improve KAT test reliability

builder/builder.mk

@@ -320,7 +320,7 @@ pytest-kat-local: push-pytest-images
 pytest-kat-envoy3: push-pytest-images # doing this all at once is too much for CI...
 	$(MAKE) pytest KAT_RUN_MODE=envoy PYTEST_ARGS="$$PYTEST_ARGS python/tests/kat"
 # ... so we have a separate rule to run things split up
-build-aux/.pytest-kat.txt.stamp: $(OSS_HOME)/venv push-pytest-images FORCE
+build-aux/.pytest-kat.txt.stamp: $(OSS_HOME)/venv push-pytest-images $(tools/kubectl) FORCE
 	. venv/bin/activate && set -o pipefail && pytest --collect-only python/tests/kat 2>&1 | sed -En 's/.*<Function (.*)>/\1/p' | cut -d. -f1 | sort -u > $@
 build-aux/pytest-kat.txt: build-aux/%: build-aux/.%.stamp $(tools/copy-ifchanged)
 	$(tools/copy-ifchanged) $< $@

python/kat/harness.py

@@ -13,22 +13,30 @@
 from collections import OrderedDict
 from functools import singledispatch
 from hashlib import sha256
-from typing import Any, Callable, Dict, List, Optional, Sequence, Tuple, Type, Union, cast
+from typing import (
+    Any,
+    Callable,
+    Dict,
+    List,
+    NamedTuple,
+    Optional,
+    Sequence,
+    Tuple,
+    Type,
+    Union,
+    cast,
+)
 
 import pytest
 import yaml as pyyaml
 from packaging import version
+from yaml.parser import ParserError as YAMLParseError
 from yaml.scanner import ScannerError as YAMLScanError
 
 import tests.integration.manifests as integration_manifests
 from ambassador.utils import parse_bool
 from tests.kubeutils import apply_kube_artifacts
-from tests.manifests import (
-    cleartext_host_manifest,
-    default_listener_manifest,
-    httpbin_manifests,
-    websocket_echo_server_manifests,
-)
+from tests.manifests import cleartext_host_manifest, default_listener_manifest
 
 from .parser import SequenceView, Tag, dump, load
 from .utils import ShellCommand
@@ -290,26 +298,17 @@ def variants(cls, *args, **kwargs) -> Tuple[Any]:
     return tuple(a for n in get_nodes(cls) for a in n.variants(*args, **kwargs))  # type: ignore
 
 
-class Name(str):
-    namespace: Optional[str]
-
-    def __new__(cls, value, namespace: Optional[str] = None):
-        s = super().__new__(cls, value)
-        s.namespace = namespace
-        return s
+class Name(NamedTuple):
+    name: str
+    namespace: str
 
     @property
-    def k8s(self):
-        return self.replace(".", "-").lower()
+    def k8s(self) -> str:
+        return self.name.replace(".", "-").lower()
 
     @property
-    def fqdn(self):
-        r = self.k8s
-
-        if self.namespace and (self.namespace != "default"):
-            r += "." + self.namespace
-
-        return r
+    def fqdn(self) -> str:
+        return ".".join([self.k8s, self.namespace, "svc", "cluster", "local"])
 
 
 class NodeLocal(threading.local):
@@ -339,30 +338,30 @@ class Node(ABC):
 
     parent: Optional["Node"]
     children: List["Node"]
-    name: Name
+    name: str
     ambassador_id: str
-    namespace: str = None  # type: ignore
+    namespace: str
     is_ambassador = False
     local_result: Optional[Dict[str, str]] = None
     xfail: Optional[str]
 
-    def __init__(self, *args, **kwargs) -> None:
+    def __init__(
+        self,
+        *args,
+        name: Optional[str] = None,
+        namespace: Optional[str] = None,
+        _clone: Optional["Node"] = None,
+        **kwargs,
+    ) -> None:
         # If self.skip is set to true, this node is skipped
         self.skip_node = False
         self.xfail: Optional[str] = None
 
-        name = kwargs.pop("name", None)
-
-        if "namespace" in kwargs:
-            self.namespace = kwargs.pop("namespace", None)
-
-        _clone: Node = kwargs.pop("_clone", None)
-
         if _clone:
             args = _clone._args  # type: ignore
             kwargs = _clone._kwargs  # type: ignore
             if name:
-                name = Name("-".join((_clone.name, name)))
+                name = "-".join((_clone.name, name))
             else:
                 name = _clone.name
             self._args = _clone._args  # type: ignore
@@ -371,14 +370,18 @@ def __init__(self, *args, **kwargs) -> None:
             self._args = args
             self._kwargs = kwargs
             if name:
-                name = Name("-".join((self.__class__.__name__, name)))
+                name = "-".join((self.__class__.__name__, name))
             else:
-                name = Name(self.__class__.__name__)
+                name = self.__class__.__name__
 
         saved = _local.current
         self.parent = _local.current
 
-        if not self.namespace:
+        if namespace:
+            self.namespace = namespace
+        if not getattr(self, "namespace", ""):
+            # We do the above `getattr` instead of just an `else` because subclasses might set a
+            # default namespace; so `self.namespace` might already be set before calling __init__().
             if self.parent and self.parent.namespace:
                 # We have no namespace assigned, but our parent does have a namespace
                 # defined. Copy the namespace down from our parent.
@@ -396,7 +399,9 @@ def __init__(self, *args, **kwargs) -> None:
         finally:
             _local.current = saved
 
-        self.name = Name(self.format(name or self.__class__.__name__))
+        # This has to come after the above to init(), because the format-string might reference
+        # things that get set by init().
+        self.name = self.format(name)
 
         names = {}  # type: ignore
         for c in self.children:
@@ -564,14 +569,10 @@ def variants(cls):
 
     @property
     def path(self) -> Name:
-        return self.relpath(None)
-
-    def relpath(self, ancestor):
-        if self.parent is ancestor:
-            return Name(self.name, namespace=self.namespace)
+        if self.parent is None:
+            return Name(self.name, self.namespace)
         else:
-            assert self.parent
-            return Name(self.parent.relpath(ancestor) + "." + self.name, namespace=self.namespace)
+            return Name(self.parent.path.name + "." + self.name, self.namespace)
 
     @property
     def traversal(self):
@@ -597,15 +598,9 @@ def depth(self):
     def format(self, st, **kwargs):
         return integration_manifests.format(st, self=self, **kwargs)
 
-    def get_fqdn(self, name: str) -> str:
-        if self.namespace and (self.namespace != "default"):
-            return f"{name}.{self.namespace}"
-        else:
-            return name
-
     @functools.lru_cache()
     def matches(self, pattern):
-        if fnmatch.fnmatch(self.path, "*%s*" % pattern):
+        if fnmatch.fnmatch(self.path.name, "*%s*" % pattern):
             return True
         for c in self.children:
             if c.matches(pattern):
@@ -696,7 +691,7 @@ def handle_local_result(self) -> bool:
     @property
     def ambassador_id(self):
         if self.parent is None:
-            return self.name.k8s
+            return self.path.k8s
         else:
             return self.parent.ambassador_id
 
@@ -781,7 +776,7 @@ def __init__(
     def as_json(self):
         assert self.parent
         result = {
-            "test": self.parent.path,
+            "test": self.parent.path.name,
             "id": id(self),
             "url": self.url,
             "insecure": self.insecure,
@@ -1180,7 +1175,7 @@ def __init__(self, *classes, scope=None):
         self.roots = tuple(v for c in classes for v in variants(c))
         self.nodes = [n for r in self.roots for n in r.traversal if not n.skip_node]
         self.tests = [n for n in self.nodes if isinstance(n, Test)]
-        self.ids = [t.path for t in self.tests]
+        self.ids = [t.path.name for t in self.tests]
         self.done = False
         self.skip_nonlocal_tests = False
         self.ids_to_strip: Dict[str, bool] = {}
@@ -1430,7 +1425,7 @@ def _setup_k8s(self, selected):
                     try:
                         for o in load(n.path, cfg, Tag.MAPPING):
                             parent_config.merge(o)
-                    except YAMLScanError as e:
+                    except (YAMLScanError, YAMLParseError) as e:
                         raise Exception("Parse Error: %s, input text:\n%s" % (e, cfg))
                 else:
                     target = cfg[0]
@@ -1444,7 +1439,7 @@ def _setup_k8s(self, selected):
                                     obj["ambassador_id"] = [n.ambassador_id]
 
                         configs[n].append((target, yaml_view))
-                    except YAMLScanError as e:
+                    except (YAMLScanError, YAMLParseError) as e:
                         raise Exception("Parse Error: %s, input text:\n%s" % (e, cfg[1]))
 
         for tgt_cfgs in configs.values():
@@ -1470,7 +1465,7 @@ def _setup_k8s(self, selected):
                             continue
                         break
                 else:
-                    assert False, "no service found for target: %s" % target.path
+                    assert False, "no service found for target: %s" % target.path.name
 
         yaml = ""
 
@@ -1784,14 +1779,6 @@ def _setup_k8s(self, selected):
                 raise RuntimeError("Could not apply manifests")
             self.applied_manifests = True
 
-        # Finally, install httpbin and the websocket-echo-server.
-        print(
-            f"applying http_manifests + websocket_echo_server_manifests to namespaces: {namespaces}"
-        )
-        for namespace in namespaces:
-            apply_kube_artifacts(namespace, httpbin_manifests)
-            apply_kube_artifacts(namespace, websocket_echo_server_manifests)
-
         for n in self.nodes:
             if n in selected and not n.xfail:
                 action = getattr(n, "post_manifest", None)

python/tests/integration/manifests/httpbin_backend.yaml

@@ -0,0 +1,34 @@
+---
+apiVersion: v1
+kind: Service
+metadata:
+  name: {self.path.k8s}
+spec:
+  selector:
+    backend: {self.path.k8s}
+  ports:
+  - name: http
+    protocol: TCP
+    port: 80
+    targetPort: http
+---
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: {self.path.k8s}
+spec:
+  replicas: 1
+  selector:
+    matchLabels:
+      backend: {self.path.k8s}
+  template:
+    metadata:
+      labels:
+        backend: {self.path.k8s}
+    spec:
+      containers:
+      - name: httpbin
+        image: docker.io/kennethreitz/httpbin
+        ports:
+        - name: http
+          containerPort: 80

python/tests/integration/manifests/statsd_backend.yaml

@@ -0,0 +1,39 @@
+---
+apiVersion: v1
+kind: Service
+metadata:
+  name: {self.path.k8s}
+spec:
+  selector:
+    backend: {self.path.k8s}
+  ports:
+  - protocol: UDP
+    port: 8125
+    targetPort: 8125
+    name: statsd-metrics
+  - protocol: TCP
+    port: 80
+    targetPort: 3000
+    name: statsd-http
+---
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: {self.path.k8s}
+spec:
+  replicas: 1
+  selector:
+    matchLabels:
+      backend: {self.path.k8s}
+  template:
+    metadata:
+      labels:
+        backend: {self.path.k8s}
+    spec:
+      containers:
+      - name: statsd
+        image: {images[test-stats]}
+        env:
+        - name: STATSD_TEST_CLUSTER
+          value: {self.target_cluster}
+      restartPolicy: Always

python/tests/integration/manifests/websocket_echo_backend.yaml

@@ -0,0 +1,30 @@
+---
+apiVersion: v1
+kind: Service
+metadata:
+  name: {self.path.k8s}
+spec:
+  selector:
+    backend: {self.path.k8s}
+  ports:
+  - protocol: TCP
+    port: 80
+    targetPort: 8080
+---
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: {self.path.k8s}
+spec:
+  replicas: 1
+  selector:
+    matchLabels:
+      backend: {self.path.k8s}
+  template:
+    metadata:
+      labels:
+        backend: {self.path.k8s}
+    spec:
+      containers:
+      - name: websocket-echo-server
+        image: docker.io/johnesmet/go-websocket-echo-server:latest