Skip to content

fix(nvca): preserve unrelated namespace metadata during reconciliation #1950

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
apartha-nv merged 3 commits into from
Sep 18, 2026
Merged

fix(nvca): preserve unrelated namespace metadata during reconciliation #1950

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5cc7faf
fix(nvca): preserve unrelated namespace metadata during reconciliation
vrv3814 Sep 17, 2026
f8e45a2
Merge branch 'main' into fix/nvca-namespace-metadata-preservation
vrv3814 Sep 17, 2026
4e2424d
fix(nvca): scope optional namespace key removal to labels or annotations
vrv3814 Sep 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/compute-plane-services/nvca/pkg/operator/reconcile/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,7 @@ go_test(
"//src/compute-plane-services/nvca/pkg/operator/reconcile/clustermgmt",
"//src/compute-plane-services/nvca/pkg/operator/types",
"//src/compute-plane-services/nvca/pkg/storage",
"//src/compute-plane-services/nvca/pkg/types",
"//src/compute-plane-services/nvca/vendor/github.com/NVIDIA/nvcf/src/libraries/go/lib/pkg/auth",
"//src/compute-plane-services/nvca/vendor/github.com/NVIDIA/nvcf/src/libraries/go/lib/pkg/core",
"//src/compute-plane-services/nvca/vendor/github.com/NVIDIA/nvcf/src/libraries/go/lib/pkg/types/nvca/config",
Expand All @@ -212,6 +213,7 @@ go_test(
"//src/compute-plane-services/nvca/vendor/k8s.io/apimachinery/pkg/labels",
"//src/compute-plane-services/nvca/vendor/k8s.io/apimachinery/pkg/runtime",
"//src/compute-plane-services/nvca/vendor/k8s.io/apimachinery/pkg/runtime/schema",
"//src/compute-plane-services/nvca/vendor/k8s.io/apimachinery/pkg/types",
"//src/compute-plane-services/nvca/vendor/k8s.io/apimachinery/pkg/util/intstr",
"//src/compute-plane-services/nvca/vendor/k8s.io/client-go/discovery",
"//src/compute-plane-services/nvca/vendor/k8s.io/client-go/discovery/fake",
Expand Down
293 changes: 293 additions & 0 deletions src/compute-plane-services/nvca/pkg/operator/reconcile/namespace_helpers_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,25 @@ limitations under the License.
package operator

import (
"context"
"fmt"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
corev1 "k8s.io/api/core/v1"
k8serrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
"k8s.io/client-go/kubernetes/fake"
k8stesting "k8s.io/client-go/testing"

nvidiaiov1 "github.com/NVIDIA/nvcf/src/compute-plane-services/nvca/pkg/apis/nvcf/v1"
"github.com/NVIDIA/nvcf/src/compute-plane-services/nvca/pkg/operator/internal/kubeclients"
"github.com/NVIDIA/nvcf/src/compute-plane-services/nvca/pkg/operator/reconcile/clustermgmt"
nvcaoptypes "github.com/NVIDIA/nvcf/src/compute-plane-services/nvca/pkg/operator/types"
nvcatypes "github.com/NVIDIA/nvcf/src/compute-plane-services/nvca/pkg/types"
)

func TestGetSystemNamespace(t *testing.T) {
Expand Down Expand Up @@ -261,3 +275,282 @@ func TestGetClusterLogLevel(t *testing.T) {
})
}
}

const (
externalLabelKey = "platform.example.com/owner"
externalAnnotationKey = "platform.example.com/audit"
)

func newNamespaceCache(t *testing.T, objects ...runtime.Object) (*BackendK8sCache, *fake.Clientset) {
t.Helper()

clientset := fake.NewSimpleClientset(objects...)
return &BackendK8sCache{
clients: &kubeclients.KubeClients{K8s: clientset},
}, clientset
}

