Skip to content

Commit e105d4c

Browse files
test: k8s unit test coverage (#604)
* test(k8s): migrate to fake clientset and increase test coverage of pkg/k8s * chore: remove docs * test(k8s): extract shared NewErrorClientset into internal/k8stest
1 parent 2cbc22e commit e105d4c

34 files changed

Lines changed: 1947 additions & 861 deletions

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ _test
2424
dist/
2525
.idea/
2626
.vscode/
27+
.claude/
2728
.vs/
2829
.DS_Store
2930
tmp/

AGENTS.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ The big-picture layers (each is one or more packages under `pkg/`):
4343
- **Config (`pkg/config/`)** — parses the YAML config dir (`clusters`, `node-groups`, `bee-configs`, `checks`, `simulations`), resolves `_inherit` inheritance, and exports into the `orchestration.*Options` types. Read from a local dir or a Git repo (`config-git-repo`).
4444
- **Orchestration (`pkg/orchestration/`)** — backend-agnostic `Cluster``NodeGroup``Node` model. The `k8s/` backend translates it into Kubernetes resources; `notset/` is the no-op fallback when K8s is disabled.
4545
- **Checks (`pkg/check/`)** — each check implements the `beekeeper.Action` interface and is registered by name in the `Checks` map in `pkg/config/check.go`; `pkg/check/runner.go` resolves names to implementations and runs them.
46-
- **Clients**`pkg/bee/` (+ `pkg/bee/api/`) is the HTTP client for a running Bee node; `pkg/k8s/` wraps the Kubernetes client (with fakes under `pkg/k8s/mocks/`); `pkg/swap/` is the Geth/blockchain client.
46+
- **Clients**`pkg/bee/` (+ `pkg/bee/api/`) is the HTTP client for a running Bee node; `pkg/k8s/` wraps the Kubernetes client (tested with client-go's fake clientset; `pkg/k8s/mocks/` holds only the `ClientConfig`/RoundTripper doubles); `pkg/swap/` is the Geth/blockchain client.
4747
- **Operational packages**`stamper`, `nuker`, `restart`, `funder` act on already-running nodes and discover them through `pkg/node/` (`NodeProvider`: Beekeeper cluster, namespace+label, or Helm), not the orchestration layer.
4848

4949
## Deployment / operating modes
@@ -58,7 +58,7 @@ Commands work against clusters provisioned in three different ways — know whic
5858

5959
- **Code style**: prefer clear, idiomatic Go that follows standard best practices and principles — small focused functions, explicit error wrapping with context, no premature abstraction. Keep changes minimal and consistent with the surrounding code.
6060
- **Commits & PR titles**: Conventional Commits, lowercase type, no trailing period, `feat(scope): …` style (enforced by `commitlint.config.js`). Do not push commits; when a commit message is requested, use the subject line only — no body/description.
61-
- **Tests**: prefer external test packages (`package foo_test`); use the `pkg/k8s/mocks` fakes instead of a live cluster. The race detector must pass.
61+
- **Tests**: prefer external test packages (`package foo_test`). Test the `pkg/k8s` clients against client-go's fake clientset instead of a live cluster`fake.NewClientset()` with `PrependReactor` for error paths, and `watch.NewRaceFreeFake` + `PrependWatchReactor` for watch paths (buffer/drive events, bound tests with `context.WithTimeout`, never `time.Sleep`). The only hand-written mock left is `pkg/k8s/mocks` (the `ClientConfig`/`RoundTripper` doubles backing `pkg/k8s/k8s_test.go`). The race detector must pass.
6262
- **Dependencies**: do not add or bump modules unless the task requires it (Dependabot handles routine bumps).
6363
- **Linting**: `gofmt` + `gofumpt` formatting and the linters in `.golangci.yml` (errorlint, errname, nilerr, goconst, misspell, unconvert, copyloopvar) must pass. Note this repo does **not** use BSD copyright file headers — don't add them.
6464

pkg/k8s/configmap/configmap_test.go

Lines changed: 19 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,22 @@
11
package configmap_test
22

33
import (
4-
"context"
4+
"errors"
55
"fmt"
66
"reflect"
77
"testing"
88

9-
"github.com/ethersphere/beekeeper/pkg/k8s/configmap"
10-
"github.com/ethersphere/beekeeper/pkg/k8s/mocks"
119
v1 "k8s.io/api/core/v1"
1210
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1311
"k8s.io/client-go/kubernetes"
1412
"k8s.io/client-go/kubernetes/fake"
13+
14+
"github.com/ethersphere/beekeeper/pkg/k8s/configmap"
15+
"github.com/ethersphere/beekeeper/pkg/k8s/internal/k8stest"
1516
)
1617

1718
func TestSet(t *testing.T) {
19+
t.Parallel()
1820
testTable := []struct {
1921
name string
2022
configName string
@@ -53,22 +55,24 @@ func TestSet(t *testing.T) {
5355
},
5456
{
5557
name: "create_error",
56-
configName: mocks.CreateBad,
57-
clientset: mocks.NewClientset(),
58-
errorMsg: fmt.Errorf("creating configmap create_bad in namespace test: mock error: cannot create config map"),
58+
configName: "test_config_map",
59+
// No object seeded, so Update returns NotFound and Set falls through
60+
// to Create, which the reactor fails.
61+
clientset: k8stest.NewErrorClientset("create", "configmaps", errors.New("mock error: cannot create config map")),
62+
errorMsg: fmt.Errorf("creating configmap test_config_map in namespace test: mock error: cannot create config map"),
5963
},
6064
{
6165
name: "update_error",
62-
configName: mocks.UpdateBad,
63-
clientset: mocks.NewClientset(),
64-
errorMsg: fmt.Errorf("updating configmap update_bad in namespace test: mock error: cannot update config map"),
66+
configName: "test_config_map",
67+
clientset: k8stest.NewErrorClientset("update", "configmaps", errors.New("mock error: cannot update config map")),
68+
errorMsg: fmt.Errorf("updating configmap test_config_map in namespace test: mock error: cannot update config map"),
6569
},
6670
}
6771

6872
for _, test := range testTable {
6973
t.Run(test.name, func(t *testing.T) {
7074
client := configmap.NewClient(test.clientset)
71-
response, err := client.Set(context.Background(), test.configName, "test", test.options)
75+
response, err := client.Set(t.Context(), test.configName, "test", test.options)
7276
if test.errorMsg == nil {
7377
if err != nil {
7478
t.Errorf("error not expected, got: %s", err.Error())
@@ -108,6 +112,7 @@ func TestSet(t *testing.T) {
108112
}
109113

110114
func TestDelete(t *testing.T) {
115+
t.Parallel()
111116
testTable := []struct {
112117
name string
113118
configName string
@@ -136,16 +141,16 @@ func TestDelete(t *testing.T) {
136141
},
137142
{
138143
name: "delete_error",
139-
configName: mocks.DeleteBad,
140-
clientset: mocks.NewClientset(),
141-
errorMsg: fmt.Errorf("deleting configmap delete_bad in namespace test: mock error: cannot delete config map"),
144+
configName: "test_config_map",
145+
clientset: k8stest.NewErrorClientset("delete", "configmaps", errors.New("mock error: cannot delete config map")),
146+
errorMsg: fmt.Errorf("deleting configmap test_config_map in namespace test: mock error: cannot delete config map"),
142147
},
143148
}
144149

145150
for _, test := range testTable {
146151
t.Run(test.name, func(t *testing.T) {
147152
client := configmap.NewClient(test.clientset)
148-
err := client.Delete(context.Background(), test.configName, "test")
153+
err := client.Delete(t.Context(), test.configName, "test")
149154
if test.errorMsg == nil {
150155
if err != nil {
151156
t.Errorf("error not expected, got: %s", err.Error())
Lines changed: 172 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,172 @@
1+
package ingressroute_test
2+
3+
import (
4+
"errors"
5+
"io"
6+
"reflect"
7+
"sort"
8+
"testing"
9+
10+
"github.com/ethersphere/beekeeper/pkg/k8s/customresource/ingressroute"
11+
"github.com/ethersphere/beekeeper/pkg/k8s/customresource/ingressroute/mock"
12+
"github.com/ethersphere/beekeeper/pkg/k8s/ingress"
13+
"github.com/ethersphere/beekeeper/pkg/logging"
14+
apierrors "k8s.io/apimachinery/pkg/api/errors"
15+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
16+
"k8s.io/apimachinery/pkg/runtime/schema"
17+
)
18+
19+
// testIRName is the canonical IngressRoute name used across the ingressroute
20+
// tests.
21+
const testIRName = "ir-0"
22+
23+
func newClient(opts ...mock.Option) *ingressroute.Client {
24+
return ingressroute.NewClient(mock.New(opts...), logging.New(io.Discard, 0))
25+
}
26+
27+
// newIR builds an IngressRoute in namespace "test" with one Route per match.
28+
func newIR(name string, matches ...string) ingressroute.IngressRoute {
29+
ir := ingressroute.IngressRoute{
30+
ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: "test"},
31+
}
32+
for _, m := range matches {
33+
ir.Spec.Routes = append(ir.Spec.Routes, ingressroute.Route{Match: m})
34+
}
35+
return ir
36+
}
37+
38+
func TestClientSet(t *testing.T) {
39+
t.Parallel()
40+
t.Run("create_when_not_found", func(t *testing.T) {
41+
client := newClient()
42+
ing, err := client.Set(t.Context(), testIRName, "test", ingressroute.Options{})
43+
if err != nil {
44+
t.Fatalf("error not expected, got: %s", err.Error())
45+
}
46+
if ing == nil || ing.Name != testIRName {
47+
t.Errorf("expected created ingress route ir-0, got: %#v", ing)
48+
}
49+
})
50+
51+
t.Run("update_when_found", func(t *testing.T) {
52+
client := newClient(mock.WithIngressRoutes(newIR(testIRName)))
53+
ing, err := client.Set(t.Context(), testIRName, "test", ingressroute.Options{})
54+
if err != nil {
55+
t.Fatalf("error not expected, got: %s", err.Error())
56+
}
57+
if ing == nil || ing.Name != testIRName {
58+
t.Errorf("expected updated ingress route ir-0, got: %#v", ing)
59+
}
60+
})
61+
62+
t.Run("get_error", func(t *testing.T) {
63+
client := newClient(mock.WithGetError(errors.New("mock error")))
64+
_, err := client.Set(t.Context(), testIRName, "test", ingressroute.Options{})
65+
if err == nil || err.Error() != "getting ingress route ir-0 in namespace test: mock error" {
66+
t.Errorf("unexpected error: %v", err)
67+
}
68+
})
69+
70+
t.Run("create_error", func(t *testing.T) {
71+
client := newClient(mock.WithCreateError(errors.New("mock error")))
72+
_, err := client.Set(t.Context(), testIRName, "test", ingressroute.Options{})
73+
if err == nil || err.Error() != "creating ingress route ir-0 in namespace test: mock error" {
74+
t.Errorf("unexpected error: %v", err)
75+
}
76+
})
77+
78+
t.Run("update_error", func(t *testing.T) {
79+
client := newClient(mock.WithIngressRoutes(newIR(testIRName)), mock.WithUpdateError(errors.New("mock error")))
80+
_, err := client.Set(t.Context(), testIRName, "test", ingressroute.Options{})
81+
if err == nil || err.Error() != "updating ingress route ir-0 in namespace test: mock error" {
82+
t.Errorf("unexpected error: %v", err)
83+
}
84+
})
85+
}
86+
87+
func TestClientDelete(t *testing.T) {
88+
t.Parallel()
89+
t.Run("delete_existing", func(t *testing.T) {
90+
client := newClient(mock.WithIngressRoutes(newIR(testIRName)))
91+
if err := client.Delete(t.Context(), testIRName, "test"); err != nil {
92+
t.Errorf("error not expected, got: %s", err.Error())
93+
}
94+
})
95+
96+
t.Run("delete_not_found_is_nil", func(t *testing.T) {
97+
client := newClient()
98+
if err := client.Delete(t.Context(), testIRName, "test"); err != nil {
99+
t.Errorf("not-found delete should be nil, got: %s", err.Error())
100+
}
101+
})
102+
103+
t.Run("delete_error", func(t *testing.T) {
104+
client := newClient(mock.WithDeleteError(errors.New("mock error")))
105+
err := client.Delete(t.Context(), testIRName, "test")
106+
if err == nil || err.Error() != "deleting ingress route ir-0 in namespace test: mock error" {
107+
t.Errorf("unexpected error: %v", err)
108+
}
109+
})
110+
}
111+
112+
func TestClientGetNodes(t *testing.T) {
113+
t.Parallel()
114+
sortNodes := func(nodes []ingress.NodeInfo) {
115+
sort.Slice(nodes, func(i, j int) bool {
116+
if nodes[i].Name != nodes[j].Name {
117+
return nodes[i].Name < nodes[j].Name
118+
}
119+
return nodes[i].Host < nodes[j].Host
120+
})
121+
}
122+
123+
t.Run("extracts_hosts", func(t *testing.T) {
124+
client := newClient(mock.WithIngressRoutes(
125+
// the PathPrefix route has no Host(...) → GetHost returns "" → skipped
126+
newIR(testIRName, `Host("a.example.com")`, "PathPrefix(`/x`)"),
127+
newIR("ir-1", `Host("b.example.com")`),
128+
))
129+
nodes, err := client.GetNodes(t.Context(), "test", "")
130+
if err != nil {
131+
t.Fatalf("error not expected, got: %s", err.Error())
132+
}
133+
sortNodes(nodes)
134+
expected := []ingress.NodeInfo{
135+
{Name: testIRName, Host: "a.example.com"},
136+
{Name: "ir-1", Host: "b.example.com"},
137+
}
138+
if !reflect.DeepEqual(nodes, expected) {
139+
t.Errorf("nodes expected: %#v, got: %#v", expected, nodes)
140+
}
141+
})
142+
143+
t.Run("no_routes", func(t *testing.T) {
144+
client := newClient()
145+
nodes, err := client.GetNodes(t.Context(), "test", "")
146+
if err != nil {
147+
t.Fatalf("error not expected, got: %s", err.Error())
148+
}
149+
if nodes != nil {
150+
t.Errorf("nodes expected nil, got: %#v", nodes)
151+
}
152+
})
153+
154+
t.Run("list_not_found_is_nil", func(t *testing.T) {
155+
client := newClient(mock.WithListError(apierrors.NewNotFound(schema.GroupResource{}, "")))
156+
nodes, err := client.GetNodes(t.Context(), "test", "")
157+
if err != nil {
158+
t.Errorf("not-found list should be nil error, got: %s", err.Error())
159+
}
160+
if nodes != nil {
161+
t.Errorf("nodes expected nil, got: %#v", nodes)
162+
}
163+
})
164+
165+
t.Run("list_error", func(t *testing.T) {
166+
client := newClient(mock.WithListError(errors.New("mock error")))
167+
_, err := client.GetNodes(t.Context(), "test", "")
168+
if err == nil || err.Error() != "list ingress routes in namespace test: mock error" {
169+
t.Errorf("unexpected error: %v", err)
170+
}
171+
})
172+
}
Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
package ingressroute_test
2+
3+
import (
4+
"encoding/json"
5+
"net/http"
6+
"net/http/httptest"
7+
"strings"
8+
"testing"
9+
10+
"github.com/ethersphere/beekeeper/pkg/k8s/customresource/ingressroute"
11+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
12+
"k8s.io/client-go/rest"
13+
)
14+
15+
// TestRESTClient exercises the real REST layer (config.NewForConfig +
16+
// ingressRouteClient CRUD/Watch) against an httptest.Server. httptest is used
17+
// instead of client-go's fake RESTClient so the custom Traefik scheme is wired
18+
// up by NewForConfig itself (no manual serializer/scheme scaffolding).
19+
func TestRESTClient(t *testing.T) {
20+
t.Parallel()
21+
apiVersion := ingressroute.GroupName + "/" + ingressroute.GroupVersion
22+
23+
ir := ingressroute.IngressRoute{
24+
TypeMeta: metav1.TypeMeta{APIVersion: apiVersion, Kind: "IngressRoute"},
25+
ObjectMeta: metav1.ObjectMeta{Name: testIRName, Namespace: "test"},
26+
Spec: ingressroute.IngressRouteSpec{Routes: []ingressroute.Route{{Match: `Host("x.example.com")`}}},
27+
}
28+
irList := ingressroute.IngressRouteList{
29+
TypeMeta: metav1.TypeMeta{APIVersion: apiVersion, Kind: "IngressRouteList"},
30+
Items: []ingressroute.IngressRoute{ir},
31+
}
32+
33+
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
34+
w.Header().Set("Content-Type", "application/json")
35+
switch {
36+
case r.Method == http.MethodDelete:
37+
_ = json.NewEncoder(w).Encode(&metav1.Status{
38+
TypeMeta: metav1.TypeMeta{APIVersion: "v1", Kind: "Status"},
39+
Status: metav1.StatusSuccess,
40+
})
41+
case r.Method == http.MethodGet && r.URL.Query().Get("watch") == "true":
42+
w.WriteHeader(http.StatusOK) // empty watch stream
43+
case r.Method == http.MethodGet && strings.HasSuffix(r.URL.Path, "/ingressroutes"):
44+
_ = json.NewEncoder(w).Encode(&irList)
45+
default: // GET by name, POST (create), PUT (update)
46+
_ = json.NewEncoder(w).Encode(&ir)
47+
}
48+
}))
49+
defer server.Close()
50+
51+
client, err := ingressroute.NewForConfig(&rest.Config{Host: server.URL})
52+
if err != nil {
53+
t.Fatalf("NewForConfig: %s", err.Error())
54+
}
55+
irs := client.IngressRoutes("test")
56+
ctx := t.Context()
57+
58+
t.Run("Get", func(t *testing.T) {
59+
got, err := irs.Get(ctx, testIRName, metav1.GetOptions{})
60+
if err != nil {
61+
t.Fatalf("Get: %s", err.Error())
62+
}
63+
if got.Name != testIRName {
64+
t.Errorf("expected name ir-0, got: %q", got.Name)
65+
}
66+
})
67+
68+
t.Run("List", func(t *testing.T) {
69+
list, err := irs.List(ctx, metav1.ListOptions{})
70+
if err != nil {
71+
t.Fatalf("List: %s", err.Error())
72+
}
73+
if len(list.Items) != 1 {
74+
t.Errorf("expected 1 item, got: %d", len(list.Items))
75+
}
76+
})
77+
78+
t.Run("Create", func(t *testing.T) {
79+
got, err := irs.Create(ctx, &ir)
80+
if err != nil {
81+
t.Fatalf("Create: %s", err.Error())
82+
}
83+
if got.Name != testIRName {
84+
t.Errorf("expected name ir-0, got: %q", got.Name)
85+
}
86+
})
87+
88+
t.Run("Update", func(t *testing.T) {
89+
got, err := irs.Update(ctx, &ir, metav1.UpdateOptions{})
90+
if err != nil {
91+
t.Fatalf("Update: %s", err.Error())
92+
}
93+
if got.Name != testIRName {
94+
t.Errorf("expected name ir-0, got: %q", got.Name)
95+
}
96+
})
97+
98+
t.Run("Delete", func(t *testing.T) {
99+
if err := irs.Delete(ctx, testIRName, metav1.DeleteOptions{}); err != nil {
100+
t.Errorf("Delete: %s", err.Error())
101+
}
102+
})
103+
104+
t.Run("Watch", func(t *testing.T) {
105+
watcher, err := irs.Watch(ctx, metav1.ListOptions{})
106+
if err != nil {
107+
t.Fatalf("Watch: %s", err.Error())
108+
}
109+
watcher.Stop()
110+
})
111+
}

0 commit comments

Comments
 (0)