Make changes as suggested in review

runtime/run.sh

@@ -42,14 +42,12 @@ if [[ "${DEBUG_SKIP_LEADER}" == "true" ]]; then
 else
   echo "Waiting for leader election..."
   LEADER=false
-  while [[ "${LEADER}" == "false" ]]; do
+  while :; do
     CURRENT_LEADER=$(curl http://127.0.0.1:4040 -s -m 2 | jq -r ".name")
     if [[ "${CURRENT_LEADER}" == "$(hostname)" ]]; then
       echo "I am the leader."
-      LEADER=true
-    else
-      sleep 1
-    fi
+      break
+    sleep 1
   done
 fi
 
@@ -58,13 +56,13 @@ tailscaled ${TAILSCALED_ARGS} &
 PID=$!
 
 UP_ARGS="--accept-dns=${TS_ACCEPT_DNS}"
-if [[ ! -z "${TS_AUTH_KEY}" ]]; then
+if [[ -n "${TS_AUTH_KEY}" ]]; then
   UP_ARGS="--authkey=${TS_AUTH_KEY} ${UP_ARGS}"
 fi
-if [[ ! -z "${TS_EXTRA_ARGS}" ]]; then
+if [[ -n "${TS_EXTRA_ARGS}" ]]; then
   UP_ARGS="${UP_ARGS} ${TS_EXTRA_ARGS:-}"
 fi
-if [[ ! -z "${TS_HOSTNAME}" ]]; then
+if [[ -n "${TS_HOSTNAME}" ]]; then
   echo "Overriding system hostname using TS_HOSTNAME: ${TS_HOSTNAME}"
   UP_ARGS="--hostname=${TS_HOSTNAME} ${UP_ARGS}"
 fi
@@ -82,7 +80,7 @@ echo "Trying to get the service ClusterIP..."
 SVC_IP_RETRIEVED=false
 while [[ "${SVC_IP_RETRIEVED}" == "false" ]]; do
   SVC_IP=$(getent hosts ${SVC_NAME}.${SVC_NAMESPACE}.svc | cut -d" " -f1)
-  if [[ ! -z "${SVC_IP}" ]]; then
+  if [[ -n "${SVC_IP}" ]]; then
     SVC_IP_RETRIEVED=true
   else
     sleep 1
@@ -100,4 +98,4 @@ echo "Updating secret with Tailscale IP"
 # patch secret with the tailscale ipv4 address
 kubectl patch secret "${TS_KUBE_SECRET}" --namespace "${PROXY_NAMESPACE}" --type=json --patch="[{\"op\":\"replace\",\"path\":\"/data/ts-ip\",\"value\":\"${TS_IP_B64}\"}]"
 
-wait ${PID}
+wait ${PID}

src/tailscale_svc_lb_controller/config.py

@@ -20,7 +20,7 @@
 # A semi-colon seperated string containing the names of any secrets that should be used
 # when pulling images. Secret must already exist and be present in the TS_PROXY_NAMESPACE
 IMAGE_PULL_SECRETS = env.get("IMAGE_PULL_SECRETS", "")
-if not re.match(r"^([a-z]|-|\d|;)*$", IMAGE_PULL_SECRETS):
+if not re.match(r"^[a-z\d;-]*$", IMAGE_PULL_SECRETS):
     logging.error("IMAGE_PULL_SECRETS invalid. Should be a semi-colon seperated list of"
                   "secret names.")
     sys.exit(1)
@@ -37,8 +37,12 @@
 # If TS_PROXY_DEPLOYMENT_TYPE is 'Deployment', this dictates the number of replicas. No effect otherwise.
 try:
     TS_PROXY_REPLICA_COUNT = int(env.get("TS_PROXY_REPLICA_COUNT", "2"))
-except Exception:
-    logging.error("TS_PROXY_REPLICA_COUNT value invalid. Should be an integer above 0.")
+except ValueError:
+    logging.error("TS_PROXY_REPLICA_COUNT value invalid. Expected integer.")
+    sys.exit(1)
+
+if TS_PROXY_REPLICA_COUNT <= 0:
+    logging.error(f"TS_PROXY_REPLICA_COUNT value invalid. Needs to be an integer greater than 0. Received ${TS_PROXY_REPLICA_COUNT}")
     sys.exit(1)
 
 # Tailscale Proxy Runtime Container Image

src/tailscale_svc_lb_controller/helpers.py

@@ -23,21 +23,17 @@ def update_service_status(namespace, service, ip):
     Update the status of the service to reflect the service Tailscale IP.
     """
 
-    try:
-        # Get the service
-        k8s = kubernetes.client.CoreV1Api()
-        service_object = k8s.read_namespaced_service(name=service, namespace=namespace)
-
-        # Update the status
-        service_object.status.load_balancer.ingress = [
-            kubernetes.client.V1LoadBalancerIngress(ip=ip)
-        ]
+    # Get the service
+    k8s = kubernetes.client.CoreV1Api()
+    service_object = k8s.read_namespaced_service(name=service, namespace=namespace)
 
-        # Patch the service with the new status
-        k8s.patch_namespaced_service_status(name=service, namespace=namespace, body=service_object)
-    except Exception as e:
-        raise e
+    # Update the status
+    service_object.status.load_balancer.ingress = [
+        kubernetes.client.V1LoadBalancerIngress(ip=ip)
+    ]
 
+    # Patch the service with the new status
+    k8s.patch_namespaced_service_status(name=service, namespace=namespace, body=service_object)
 
 def get_hostname(target_service_name: str, target_service_namespace: str) -> str:
     """
@@ -61,16 +57,21 @@ def get_hostname(target_service_name: str, target_service_namespace: str) -> str
     return ""
 
 
-def get_image_pull_secrets() -> [str]:
+def get_image_pull_secrets() -> [kubernetes.client.V1LocalObjectReference]:
     """
     Generates the imagePullSecrets to use, based on the semi-colon seperated string
     config.IMAGE_PULL_SECRETS.
     """
-    if config.IMAGE_PULL_SECRETS is not None:
-        logging.debug(f"Image Pull Secrets: {config.IMAGE_PULL_SECRETS}")
-        retval = []
-        secrets = config.IMAGE_PULL_SECRETS.split(";")
-        for secret in secrets:
-            retval.append(kubernetes.client.V1LocalObjectReference(name=secret))
-        return retval
-    return []
+    if not config.IMAGE_PULL_SECRETS:
+        return []
+    logging.debug(f"Image Pull Secrets: {config.IMAGE_PULL_SECRETS}")
+    secrets = config.IMAGE_PULL_SECRETS.split(";")
+    return [kubernetes.client.V1LocalObjectReference(name=secret) for secret in secrets]
+
+@contextlib.contextmanager
+def ignore_k8s_statuses(*ignored_statuses):
+    try:
+        yield
+    except kubernetes.client.exceptions.ApiException as e:
+        if e.status not in ignored_statuses:
+            raise e

src/tailscale_svc_lb_controller/resources/base.py

@@ -1,24 +1,41 @@
+import abc
+import contextlib
 import kubernetes
 
 from src.tailscale_svc_lb_controller import config
 from src.tailscale_svc_lb_controller import helpers
 
 
 class BaseResource:
-    target_service_name = ""
-    target_service_namespace = ""
-    tailscale_proxy_namespace = ""
 
-    # All resources that inherit this class need to implement the following methods
-    def new(self): raise NotImplementedError
+    def __init__(self, target_service_name: str, target_service_namespace: str, namespace: str):
+        self.target_service_name = target_service_name
+        self.target_service_namespace = target_service_namespace
+        self.tailscale_proxy_namespace = namespace
 
-    def create(self): raise NotImplementedError
+    @abstractmethod
+    def new(self):
+        pass
 
-    def delete(self): raise NotImplementedError
+    @abstractmethod
+    def create(self):
+        pass
 
-    def get(self): raise NotImplementedError
+    @abstractmethod
+    def delete(self):
+        pass
 
-    def reconcile(self): raise NotImplementedError
+    @abstractmethod
+    def get(self):
+        pass
+
+    def reconcile(self):
+        """
+        Creates the resource if it doesn't already exist
+        """
+        existing = self.get()
+        if existing is None:
+            self.create()
 
     def _generate_pod_template_spec(self) -> kubernetes.client.V1PodTemplateSpec:
         node_selector = None
@@ -133,4 +150,4 @@ def _generate_pod_template_spec(self) -> kubernetes.client.V1PodTemplateSpec:
                     )
                 ],
             ),