func TestBackendK8sCache_CreateOrUpdateNamespaceMetadataOwnership(t *testing.T) {
ctx := context.Background()

t.Run("external labels and annotations survive reconciliation", func(t *testing.T) {
bc, clientset := newNamespaceCache(t, &corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{
Name: "test-ns",
Labels: map[string]string{externalLabelKey: "platform", InstanceLabelKey: "stale"},
Annotations: map[string]string{externalAnnotationKey: "enabled"},
},
})

desired := &corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{
Name: "test-ns",
Labels: getAppLabels(),
Annotations: map[string]string{ClusterName: "test-cluster"},
},
}

require.NoError(t, bc.createOrUpdateNamespace(ctx, desired))

got, err := clientset.CoreV1().Namespaces().Get(ctx, "test-ns", metav1.GetOptions{})
require.NoError(t, err)
assert.Equal(t, "platform", got.Labels[externalLabelKey])
assert.Equal(t, "enabled", got.Annotations[externalAnnotationKey])
assert.Equal(t, nvcaoptypes.NVCAModuleName, got.Labels[InstanceLabelKey])
assert.Equal(t, "test-cluster", got.Annotations[ClusterName])
})

t.Run("namespace identity and other metadata survive reconciliation", func(t *testing.T) {
bc, clientset := newNamespaceCache(t, &corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{
Name: "test-ns",
UID: "namespace-uid",
Finalizers: []string{"example.com/protect"},
OwnerReferences: []metav1.OwnerReference{{APIVersion: "v1", Kind: "ConfigMap", Name: "owner"}},
},
Spec: corev1.NamespaceSpec{Finalizers: []corev1.FinalizerName{corev1.FinalizerKubernetes}},
})

desired := &corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{Name: "test-ns", Labels: getAppLabels()},
}

require.NoError(t, bc.createOrUpdateNamespace(ctx, desired))

got, err := clientset.CoreV1().Namespaces().Get(ctx, "test-ns", metav1.GetOptions{})
require.NoError(t, err)
assert.Equal(t, types.UID("namespace-uid"), got.UID)
assert.Equal(t, []string{"example.com/protect"}, got.Finalizers)
require.Len(t, got.OwnerReferences, 1)
assert.Equal(t, "owner", got.OwnerReferences[0].Name)
assert.Equal(t, []corev1.FinalizerName{corev1.FinalizerKubernetes}, got.Spec.Finalizers)
})

t.Run("optional owned key is removed when the desired object omits it", func(t *testing.T) {
bc, clientset := newNamespaceCache(t, &corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{
Name: "test-ns",
Labels: map[string]string{
clustermgmt.ShaderCacheLabelKey: "true",
externalLabelKey: "platform",
},
},
})

desired := &corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{Name: "test-ns", Labels: getAppLabels()},
}

require.NoError(t, bc.createOrUpdateNamespace(ctx, desired, clustermgmt.ShaderCacheLabelKey))

got, err := clientset.CoreV1().Namespaces().Get(ctx, "test-ns", metav1.GetOptions{})
require.NoError(t, err)
assert.NotContains(t, got.Labels, clustermgmt.ShaderCacheLabelKey)
assert.Equal(t, "platform", got.Labels[externalLabelKey])
})

t.Run("optional owned key is kept when the desired object sets it", func(t *testing.T) {
bc, clientset := newNamespaceCache(t, &corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{Name: "test-ns"},
})

desired := &corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{
Name: "test-ns",
Labels: map[string]string{clustermgmt.ShaderCacheLabelKey: "true"},
},
}

require.NoError(t, bc.createOrUpdateNamespace(ctx, desired, clustermgmt.ShaderCacheLabelKey))

got, err := clientset.CoreV1().Namespaces().Get(ctx, "test-ns", metav1.GetOptions{})
require.NoError(t, err)
assert.Equal(t, "true", got.Labels[clustermgmt.ShaderCacheLabelKey])
})

t.Run("nil metadata maps on either side are handled", func(t *testing.T) {
bc, clientset := newNamespaceCache(t, &corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{Name: "test-ns"},
})

require.NoError(t, bc.createOrUpdateNamespace(ctx, &corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{Name: "test-ns", Labels: getAppLabels()},
}))
require.NoError(t, bc.createOrUpdateNamespace(ctx, &corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{Name: "test-ns"},
}))

