Skip to content

Commit de18545

Browse files
author
Dmitriy Matrenichev
committed
fix: ensure that controller conformance tests work over net too
Otherwise, buggy `FromProto|MarshalProto` methods wil go unnoticed. Signed-off-by: Dmitriy Matrenichev <dmitry.matrenichev@siderolabs.com>
1 parent aa632ea commit de18545

File tree

4 files changed

+89
-19
lines changed

4 files changed

+89
-19
lines changed

go.mod

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,4 +47,7 @@ require (
4747
google.golang.org/genproto/googleapis/rpc v0.0.0-20241202173237-19429a94021a // indirect
4848
)
4949

50-
retract v0.4.7 // Wait with locked mutex leads to the deadlock
50+
retract (
51+
v0.7.3 // Typo in the test type result
52+
v0.4.7 // Wait with locked mutex leads to the deadlock
53+
)

pkg/controller/conformance/resources.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ func NewStrResource(ns resource.Namespace, id resource.ID, value string) *StrRes
6161
type strSpec struct{ ValueGetSet[string] } //nolint:recvcheck
6262

6363
func (s *strSpec) FromProto(bytes []byte) { s.value = string(bytes) }
64-
func (s strSpec) MarshalProto() ([]byte, error) { return []byte(s.value + "stuff"), nil }
64+
func (s strSpec) MarshalProto() ([]byte, error) { return []byte(s.value), nil }
6565

6666
// SentenceResourceType is the type of SentenceResource.
6767
const SentenceResourceType = resource.Type("test/sentence")

pkg/controller/conformance/runtime.go

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"expvar"
1010
"fmt"
1111
"math/rand"
12+
"strings"
1213
"sync"
1314
"time"
1415