-        )
+        )

src/tailscale_svc_lb_controller/resources/daemonset.py

@@ -6,11 +6,6 @@
 
 class DaemonSet(BaseResource):
 
-    def __init__(self, target_service_name: str, target_service_namespace: str, namespace: str):
-        self.target_service_name = target_service_name
-        self.target_service_namespace = target_service_namespace
-        self.tailscale_proxy_namespace = namespace
-
     def new(self) -> kubernetes.client.V1DaemonSet:
         """
         Returns the kubernetes.client.V1DaemonSet that runs the tailscale proxy instance
@@ -34,11 +29,11 @@ def create(self) -> kubernetes.client.V1DaemonSet:
         Creates the DaemonSet that runs the Tailscale Proxy
         """
         k8s = kubernetes.client.AppsV1Api()
-        deployment = self.new()
+        daemonset = self.new()
 
         return k8s.create_namespaced_daemon_set(
             namespace=self.tailscale_proxy_namespace,
-            body=deployment
+            body=daemonset
         )
 
     def delete(self) -> None:
@@ -47,36 +42,19 @@ def delete(self) -> None:
         """
         k8s = kubernetes.client.AppsV1Api()
         # Delete all DaemonSets with svc-name label
-        try:
+        with(helpers.ignore_k8s_statuses(404)):
             k8s.delete_collection_namespaced_daemon_set(
                 namespace=self.tailscale_proxy_namespace,
                 label_selector=f"{config.SERVICE_NAME_LABEL}={self.target_service_name}"
             )
-        except kubernetes.client.exceptions.ApiException as e:
-            if e.status == 404:
-                return None
-            raise e
 
     def get(self) -> kubernetes.client.V1DaemonSet | None:
         """
         Fetches the current DaemonSet that should have been deployed as part of the proxy instance
         """
         k8s = kubernetes.client.AppsV1Api()
-        try:
+        with(helpers.ignore_k8s_statuses(404)):
             return k8s.read_namespaced_daemon_set(
                 namespace=self.tailscale_proxy_namespace,
                 name=f"{config.RESOURCE_PREFIX}{self.target_service_name}"
-            )
-        except kubernetes.client.exceptions.ApiException as e:
-            if e.status == 404:
-                return None
-            else:
-                raise e
-
-    def reconcile(self):
-        """
-        Creates the resource if it doesn't already exist
-        """
-        existing = self.get()
-        if existing is None:
-            self.create()
+            )

src/tailscale_svc_lb_controller/resources/deployment.py

@@ -6,11 +6,6 @@
 
 class Deployment(BaseResource):
 
-    def __init__(self, target_service_name: str, target_service_namespace: str, namespace: str):
-        self.target_service_name = target_service_name
-        self.target_service_namespace = target_service_namespace
-        self.tailscale_proxy_namespace = namespace
-
     def new(self) -> kubernetes.client.V1Deployment:
         """
         Returns the kubernetes.client.V1Deployment that runs the tailscale proxy instance