got, err := clientset.CoreV1().Namespaces().Get(ctx, "test-ns", metav1.GetOptions{})
require.NoError(t, err)
assert.Equal(t, nvcaoptypes.NVCAModuleName, got.Labels[InstanceLabelKey])
assert.Nil(t, got.Annotations)
})

t.Run("missing namespace is created with the desired metadata", func(t *testing.T) {
bc, clientset := newNamespaceCache(t)

desired := &corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{
Name: "test-ns",
Labels: getAppLabels(),
Annotations: map[string]string{ClusterName: "test-cluster"},
},
}

require.NoError(t, bc.createOrUpdateNamespace(ctx, desired))

got, err := clientset.CoreV1().Namespaces().Get(ctx, "test-ns", metav1.GetOptions{})
require.NoError(t, err)
assert.Equal(t, nvcaoptypes.NVCAModuleName, got.Labels[InstanceLabelKey])
assert.Equal(t, "test-cluster", got.Annotations[ClusterName])
})

t.Run("no write is issued when the owned metadata already matches", func(t *testing.T) {
bc, clientset := newNamespaceCache(t, &corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{
Name: "test-ns",
Labels: getAppLabels(),
Annotations: map[string]string{externalAnnotationKey: "enabled"},
},
})
clientset.ClearActions()

require.NoError(t, bc.createOrUpdateNamespace(ctx, &corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{Name: "test-ns", Labels: getAppLabels()},
}))

for _, action := range clientset.Actions() {
assert.NotEqual(t, "update", action.GetVerb(), "namespace was rewritten with no metadata change")
}
})

t.Run("concurrent external metadata change is merged on conflict", func(t *testing.T) {
bc, clientset := newNamespaceCache(t, &corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{Name: "test-ns"},
})

nsResource := corev1.SchemeGroupVersion.WithResource("namespaces")
conflicts := 0
clientset.PrependReactor("update", "namespaces", func(action k8stesting.Action) (bool, runtime.Object, error) {
if conflicts > 0 {
return false, nil, nil
}
conflicts++

// Simulate another controller writing the namespace between our read and our update.
external, err := clientset.Tracker().Get(nsResource, "", "test-ns")
if err != nil {
return true, nil, err
}
externalNS, ok := external.(*corev1.Namespace)
if !ok {
return true, nil, fmt.Errorf("unexpected object type %T", external)
}
externalNS.Annotations = map[string]string{externalAnnotationKey: "enabled"}
if err := clientset.Tracker().Update(nsResource, externalNS, ""); err != nil {
return true, nil, err
}

return true, nil, k8serrors.NewConflict(
corev1.Resource("namespaces"), "test-ns", fmt.Errorf("object was modified"))
})

require.NoError(t, bc.createOrUpdateNamespace(ctx, &corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{Name: "test-ns", Labels: getAppLabels()},
}))

got, err := clientset.CoreV1().Namespaces().Get(ctx, "test-ns", metav1.GetOptions{})
require.NoError(t, err)
assert.Equal(t, 1, conflicts)
assert.Equal(t, "enabled", got.Annotations[externalAnnotationKey])
assert.Equal(t, nvcaoptypes.NVCAModuleName, got.Labels[InstanceLabelKey])
})
}

func TestSetupNamespacesPreserveExternalMetadata(t *testing.T) {
ctx := context.Background()

nb := &nvidiaiov1.NVCFBackend{
Spec: nvidiaiov1.NVCFBackendSpec{
NVCFBackendSpecT: nvidiaiov1.NVCFBackendSpecT{
ClusterConfig: nvidiaiov1.ClusterConfig{
ClusterName: "test-cluster",
ClusterGroupName: "test-group",
},
},
},
}

externalMetadata := func(name string) *corev1.Namespace {
return &corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{
Name: name,
Labels: map[string]string{externalLabelKey: "platform"},
Annotations: map[string]string{externalAnnotationKey: "enabled"},
},
}
}

