Skip to content

Commit 8bb03d2

Browse files
Add unit tests for internal/common helpers
Add unit tests for EnsureSecret, EnsureNetworkAttachments, and EnsureTopology. Extend placement functional coverage for missing and incomplete input secrets. Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 717a627 commit 8bb03d2

8 files changed

Lines changed: 965 additions & 5 deletions

File tree

Makefile

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,10 @@ test: manifests generate fmt vet envtest ginkgo ## Run tests.
155155
OPERATOR_TEMPLATES="$(PWD)/templates" \
156156
$(GINKGO) --trace --cover --coverpkg=../../internal/...,../../api/nova/v1beta1,../../api/placement/v1beta1 --coverprofile cover.out --covermode=atomic --randomize-all ${PROC_CMD} $(GINKGO_ARGS) ./test/...
157157

158+
.PHONY: gotest-unit
159+
gotest-unit: ## Run unit tests under test/unit/ (also run via ginkgo in the test target).
160+
go test -v ./test/unit/...
161+
158162
##@ Build
159163

160164
.PHONY: build

go.mod

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,7 @@ require (
102102
google.golang.org/genproto/googleapis/rpc v0.0.0-20250115164207-1a7da9e5054f // indirect
103103
google.golang.org/grpc v1.71.1 // indirect
104104
google.golang.org/protobuf v1.36.7 // indirect
105+
gopkg.in/evanphx/json-patch.v4 v4.12.0 // indirect
105106
gopkg.in/inf.v0 v0.9.1 // indirect
106107
gopkg.in/yaml.v2 v2.4.0 // indirect
107108
k8s.io/apiextensions-apiserver v0.33.2 // indirect

test/functional/placement/api_controller_test.go

Lines changed: 39 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,35 @@ var _ = Describe("PlacementAPI controller", func() {
176176
})
177177
})
178178