@@ -47,36 +42,19 @@ def delete(self) -> None:
         """
         k8s = kubernetes.client.AppsV1Api()
         # Delete all Deployments with svc-name label
-        try:
+        with(helpers.ignore_k8s_statuses(404)):
             k8s.delete_collection_namespaced_deployment(
                 namespace=self.tailscale_proxy_namespace,
                 label_selector=f"{config.SERVICE_NAME_LABEL}={self.target_service_name}"
             )
-        except kubernetes.client.exceptions.ApiException as e:
-            if e.status == 404:
-                return None
-            raise e
 
     def get(self) -> kubernetes.client.V1Deployment | None:
         """
         Fetches the current Deployment that should have been deployed as part of the proxy instance
         """
         k8s = kubernetes.client.AppsV1Api()
-        try:
+        with(helpers.ignore_k8s_statuses(404)):
             return k8s.read_namespaced_deployment(
                 namespace=self.tailscale_proxy_namespace,
                 name=f"{config.RESOURCE_PREFIX}{self.target_service_name}"
-            )
-        except kubernetes.client.exceptions.ApiException as e:
-            if e.status == 404:
-                return None
-            else:
-                raise e
-
-    def reconcile(self):
-        """
-        Creates the resource if it doesn't already exist
-        """
-        existing = self.get()
-        if existing is None:
-            self.create()
+            )

src/tailscale_svc_lb_controller/resources/role.py

@@ -6,11 +6,6 @@
 
 class Role(BaseResource):
 
-    def __init__(self, target_service_name: str, target_service_namespace: str, namespace: str):
-        self.target_service_name = target_service_name
-        self.target_service_namespace = target_service_namespace
-        self.tailscale_proxy_namespace = namespace
-
     def new(self) -> kubernetes.client.V1Role:
         """
         Returns the kubernetes.client.V1Role that
@@ -57,36 +52,19 @@ def delete(self) -> None:
         Delete the Role deployed as part of a proxy instance
         """
         k8s = kubernetes.client.RbacAuthorizationV1Api()
-        try:
+        with(helpers.ignore_k8s_statuses(404)):
             k8s.delete_collection_namespaced_role(
                 namespace=self.tailscale_proxy_namespace,
                 label_selector=f"{config.SERVICE_NAME_LABEL}={self.target_service_name}"
             )
-        except kubernetes.client.exceptions.ApiException as e:
-            if e.status == 404:
-                return None
-            raise e
 
     def get(self) -> kubernetes.client.V1Role | None:
         """
         Fetches the current Role that should have been deployed as part of the proxy instance
         """
         k8s = kubernetes.client.RbacAuthorizationV1Api()
-        try:
+        with(helpers.ignore_k8s_statuses(404)):
             return k8s.read_namespaced_role(
                 namespace=self.tailscale_proxy_namespace,
                 name=f"{config.RESOURCE_PREFIX}{self.target_service_name}"
-            )
-        except kubernetes.client.exceptions.ApiException as e:
-            if e.status == 404:
-                return None
-            else:
-                raise e
-
-    def reconcile(self):
-        """
-        Creates the resource if it doesn't already exist
-        """
-        existing = self.get()
-        if existing is None:
-            self.create()
+            )