Skip to content

Commit c2ab79b

Browse files
committed
refactor: split parts of the adapter as owned.State
The idea is to split the part which enforces the ownership (but skip things like rate-limiting protection and controller-specific access checks) into a separate package which has exactly same interface. This allows one to easily build a `controller.Reader/Writer` out of `state.State` (e.g. in the unit-tests). Signed-off-by: Andrey Smirnov <[email protected]>
1 parent 5e5068b commit c2ab79b

File tree

7 files changed

+388
-109
lines changed

7 files changed

+388
-109
lines changed

pkg/controller/runtime.go

Lines changed: 9 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212

1313
"github.com/cosi-project/runtime/pkg/resource"
1414
"github.com/cosi-project/runtime/pkg/state"
15+
"github.com/cosi-project/runtime/pkg/state/owned"
1516
)
1617

1718
// ReconcileEvent is a signal for controller to reconcile its resources.
@@ -111,11 +112,7 @@ type Output struct {
111112
// Reader provides read-only access to the state.
112113
//
113114
// Interface [state.State] also satisfies this interface.
114-
type Reader interface {
115-
Get(context.Context, resource.Pointer, ...state.GetOption) (resource.Resource, error)
116-
List(context.Context, resource.Kind, ...state.ListOption) (resource.List, error)
117-
ContextWithTeardown(context.Context, resource.Pointer) (context.Context, error)
118-
}
115+
type Reader = owned.Reader
119116

120117
// UncachedReader provides read-only access to the state without cache.
121118
//
@@ -128,23 +125,10 @@ type UncachedReader interface {
128125
// Writer provides write access to the state.
129126
//
130127
// Only output objects can be written to by the controller.
131-
type Writer interface {
132-
Create(context.Context, resource.Resource) error
133-
Update(context.Context, resource.Resource) error
134-
Modify(context.Context, resource.Resource, func(resource.Resource) error, ...ModifyOption) error
135-
ModifyWithResult(context.Context, resource.Resource, func(resource.Resource) error, ...ModifyOption) (resource.Resource, error)
136-
Teardown(context.Context, resource.Pointer, ...DeleteOption) (bool, error)
137-
Destroy(context.Context, resource.Pointer, ...DeleteOption) error
138-
139-
AddFinalizer(context.Context, resource.Pointer, ...resource.Finalizer) error
140-
RemoveFinalizer(context.Context, resource.Pointer, ...resource.Finalizer) error
141-
}
128+
type Writer = owned.Writer
142129

143130
// ReaderWriter combines Reader and Writer interfaces.
144-
type ReaderWriter interface {
145-
Reader
146-
Writer
147-
}
131+
type ReaderWriter = owned.ReaderWriter
148132

149133
// OutputTracker provides automatic cleanup of the outputs based on the calls to Modify function.
150134
//
@@ -159,64 +143,22 @@ type OutputTracker interface {
159143
}
160144

161145
// DeleteOption for operation Teardown/Destroy.
162-
type DeleteOption func(*DeleteOptions)
163-
164-
// DeleteOptions for operation Teardown/Destroy.
165-
type DeleteOptions struct {
166-
Owner *string
167-
}
146+
type DeleteOption = owned.DeleteOption
168147

169148
// WithOwner allows to specify owner of the resource.
170149
func WithOwner(owner string) DeleteOption {
171-
return func(o *DeleteOptions) {
172-
o.Owner = &owner
173-
}
174-
}
175-
176-
// ToDeleteOptions converts variadic options to DeleteOptions.
177-
func ToDeleteOptions(opts ...DeleteOption) DeleteOptions {
178-
var options DeleteOptions
179-
180-
for _, opt := range opts {
181-
opt(&options)
182-
}
183-
184-
return options
150+
return owned.WithOwner(owner)
185151
}
186152

187153
// ModifyOption for operation Modify.
188-
type ModifyOption func(*ModifyOptions)
189-
190-
// ModifyOptions for operation Modify.
191-
type ModifyOptions struct {
192-
ExpectedPhase *resource.Phase
193-
}
154+
type ModifyOption = owned.ModifyOption
194155

195156
// WithExpectedPhase allows to specify expected phase of the resource.
196157
func WithExpectedPhase(phase resource.Phase) ModifyOption {
197-
return func(o *ModifyOptions) {
198-
o.ExpectedPhase = &phase
199-
}
158+
return owned.WithExpectedPhase(phase)
200159
}
201160

202161
// WithExpectedPhaseAny allows to specify any phase of the resource.
203162
func WithExpectedPhaseAny() ModifyOption {
204-
return func(o *ModifyOptions) {
205-
o.ExpectedPhase = nil
206-
}
207-
}
208-
209-
// ToModifyOptions converts variadic options to ModifyOptions.
210-
func ToModifyOptions(opts ...ModifyOption) ModifyOptions {
211-
phase := resource.PhaseRunning
212-
213-
options := ModifyOptions{
214-
ExpectedPhase: &phase,
215-
}
216-
217-
for _, opt := range opts {
218-
opt(&options)
219-
}
220-
221-
return options
163+
return owned.WithExpectedPhaseAny()
222164
}

pkg/controller/runtime/internal/controllerstate/adapter.go

Lines changed: 14 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -17,15 +17,16 @@ import (
1717
"github.com/cosi-project/runtime/pkg/controller/runtime/internal/cache"
1818
"github.com/cosi-project/runtime/pkg/resource"
1919
"github.com/cosi-project/runtime/pkg/state"
20+
"github.com/cosi-project/runtime/pkg/state/owned"
2021
)
2122

2223
// StateAdapter implements filtered access to the resource state by controller inputs/outputs.
2324
//
2425
// If the read cache is enabled for a resource type, controller.Reader interface will be redirected to the cache.
2526
type StateAdapter struct {
26-
State state.State
27-
Cache *cache.ResourceCache
28-
Name string
27+
OwnedState *owned.State
28+
Cache *cache.ResourceCache
29+
Name string
2930

3031
UpdateLimiter *rate.Limiter
3132
Logger *zap.Logger
@@ -120,7 +121,7 @@ func (adapter *StateAdapter) get(ctx context.Context, disableCache bool, resourc
120121
adapter.Logger.Warn("get uncached resource", zap.String("namespace", resourcePointer.Namespace()), zap.String("type", resourcePointer.Type()), zap.String("id", resourcePointer.ID()))
121122
}
122123

123-
return adapter.State.Get(ctx, resourcePointer, opts...)
124+
return adapter.OwnedState.Get(ctx, resourcePointer, opts...)
124125
}
125126

126127
// List implements controller.Runtime interface.
@@ -147,7 +148,7 @@ func (adapter *StateAdapter) list(ctx context.Context, disableCache bool, resour
147148
adapter.Logger.Warn("list uncached resource", zap.String("namespace", resourceKind.Namespace()), zap.String("type", resourceKind.Type()))
148149
}
149150

150-
return adapter.State.List(ctx, resourceKind, opts...)
151+
return adapter.OwnedState.List(ctx, resourceKind, opts...)
151152
}
152153

153154
// ContextWithTeardown implements controller.Runtime interface.
@@ -164,7 +165,7 @@ func (adapter *StateAdapter) ContextWithTeardown(ctx context.Context, resourcePo
164165
adapter.Logger.Warn("context with teardown on uncached resource", zap.String("namespace", resourcePointer.Namespace()), zap.String("type", resourcePointer.Type()))
165166
}
166167

167-
return adapter.State.ContextWithTeardown(ctx, resourcePointer)
168+
return adapter.OwnedState.ContextWithTeardown(ctx, resourcePointer)
168169
}
169170

