From b9da3e8e46bef1bca20b427a92fb190f006f586b Mon Sep 17 00:00:00 2001 From: NopeCharon <115794254+NopeCharon@users.noreply.github.com> Date: Sun, 25 Oct 2026 16:17:24 +0530 Subject: [PATCH] fix: honour optional on ConfigMap and Secret volumes A configMap or secret volume marked optional whose object does not exist was treated like a required one: remoteExecutionHandleVolumes kept the pod in Initializing, polled for the object every 2s and, after 5 minutes, failed the pod with "CFGMaps/Secrets not found". A configMap source of a projected volume failed the pod right away, also when it was optional or only one of its listed items was missing. The kubelet mounts an empty volume for a missing optional object, and skips the missing keys of an optional projected source. Do the same: - configMap / secret volumes: on NotFound for an optional object, send the plugin an empty ConfigMap/Secret with that name instead of waiting. Plugins look retrieved objects up by name (the Slurm plugin fails the mount when one is absent), so an empty object is what yields an empty mount there. - projected configMap sources: an optional ConfigMap that is missing projects no keys, and missing keys listed in items are skipped. Other errors, and objects that are not optional, keep the existing behaviour. Fixes #589 Signed-off-by: NopeCharon <115794254+NopeCharon@users.noreply.github.com> --- pkg/virtualkubelet/execute.go | 37 +++++++++--- pkg/virtualkubelet/execute_test.go | 90 ++++++++++++++++++++++++++++++ 2 files changed, 120 insertions(+), 7 deletions(-) diff --git a/pkg/virtualkubelet/execute.go b/pkg/virtualkubelet/execute.go index 7b0b763d..44a7ba6f 100644 --- a/pkg/virtualkubelet/execute.go +++ b/pkg/virtualkubelet/execute.go @@ -28,6 +28,7 @@ import ( "go.opentelemetry.io/otel/attribute" trace "go.opentelemetry.io/otel/trace" v1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -803,9 +804,15 @@ func remoteExecutionHandleProjectedSource( // With InterLink, the Kubernetes internal network is not accessible so this default ca.crt is probably useless. log.G(ctx).Warning("using default Kubernetes API kube-root-ca.crt (no override found), but the default one might not be compatible with the subject: ", p.config.KubernetesAPIAddr) } + optional := source.ConfigMap.Optional != nil && *source.ConfigMap.Optional // Fetch the ConfigMap once, then iterate over items (or all keys if no items are specified). cfgmap, err := p.clientSet.CoreV1().ConfigMaps(pod.Namespace).Get(ctx, source.ConfigMap.Name, metav1.GetOptions{}) if err != nil { + if optional && apierrors.IsNotFound(err) { + // Like the kubelet, a missing optional ConfigMap projects no keys. + log.G(ctx).Infof("optional ConfigMap %s of pod %s not found, projecting no keys from it", source.ConfigMap.Name, pod.Name) + return nil + } return fmt.Errorf("error during retrieval of ConfigMap %s error: %w", source.ConfigMap.Name, err) } if len(source.ConfigMap.Items) == 0 { @@ -817,7 +824,7 @@ func remoteExecutionHandleProjectedSource( for _, item := range source.ConfigMap.Items { if value, ok := cfgmap.Data[item.Key]; ok { projectedVolume.Data[item.Path] = value - } else { + } else if !optional { return fmt.Errorf("error during retrieval of key %s of (existing) ConfigMap %s", item.Key, source.ConfigMap.Name) } } @@ -931,13 +938,21 @@ func remoteExecutionHandleVolumes(ctx context.Context, p *Provider, pod *v1.Pod, switch { case volume.ConfigMap != nil: cfgmap, err := p.clientSet.CoreV1().ConfigMaps(pod.Namespace).Get(ctx, volume.ConfigMap.Name, metav1.GetOptions{}) - if err != nil { + switch { + case err == nil: + req.ConfigMaps = append(req.ConfigMaps, *cfgmap) + case volume.ConfigMap.Optional != nil && *volume.ConfigMap.Optional && apierrors.IsNotFound(err): + // Like the kubelet, mount an empty volume for a missing optional ConfigMap + // instead of waiting for it. The plugin still gets an object to mount by name. + log.G(ctx).Infof("optional ConfigMap %s of pod %s not found, mounting an empty volume", volume.ConfigMap.Name, pod.Name) + req.ConfigMaps = append(req.ConfigMaps, v1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: volume.ConfigMap.Name, Namespace: pod.Namespace}, + }) + default: err = failedMount(ctx, &failedAndWait, volume.ConfigMap.Name, pod, p, err) if err != nil { return err } - } else { - req.ConfigMaps = append(req.ConfigMaps, *cfgmap) } case volume.Projected != nil: @@ -1007,13 +1022,21 @@ func remoteExecutionHandleVolumes(ctx context.Context, p *Provider, pod *v1.Pod, case volume.Secret != nil: scrt, err := p.clientSet.CoreV1().Secrets(pod.Namespace).Get(ctx, volume.Secret.SecretName, metav1.GetOptions{}) - if err != nil { + switch { + case err == nil: + req.Secrets = append(req.Secrets, *scrt) + case volume.Secret.Optional != nil && *volume.Secret.Optional && apierrors.IsNotFound(err): + // Like the kubelet, mount an empty volume for a missing optional Secret + // instead of waiting for it. The plugin still gets an object to mount by name. + log.G(ctx).Infof("optional Secret %s of pod %s not found, mounting an empty volume", volume.Secret.SecretName, pod.Name) + req.Secrets = append(req.Secrets, v1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: volume.Secret.SecretName, Namespace: pod.Namespace}, + }) + default: err = failedMount(ctx, &failedAndWait, volume.Secret.SecretName, pod, p, err) if err != nil { return err } - } else { - req.Secrets = append(req.Secrets, *scrt) } case volume.EmptyDir != nil: diff --git a/pkg/virtualkubelet/execute_test.go b/pkg/virtualkubelet/execute_test.go index 48b0bd77..6c0bf5b1 100644 --- a/pkg/virtualkubelet/execute_test.go +++ b/pkg/virtualkubelet/execute_test.go @@ -379,6 +379,7 @@ func TestRemoteExecutionHandleProjectedSourceConfigMap(t *testing.T) { configMapData map[string]string sourceItems []v1.KeyToPath overrideCaCrt string + optional bool expectedData map[string]string expectErr bool }{ @@ -461,6 +462,35 @@ func TestRemoteExecutionHandleProjectedSourceConfigMap(t *testing.T) { expectedData: nil, expectErr: true, }, + { + name: "missing key in items of optional configmap is skipped", + configMapName: testConfigMapName, + configMapData: map[string]string{ + testConfigKey1: testConfigValue1, + }, + sourceItems: []v1.KeyToPath{ + {Key: testConfigKey1, Path: testConfigKey1}, + {Key: "missing-key", Path: "some-path"}, + }, + optional: true, + expectedData: map[string]string{ + testConfigKey1: testConfigValue1, + }, + }, + { + name: "missing configmap returns error", + configMapName: testConfigMapName, + configMapData: nil, + expectedData: nil, + expectErr: true, + }, + { + name: "missing optional configmap projects no keys", + configMapName: testConfigMapName, + configMapData: nil, + optional: true, + expectedData: map[string]string{}, + }, } for _, tt := range tests { @@ -486,10 +516,12 @@ func TestRemoteExecutionHandleProjectedSourceConfigMap(t *testing.T) { }, } + optional := tt.optional source := v1.VolumeProjection{ ConfigMap: &v1.ConfigMapProjection{ LocalObjectReference: v1.LocalObjectReference{Name: tt.configMapName}, Items: tt.sourceItems, + Optional: &optional, }, } @@ -657,6 +689,64 @@ func TestRemoteExecutionHandleVolumesDownwardAPIDisabledProjectedVolumes(t *test assert.Nil(t, pod.Spec.Volumes[0].Projected) } +func TestRemoteExecutionHandleVolumesOptionalMissing(t *testing.T) { + const ( + missingConfigMap = "missing-config" + missingSecret = "missing-secret" + ) + ctx := context.Background() + namespace := testNamespace + optional := true + pod := &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: testPodName, + Namespace: namespace, + }, + Spec: v1.PodSpec{ + Volumes: []v1.Volume{ + { + Name: "config", + VolumeSource: v1.VolumeSource{ + ConfigMap: &v1.ConfigMapVolumeSource{ + LocalObjectReference: v1.LocalObjectReference{Name: missingConfigMap}, + Optional: &optional, + }, + }, + }, + { + Name: "secret", + VolumeSource: v1.VolumeSource{ + Secret: &v1.SecretVolumeSource{ + SecretName: missingSecret, + Optional: &optional, + }, + }, + }, + }, + }, + } + + fakeClient := fake.NewSimpleClientset(pod.DeepCopy()) + p := &Provider{ + clientSet: fakeClient, + notifier: func(*v1.Pod) {}, + } + req := &types.PodCreateRequests{} + + // Without the optional handling this waits five minutes for the objects to appear. + err := remoteExecutionHandleVolumes(ctx, p, pod, req) + require.NoError(t, err) + assert.Equal(t, v1.PodPending, pod.Status.Phase) + require.Len(t, req.ConfigMaps, 1) + assert.Equal(t, missingConfigMap, req.ConfigMaps[0].Name) + assert.Equal(t, namespace, req.ConfigMaps[0].Namespace) + assert.Empty(t, req.ConfigMaps[0].Data) + require.Len(t, req.Secrets, 1) + assert.Equal(t, missingSecret, req.Secrets[0].Name) + assert.Equal(t, namespace, req.Secrets[0].Namespace) + assert.Empty(t, req.Secrets[0].Data) +} + func TestResolveEnvFromRefs(t *testing.T) { ctx := context.Background() namespace := testNamespace