Skip to content

Commit

Permalink
Fix Plugin Config Daemon node selector
Browse files Browse the repository at this point in the history
When node selectors are added via "configDaemonNodeSelector"
via the sriovoperatorconfigs CRD then completely removed, the previous
node selectors would remain for the plugin config daemons
(sriov-device-plugin). This was not cleaned up, because the function
"syncPluginDaemonSet" bails out when "configDaemonNodeSelector" is
empty. For example:

Step 1) Create 3 labels in node selector:
configDaemonNodeSelector = {"labelA": "", "labelB": "", "labelC": ""}
device-plugin DS NodeSelector = {"labelA": "", "labelB": "", "labelC": ""}
Step 2) Remove all labels in node selector:
configDaemonNodeSelector = {}
device-plugin DS NodeSelector = {"labelA": "", "labelB": "", "labelC": ""}

The expectation is that the device plugin will revert to the default
node selectors (e.g. {"node-role.kubernetes.io/worker": "",
"kubernetes.io/os": "linux"})

This change will make sure that the default values are taken from the
rendered plugin daemonset when "configDaemonNodeSelector" is empty.

Signed-off-by: William Zhao <[email protected]>
  • Loading branch information
wizhaoredhat committed Aug 2, 2023
1 parent 990648e commit ad92936
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 11 deletions.
11 changes: 11 additions & 0 deletions controllers/helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"strings"

constants "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/consts"
"github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/render"
)

var webhooks = map[string](string){
Expand Down Expand Up @@ -56,3 +57,13 @@ func formatJSON(str string) (string, error) {
}
return prettyJSON.String(), nil
}

// When given an empty render data structure, this function will fill it with the
// default data for plugin daemon sets (e.g. sriov-device-plugin).
func setPluginRenderData(data *render.RenderData) {
(*data).Data["Namespace"] = namespace
(*data).Data["SRIOVDevicePluginImage"] = os.Getenv("SRIOV_DEVICE_PLUGIN_IMAGE")
(*data).Data["ReleaseVersion"] = os.Getenv("RELEASEVERSION")
(*data).Data["ResourcePrefix"] = os.Getenv("RESOURCE_PREFIX")
(*data).Data["ImagePullSecrets"] = GetImagePullSecrets()
}
7 changes: 1 addition & 6 deletions controllers/sriovnetworknodepolicy_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import (
"context"
"encoding/json"
"fmt"
"os"
"sort"
"strings"

Expand Down Expand Up @@ -330,11 +329,7 @@ func (r *SriovNetworkNodePolicyReconciler) syncPluginDaemonObjs(ctx context.Cont

// render plugin manifests
data := render.MakeRenderData()
data.Data["Namespace"] = namespace
data.Data["SRIOVDevicePluginImage"] = os.Getenv("SRIOV_DEVICE_PLUGIN_IMAGE")
data.Data["ReleaseVersion"] = os.Getenv("RELEASEVERSION")
data.Data["ResourcePrefix"] = os.Getenv("RESOURCE_PREFIX")
data.Data["ImagePullSecrets"] = GetImagePullSecrets()
setPluginRenderData(&data)

objs, err := renderDsForCR(constants.PluginPath, &data)
if err != nil {
Expand Down
50 changes: 45 additions & 5 deletions controllers/sriovoperatorconfig_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,16 +147,48 @@ func (r *SriovOperatorConfigReconciler) SetupWithManager(mgr ctrl.Manager) error
Complete(r)
}

// This function will attempt to render the plugin daemon sets to retrieve the default node selector
// when successful it will set the input node selector.
func (r *SriovOperatorConfigReconciler) getDefaultPluginDsNodeSelector(name string, m *map[string]string) error {
logger := log.Log.WithName("syncPluginDaemonSet")
logger.Info("Set default plugin DaemonSet node selector")

data := render.MakeRenderData()
setPluginRenderData(&data)

objs, err := render.RenderDir(constants.PluginPath, &data)
if err != nil {
logger.Error(err, "Fail to render plugin DaemonSet manifests")
return err
}

for _, obj := range objs {
if obj.GetKind() == constants.DaemonSet {
scheme := kscheme.Scheme
ds := &appsv1.DaemonSet{}
err = scheme.Convert(obj, ds, nil)
if err != nil {
logger.Error(err, "Fail to convert to plugin DaemonSet")
return err
}
// Set the default NodeSelector value from the rendered ds.
if ds.Name == name {
logger.Info("Setting the default node selector from rendered plugin DaemonSet", "name", name)
*m = ds.Spec.Template.Spec.NodeSelector
}
}
}

return nil
}

func (r *SriovOperatorConfigReconciler) syncPluginDaemonSet(ctx context.Context, dc *sriovnetworkv1.SriovOperatorConfig) error {
logger := log.Log.WithName("syncConfigDaemonset")
logger := log.Log.WithName("syncPluginDaemonSet")
logger.Info("Start to sync SRIOV plugin daemonsets nodeSelector")
ds := &appsv1.DaemonSet{}

names := []string{"sriov-cni", "sriov-device-plugin"}

if len(dc.Spec.ConfigDaemonNodeSelector) == 0 {
return nil
}
for _, name := range names {
err := r.Client.Get(ctx, types.NamespacedName{Name: name, Namespace: namespace}, ds)
if err != nil {
Expand All @@ -166,7 +198,15 @@ func (r *SriovOperatorConfigReconciler) syncPluginDaemonSet(ctx context.Context,
logger.Error(err, "Couldn't get daemonset", "name", name)
return err
}
ds.Spec.Template.Spec.NodeSelector = dc.Spec.ConfigDaemonNodeSelector
if len(dc.Spec.ConfigDaemonNodeSelector) == 0 {
err = r.getDefaultPluginDsNodeSelector(name, &ds.Spec.Template.Spec.NodeSelector)
if err != nil {
logger.Error(err, "Couldn't get the default daemonset node selector", "name", name)
return err
}
} else {
ds.Spec.Template.Spec.NodeSelector = dc.Spec.ConfigDaemonNodeSelector
}
err = r.Client.Update(ctx, ds)
if err != nil {
logger.Error(err, "Couldn't update daemonset", "name", name)
Expand Down

0 comments on commit ad92936

Please sign in to comment.