170171
// Create implements controller.Runtime interface.
@@ -178,7 +179,7 @@ func (adapter *StateAdapter) Create(ctx context.Context, r resource.Resource) er
178179
r.Metadata().Namespace(), r.Metadata().Type(), adapter.Name, r.Metadata().ID())
179180
}
180181

181-
return adapter.State.Create(ctx, r, state.WithCreateOwner(adapter.Name))
182+
return adapter.OwnedState.Create(ctx, r)
182183
}
183184

184185
// Update implements controller.Runtime interface.
@@ -192,7 +193,7 @@ func (adapter *StateAdapter) Update(ctx context.Context, newResource resource.Re
192193
newResource.Metadata().Namespace(), newResource.Metadata().Type(), adapter.Name, newResource.Metadata().ID())
193194
}
194195

195-
return adapter.State.Update(ctx, newResource, state.WithUpdateOwner(adapter.Name))
196+
return adapter.OwnedState.Update(ctx, newResource)
196197
}
197198

198199
// Modify implements controller.Runtime interface.
@@ -223,16 +224,7 @@ func (adapter *StateAdapter) modify(
223224
emptyResource.Metadata().Namespace(), emptyResource.Metadata().Type(), adapter.Name, emptyResource.Metadata().ID())
224225
}
225226