@@ -31,8 +32,8 @@ type RuntimeSuite struct { //nolint:govet
3132

3233
Runtime controller.Engine
3334

34-
SetupRuntime func()
35-
TearDownRuntime func()
35+
SetupRuntime func(suite *RuntimeSuite)
36+
TearDownRuntime func(suite *RuntimeSuite)
3637

3738
MetricsReadCacheEnabled bool
3839

@@ -42,12 +43,15 @@ type RuntimeSuite struct { //nolint:govet
4243
ctxCancel context.CancelFunc
4344
}
4445

46+
// Context provides the context for the test suite.
47+
func (suite *RuntimeSuite) Context() context.Context { return suite.ctx }
48+
4549
// SetupTest ...
4650
func (suite *RuntimeSuite) SetupTest() {
4751
suite.ctx, suite.ctxCancel = context.WithTimeout(context.Background(), 3*time.Minute)
4852

4953
if suite.SetupRuntime != nil {
50-
suite.SetupRuntime()
54+
suite.SetupRuntime(suite)
5155
}
5256
}
5357

@@ -57,7 +61,12 @@ func (suite *RuntimeSuite) startRuntime(ctx context.Context) {
5761
go func() {
5862
defer suite.wg.Done()
5963

60-
suite.Assert().NoError(suite.Runtime.Run(ctx))
64+
err := suite.Runtime.Run(ctx)
65+
// we can safely ignore canceled error,
66+
// but we can't check for it using errors.Is because it's a rpc error
67+
if err != nil && !strings.Contains(err.Error(), "context canceled") {
68+
suite.Assert().NoError(err)
69+
}
6170
}()
6271
}
6372

@@ -161,7 +170,7 @@ func (suite *RuntimeSuite) TearDownTest() {
161170
suite.Assert().NoError(suite.State.Create(context.Background(), NewSentenceResource("sentences", "xxx", "")))
162171

163172
if suite.TearDownRuntime != nil {
164-
suite.TearDownRuntime()
173+
suite.TearDownRuntime(suite)
165174
}
166175
}
167176

pkg/controller/runtime/runtime_test.go

Lines changed: 70 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,31 +6,51 @@ package runtime_test
66

77
import (
88
"context"
9+
"net"
910
goruntime "runtime"
1011
"strconv"
1112
"testing"
1213
"time"
1314

15+
"github.com/siderolabs/gen/xtesting/must"
1416
"github.com/stretchr/testify/assert"
1517
"github.com/stretchr/testify/require"
1618
suiterunner "github.com/stretchr/testify/suite"
1719
"go.uber.org/goleak"
1820
"go.uber.org/zap/zaptest"
21+
"google.golang.org/grpc"
22+
"google.golang.org/grpc/credentials/insecure"
1923

24+
"github.com/cosi-project/runtime/api/v1alpha1"
2025
"github.com/cosi-project/runtime/pkg/controller/conformance"
2126
"github.com/cosi-project/runtime/pkg/controller/runtime"
2227
"github.com/cosi-project/runtime/pkg/controller/runtime/options"
2328
"github.com/cosi-project/runtime/pkg/future"
2429
"github.com/cosi-project/runtime/pkg/resource"
30+
"github.com/cosi-project/runtime/pkg/resource/protobuf"
2531
"github.com/cosi-project/runtime/pkg/safe"
2632
"github.com/cosi-project/runtime/pkg/state"
2733
stateconformance "github.com/cosi-project/runtime/pkg/state/conformance"
2834
"github.com/cosi-project/runtime/pkg/state/impl/inmem"
2935
"github.com/cosi-project/runtime/pkg/state/impl/namespaced"
36+
"github.com/cosi-project/runtime/pkg/state/protobuf/client"
37+
"github.com/cosi-project/runtime/pkg/state/protobuf/server"
3038
)
3139

40+
func noError(err error) {
41+
if err != nil {
42+
panic(err)
43+
}
44+
}
45+
46+
func init() {
47+
noError(protobuf.RegisterResource(conformance.IntResourceType, &conformance.IntResource{}))
48+
noError(protobuf.RegisterResource(conformance.StrResourceType, &conformance.StrResource{}))
49+
noError(protobuf.RegisterResource(conformance.SentenceResourceType, &conformance.SentenceResource{}))
50+
}
51+
3252
func TestRuntimeConformance(t *testing.T) {
33-
for _, tt := range []struct {
53+
tests := []struct {
3454
name string
3555
opts []options.Option
3656
metricsReadCacheEnabled bool
@@ -68,25 +88,63 @@ func TestRuntimeConformance(t *testing.T) {
6888
options.WithWarnOnUncachedReads(true),
6989
},
7090
},
71-
} {
91+
}
92+
93+
for _, tt := range tests {
7294
t.Run(tt.name, func(t *testing.T) {
7395
t.Cleanup(func() { goleak.VerifyNone(t, goleak.IgnoreCurrent()) })
7496

75-
suite := &conformance.RuntimeSuite{
97+
suiterunner.Run(t, &conformance.RuntimeSuite{
7698
MetricsReadCacheEnabled: tt.metricsReadCacheEnabled,
77-
}
78-
suite.SetupRuntime = func() {
79-
suite.State = state.WrapCore(namespaced.NewState(inmem.Build))
99+
SetupRuntime: func(rs *conformance.RuntimeSuite) {
100+
rs.State = state.WrapCore(namespaced.NewState(inmem.Build))
101+
logger := zaptest.NewLogger(rs.T())
102+
rs.Runtime = must.Value(runtime.NewRuntime(rs.State, logger, tt.opts...))(rs.T())
103+
},
104+
})
105+
})
106+
}
80107

81-
var err error
108+
const listenOn = "127.0.0.1:0"
82109

83-
logger := zaptest.NewLogger(t)
110+
t.Log("testing networked runtime")
84111

85-
suite.Runtime, err = runtime.NewRuntime(suite.State, logger, tt.opts...)
86-
suite.Require().NoError(err)
87-
}
112+
for _, tt := range tests {
113+
t.Run(tt.name+"_over-network", func(t *testing.T) {
114+
t.Cleanup(func() { goleak.VerifyNone(t, goleak.IgnoreCurrent()) })
115+
116+
suiterunner.Run(t, &conformance.RuntimeSuite{
117+
MetricsReadCacheEnabled: tt.metricsReadCacheEnabled,
118+
SetupRuntime: func(rs *conformance.RuntimeSuite) {
119+
l := must.Value(net.Listen("tcp", listenOn))(rs.T())
120+
121+
grpcServer := grpc.NewServer()
122+
inmemState := state.WrapCore(namespaced.NewState(inmem.Build))
123+
v1alpha1.RegisterStateServer(grpcServer, server.NewState(inmemState))
124+
125+
go func() { assert.NoError(rs.T(), grpcServer.Serve(l)) }()
126+
127+
rs.T().Cleanup(func() { grpcServer.Stop() })
128+
129+
grpcConn := must.Value(grpc.NewClient(
130+
l.Addr().String(),
131+
grpc.WithTransportCredentials(insecure.NewCredentials()),
132+
))(rs.T())
133+
134+
rs.T().Cleanup(func() { assert.NoError(rs.T(), grpcConn.Close()) })
135+
136+
stateClient := v1alpha1.NewStateClient(grpcConn)
137+
rs.State = state.WrapCore(client.NewAdapter(stateClient))
138+
139+
must.Value(rs.State.List(rs.Context(), conformance.NewIntResource("default", "zero", 0).Metadata()))(rs.T())
88140

89-
suiterunner.Run(t, suite)
141+
rs.Runtime = must.Value(runtime.NewRuntime(
142+
rs.State,
143+
zaptest.NewLogger(rs.T()),
144+
tt.opts...,
145+
))(rs.T())
146+
},
147+
})
90148
})
91149
}
92150
}

0 commit comments

Comments
 (0)