179+
When("a PlacementAPI CR is created pointing to a non existent Secret", func() {
180+
BeforeEach(func() {
181+
DeferCleanup(
182+
th.DeleteInstance,
183+
CreatePlacementAPI(names.PlacementAPIName, GetDefaultPlacementAPISpec()),
184+
)
185+
})
186+
187+
It("is not Ready", func() {
188+
th.ExpectCondition(
189+
names.PlacementAPIName,
190+
ConditionGetterFunc(PlacementConditionGetter),
191+
condition.ReadyCondition,
192+
corev1.ConditionFalse,
193+
)
194+
})
195+
196+
It("is missing the secret", func() {
197+
th.ExpectConditionWithDetails(
198+
names.PlacementAPIName,
199+
ConditionGetterFunc(PlacementConditionGetter),
200+
condition.InputReadyCondition,
201+
corev1.ConditionFalse,
202+
condition.ErrorReason,
203+
fmt.Sprintf("Input data resources missing: secret/%s", SecretName),
204+
)
205+
})
206+
})
207+
179208
When("a secret is provided with missing fields", func() {
180209
BeforeEach(func() {
181210
DeferCleanup(
@@ -190,15 +219,20 @@ var _ = Describe("PlacementAPI controller", func() {
190219
)
191220
})
192221
It("reports that input is not ready", func() {
193-
// FIXME(gibi): This is a bug as placement controller does not
194-
// check the content of the Secret so eventually a dbsync job is
195-
// created with incorrect config
196222
th.ExpectCondition(
197223
names.PlacementAPIName,
198224
ConditionGetterFunc(PlacementConditionGetter),
199225
condition.InputReadyCondition,
200226
corev1.ConditionFalse,
201227
)
228+
th.ExpectConditionWithDetails(
229+
names.PlacementAPIName,
230+
ConditionGetterFunc(PlacementConditionGetter),
231+
condition.InputReadyCondition,
232+
corev1.ConditionFalse,
233+
condition.ErrorReason,
234+
fmt.Sprintf("Input data error occurred field not found in Secret: 'PlacementPassword' not found in secret/%s", SecretName),
235+
)
202236
})
203237
})
204238

@@ -1749,11 +1783,11 @@ var _ = Describe("PlacementAPI reconfiguration", func() {
17491783
mariadb.SimulateMariaDBAccountCompleted(names.MariaDBAccount)
17501784
})
17511785

1752-
It("should set ServiceConfigReady to False", func() {
1786+
It("should set InputReady to False while waiting for the ApplicationCredential secret", func() {
17531787
th.ExpectCondition(
17541788
names.PlacementAPIName,
17551789
ConditionGetterFunc(PlacementConditionGetter),
1756-
condition.ServiceConfigReadyCondition,
1790+
condition.InputReadyCondition,
17571791
corev1.ConditionFalse,
17581792
)
17591793
})

test/unit/common/helpers_test.go

Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,133 @@
1+
/*
2+
Copyright 2026.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package common_test
18+
19+
import (
20+
"context"
21+
"errors"
22+
"testing"
23+
24+
networkv1 "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1"
25+
topologyv1 "github.com/openstack-k8s-operators/infra-operator/apis/topology/v1beta1"
26+
helper "github.com/openstack-k8s-operators/lib-common/modules/common/helper"
27+
corev1 "k8s.io/api/core/v1"
28+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
29+
"k8s.io/apimachinery/pkg/runtime"
30+
"k8s.io/client-go/kubernetes/scheme"
31+
"sigs.k8s.io/controller-runtime/pkg/client"
32+
"sigs.k8s.io/controller-runtime/pkg/client/fake"
33+
"sigs.k8s.io/controller-runtime/pkg/log"
34+
)
35+
36+
const testNamespace = "test-ns"
37+
38+
var (
39+
errTestSecretGet = errors.New("connection refused")
40+
errTestNADGet = errors.New("apiserver unavailable")
41+
)
42+
43+
func newTestInstance() *corev1.ConfigMap {
44+
return &corev1.ConfigMap{
45+
ObjectMeta: metav1.ObjectMeta{
46+
Name: "test-instance",
47+
Namespace: testNamespace,
48+
},
49+
}
50+
}
51+
52+
func newTestHelper(t *testing.T, s *runtime.Scheme, objs ...client.Object) *helper.Helper {
53+
t.Helper()
54+
55+
instance := newTestInstance()
56+
allObjs := append([]client.Object{instance}, objs...)
57+
cl := fake.NewClientBuilder().WithScheme(s).WithObjects(allObjs...).Build()
58+
59+
h, err := helper.NewHelper(instance, cl, nil, s, log.Log)
60+
if err != nil {
61+
t.Fatalf("helper.NewHelper: %v", err)
62+
}
63+
return h
64+
}
65+
66+
func newNetworkTestScheme(t *testing.T) *runtime.Scheme {
67+
t.Helper()
68+
69+
s := runtime.NewScheme()
70+
if err := scheme.AddToScheme(s); err != nil {
71+
t.Fatalf("add core scheme: %v", err)
72+
}
73+
if err := networkv1.AddToScheme(s); err != nil {
74+
t.Fatalf("add network scheme: %v", err)
75+
}
76+
return s
77+
}
78+
79+
func newTopologyTestScheme(t *testing.T) *runtime.Scheme {
80+
t.Helper()
81+
82+
s := runtime.NewScheme()
83+
if err := scheme.AddToScheme(s); err != nil {
84+
t.Fatalf("add core scheme: %v", err)
85+
}
86+
if err := topologyv1.AddToScheme(s); err != nil {
87+
t.Fatalf("add topology scheme: %v", err)
88+
}
89+
return s
90+
}
91+
92+
// errorReader returns a fixed error from Get for EnsureSecret tests.
93+
type errorReader struct {
94+
err error
95+
}
96+
97+
func (r errorReader) Get(_ context.Context, _ client.ObjectKey, _ client.Object, _ ...client.GetOption) error {
98+
return r.err
99+
}
100+
101+
func (r errorReader) List(_ context.Context, _ client.ObjectList, _ ...client.ListOption) error {
102+
return r.err
103+
}
104+
105+
// errorInjectingClient fails Get for NetworkAttachmentDefinition objects.
106+
type errorInjectingClient struct {
107+
client.Client
108+
getErr error
109+
}
110+
111+
func (c *errorInjectingClient) Get(ctx context.Context, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error {
112+
if c.getErr != nil {
113+
if _, ok := obj.(*networkv1.NetworkAttachmentDefinition); ok {
114+
return c.getErr
115+
}
116+
}
117+
return c.Client.Get(ctx, key, obj, opts...)
118+
}
119+
120+
func newTestHelperWithGetError(t *testing.T, s *runtime.Scheme, getErr error, objs ...client.Object) *helper.Helper {
121+
t.Helper()
122+
123+
instance := newTestInstance()
124+
allObjs := append([]client.Object{instance}, objs...)
125+
base := fake.NewClientBuilder().WithScheme(s).WithObjects(allObjs...).Build()
126+
cl := &errorInjectingClient{Client: base, getErr: getErr}
127+
128+
h, err := helper.NewHelper(instance, cl, nil, s, log.Log)
129+
if err != nil {
130+
t.Fatalf("helper.NewHelper: %v", err)
131+
}
132+
return h
133+
}

test/unit/common/network_test.go

Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,162 @@
1+
/*
2+
Copyright 2026.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package common_test
18+
19+
import (
20+
"context"
21+
"fmt"
22+
"strings"
23+
"testing"
24+
"time"
25+
26+
networkv1 "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1"
27+
condition "github.com/openstack-k8s-operators/lib-common/modules/common/condition"
28+
internalcommon "github.com/openstack-k8s-operators/nova-operator/internal/common"
29+
corev1 "k8s.io/api/core/v1"
30+
k8s_errors "k8s.io/apimachinery/pkg/api/errors"
31+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
32+
ctrl "sigs.k8s.io/controller-runtime"
33+
)
34+
35+
func TestEnsureNetworkAttachments_noAttachments(t *testing.T) {
36+
ctx := context.Background()
37+
requeueTimeout := 5 * time.Second
38+
conditions := &condition.Conditions{}
39+
h := newTestHelper(t, newNetworkTestScheme(t))
40+
41+
annotations, result, err := internalcommon.EnsureNetworkAttachments(
42+
ctx, h, nil, conditions, requeueTimeout,
43+
)
44+
if err != nil {
45+
t.Fatalf("expected no error, got %v", err)
46+
}
47+
if result != (ctrl.Result{}) {
48+
t.Fatalf("expected empty result, got %#v", result)
49+
}
50+
want := map[string]string{networkv1.NetworkAttachmentAnnot: "[]"}
51+
if annotations == nil || annotations[networkv1.NetworkAttachmentAnnot] != want[networkv1.NetworkAttachmentAnnot] {
52+
t.Fatalf("expected annotations %v, got %v", want, annotations)
53+
}
54+
if conditions.Get(condition.NetworkAttachmentsReadyCondition) != nil {
55+
t.Fatal("expected NetworkAttachmentsReady condition to be unset")
56+
}
57+
}
58+
59+
func TestEnsureNetworkAttachments_missingNAD(t *testing.T) {
60+
const nadName = "missing-nad"
61+
ctx := context.Background()
62+
requeueTimeout := 5 * time.Second
63+
conditions := &condition.Conditions{}
64+
h := newTestHelper(t, newNetworkTestScheme(t))
65+
66+
annotations, result, err := internalcommon.EnsureNetworkAttachments(
67+
ctx, h, []string{nadName}, conditions, requeueTimeout,
68+
)
69+
if err != nil {
70+
t.Fatalf("expected no error, got %v", err)
71+
}
72+
if result.RequeueAfter != requeueTimeout {
73+
t.Fatalf("expected RequeueAfter %s, got %s", requeueTimeout, result.RequeueAfter)
74+
}
75+
if annotations != nil {
76+
t.Fatalf("expected nil annotations, got %v", annotations)
77+
}
78+
79+
nadReady := conditions.Get(condition.NetworkAttachmentsReadyCondition)
80+
if nadReady == nil {
81+
t.Fatal("expected NetworkAttachmentsReady condition to be set")
82+
}
83+
if nadReady.Status != corev1.ConditionFalse {
84+
t.Fatalf("expected status False, got %q", nadReady.Status)
85+
}
86+
if nadReady.Reason != condition.ErrorReason {
87+
t.Fatalf("expected reason %q, got %q", condition.ErrorReason, nadReady.Reason)
88+
}
89+
wantMessage := fmt.Sprintf(condition.NetworkAttachmentsReadyWaitingMessage, nadName)
90+
if nadReady.Message != wantMessage {
91+
t.Fatalf("expected message %q, got %q", wantMessage, nadReady.Message)
92+
}
93+
}
94+
95+
func TestEnsureNetworkAttachments_getError(t *testing.T) {
96+
const nadName = "broken-nad"
97+
ctx := context.Background()
98+
requeueTimeout := 5 * time.Second
99+
conditions := &condition.Conditions{}
100+
getErr := k8s_errors.NewInternalError(errTestNADGet)
101+
h := newTestHelperWithGetError(t, newNetworkTestScheme(t), getErr)
102+
103+
annotations, result, err := internalcommon.EnsureNetworkAttachments(
104+
ctx, h, []string{nadName}, conditions, requeueTimeout,
105+
)
106+
if err == nil {
107+
t.Fatal("expected error, got nil")
108+
}
109+
if !strings.Contains(err.Error(), "apiserver unavailable") {
110+
t.Fatalf("expected wrapped apiserver error, got %v", err)
111+
}
112+
if result != (ctrl.Result{}) {
113+
t.Fatalf("expected empty result, got %#v", result)
114+
}
115+
if annotations != nil {
116+
t.Fatalf("expected nil annotations, got %v", annotations)
117+
}
118+
119+
nadReady := conditions.Get(condition.NetworkAttachmentsReadyCondition)
120+
if nadReady == nil {
121+
t.Fatal("expected NetworkAttachmentsReady condition to be set")
122+
}
123+
wantMessage := fmt.Sprintf(condition.NetworkAttachmentsErrorMessage, err.Error())
124+
if nadReady.Message != wantMessage {
125+
t.Fatalf("expected condition message %q, got %q", wantMessage, nadReady.Message)
126+
}
127+
}
128+
129+
func TestEnsureNetworkAttachments_nadPresent(t *testing.T) {
130+
const nadName = "internalapi"
131+
ctx := context.Background()
132+
requeueTimeout := 5 * time.Second
133+
conditions := &condition.Conditions{}
134+
scheme := newNetworkTestScheme(t)
135+
nad := &networkv1.NetworkAttachmentDefinition{
136+
ObjectMeta: metav1.ObjectMeta{
137+
Name: nadName,
138+
Namespace: testNamespace,
139+
},
140+
}
141+
h := newTestHelper(t, scheme, nad)
142+
143+
annotations, result, err := internalcommon.EnsureNetworkAttachments(
144+
ctx, h, []string{nadName}, conditions, requeueTimeout,
145+
)
146+
if err != nil {
147+
t.Fatalf("expected no error, got %v", err)
148+
}
149+
if result != (ctrl.Result{}) {
150+
t.Fatalf("expected empty result, got %#v", result)
151+
}
152+
want := fmt.Sprintf(
153+
`[{"name":"%s","namespace":"%s","interface":"%s"}]`,
154+
nadName, testNamespace, nadName,
155+
)
156+
if annotations == nil || annotations[networkv1.NetworkAttachmentAnnot] != want {
157+
t.Fatalf("expected annotation %q, got %v", want, annotations)
158+
}
159+
if conditions.Get(condition.NetworkAttachmentsReadyCondition) != nil {
160+
t.Fatal("expected NetworkAttachmentsReady condition to be unset")
161+
}
162+
}

0 commit comments

Comments
 (0)