ovh · yesnault · Oct 18, 2023 · Oct 17, 2023 · Oct 17, 2023 · Oct 18, 2023
@@ -133,6 +133,11 @@ func (h *HatcheryKubernetes) killAwolWorkers(ctx context.Context) error {
 				globalErr = err
 				log.Error(ctx, "hatchery:kubernetes> killAwolWorkers> Cannot delete pod %s (%s)", pod.Name, err)
 			}
+
+			if err := h.deleteSecretByWorkerName(ctx, labels[LABEL_WORKER_NAME]); err != nil {
+				log.ErrorWithStackTrace(ctx, sdk.WrapError(err, "cannot delete secret for worker %s", labels[LABEL_WORKER_NAME]))
+			}
+
 			log.Debug(ctx, "pod %s/%s killed", pod.Namespace, pod.Name)
 		}
 	}

@@ -371,9 +371,9 @@ func (h *HatcheryKubernetes) SpawnWorker(ctx context.Context, spawnArgs hatchery
 
 	// Check here to add secret if needed
 	if spawnArgs.Model.IsPrivate() || (spawnArgs.Model.GetDockerUsername() != "" && spawnArgs.Model.GetDockerPassword() != "") {
-		secretRegistryName, err := h.createRegistrySecret(ctx, spawnArgs.Model)
+		secretRegistryName, err := h.createRegistrySecret(ctx, spawnArgs.Model, workerConfig.Name)
 		if err != nil {
-			return sdk.WrapError(err, "cannot create secret for model %s", spawnArgs.Model.GetPath())
+			return sdk.WrapError(err, "cannot create registry secret for worker %s", workerConfig.Name)
 		}
 		podSchema.Spec.ImagePullSecrets = []apiv1.LocalObjectReference{{Name: secretRegistryName}}
 	}
@@ -496,6 +496,14 @@ func (h *HatcheryKubernetes) routines(ctx context.Context) {
 	ticker := time.NewTicker(10 * time.Second)
 	defer ticker.Stop()
 
+	deleteSecretsInterval := 10 * time.Minute
+	if h.Config.DeleteSecretsInterval > 0 {
+		deleteSecretsInterval = time.Duration(h.Config.DeleteSecretsInterval) * time.Second
+	}
+
+	tickerSecrets := time.NewTicker(deleteSecretsInterval)
+	defer tickerSecrets.Stop()
+
 	for {
 		select {
 		case <-ticker.C:
@@ -511,6 +519,7 @@ func (h *HatcheryKubernetes) routines(ctx context.Context) {
 				}
 			})
 
+		case <-tickerSecrets.C:
 			h.GoRoutines.Exec(ctx, "deleteSecrets", func(ctx context.Context) {
 				if err := h.deleteSecrets(ctx); err != nil {
 					log.ErrorWithStackTrace(ctx, sdk.WrapError(err, "cannot delete secrets"))

@@ -3,6 +3,7 @@ package kubernetes
 import (
 	"context"
 	"fmt"
+	"time"
 
 	"github.com/docker/distribution/reference"
 	"github.com/rockbears/log"
@@ -15,6 +16,7 @@ import (
 )
 
 // Delete worker model registry and worker config secrets that are not used by any pods.
+// This is used as a "gc", in the nominal case, the deletion of secrets is done when removing workers with killAwolWorkers
 func (h *HatcheryKubernetes) deleteSecrets(ctx context.Context) error {
 	pods, err := h.kubeClient.PodList(ctx, h.Config.Namespace, metav1.ListOptions{
 		LabelSelector: fmt.Sprintf("%s=%s,%s", LABEL_HATCHERY_NAME, h.Config.Name, LABEL_WORKER_NAME),
@@ -29,6 +31,10 @@ func (h *HatcheryKubernetes) deleteSecrets(ctx context.Context) error {
 	}
 
 	for _, secret := range secrets.Items {
+		// created last 10min, too young to delete it
+		if time.Since(secret.GetCreationTimestamp().Time) > 10*time.Minute {
+			continue
+		}
 		secretLabels := secret.GetLabels()
 		if secretLabels == nil {
 			continue
@@ -39,10 +45,6 @@ func (h *HatcheryKubernetes) deleteSecrets(ctx context.Context) error {
 			if podLabels == nil {
 				continue
 			}
-			if wm, ok := secretLabels[LABEL_WORKER_MODEL_PATH]; ok && podLabels[LABEL_WORKER_MODEL_PATH] == wm {
-				found = true
-				break
-			}
 			if w, ok := secretLabels[LABEL_WORKER_NAME]; ok && podLabels[LABEL_WORKER_NAME] == w {
 				found = true
 				break
@@ -59,11 +61,31 @@ func (h *HatcheryKubernetes) deleteSecrets(ctx context.Context) error {
 	return nil
 }
 
-func (h *HatcheryKubernetes) createRegistrySecret(ctx context.Context, model sdk.WorkerStarterWorkerModel) (string, error) {
-	secretName := slug.Convert("cds-worker-registry-" + model.GetPath())
+// deleteSecretByName deletes a secret. If the secret does not exit, return nil
+func (h *HatcheryKubernetes) deleteSecretByWorkerName(ctx context.Context, workerName string) error {
+	secrets, err := h.kubeClient.SecretList(ctx, h.Config.Namespace, metav1.ListOptions{LabelSelector: LABEL_HATCHERY_NAME})
+	if err != nil {
+		return sdk.WrapError(err, "cannot get secrets")
+	}
 
-	_ = h.kubeClient.SecretDelete(ctx, h.Config.Namespace, secretName, metav1.DeleteOptions{})
+	for _, secret := range secrets.Items {
+		secretLabels := secret.GetLabels()
+		if secretLabels == nil {
+			continue
+		}
+		if _, ok := secretLabels[LABEL_WORKER_NAME]; ok {
+			if err := h.kubeClient.SecretDelete(ctx, h.Config.Namespace, secret.Name, metav1.DeleteOptions{}); err != nil {
+				return sdk.WrapError(err, "cannot delete secret %s from worker %s", secret.Name, secretLabels[LABEL_WORKER_NAME])
+			}
+			// no break, we can have two secrets used by a worker (registry and config secrets)
+		}
+	}
+
+	return nil
+}
 
+func (h *HatcheryKubernetes) createRegistrySecret(ctx context.Context, model sdk.WorkerStarterWorkerModel, workerName string) (string, error) {
+	secretName := slug.Convert("cds-worker-registry-" + workerName)
 	registry := "https://index.docker.io/v1/"
 	if model.ModelV1 != nil && model.ModelV1.ModelDocker.Registry != "" {
 		registry = model.ModelV1.ModelDocker.Registry
@@ -82,8 +104,8 @@ func (h *HatcheryKubernetes) createRegistrySecret(ctx context.Context, model sdk
 			Name:      secretName,
 			Namespace: h.Config.Namespace,
 			Labels: map[string]string{
-				LABEL_HATCHERY_NAME:     h.Configuration().Name,
-				LABEL_WORKER_MODEL_PATH: slug.Convert(model.GetPath()),
+				LABEL_HATCHERY_NAME: h.Configuration().Name,
+				LABEL_WORKER_NAME:   workerName,
 			},
 		},
 		Type: apiv1.SecretTypeDockerConfigJson,
@@ -99,7 +121,6 @@ func (h *HatcheryKubernetes) createRegistrySecret(ctx context.Context, model sdk
 
 func (h *HatcheryKubernetes) createConfigSecret(ctx context.Context, workerConfig workerruntime.WorkerConfig) (string, error) {
 	secretName := slug.Convert("cds-worker-config-" + workerConfig.Name)
-
 	_ = h.kubeClient.SecretDelete(ctx, h.Config.Namespace, secretName, metav1.DeleteOptions{})
 
 	if _, err := h.kubeClient.SecretCreate(ctx, h.Config.Namespace, &apiv1.Secret{

@@ -52,6 +52,8 @@ type HatcheryConfiguration struct {
 	KubernetesClientKeyData string `mapstructure:"clientKeyData" toml:"clientKeyData" default:"" commented:"true" comment:"Client certificate data (content, not path and not base64 encoded) for tls kubernetes (optional if no tls needed)" json:"-"`
 	// CustomAnnotations that will be added to pods spawned by the hatchery
 	CustomAnnotations []CustomAnnotation `mapstructure:"customAnnotations" toml:"customAnnotations" default:"" commented:"true" comment:"CustomAnnotations that will be added to pods spawned by the hatchery" json:"-"`
+	// DeleteSecretsInterval used by deleteSecrets to clean secrets not used
+	DeleteSecretsInterval int `mapstructure:"deleteSecretsInterval" toml:"deleteSecretsInterval" commented:"true" comment:"Delete kubernetes worker secrets not used (seconds)" json:"deleteSecretsInterval"`
 }
 
 type CustomAnnotation struct {