Skip to content

Commit 0da36a9

Browse files
committed
refactor(plugin): move crToConfigName tracking to reconciler and extract resolveConfigName helper
Signed-off-by: cannarelladev <cannarella.dev@gmail.com>
1 parent 3b25ce3 commit 0da36a9

File tree

2 files changed

+68
-6
lines changed

2 files changed

+68
-6
lines changed

controllers/artifact/plugin/controller.go

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ func NewPluginReconciler(cl client.Client, scheme *runtime.Scheme, nodeName, nam
5252
artifactManager: artifact.NewManager(cl, namespace),
5353
PluginsConfig: &PluginsConfig{},
5454
nodeName: nodeName,
55+
crToConfigName: make(map[string]string),
5556
}
5657
}
5758

@@ -63,6 +64,7 @@ type PluginReconciler struct {
6364
artifactManager *artifact.Manager
6465
PluginsConfig *PluginsConfig
6566
nodeName string
67+
crToConfigName map[string]string
6668
}
6769

6870
// Reconcile is part of the main kubernetes reconciliation loop which aims to
@@ -165,6 +167,7 @@ func (r *PluginReconciler) handleDeletion(ctx context.Context, plugin *artifactv
165167

166168
// Remove the plugin configuration.
167169
r.PluginsConfig.removeConfig(plugin)
170+
delete(r.crToConfigName, plugin.Name)
168171

169172
// Write the updated configuration to the file.
170173
if err := r.removePluginConfig(ctx, plugin); err != nil {
@@ -191,6 +194,13 @@ func (r *PluginReconciler) handleDeletion(ctx context.Context, plugin *artifactv
191194
func (r *PluginReconciler) ensurePluginConfig(ctx context.Context, plugin *artifactv1alpha1.Plugin) error {
192195
logger := log.FromContext(ctx)
193196
logger.Info("Ensuring plugin configuration")
197+
198+
configName := resolveConfigName(plugin)
199+
if oldName, ok := r.crToConfigName[plugin.Name]; ok && oldName != configName {
200+
r.PluginsConfig.removeByName(oldName)
201+
}
202+
r.crToConfigName[plugin.Name] = configName
203+
194204
r.PluginsConfig.addConfig(plugin)
195205
// Convert the struct to string.
196206
pluginConfigString, err := r.PluginsConfig.toString()
@@ -271,6 +281,13 @@ type PluginsConfig struct {
271281
LoadPlugins []string `yaml:"load_plugins,omitempty"`
272282
}
273283

284+
func resolveConfigName(plugin *artifactv1alpha1.Plugin) string {
285+
if plugin.Spec.Config != nil && plugin.Spec.Config.Name != "" {
286+
return plugin.Spec.Config.Name
287+
}
288+
return plugin.Name
289+
}
290+
274291
func (pc *PluginsConfig) addConfig(plugin *artifactv1alpha1.Plugin) {
275292
config := PluginConfig{
276293
LibraryPath: artifact.Path(plugin.Name, priority.DefaultPriority, artifact.MediumOCI, artifact.TypePlugin),
@@ -315,12 +332,10 @@ func (pc *PluginsConfig) addConfig(plugin *artifactv1alpha1.Plugin) {
315332
}
316333

317334
func (pc *PluginsConfig) removeConfig(plugin *artifactv1alpha1.Plugin) {
318-
// Resolve the effective config name (same logic as addConfig).
319-
name := plugin.Name
320-
if plugin.Spec.Config != nil && plugin.Spec.Config.Name != "" {
321-
name = plugin.Spec.Config.Name
322-
}
335+
pc.removeByName(resolveConfigName(plugin))
336+
}
323337

338+
func (pc *PluginsConfig) removeByName(name string) {
324339
for i, c := range pc.Configs {
325340
if c.Name == name {
326341
pc.Configs = append(pc.Configs[:i], pc.Configs[i+1:]...)
@@ -337,7 +352,6 @@ func (pc *PluginsConfig) removeConfig(plugin *artifactv1alpha1.Plugin) {
337352
}
338353

339354
func (pc *PluginsConfig) toString() (string, error) {
340-
// Convert the struct to YAML.
341355
data, err := yaml.Marshal(pc)
342356
if err != nil {
343357
return "", err

controllers/artifact/plugin/controller_test.go

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -336,6 +336,54 @@ func TestPluginsConfig_AddThenRemove_RoundTrip(t *testing.T) {
336336
assert.True(t, pc.isEmpty())
337337
})
338338

339+
t.Run("changing spec.config.name removes stale entry via reconciler tracking", func(t *testing.T) {
340+
pc := &PluginsConfig{}
341+
crToConfigName := make(map[string]string)
342+
343+
// Initial: CR "my-plugin" with spec.config.name = "json".
344+
plugin := &artifactv1alpha1.Plugin{
345+
ObjectMeta: metav1.ObjectMeta{Name: "my-plugin"},
346+
Spec: artifactv1alpha1.PluginSpec{
347+
Config: &artifactv1alpha1.PluginConfig{
348+
Name: "json",
349+
},
350+
},
351+
}
352+
crToConfigName[plugin.Name] = resolveConfigName(plugin)
353+
pc.addConfig(plugin)
354+
require.Len(t, pc.Configs, 1)
355+
assert.Equal(t, "json", pc.Configs[0].Name)
356+
assert.Equal(t, []string{"json"}, pc.LoadPlugins)
357+
358+
// User changes spec.config.name from "json" to "json-v2".
359+
pluginRenamed := &artifactv1alpha1.Plugin{
360+
ObjectMeta: metav1.ObjectMeta{Name: "my-plugin"},
361+
Spec: artifactv1alpha1.PluginSpec{
362+
Config: &artifactv1alpha1.PluginConfig{
363+
Name: "json-v2",
364+
},
365+
},
366+
}
367+
368+
// Reconciler detects name change and removes stale entry before addConfig.
369+
newName := resolveConfigName(pluginRenamed)
370+
if oldName, ok := crToConfigName[pluginRenamed.Name]; ok && oldName != newName {
371+
pc.removeByName(oldName)
372+
}
373+
crToConfigName[pluginRenamed.Name] = newName
374+
pc.addConfig(pluginRenamed)
375+
376+
// The old "json" entry must be gone, only "json-v2" should remain.
377+
require.Len(t, pc.Configs, 1)
378+
assert.Equal(t, "json-v2", pc.Configs[0].Name)
379+
assert.Equal(t, []string{"json-v2"}, pc.LoadPlugins)
380+
381+
// Deletion should clean up fully.
382+
pc.removeConfig(pluginRenamed)
383+
delete(crToConfigName, pluginRenamed.Name)
384+
assert.True(t, pc.isEmpty())
385+
})
386+
339387
t.Run("add, update, then remove", func(t *testing.T) {
340388
pc := &PluginsConfig{}
341389

0 commit comments

Comments
 (0)