Sitelet https://github.com/kubernetes/kops/commit/1f1729523ef646483e1231dd641c39cb7aa62d82
Skip to content

Commit 1f17295

Browse files
kubernetes-prow[bot]kubernetes-prow[bot]
authored
Merge pull request #18690 from hakman/fi-constant-method-names
upup/pkg/fi: use literal method names for task dispatch
2 parents 0ef4755 + b8f6034 commit 1f17295

3 files changed

Lines changed: 56 additions & 51 deletions

File tree

‎upup/pkg/fi/context.go‎

Lines changed: 36 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -196,19 +196,41 @@ func (c *Context[T]) Render(a, e, changes Task[T]) error {
196196

197197
targetType := reflect.ValueOf(c.Target).Type()
198198

199-
var renderer *reflect.Method
199+
// Probe renderers with literal method names only: enumerating the method set or passing a
200+
// variable name to MethodByName would disable linker pruning of every unused exported method.
201+
candidates := []struct {
202+
name string
203+
method reflect.Value
204+
}{
205+
{"Render", v.MethodByName("Render")},
206+
{"RenderAWS", v.MethodByName("RenderAWS")},
207+
{"RenderAzure", v.MethodByName("RenderAzure")},
208+
{"RenderDO", v.MethodByName("RenderDO")},
209+
{"RenderGCE", v.MethodByName("RenderGCE")},
210+
{"RenderHetzner", v.MethodByName("RenderHetzner")},
211+
{"RenderInstall", v.MethodByName("RenderInstall")},
212+
{"RenderLinode", v.MethodByName("RenderLinode")},
213+
{"RenderLocal", v.MethodByName("RenderLocal")},
214+
{"RenderOpenstack", v.MethodByName("RenderOpenstack")},
215+
{"RenderScw", v.MethodByName("RenderScw")},
216+
{"RenderSubnet", v.MethodByName("RenderSubnet")},
217+
{"RenderTerraform", v.MethodByName("RenderTerraform")},
218+
}
219+
220+
var rendererName string
221+
var renderer reflect.Value
200222
var rendererArgs []reflect.Value
201223

202-
for i := 0; i < vType.NumMethod(); i++ {
203-
method := vType.Method(i)
204-
if !strings.HasPrefix(method.Name, "Render") {
224+
for _, candidate := range candidates {
225+
if !candidate.method.IsValid() {
205226
continue
206227
}
228+
mType := candidate.method.Type()
207229
match := true
208230

209231
var args []reflect.Value
210-
for j := 0; j < method.Type.NumIn(); j++ {
211-
arg := method.Type.In(j)
232+
for j := 0; j < mType.NumIn(); j++ {
233+
arg := mType.In(j)
212234
if vType.ConvertibleTo(arg) {
213235
continue
214236
}
@@ -224,28 +246,28 @@ func (c *Context[T]) Render(a, e, changes Task[T]) error {
224246
break
225247
}
226248
if match {
227-
if renderer != nil {
228-
if method.Name == "Render" {
249+
if renderer.IsValid() {
250+
if candidate.name == "Render" {
229251
continue
230252
}
231-
if renderer.Name != "Render" {
253+
if rendererName != "Render" {
232254
return fmt.Errorf("found multiple Render methods that could be involved on %T", e)
233255
}
234256
}
235-
renderer = &method
257+
rendererName = candidate.name
258+
renderer = candidate.method
236259
rendererArgs = args
237260
}
238261

239262
}
240-
if renderer == nil {
263+
if !renderer.IsValid() {
241264
return fmt.Errorf("could not find Render method on type %T (target %T)", e, c.Target)
242265
}
243266
rendererArgs = append(rendererArgs, reflect.ValueOf(a))
244267
rendererArgs = append(rendererArgs, reflect.ValueOf(e))
245268
rendererArgs = append(rendererArgs, reflect.ValueOf(changes))
246-
klog.V(11).Infof("Calling method %s on %T", renderer.Name, e)
247-
m := v.MethodByName(renderer.Name)
248-
rv := m.Call(rendererArgs)
269+
klog.V(11).Infof("Calling method %s on %T", rendererName, e)
270+
rv := renderer.Call(rendererArgs)
249271
var rvErr error
250272
if !rv[0].IsNil() {
251273
rvErr = rv[0].Interface().(error)

‎upup/pkg/fi/default_methods.go‎

Lines changed: 20 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -137,25 +137,30 @@ func defaultDeltaRunMethod[T SubContext](e Task[T], c *Context[T]) error {
137137
return nil
138138
}
139139

140-
// invokeCheckChanges calls the checkChanges method by reflection
140+
// invokeCheckChanges calls the CheckChanges method by reflection. Here and below, MethodByName
141+
// must be called with literal names to keep linker method pruning working (see Context.Render).
141142
func invokeCheckChanges[T SubContext](a, e, changes Task[T]) error {
142-
rv, err := reflectutils.InvokeMethod(e, "CheckChanges", a, e, changes)
143-
if err != nil {
144-
return err
143+
m := reflect.ValueOf(e).MethodByName("CheckChanges")
144+
if !m.IsValid() {
145+
return &reflectutils.MethodNotFoundError{Name: "CheckChanges", Target: e}
145146
}
147+
rv := m.Call([]reflect.Value{reflect.ValueOf(a), reflect.ValueOf(e), reflect.ValueOf(changes)})
148+
var err error
146149
if !rv[0].IsNil() {
147150
err = rv[0].Interface().(error)
148151
}
149152
return err
150153
}
151154

152-
// invokeFind calls the find method by reflection
155+
// invokeFind calls the Find method by reflection.
153156
func invokeFind[T SubContext](e Task[T], c *Context[T]) (Task[T], error) {
154-
rv, err := reflectutils.InvokeMethod(e, "Find", c)
155-
if err != nil {
156-
return nil, err
157+
m := reflect.ValueOf(e).MethodByName("Find")
158+
if !m.IsValid() {
159+
return nil, &reflectutils.MethodNotFoundError{Name: "Find", Target: e}
157160
}
161+
rv := m.Call([]reflect.Value{reflect.ValueOf(c)})
158162
var task Task[T]
163+
var err error
159164
if !rv[0].IsNil() {
160165
task = rv[0].Interface().(Task[T])
161166
}
@@ -165,16 +170,16 @@ func invokeFind[T SubContext](e Task[T], c *Context[T]) (Task[T], error) {
165170
return task, err
166171
}
167172

168-
// invokeShouldCreate calls the ShouldCreate method by reflection, if it exists
173+
// invokeShouldCreate calls the ShouldCreate method by reflection; tasks without it are created
174+
// by default.
169175
func invokeShouldCreate[T SubContext](a, e, changes Task[T]) (bool, error) {
170-
rv, err := reflectutils.InvokeMethod(e, "ShouldCreate", a, e, changes)
171-
if err != nil {
172-
if reflectutils.IsMethodNotFound(err) {
173-
return true, nil
174-
}
175-
return false, err
176+
m := reflect.ValueOf(e).MethodByName("ShouldCreate")
177+
if !m.IsValid() {
178+
return true, nil
176179
}
180+
rv := m.Call([]reflect.Value{reflect.ValueOf(a), reflect.ValueOf(e), reflect.ValueOf(changes)})
177181
shouldCreate := rv[0].Interface().(bool)
182+
var err error
178183
if !rv[1].IsNil() {
179184
err = rv[1].Interface().(error)
180185
}

‎util/pkg/reflectutils/walk.go‎

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -57,28 +57,6 @@ func JSONMergeStruct(dest, src interface{}) {
5757
}
5858
}
5959

60-
// InvokeMethod calls the specified method by reflection
61-
func InvokeMethod(target interface{}, name string, args ...interface{}) ([]reflect.Value, error) {
62-
v := reflect.ValueOf(target)
63-
64-
method, found := v.Type().MethodByName(name)
65-
if !found {
66-
return nil, &MethodNotFoundError{
67-
Name: name,
68-
Target: target,
69-
}
70-
}
71-
72-
var argValues []reflect.Value
73-
for _, a := range args {
74-
argValues = append(argValues, reflect.ValueOf(a))
75-
}
76-
klog.V(12).Infof("Calling method %s on %T", method.Name, target)
77-
m := v.MethodByName(method.Name)
78-
rv := m.Call(argValues)
79-
return rv, nil
80-
}
81-
8260
func BuildTypeName(t reflect.Type) string {
8361
switch t.Kind() {
8462
case reflect.Ptr:

0 commit comments

Comments
 (0)