From e8b2ad014bde5aff997c94033e963a5e223dc0cc Mon Sep 17 00:00:00 2001 From: Eric <5089238+emizzle@users.noreply.github.com> Date: Mon, 15 Jun 2026 15:50:16 +1000 Subject: [PATCH] feat: remove usage of PVC disks (#128) * fix: delete PVCs after stopping containers * Didn't work, instead try to delete all PVCs just before the namespace is deleted, after all pods destroyed. * Didn't work, force kill pods, then delete pvcs Force kill pods, wait for them to be killed. Then remove the pvc finaliser that protects the pvc from deletion. Finally, delete the pvc. The finaliser deletion step is there in case the force kill pod times out. * try without waiting for pods to be killed * prevent double delete race * remove unneeded method, improve log output in pvc deletion * Use emptyDir ephemeral volumes instead of PVCs * fix dist tests workflow summary After kubeconfig was replaced with an in-cluster service account, k8sClient was returning null and thus no test summaries were being written to ConfigMaps. This change returns a default Kubeconfig for the k8sClient when one is not passed in an environment variable. * remove PVC volume deletion since PVCs are no longer created --- CONTRIBUTINGPLUGINS.MD | 2 +- Framework/KubernetesWorkflow/K8sController.cs | 67 +++++-------------- Tests/DistTestCore/Global.cs | 16 +++-- 3 files changed, 29 insertions(+), 56 deletions(-) diff --git a/CONTRIBUTINGPLUGINS.MD b/CONTRIBUTINGPLUGINS.MD index 38420918..d05175d2 100644 --- a/CONTRIBUTINGPLUGINS.MD +++ b/CONTRIBUTINGPLUGINS.MD @@ -120,7 +120,7 @@ Should your deploy methods not return framework-types like RunningContainers, pl The primary reason to decouple deploying and wrapping functionalities is that some use cases require these steps to be performed by separate applications, and different moments in time. For this reason, whatever is returned by the deploy methods should be serializable. After deserialization at some later time, it should then be valid input for the wrap method. ## Container Recipes -In order to run a container of your application, the framework needs to know how to create that container. Think of a container recipe as being similar to a docker-compose.yaml file: You specify the docker image, ports, environment variables, persistent volumes, and secrets. However, container recipes are code. This allows you to add conditional behaviour to how your container is constructed. For example: The 'user' of your plugin specifies in their call input that they want to run your application in a certain mode. This would cause your container recipe to set certain environment variables, which cause the application to behave in the requested way. +In order to run a container of your application, the framework needs to know how to create that container. Think of a container recipe as being similar to a docker-compose.yaml file: You specify the docker image, ports, environment variables, volumes, and secrets. However, container recipes are code. This allows you to add conditional behaviour to how your container is constructed. For example: The 'user' of your plugin specifies in their call input that they want to run your application in a certain mode. This would cause your container recipe to set certain environment variables, which cause the application to behave in the requested way. ### Addresses and ports In a docker-compose.yaml file, it is perfectly normal to specify which ports should be exposed on your container. However, in the framework there's more to consider. When your application container starts, who knows on what kind of machine it runs, and what other processes it's sharing space with? Well, Kubernetes knows. Therefore, it is recommended that container recipes *do not* specify exact port numbers. The framework allows container recipes to declare "a port" without specifying its port number. This allows the framework and Kubernetes to figure out which ports are available when it's time to deploy. In order to find out which port numbers were assigned post-deployment, you can look up the port by tag (which is just an identifying string). When you specify a port to be mapped in your container recipe, you must specify: diff --git a/Framework/KubernetesWorkflow/K8sController.cs b/Framework/KubernetesWorkflow/K8sController.cs index 5ca3a609..5eebb4bf 100644 --- a/Framework/KubernetesWorkflow/K8sController.cs +++ b/Framework/KubernetesWorkflow/K8sController.cs @@ -120,21 +120,23 @@ namespace KubernetesWorkflow log.Debug(); var all = client.Run(c => c.ListNamespace().Items); - var namespaces = all.Select(n => n.Name()).Where(n => n.StartsWith(prefix)); - - if (wait) - { - // If we're going to wait, trigger the delete for all the namespaces immediately. - // Then wait for them to finish one by one. - foreach (var ns in namespaces) - { - DeleteNamespace(ns, false); - } - } + var namespaces = all.Select(n => n.Name()).Where(n => n.StartsWith(prefix)).ToArray(); + // Trigger all deletions without waiting so they proceed in parallel. foreach (var ns in namespaces) { - DeleteNamespace(ns, wait); + DeleteNamespace(ns, false); + } + + // Optionally wait for each namespace to finish, one by one. + // Separated from the trigger loop to avoid re-entering PrepareNamespaceForDeletion + // on namespaces already in Terminating state (IsNamespaceOnline returns true for those). + if (wait) + { + foreach (var ns in namespaces) + { + WaitUntilNamespaceDeleted(ns); + } } } @@ -546,8 +548,6 @@ namespace KubernetesWorkflow private V1Volume CreateVolume(VolumeMount v) { - CreatePersistentVolumeClaimIfNeeded(v); - if (!string.IsNullOrEmpty(v.HostPath)) { return new V1Volume @@ -572,36 +572,13 @@ namespace KubernetesWorkflow return new V1Volume { Name = v.VolumeName, - PersistentVolumeClaim = new V1PersistentVolumeClaimVolumeSource + EmptyDir = new V1EmptyDirVolumeSource { - ClaimName = v.VolumeName + SizeLimit = v.ResourceQuantity != null ? new ResourceQuantity(v.ResourceQuantity) : null } }; } - private void CreatePersistentVolumeClaimIfNeeded(VolumeMount v) - { - var pvcs = client.Run(c => c.ListNamespacedPersistentVolumeClaim(K8sNamespace)); - if (pvcs != null && pvcs.Items.Any(i => i.Name() == v.VolumeName)) return; - - client.Run(c => c.CreateNamespacedPersistentVolumeClaim(new V1PersistentVolumeClaim - { - ApiVersion = "v1", - Metadata = new V1ObjectMeta - { - Name = v.VolumeName, - }, - Spec = new V1PersistentVolumeClaimSpec - { - AccessModes = new List - { - "ReadWriteOnce" - }, - Resources = CreateVolumeResourceRequirements(v), - }, - }, K8sNamespace)); - } - private V1SecretVolumeSource CreateVolumeSecret(VolumeMount v) { if (string.IsNullOrWhiteSpace(v.Secret)) return null!; @@ -611,18 +588,6 @@ namespace KubernetesWorkflow }; } - private V1VolumeResourceRequirements CreateVolumeResourceRequirements(VolumeMount v) - { - if (v.ResourceQuantity == null) return null!; - return new V1VolumeResourceRequirements - { - Requests = new Dictionary() - { - {"storage", new ResourceQuantity(v.ResourceQuantity) } - } - }; - } - private List CreateEnv(ContainerRecipe recipe) { return recipe.EnvVars.Select(CreateEnvVar).ToList(); diff --git a/Tests/DistTestCore/Global.cs b/Tests/DistTestCore/Global.cs index 83060c52..f0eff4b7 100644 --- a/Tests/DistTestCore/Global.cs +++ b/Tests/DistTestCore/Global.cs @@ -20,10 +20,18 @@ namespace DistTestCore private static IKubernetes? CreateK8sClient() { - var kubeconfig = Environment.GetEnvironmentVariable("KUBECONFIG"); - if (string.IsNullOrEmpty(kubeconfig)) return null; - var config = KubernetesClientConfiguration.BuildConfigFromConfigFile(kubeconfig); - return new Kubernetes(config); + try + { + var kubeconfig = Environment.GetEnvironmentVariable("KUBECONFIG"); + var config = string.IsNullOrEmpty(kubeconfig) + ? KubernetesClientConfiguration.BuildDefaultConfig() + : KubernetesClientConfiguration.BuildConfigFromConfigFile(kubeconfig); + return new Kubernetes(config); + } + catch + { + return null; + } } public Global()