Skip to content

Commit

Permalink
Switch fetchNamespace to Get instead of list
Browse files Browse the repository at this point in the history
Was using List with a label, that's set by k8s to the name of the
namespace. Only returned the first result.
Testdata assumed label and namespace name could be different.
  • Loading branch information
Mario Manno authored and manno committed Mar 5, 2024
1 parent 396e720 commit a310a17
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 16 deletions.
14 changes: 5 additions & 9 deletions internal/cmd/agent/deployer/deployer.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (

corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/log"
)
Expand Down Expand Up @@ -161,20 +162,15 @@ func (d *Deployer) updateNamespace(ctx context.Context, ns *corev1.Namespace) er
}

// fetchNamespace gets the namespace matching the release ID. Returns an error if none is found.
// releaseID is composed of release.Namespace/release.Name/release.Version
func (d *Deployer) fetchNamespace(ctx context.Context, releaseID string) (*corev1.Namespace, error) {
// releaseID is composed of release.Namespace/release.Name/release.Version
namespace := strings.Split(releaseID, "/")[0]
list := &corev1.NamespaceList{}
err := d.client.List(ctx, list, client.MatchingLabels{
corev1.LabelMetadataName: namespace,
})
ns := &corev1.Namespace{}
err := d.client.Get(ctx, types.NamespacedName{Name: namespace}, ns)
if err != nil {
return nil, err
}
if len(list.Items) == 0 {
return nil, fmt.Errorf("namespace %s not found", namespace)
}
return &list.Items[0], nil
return ns, nil
}

// addLabelsFromOptions updates nsLabels so that it only contains all labels specified in optLabels, plus the `kubernetes.io/metadata.name` labels added by kubernetes when creating the namespace.
Expand Down
16 changes: 9 additions & 7 deletions internal/cmd/agent/deployer/deployer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@ package deployer

import (
"context"
"errors"
"testing"

fleet "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1"

corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
Expand All @@ -32,13 +32,14 @@ func TestSetNamespaceLabelsAndAnnotations(t *testing.T) {
}},
ns: corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{
Name: "namespace1234",
Name: "namespace",
Labels: map[string]string{"kubernetes.io/metadata.name": "namespace"},
},
},
release: "namespace/foo/bar",
expectedNs: corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{
Name: "namespace",
Labels: map[string]string{"kubernetes.io/metadata.name": "namespace", "optLabel1": "optValue1", "optLabel2": "optValue2"},
Annotations: map[string]string{"optAnn1": "optValue1"},
},
Expand All @@ -54,14 +55,15 @@ func TestSetNamespaceLabelsAndAnnotations(t *testing.T) {
}},
ns: corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{
Name: "namespace1234",
Name: "namespace",
Labels: map[string]string{"nsLabel": "nsValue", "kubernetes.io/metadata.name": "namespace"},
Annotations: map[string]string{"nsAnn": "nsValue"},
},
},
release: "namespace/foo/bar",
expectedNs: corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{
Name: "namespace",
Labels: map[string]string{"optLabel": "optValue", "kubernetes.io/metadata.name": "namespace"},
Annotations: map[string]string{},
},
Expand All @@ -77,14 +79,15 @@ func TestSetNamespaceLabelsAndAnnotations(t *testing.T) {
}},
ns: corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{
Name: "namespace1234",
Name: "namespace",
Labels: map[string]string{"bdLabel": "nsValue", "kubernetes.io/metadata.name": "namespace"},
Annotations: map[string]string{"bdAnn": "nsValue"},
},
},
release: "namespace/foo/bar",
expectedNs: corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{
Name: "namespace",
Labels: map[string]string{"bdLabel": "labelUpdated", "kubernetes.io/metadata.name": "namespace"},
Annotations: map[string]string{"bdAnn": "annUpdated"},
},
Expand Down Expand Up @@ -132,7 +135,6 @@ func TestSetNamespaceLabelsAndAnnotationsError(t *testing.T) {
},
}}
release := "test/foo/bar"
expectedErr := errors.New("namespace test not found")

scheme := runtime.NewScheme()
utilruntime.Must(clientgoscheme.AddToScheme(scheme))
Expand All @@ -143,7 +145,7 @@ func TestSetNamespaceLabelsAndAnnotationsError(t *testing.T) {

err := h.setNamespaceLabelsAndAnnotations(context.Background(), bd, release)

if err.Error() != expectedErr.Error() {
t.Errorf("expected error %v: got %v", expectedErr, err)
if !apierrors.IsNotFound(err) {
t.Errorf("expected not found error: got %v", err)
}
}

0 comments on commit a310a17

Please sign in to comment.