Merge pull request #574 from element-hq/bbz/more-hostAliases

More hostAliases tweaking

Author: benbz

charts/matrix-stack/source/matrix-rtc.json

@@ -165,9 +165,6 @@
         "containersSecurityContext": {
           "$ref": "file://common/containersSecurityContext.json"
         },
-        "hostAliases": {
-          "$ref": "file://common/hostAliases.json"
-        },
         "nodeSelector": {
           "$ref": "file://common/nodeSelector.json"
         },

charts/matrix-stack/source/matrix-rtc.yaml.j2

@@ -66,7 +66,6 @@ sfu:
 {{- sub_schema_values.labels() | indent(2) }}
 {{- sub_schema_values.workloadAnnotations() | indent(2) }}
 {{- sub_schema_values.containersSecurityContext() | indent(2) }}
-{{- sub_schema_values.hostAliases() | indent(2) }}
 {{- sub_schema_values.nodeSelector() | indent(2) }}
 {{- sub_schema_values.podSecurityContext(user_id='10030', group_id='10030') | indent(2) }}
 {{- sub_schema_values.resources(requests_memory='150Mi', requests_cpu='100m', limits_memory='4Gi') | indent(2) }}

charts/matrix-stack/templates/synapse/_synapse_pod.tpl

@@ -48,10 +48,12 @@ template:
                                           "deployment" false
                                           "usesMatrixTools" true)
                                     ) | nindent 4 }}
+{{- if not $isHook }}
 {{- with .hostAliases }}
     hostAliases:
       {{- tpl (toYaml . | nindent 6) $root }}
 {{- end }}
+{{- end }}
 {{- /*
 We have an init container to render & merge the config for several reasons:
 * We have external, user-supplied Secrets and don't want to use `lookup` as that doesn't work with things like ArgoCD

charts/matrix-stack/values.schema.json

@@ -1805,24 +1805,6 @@
               "type": "object",
               "additionalProperties": false
             },
-            "hostAliases": {
-              "type": "array",
-              "items": {
-                "type": "object",
-                "properties": {
-                  "ip": {
-                    "type": "string"
-                  },
-                  "hostnames": {
-                    "type": "array",
-                    "items": {
-                      "type": "string"
-                    }
-                  }
-                },
-                "additionalProperties": false
-              }
-            },
             "nodeSelector": {
               "type": "object",
               "additionalProperties": {

charts/matrix-stack/values.yaml

@@ -742,18 +742,6 @@ matrixRTC:
       ## localhostProfile must only be set set if type Localhost. It indicates the path of the pre-configured profile on the node, relative to the kubelet's configured Seccomp profile location (configured with the --root-dir flag).
       # seccompProfile:
       #  type: RuntimeDefault
-    ## The list of hosts aliases to configure on the pod spec.
-    ## It should be avoid as much as possible to use this feature.
-    ## Please prefer using an DNS entry to resolve your hostnames.
-    ## This can be used as a workaround when entries cannot be resolved using DNS, for example for our automated testings.
-    ## e.g.
-    ## hostAliases:
-    ## - ip: 192.0.2.1  # An IP resolution to add to /etc/hosts
-    ##   # A list of hostnames to be associated with the above IP
-    ##   hostnames:
-    ##   - ess.localhost
-    ##   - synapse.ess.localhost
-    hostAliases: []
     ## NodeSelector is a selector which must be true for the pod to fit on a node. Selector which must match a node's labels for the pod to be scheduled on that node. More info: https://kubernetes.io/docs/concepts/configuration/assign-pod-node/
     # nodeSelector: {}
     ## A subset of PodSecurityContext. PodSecurityContext holds pod-level security attributes and common container settings

newsfragments/574.changed.1.md

@@ -0,0 +1 @@
+Don't set `hostAliases` on the Synapse config job as it just operates on the config files.

newsfragments/574.changed.md

@@ -0,0 +1 @@
+Removed `hostAliases` support from Matrix RTC SFU as it doesn't make outbound requests.

newsfragments/574.internal.md

@@ -0,0 +1 @@
+CI: test that `hostAliases` are correctly set for all workloads that make outbound requests.

tests/manifests/__init__.py

@@ -204,6 +204,9 @@ def deployable_details_for_container(self, container_name: str) -> "DeployableDe
 
 @dataclass(unsafe_hash=True)
 class SidecarDetails(DeployableDetails):
+    # We have to be a workload as we're a sidecar
+    has_workloads: bool = True
+
     parent: DeployableDetails = field(default=None, init=False, hash=False)  # type: ignore[assignment]
 
     def __post_init__(self):
@@ -223,12 +226,19 @@ def __post_init__(self):
         # Not possible, will come from the parent components
         self.has_topology_spread_constraints = False
 
-        # We have to be a workload as we're a sidecar
-        self.has_workloads = True
-
         # We dont support replicas
         self.has_replicas = False
 
+    def create_ownership_link(self, parent: "ComponentDetails | SubComponentDetails"):
+        self.parent = parent
+
+        # If the sidecar makes outbound requests, the parent will need hostAlias support
+        # even if it itself doesn't make outbound requests
+        if self.makes_outbound_requests:
+            self.parent.makes_outbound_requests = True
+            # As we won't have the properties ourselves
+            self.makes_outbound_requests = False
+
     def owns_manifest_named(self, manifest_name: str) -> bool:
         # Sidecars shouldn't own anything that their parent could possibly own
         if self.parent.owns_manifest_named(manifest_name):
@@ -248,7 +258,7 @@ def __post_init__(self):
         super().__post_init__()
 
         for sidecar in self.sidecars:
-            sidecar.parent = self
+            sidecar.create_ownership_link(self)
 
     def owns_manifest_named(self, manifest_name: str) -> bool:
         return manifest_name.startswith(self.name)
@@ -285,7 +295,7 @@ def __post_init__(
         super().__post_init__()
 
         for sidecar in self.sidecars:
-            sidecar.parent = self
+            sidecar.create_ownership_link(self)
 
         if not self.value_file_prefix:
             self.value_file_prefix = self.name
@@ -461,6 +471,7 @@ def make_synapse_worker_sub_component(worker_name: str, worker_type: str) -> Sub
                 has_topology_spread_constraints=False,
                 has_ingress=False,
                 has_replicas=False,
+                makes_outbound_requests=False,
             ),
         ),
         shared_component_names=("init-secrets",),
@@ -546,7 +557,6 @@ def make_synapse_worker_sub_component(worker_name: str, worker_type: str) -> Sub
                 values_file_path=ValuesFilePath.read_write("synapse", "checkConfigHook"),
                 values_file_path_overrides={
                     PropertyType.Env: ValuesFilePath.read_elsewhere("synapse", "extraEnv"),
-                    PropertyType.HostAliases: ValuesFilePath.read_elsewhere("synapse", "hostAliases"),
                     PropertyType.Image: ValuesFilePath.read_elsewhere("synapse", "image"),
                     # Job so no livenessProbe
                     PropertyType.LivenessProbe: ValuesFilePath.not_supported(),
@@ -565,6 +575,7 @@ def make_synapse_worker_sub_component(worker_name: str, worker_type: str) -> Sub
                 has_ingress=False,
                 has_service_monitor=False,
                 has_replicas=False,
+                makes_outbound_requests=False,
             ),
         ),
         shared_component_names=("deployment-markers", "init-secrets", "haproxy", "postgres"),