Skip to content

Commit b2a5f12

Browse files
lexfreilexfrei
committed
build(kubernetes): run the tests of the packages the patches touch
The cluster-autoscaler and kubevirt-cloud-provider image builds ran go build only, and go build never compiles a _test.go, so nothing ran the tests of the code the patches change. Run go test in the builder stage on the packages the patches touch. The list is read from the patches themselves, so a new patch is covered without an edit to the Dockerfile. Both builder stages export GOOS and GOARCH for the target, so the tests unset them: a local make image builds amd64 and arm64 by default, and a test binary for the other architecture runs on the build host only under binfmt emulation. The first run of that step on cluster-autoscaler found the carried downscale patch out of step with the upstream tests it leaves in place: SetSize no longer refuses a size above maxSize, while one case of TestNodeGroupIncreaseSizeErrors still expected that refusal. The patch now drops that case, and carries a test for the downscale it is named for instead. DeleteNodes lowers a group one node at a time through SetSize, so with the refusal in place a group standing above its max size could not shed a node at all. The new test deletes a node from such a group and expects the replica count to drop; against the unpatched SetSize it fails with the refusal the patch removes. Scale-up past maxSize stays bounded by the core scale-up orchestrator, which skips a node group already at MaxSize. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <[email protected]>
1 parent 070d235 commit b2a5f12

3 files changed

Lines changed: 105 additions & 0 deletions

File tree

‎packages/apps/kubernetes/images/cluster-autoscaler/Dockerfile‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,22 @@ RUN git clone https://github.com/kubernetes/autoscaler /src/autoscaler \
1616
WORKDIR /src/autoscaler/cluster-autoscaler
1717
COPY fix-downscale.diff /fix-downscale.diff
1818
RUN git apply /fix-downscale.diff
19+
20+
# go build never compiles a _test.go, so nothing else runs the tests of the
21+
# packages the patch touches. Each package is tested from its own directory,
22+
# below the clone root because the patch names paths from there: that finds
23+
# the module it belongs to, and an empty list runs nothing.
24+
# The list is read into a variable first: a for loop drops the exit status
25+
# of a command substitution, so a patch git cannot read would leave the list
26+
# empty and the step green.
27+
# The stage sets GOOS and GOARCH to the target for the whole stage, so the
28+
# tests drop them: a test binary built for another architecture cannot run
29+
# on the build host without emulation.
30+
RUN cd /src/autoscaler && files=$(git apply --numstat /fix-downscale.diff) || exit 1; \
31+
for dir in $(printf '%s\n' "$files" | cut -f3 | grep '\.go$' | xargs -r -n1 dirname | sort -u); do \
32+
(cd "$dir" && env -u GOOS -u GOARCH go test .) || exit 1; \
33+
done
34+
1935
RUN make build
2036

2137
FROM $BASEIMAGE