226-
updateOptions := []state.UpdateOption{state.WithUpdateOwner(adapter.Name)}
227-
228-
modifyOptions := controller.ToModifyOptions(options...)
229-
if modifyOptions.ExpectedPhase != nil {
230-
updateOptions = append(updateOptions, state.WithExpectedPhase(*modifyOptions.ExpectedPhase))
231-
} else {
232-
updateOptions = append(updateOptions, state.WithExpectedPhaseAny())
233-
}
234-
235-
return adapter.State.ModifyWithResult(ctx, emptyResource, updateFunc, updateOptions...)
227+
return adapter.OwnedState.ModifyWithResult(ctx, emptyResource, updateFunc, options...)
236228
}
237229

238230
// AddFinalizer implements controller.Runtime interface.
@@ -245,7 +237,7 @@ func (adapter *StateAdapter) AddFinalizer(ctx context.Context, resourcePointer r
245237
return err
246238
}
247239

248-
return adapter.State.AddFinalizer(ctx, resourcePointer, fins...)
240+
return adapter.OwnedState.AddFinalizer(ctx, resourcePointer, fins...)
249241
}
250242

251243
// RemoveFinalizer implements controller.Runtime interface.
@@ -258,7 +250,7 @@ func (adapter *StateAdapter) RemoveFinalizer(ctx context.Context, resourcePointe
258250
return err
259251
}
260252

261-
err := adapter.State.RemoveFinalizer(ctx, resourcePointer, fins...)
253+
err := adapter.OwnedState.RemoveFinalizer(ctx, resourcePointer, fins...)
262254
if state.IsNotFoundError(err) {
263255
err = nil
264256
}
@@ -276,16 +268,7 @@ func (adapter *StateAdapter) Teardown(ctx context.Context, resourcePointer resou
276268
return false, fmt.Errorf("resource %q/%q is not an output for controller %q, teardown attempted on %q", resourcePointer.Namespace(), resourcePointer.Type(), adapter.Name, resourcePointer.ID())
277269
}
278270

279-
var opts []state.TeardownOption
280-
281-
opOpt := controller.ToDeleteOptions(opOpts...)
282-
if opOpt.Owner != nil {
283-
opts = append(opts, state.WithTeardownOwner(*opOpt.Owner))
284-
} else {
285-
opts = append(opts, state.WithTeardownOwner(adapter.Name))
286-
}
287-
288-
return adapter.State.Teardown(ctx, resourcePointer, opts...)
271+
return adapter.OwnedState.Teardown(ctx, resourcePointer, opOpts...)
289272
}
290273

291274
// Destroy implements controller.Runtime interface.
@@ -298,14 +281,5 @@ func (adapter *StateAdapter) Destroy(ctx context.Context, resourcePointer resour
298281
return fmt.Errorf("resource %q/%q is not an output for controller %q, destroy attempted on %q", resourcePointer.Namespace(), resourcePointer.Type(), adapter.Name, resourcePointer.ID())
299282
}
300283

301-
var opts []state.DestroyOption
302-
303-
opOpt := controller.ToDeleteOptions(opOpts...)
304-
if opOpt.Owner != nil {
305-
opts = append(opts, state.WithDestroyOwner(*opOpt.Owner))
306-
} else {
307-
opts = append(opts, state.WithDestroyOwner(adapter.Name))
308-
}
309-
310-
return adapter.State.Destroy(ctx, resourcePointer, opts...)
284+
return adapter.OwnedState.Destroy(ctx, resourcePointer, opOpts...)
311285
}

pkg/controller/runtime/internal/qruntime/qruntime.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import (
2727
"github.com/cosi-project/runtime/pkg/controller/runtime/metrics"
2828
"github.com/cosi-project/runtime/pkg/logging"
2929
"github.com/cosi-project/runtime/pkg/resource"
30+
"github.com/cosi-project/runtime/pkg/state/owned"
3031
)
3132

