Skip to content

Commit b2d9e15

Browse files
committed
apis/nfd: drop the private template caching fields
Drop the private fields – that were supposed to be used for caching parsed templates – from the Rule type. Keep the API typedefs cleaner and simpler. Moreover, the caching was not even used in practice, effectively complicating code without any benefit: the way the types are used in nfd-master creates a local copy of Rule type storing the cached template in the copy, wasting it from any future users. There are also other possible caveats in caching like we tried to do it. For example the objects returned by the api lister are supposed to be treated as read-only - in particular if we would be to modify them there should at least be proper locking in place as nfd-master potentially processes the same rule (the same Go object) in parallel for multiple nodes. If any optimization like this will be pursued it should be done properly, probably with private type(s) at the consumer's end, not contaminating the API types.
1 parent 443ff80 commit b2d9e15

File tree

4 files changed

+9
-47
lines changed

4 files changed

+9
-47
lines changed

pkg/apis/nfd/v1alpha1/rule.go

Lines changed: 9 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -112,15 +112,12 @@ func (r *Rule) executeLabelsTemplate(in matchedFeatures, out map[string]string)
112112
return nil
113113
}
114114

115-
if r.labelsTemplate == nil {
116-
t, err := newTemplateHelper(r.LabelsTemplate)
117-
if err != nil {
118-
return fmt.Errorf("failed to parse LabelsTemplate: %w", err)
119-
}
120-
r.labelsTemplate = t
115+
th, err := newTemplateHelper(r.LabelsTemplate)
116+
if err != nil {
117+
return fmt.Errorf("failed to parse LabelsTemplate: %w", err)
121118
}
122119

123-
labels, err := r.labelsTemplate.expandMap(in)
120+
labels, err := th.expandMap(in)
124121
if err != nil {
125122
return fmt.Errorf("failed to expand LabelsTemplate: %w", err)
126123
}
@@ -134,15 +131,13 @@ func (r *Rule) executeVarsTemplate(in matchedFeatures, out map[string]string) er
134131
if r.VarsTemplate == "" {
135132
return nil
136133
}
137-
if r.varsTemplate == nil {
138-
t, err := newTemplateHelper(r.VarsTemplate)
139-
if err != nil {
140-
return err
141-
}
142-
r.varsTemplate = t
134+
135+
th, err := newTemplateHelper(r.VarsTemplate)
136+
if err != nil {
137+
return err
143138
}
144139

145-
vars, err := r.varsTemplate.expandMap(in)
140+
vars, err := th.expandMap(in)
146141
if err != nil {
147142
return err
148143
}
@@ -216,22 +211,6 @@ func newTemplateHelper(name string) (*templateHelper, error) {
216211
return &templateHelper{template: tmpl}, nil
217212
}
218213

219-
// DeepCopy is a stub to augment the auto-generated code
220-
func (h *templateHelper) DeepCopy() *templateHelper {
221-
if h == nil {
222-
return nil
223-
}
224-
out := new(templateHelper)
225-
h.DeepCopyInto(out)
226-
return out
227-
}
228-
229-
// DeepCopyInto is a stub to augment the auto-generated code
230-
func (h *templateHelper) DeepCopyInto(out *templateHelper) {
231-
// HACK: just re-use the template
232-
out.template = h.template
233-
}
234-
235214
func (h *templateHelper) execute(data interface{}) (string, error) {
236215
var tmp bytes.Buffer
237216
if err := h.template.Execute(&tmp, data); err != nil {

pkg/apis/nfd/v1alpha1/rule_test.go

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -369,30 +369,25 @@ var-2=
369369
assert.Equal(t, map[string]string{"foo": "bar"}, m.Labels, "instances should have matched")
370370
assert.Empty(t, m.Vars)
371371

372-
r2.labelsTemplate = nil
373372
r2.LabelsTemplate = "foo"
374373
_, err = r2.Execute(f)
375374
assert.Error(t, err)
376375

377-
r2.labelsTemplate = nil
378376
r2.LabelsTemplate = "{{"
379377
_, err = r2.Execute(f)
380378
assert.Error(t, err)
381379

382-
r2.labelsTemplate = nil
383380
r2.LabelsTemplate = ""
384381
r2.VarsTemplate = "bar=baz"
385382
m, err = r2.Execute(f)
386383
assert.Nil(t, err)
387384
assert.Empty(t, m.Labels)
388385
assert.Equal(t, map[string]string{"bar": "baz"}, m.Vars, "instances should have matched")
389386

390-
r2.varsTemplate = nil
391387
r2.VarsTemplate = "bar"
392388
_, err = r2.Execute(f)
393389
assert.Error(t, err)
394390

395-
r2.varsTemplate = nil
396391
r2.VarsTemplate = "{{"
397392
_, err = r2.Execute(f)
398393
assert.Error(t, err)

pkg/apis/nfd/v1alpha1/types.go

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -178,10 +178,6 @@ type Rule struct {
178178
// MatchAny specifies a list of matchers one of which must match.
179179
// +optional
180180
MatchAny []MatchAnyElem `json:"matchAny"`
181-
182-
// private helpers/cache for handling golang templates
183-
labelsTemplate *templateHelper `json:"-"`
184-
varsTemplate *templateHelper `json:"-"`
185181
}
186182

187183
// MatchAnyElem specifies one sub-matcher of MatchAny.

pkg/apis/nfd/v1alpha1/zz_generated.deepcopy.go

Lines changed: 0 additions & 8 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)