‎packages/apps/kubernetes/images/cluster-autoscaler/fix-downscale.diff‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,3 +11,78 @@ index 4eec0e4bf..f28fd9241 100644
1111
case nreplicas < r.minSize:
1212
return fmt.Errorf("size decrease too large - desired:%d min:%d", nreplicas, r.minSize)
1313
}
14+
diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup_test.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup_test.go
15+
--- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup_test.go
16+
+++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup_test.go
17+
@@ -241,11 +241,6 @@
18+
delta: -1,
19+
initial: 3,
20+
errorMsg: "size increase must be positive",
21+
- }, {
22+
- description: "errors because initial+delta > maxSize",
23+
- delta: 8,
24+
- initial: 3,
25+
- errorMsg: "size increase too large - desired:11 max:10",
26+
}}
27+
28+
test := func(t *testing.T, tc *testCase, testConfig *testConfig) {
29+
@@ -767,7 +762,59 @@
30+
)
31+
})
32+
}
33+
+
34+
+// A group can stand above its max size, for one when the annotation is
35+
+// lowered under it, and deleting a node from it must not be refused as an
36+
+// increase.
37+
+func TestNodeGroupDeleteNodesAboveMaxSize(t *testing.T) {
38+
+ test := func(t *testing.T, testConfig *testConfig) {
39+
+ controller, stop := mustCreateTestController(t, testConfig)
40+
+ defer stop()
41+
+
42+
+ nodegroups, err := controller.nodeGroups()
43+
+ if err != nil {
44+
+ t.Fatalf("unexpected error: %v", err)
45+
+ }
46+
47+
+ if l := len(nodegroups); l != 1 {
48+
+ t.Fatalf("expected 1 nodegroup, got %d", l)
49+
+ }
50+
+
51+
+ ng := nodegroups[0].(*nodegroup)
52+
+ if err := ng.DeleteNodes(testConfig.nodes[:1]); err != nil {
53+
+ t.Fatalf("unexpected error: %v", err)
54+
+ }
55+
+
56+
+ gvr, err := ng.scalableResource.GroupVersionResource()
57+
+ if err != nil {
58+
+ t.Fatalf("unexpected error: %v", err)
59+
+ }
60+
+
61+
+ scalableResource, err := ng.machineController.managementScaleClient.Scales(testConfig.spec.namespace).
62+
+ Get(context.TODO(), gvr.GroupResource(), ng.scalableResource.Name(), metav1.GetOptions{})
63+
+ if err != nil {
64+
+ t.Fatalf("unexpected error: %v", err)
65+
+ }
66+
+
67+
+ if scalableResource.Spec.Replicas != 3 {
68+
+ t.Errorf("expected 3, got %v", scalableResource.Spec.Replicas)
69+
+ }
70+
+ }
71+
+
72+
+ annotations := map[string]string{
73+
+ nodeGroupMinSizeAnnotationKey: "1",
74+
+ nodeGroupMaxSizeAnnotationKey: "2",
75+
+ }
76+
+
77+
+ t.Run("MachineSet", func(t *testing.T) {
78+
+ test(t, createMachineSetTestConfig(RandomString(6), RandomString(6), RandomString(6), 4, annotations, nil))
79+
+ })
80+
+
81+
+ t.Run("MachineDeployment", func(t *testing.T) {
82+
+ test(t, createMachineDeploymentTestConfig(RandomString(6), RandomString(6), RandomString(6), 4, annotations, nil))
83+
+ })
84+
+}
85+
+
86+
func TestNodeGroupMachineSetDeleteNodesWithMismatchedNodes(t *testing.T) {
87+
test := func(t *testing.T, expected int, testConfigs []*testConfig) {
88+
testConfig0, testConfig1 := testConfigs[0], testConfigs[1]

‎packages/apps/kubernetes/images/kubevirt-cloud-provider/Dockerfile‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,20 @@ RUN go get 'k8s.io/endpointslice/[email protected]' 'k8s.io/[email protected]'
1919
RUN go mod tidy
2020
RUN go mod vendor
2121

22+
# go build never compiles a _test.go, so nothing else runs the tests of the
23+
# packages the patches touch. Each package is tested from its own directory:
24+
# that finds the module it belongs to, and an empty list runs nothing.
25+
# The list is read into a variable first: a for loop drops the exit status
26+
# of a command substitution, so a patch git cannot read would leave the list
27+
# empty and the step green.
28+
# The stage sets GOOS and GOARCH to the target for the whole stage, so the
29+
# tests drop them: a test binary built for another architecture cannot run
30+
# on the build host without emulation.
31+
RUN files=$(git apply --numstat /patches/*.diff) || exit 1; \
32+
for dir in $(printf '%s\n' "$files" | cut -f3 | grep '\.go$' | xargs -r -n1 dirname | sort -u); do \
33+
(cd "$dir" && env -u GOOS -u GOARCH go test -mod=vendor .) || exit 1; \
34+
done
35+
2236
RUN CGO_ENABLED=0 go build -mod=vendor -ldflags="-s -w" -o bin/kubevirt-cloud-controller-manager ./cmd/kubevirt-cloud-controller-manager
2337

2438
FROM scratch

0 commit comments

Comments
 (0)