t.Run("system namespace", func(t *testing.T) {
bc, clientset := newNamespaceCache(t, externalMetadata(getSystemNamespace(nb)))

require.NoError(t, bc.setupSystemNamespace(ctx, nb))

got, err := clientset.CoreV1().Namespaces().Get(ctx, getSystemNamespace(nb), metav1.GetOptions{})
require.NoError(t, err)
assert.Equal(t, "platform", got.Labels[externalLabelKey])
assert.Equal(t, "enabled", got.Annotations[externalAnnotationKey])
assert.Equal(t, nvcaoptypes.NVCAModuleName, got.Labels[InstanceLabelKey])
assert.Equal(t, "test-cluster", got.Annotations[ClusterName])
assert.Equal(t, "test-group", got.Annotations[ClusterGroupKey])
})

t.Run("requests namespace with gxcache enabled", func(t *testing.T) {
bc, clientset := newNamespaceCache(t, externalMetadata(getRequestsNamespace(nb)))
bc.enableGXCache = true

require.NoError(t, bc.setupRequestsNamespace(ctx, nb))

got, err := clientset.CoreV1().Namespaces().Get(ctx, getRequestsNamespace(nb), metav1.GetOptions{})
require.NoError(t, err)
assert.Equal(t, "platform", got.Labels[externalLabelKey])
assert.Equal(t, "enabled", got.Annotations[externalAnnotationKey])
assert.Equal(t, nvcaoptypes.NVCAModuleName, got.Labels[ManagedbyLabelKey])
assert.Equal(t, WorkloadInstanceTypeValuePodSpec, got.Labels[nvcatypes.WorkloadInstanceTypeLabel])
assert.Equal(t, "true", got.Labels[clustermgmt.ShaderCacheLabelKey])
})

t.Run("requests namespace drops the gxcache label when disabled", func(t *testing.T) {
existing := externalMetadata(getRequestsNamespace(nb))
existing.Labels[clustermgmt.ShaderCacheLabelKey] = "true"
bc, clientset := newNamespaceCache(t, existing)

require.NoError(t, bc.setupRequestsNamespace(ctx, nb))

got, err := clientset.CoreV1().Namespaces().Get(ctx, getRequestsNamespace(nb), metav1.GetOptions{})
require.NoError(t, err)
assert.NotContains(t, got.Labels, clustermgmt.ShaderCacheLabelKey)
assert.Equal(t, "platform", got.Labels[externalLabelKey])
assert.Equal(t, "enabled", got.Annotations[externalAnnotationKey])
})
}
23 changes: 8 additions & 15 deletions src/compute-plane-services/nvca/pkg/operator/reconcile/nvcaagent_reconcile.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,28 +250,21 @@ func (bc *BackendK8sCache) setupRequestsNamespace(ctx context.Context, nb *nvidi
nvcatypes.WorkloadInstanceTypeLabel: WorkloadInstanceTypeValuePodSpec,
}

// set the label for gxcache if enabled
if bc.enableGXCache {
labels[clustermgmt.ShaderCacheLabelKey] = strconv.FormatBool(true)
}

reqNSObj := &corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{
Name: requestsNamespace,
Labels: labels,
},
}

// set the labels for gxcache if enabled
if bc.enableGXCache {
if _, ok := reqNSObj.Labels[clustermgmt.ShaderCacheLabelKey]; !ok {
if reqNSObj.Labels == nil {
reqNSObj.Labels = make(map[string]string)
}
reqNSObj.Labels[clustermgmt.ShaderCacheLabelKey] = strconv.FormatBool(true)
}
} else {
if _, ok := reqNSObj.Labels[clustermgmt.ShaderCacheLabelKey]; !ok {
delete(reqNSObj.Labels, clustermgmt.ShaderCacheLabelKey)
}
}

if err := bc.createOrUpdateNamespace(ctx, reqNSObj); err != nil {
// The GXCache label is NVCA-owned but conditional, so name it explicitly: reconciliation drops it from the
// namespace once the feature is turned off, while leaving metadata owned by other controllers alone.
if err := bc.createOrUpdateNamespace(ctx, reqNSObj, clustermgmt.ShaderCacheLabelKey); err != nil {
return fmt.Errorf("failed to setup namespace %v", reqNSObj.Name)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
}

Expand Down
Loading
Loading