Fix index out of range if multiple default plugins are overridden

Signed-off-by: Dave Chen <dave.chen@arm.com>
This commit is contained in:
Dave Chen 2021-07-08 13:00:23 +08:00
parent f915aa39e8
commit 1727cea64c
2 changed files with 52 additions and 6 deletions

View File

@ -156,6 +156,8 @@ type pluginIndex struct {
func mergePluginSet(defaultPluginSet, customPluginSet v1beta2.PluginSet) v1beta2.PluginSet { func mergePluginSet(defaultPluginSet, customPluginSet v1beta2.PluginSet) v1beta2.PluginSet {
disabledPlugins := sets.NewString() disabledPlugins := sets.NewString()
enabledCustomPlugins := make(map[string]pluginIndex) enabledCustomPlugins := make(map[string]pluginIndex)
// replacedPluginIndex is a set of index of plugins, which have replaced the default plugins.
replacedPluginIndex := sets.NewInt()
for _, disabledPlugin := range customPluginSet.Disabled { for _, disabledPlugin := range customPluginSet.Disabled {
disabledPlugins.Insert(disabledPlugin.Name) disabledPlugins.Insert(disabledPlugin.Name)
} }
@ -170,16 +172,22 @@ func mergePluginSet(defaultPluginSet, customPluginSet v1beta2.PluginSet) v1beta2
} }
// The default plugin is explicitly re-configured, update the default plugin accordingly. // The default plugin is explicitly re-configured, update the default plugin accordingly.
if customPlugin, ok := enabledCustomPlugins[defaultEnabledPlugin.Name]; ok { if customPlugin, ok := enabledCustomPlugins[defaultEnabledPlugin.Name]; ok {
klog.InfoS("Defaut plugin is explicitly re-configured and is overriding.", "plugin", defaultEnabledPlugin.Name) klog.InfoS("Default plugin is explicitly re-configured; overriding", "plugin", defaultEnabledPlugin.Name)
// update the default plugin in place to preserve order // Update the default plugin in place to preserve order.
defaultEnabledPlugin = customPlugin.plugin defaultEnabledPlugin = customPlugin.plugin
// kick the plugin from the enabled list of custom plugins replacedPluginIndex.Insert(customPlugin.index)
customPluginSet.Enabled = append(customPluginSet.Enabled[:customPlugin.index], customPluginSet.Enabled[customPlugin.index+1:]...)
} }
enabledPlugins = append(enabledPlugins, defaultEnabledPlugin) enabledPlugins = append(enabledPlugins, defaultEnabledPlugin)
} }
} }
enabledPlugins = append(enabledPlugins, customPluginSet.Enabled...) // Append all the custom plugins which haven't replaced any default plugins.
// Note: duplicated custom plugins will still be appended here.
// If so, the instantiation of scheduler framework will detect it and abort.
for index, plugin := range customPluginSet.Enabled {
if !replacedPluginIndex.Has(index) {
enabledPlugins = append(enabledPlugins, plugin)
}
}
return v1beta2.PluginSet{Enabled: enabledPlugins} return v1beta2.PluginSet{Enabled: enabledPlugins}
} }

View File

@ -367,6 +367,7 @@ func TestMergePlugins(t *testing.T) {
Filter: v1beta2.PluginSet{ Filter: v1beta2.PluginSet{
Enabled: []v1beta2.Plugin{ Enabled: []v1beta2.Plugin{
{Name: "Plugin1", Weight: pointer.Int32Ptr(2)}, {Name: "Plugin1", Weight: pointer.Int32Ptr(2)},
{Name: "Plugin3", Weight: pointer.Int32Ptr(3)},
}, },
}, },
}, },
@ -374,6 +375,8 @@ func TestMergePlugins(t *testing.T) {
Filter: v1beta2.PluginSet{ Filter: v1beta2.PluginSet{
Enabled: []v1beta2.Plugin{ Enabled: []v1beta2.Plugin{
{Name: "Plugin1"}, {Name: "Plugin1"},
{Name: "Plugin2"},
{Name: "Plugin3"},
}, },
}, },
}, },
@ -381,6 +384,8 @@ func TestMergePlugins(t *testing.T) {
Filter: v1beta2.PluginSet{ Filter: v1beta2.PluginSet{
Enabled: []v1beta2.Plugin{ Enabled: []v1beta2.Plugin{
{Name: "Plugin1", Weight: pointer.Int32Ptr(2)}, {Name: "Plugin1", Weight: pointer.Int32Ptr(2)},
{Name: "Plugin2"},
{Name: "Plugin3", Weight: pointer.Int32Ptr(3)},
}, },
}, },
}, },
@ -391,6 +396,38 @@ func TestMergePlugins(t *testing.T) {
Filter: v1beta2.PluginSet{ Filter: v1beta2.PluginSet{
Enabled: []v1beta2.Plugin{ Enabled: []v1beta2.Plugin{
{Name: "Plugin2", Weight: pointer.Int32Ptr(2)}, {Name: "Plugin2", Weight: pointer.Int32Ptr(2)},
{Name: "Plugin1", Weight: pointer.Int32Ptr(1)},
},
},
},
defaultPlugins: &v1beta2.Plugins{
Filter: v1beta2.PluginSet{
Enabled: []v1beta2.Plugin{
{Name: "Plugin1"},
{Name: "Plugin2"},
{Name: "Plugin3"},
},
},
},
expectedPlugins: &v1beta2.Plugins{
Filter: v1beta2.PluginSet{
Enabled: []v1beta2.Plugin{
{Name: "Plugin1", Weight: pointer.Int32Ptr(1)},
{Name: "Plugin2", Weight: pointer.Int32Ptr(2)},
{Name: "Plugin3"},
},
},
},
},
{
name: "RepeatedCustomPlugin",
customPlugins: &v1beta2.Plugins{
Filter: v1beta2.PluginSet{
Enabled: []v1beta2.Plugin{
{Name: "Plugin1"},
{Name: "Plugin2", Weight: pointer.Int32Ptr(2)},
{Name: "Plugin3"},
{Name: "Plugin2", Weight: pointer.Int32Ptr(4)},
}, },
}, },
}, },
@ -407,8 +444,9 @@ func TestMergePlugins(t *testing.T) {
Filter: v1beta2.PluginSet{ Filter: v1beta2.PluginSet{
Enabled: []v1beta2.Plugin{ Enabled: []v1beta2.Plugin{
{Name: "Plugin1"}, {Name: "Plugin1"},
{Name: "Plugin2", Weight: pointer.Int32Ptr(2)}, {Name: "Plugin2", Weight: pointer.Int32Ptr(4)},
{Name: "Plugin3"}, {Name: "Plugin3"},
{Name: "Plugin2", Weight: pointer.Int32Ptr(2)},
}, },
}, },
}, },