3233
type resourceNamespaceType struct {
@@ -105,7 +106,7 @@ func NewAdapter(
105106

106107
return &Adapter{
107108
StateAdapter: controllerstate.StateAdapter{
108-
State: state,
109+
OwnedState: owned.New(state, name),
109110
Cache: adapterOptions.Cache,
110111
Name: name,
111112
UpdateLimiter: rate.NewLimiter(adapterOptions.RuntimeOptions.ChangeRateLimit, adapterOptions.RuntimeOptions.ChangeBurst),

pkg/controller/runtime/internal/rruntime/rruntime.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
"github.com/cosi-project/runtime/pkg/controller/runtime/metrics"
2323
"github.com/cosi-project/runtime/pkg/controller/runtime/options"
2424
"github.com/cosi-project/runtime/pkg/resource"
25+
"github.com/cosi-project/runtime/pkg/state/owned"
2526
)
2627

2728
// Adapter connects common controller-runtime and rruntime controller.
@@ -64,7 +65,7 @@ func NewAdapter(
6465
adapter := &Adapter{
6566
StateAdapter: controllerstate.StateAdapter{
6667
Name: ctrl.Name(),
67-
State: state,
68+
OwnedState: owned.New(state, ctrl.Name()),
6869
Cache: adapterOptions.Cache,
6970
Outputs: slices.Clone(ctrl.Outputs()),
7071
UpdateLimiter: rate.NewLimiter(adapterOptions.RuntimeOptions.ChangeRateLimit, adapterOptions.RuntimeOptions.ChangeBurst),

pkg/state/owned/owned.go

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
// This Source Code Form is subject to the terms of the Mozilla Public
2+
// License, v. 2.0. If a copy of the MPL was not distributed with this
3+
// file, You can obtain one at http://mozilla.org/MPL/2.0/.
4+
5+
// Package owned contains the wrapping state for enforcing ownership.
6+
package owned
7+
8+
import (
9+
"context"
10+
11+
"github.com/cosi-project/runtime/pkg/resource"
12+
"github.com/cosi-project/runtime/pkg/state"
13+
)
14+
15+
// Reader provides read-only access to the state.
16+
//
17+
// Is is identical to the [state.State] interface, and it does not
18+
// provide any additional methods.
19+
type Reader interface {
20+
Get(context.Context, resource.Pointer, ...state.GetOption) (resource.Resource, error)
21+
List(context.Context, resource.Kind, ...state.ListOption) (resource.List, error)
22+
ContextWithTeardown(context.Context, resource.Pointer) (context.Context, error)
23+
}
24+
25+
// Writer provides write access to the state.
26+
//
27+
// Write methods enforce that the resources are owned by the designated owner.
28+
type Writer interface {
29+
Create(context.Context, resource.Resource) error
30+
Update(context.Context, resource.Resource) error
31+
Modify(context.Context, resource.Resource, func(resource.Resource) error, ...ModifyOption) error
32+
ModifyWithResult(context.Context, resource.Resource, func(resource.Resource) error, ...ModifyOption) (resource.Resource, error)
33+
Teardown(context.Context, resource.Pointer, ...DeleteOption) (bool, error)
34+
Destroy(context.Context, resource.Pointer, ...DeleteOption) error
35+
36+
AddFinalizer(context.Context, resource.Pointer, ...resource.Finalizer) error
37+
RemoveFinalizer(context.Context, resource.Pointer, ...resource.Finalizer) error
38+
}
39+
40+
// ReaderWriter combines Reader and Writer interfaces.
41+
type ReaderWriter interface {
42+
Reader
43+
Writer
44+
}
45+
46+
// DeleteOption for operation Teardown/Destroy.
47+
type DeleteOption func(*DeleteOptions)
48+
49+
// DeleteOptions for operation Teardown/Destroy.
50+
type DeleteOptions struct {
51+
Owner *string
52+
}
53+
54+
// WithOwner allows to specify owner of the resource.
55+
func WithOwner(owner string) DeleteOption {
56+
return func(o *DeleteOptions) {
57+
o.Owner = &owner
58+
}
59+
}
60+
61+
// ToDeleteOptions converts variadic options to DeleteOptions.
62+
func ToDeleteOptions(opts ...DeleteOption) DeleteOptions {
63+
var options DeleteOptions
64+
65+
for _, opt := range opts {
66+
opt(&options)
67+
}
68+
69+
return options
70+
}
71+
72+
// ModifyOption for operation Modify.
73+
type ModifyOption func(*ModifyOptions)
74+
75+
// ModifyOptions for operation Modify.
76+
type ModifyOptions struct {
77+
ExpectedPhase *resource.Phase
78+
}
79+
80+
// WithExpectedPhase allows to specify expected phase of the resource.
81+
func WithExpectedPhase(phase resource.Phase) ModifyOption {
82+
return func(o *ModifyOptions) {
83+
o.ExpectedPhase = &phase
84+
}
85+
}
86+
87+
// WithExpectedPhaseAny allows to specify any phase of the resource.
88+
func WithExpectedPhaseAny() ModifyOption {
89+
return func(o *ModifyOptions) {
90+
o.ExpectedPhase = nil
91+
}
92+
}
93+
94+
// ToModifyOptions converts variadic options to ModifyOptions.
95+
func ToModifyOptions(opts ...ModifyOption) ModifyOptions {
96+
phase := resource.PhaseRunning
97+
98+
options := ModifyOptions{
99+
ExpectedPhase: &phase,
100+
}
101+
102+
for _, opt := range opts {
103+
opt(&options)
104+
}
105+
106+
return options
107+
}

0 commit comments

Comments
 (0)