Merge pull request #1579 from tkatila/operator/multiple-crs

operator: remove one-cr-per-kind limitation

Author: mythi

cmd/operator/README.md

@@ -103,6 +103,8 @@ NAME                     DESIRED   READY   NODE SELECTOR   AGE
 gpudeviceplugin-sample   1         1                       5s
 ```
 
+**NOTE:** Intel Device Plugin Operator supports multiple custom resources per Kind (QAT, DSA, etc.). With multiple custom resources and different `nodeSelectors`, it is possible to customize device plugin configuration per node or per group of nodes. See also [known issues](#multiple-custom-resources).
+
 ## Upgrade
 
 The upgrade of the deployed plugins can be done by simply installing a new release of the operator.
@@ -135,6 +137,10 @@ command line argument multiple times.
 
 ## Known issues
 
+### Multiple Custom Resources
+
+With multiple custom resources, `nodeSelector` has to be carefully set to avoid device plugin DaemonSet getting deployed multiple times on the same node, as operator does not check or prevent this. Multiple plugins managing same resource on a node can cause invalid behavior and/or duplicate device resources on node.
+
 ### Cluster behind a proxy
 
 If your cluster operates behind a corporate proxy make sure that the API

pkg/apis/deviceplugin/v1/dlbdeviceplugin_webhook.go

@@ -15,7 +15,6 @@
 package v1
 
 import (
-	"github.com/pkg/errors"
 	"k8s.io/apimachinery/pkg/runtime"
 	ctrl "sigs.k8s.io/controller-runtime"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
@@ -25,10 +24,6 @@ import (
 	"github.com/intel/intel-device-plugins-for-kubernetes/pkg/controllers"
 )
 
-const (
-	dlbPluginKind = "DlbDevicePlugin"
-)
-
 var (
 	// dlbdevicepluginlog is for logging in this package.
 	dlbdevicepluginlog = logf.Log.WithName("dlbdeviceplugin-resource")
@@ -64,10 +59,6 @@ var _ webhook.Validator = &DlbDevicePlugin{}
 func (r *DlbDevicePlugin) ValidateCreate() (admission.Warnings, error) {
 	dlbdevicepluginlog.Info("validate create", "name", r.Name)
 
-	if controllers.GetDevicePluginCount(dlbPluginKind) > 0 {
-		return nil, errors.Errorf("an instance of %q already exists in the cluster", dlbPluginKind)
-	}
-
 	return nil, r.validatePlugin()
 }
 

pkg/apis/deviceplugin/v1/dsadeviceplugin_webhook.go

@@ -25,10 +25,6 @@ import (
 	"github.com/intel/intel-device-plugins-for-kubernetes/pkg/controllers"
 )
 
-const (
-	dsaPluginKind = "DsaDevicePlugin"
-)
-
 var (
 	// dsadevicepluginlog is for logging in this package.
 	dsadevicepluginlog = logf.Log.WithName("dsadeviceplugin-resource")
@@ -64,10 +60,6 @@ var _ webhook.Validator = &DsaDevicePlugin{}
 func (r *DsaDevicePlugin) ValidateCreate() (admission.Warnings, error) {
 	dsadevicepluginlog.Info("validate create", "name", r.Name)
 
-	if controllers.GetDevicePluginCount(dsaPluginKind) > 0 {
-		return nil, errors.Errorf("an instance of %q already exists in the cluster", dsaPluginKind)
-	}
-
 	return nil, r.validatePlugin()
 }
 

pkg/apis/deviceplugin/v1/fpgadeviceplugin_webhook.go

@@ -15,7 +15,6 @@
 package v1
 
 import (
-	"github.com/pkg/errors"
 	"k8s.io/apimachinery/pkg/runtime"
 	ctrl "sigs.k8s.io/controller-runtime"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
@@ -25,10 +24,6 @@ import (
 	"github.com/intel/intel-device-plugins-for-kubernetes/pkg/controllers"
 )
 
-const (
-	fpgaPluginKind = "FpgaDevicePlugin"
-)
-
 var (
 	// fpgadevicepluginlog is for logging in this package.
 	fpgadevicepluginlog = logf.Log.WithName("fpgadeviceplugin-resource")
@@ -68,10 +63,6 @@ var _ webhook.Validator = &FpgaDevicePlugin{}
 func (r *FpgaDevicePlugin) ValidateCreate() (admission.Warnings, error) {
 	fpgadevicepluginlog.Info("validate create", "name", r.Name)
 
-	if controllers.GetDevicePluginCount(fpgaPluginKind) > 0 {
-		return nil, errors.Errorf("an instance of %q already exists in the cluster", fpgaPluginKind)
-	}
-
 	return nil, r.validatePlugin()
 }
 

pkg/apis/deviceplugin/v1/gpudeviceplugin_webhook.go

@@ -15,29 +15,32 @@
 package v1
 
 import (
+	"context"
+
 	"github.com/pkg/errors"
 	"k8s.io/apimachinery/pkg/runtime"
 	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 
 	"github.com/intel/intel-device-plugins-for-kubernetes/pkg/controllers"
 )
 
-const (
-	gpuPluginKind = "GpuDevicePlugin"
-)
-
 var (
 	// gpudevicepluginlog is for logging in this package.
 	gpudevicepluginlog = logf.Log.WithName("gpudeviceplugin-resource")
 
 	gpuMinVersion = controllers.ImageMinVersion
 )
 
+var cli client.Client
+
 // SetupWebhookWithManager sets up a webhook for GpuDevicePlugin custom resources.
 func (r *GpuDevicePlugin) SetupWebhookWithManager(mgr ctrl.Manager) error {
+	cli = mgr.GetClient()
+
 	return ctrl.NewWebhookManagedBy(mgr).
 		For(r).
 		Complete()
@@ -64,10 +67,6 @@ var _ webhook.Validator = &GpuDevicePlugin{}
 func (r *GpuDevicePlugin) ValidateCreate() (admission.Warnings, error) {
 	gpudevicepluginlog.Info("validate create", "name", r.Name)
 
-	if controllers.GetDevicePluginCount(gpuPluginKind) > 0 {
-		return nil, errors.Errorf("an instance of %q already exists in the cluster", gpuPluginKind)
-	}
-
 	return nil, r.validatePlugin()
 }
 
@@ -85,6 +84,30 @@ func (r *GpuDevicePlugin) ValidateDelete() (admission.Warnings, error) {
 	return nil, nil
 }
 
+func (r *GpuDevicePlugin) crossCheckResourceManagement() bool {
+	ctx := context.Background()
+	gpuCrs := GpuDevicePluginList{}
+
+	if err := cli.List(ctx, &gpuCrs); err != nil {
+		gpudevicepluginlog.Info("unable to list GPU CRs")
+
+		return false
+	}
+
+	for _, cr := range gpuCrs.Items {
+		// Ignore itself.
+		if cr.Name == r.Name {
+			continue
+		}
+
+		if cr.Spec.ResourceManager != r.Spec.ResourceManager {
+			return false
+		}
+	}
+
+	return true
+}
+
 func (r *GpuDevicePlugin) validatePlugin() error {
 	if r.Spec.SharedDevNum == 1 && r.Spec.PreferredAllocationPolicy != "none" {
 		return errors.Errorf("PreferredAllocationPolicy is valid only when setting sharedDevNum > 1")
@@ -94,5 +117,9 @@ func (r *GpuDevicePlugin) validatePlugin() error {
 		return errors.Errorf("resourceManager is valid only when setting sharedDevNum > 1")
 	}
 
+	if !r.crossCheckResourceManagement() {
+		return errors.Errorf("All GPU CRs must be with or without resource management")
+	}
+
 	return validatePluginImage(r.Spec.Image, "intel-gpu-plugin", gpuMinVersion)
 }

pkg/apis/deviceplugin/v1/iaadeviceplugin_webhook.go

@@ -25,10 +25,6 @@ import (
 	"github.com/intel/intel-device-plugins-for-kubernetes/pkg/controllers"
 )
 
-const (
-	iaaPluginKind = "IaaDevicePlugin"
-)
-
 var (
 	// iaadevicepluginlog is for logging in this package.
 	iaadevicepluginlog = logf.Log.WithName("iaadeviceplugin-resource")
@@ -64,10 +60,6 @@ var _ webhook.Validator = &IaaDevicePlugin{}
 func (r *IaaDevicePlugin) ValidateCreate() (admission.Warnings, error) {
 	iaadevicepluginlog.Info("validate create", "name", r.Name)
 
-	if controllers.GetDevicePluginCount(iaaPluginKind) > 0 {
-		return nil, errors.Errorf("an instance of %q already exists in the cluster", iaaPluginKind)
-	}
-
 	return nil, r.validatePlugin()
 }
 

pkg/apis/deviceplugin/v1/qatdeviceplugin_webhook.go

@@ -25,10 +25,6 @@ import (
 	"github.com/intel/intel-device-plugins-for-kubernetes/pkg/controllers"
 )
 
-const (
-	qatPluginKind = "QatDevicePlugin"
-)
-
 var (
 	// qatdevicepluginlog is for logging in this package.
 	qatdevicepluginlog = logf.Log.WithName("qatdeviceplugin-resource")
@@ -64,10 +60,6 @@ var _ webhook.Validator = &QatDevicePlugin{}
 func (r *QatDevicePlugin) ValidateCreate() (admission.Warnings, error) {
 	qatdevicepluginlog.Info("validate create", "name", r.Name)
 
-	if controllers.GetDevicePluginCount(qatPluginKind) > 0 {
-		return nil, errors.Errorf("an instance of %q already exists in the cluster", qatPluginKind)
-	}
-
 	return nil, r.validatePlugin()
 }
 

pkg/apis/deviceplugin/v1/sgxdeviceplugin_webhook.go

@@ -15,7 +15,6 @@
 package v1
 
 import (
-	"github.com/pkg/errors"
 	"k8s.io/apimachinery/pkg/runtime"
 	ctrl "sigs.k8s.io/controller-runtime"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
@@ -25,10 +24,6 @@ import (
 	"github.com/intel/intel-device-plugins-for-kubernetes/pkg/controllers"
 )
 
-const (
-	sgxPluginKind = "SgxDevicePlugin"
-)
-
 var (
 	// sgxdevicepluginlog is for logging in this package.
 	sgxdevicepluginlog = logf.Log.WithName("sgxdeviceplugin-resource")
@@ -64,10 +59,6 @@ var _ webhook.Validator = &SgxDevicePlugin{}
 func (r *SgxDevicePlugin) ValidateCreate() (admission.Warnings, error) {
 	sgxdevicepluginlog.Info("validate create", "name", r.Name)
 
-	if controllers.GetDevicePluginCount(sgxPluginKind) > 0 {
-		return nil, errors.Errorf("an instance of %q already exists in the cluster", sgxPluginKind)
-	}
-
 	return nil, r.validatePlugin()
 }
 

pkg/controllers/dlb/controller.go

@@ -71,19 +71,12 @@ func (c *controller) Upgrade(ctx context.Context, obj client.Object) bool {
 	return controllers.UpgradeImages(ctx, &dp.Spec.Image, &dp.Spec.InitImage)
 }
 
-func (c *controller) GetTotalObjectCount(ctx context.Context, clnt client.Client) (int, error) {
-	var list devicepluginv1.DlbDevicePluginList
-	if err := clnt.List(ctx, &list); err != nil {
-		return 0, err
-	}
-
-	return len(list.Items), nil
-}
-
 func (c *controller) NewDaemonSet(rawObj client.Object) *apps.DaemonSet {
 	devicePlugin := rawObj.(*devicepluginv1.DlbDevicePlugin)
 
 	ds := deployments.DLBPluginDaemonSet()
+	ds.Name = controllers.SuffixedName(ds.Name, devicePlugin.Name)
+
 	if len(devicePlugin.Spec.NodeSelector) > 0 {
 		ds.Spec.Template.Spec.NodeSelector = devicePlugin.Spec.NodeSelector
 	}

pkg/controllers/dlb/controller_test.go

@@ -42,7 +42,7 @@ func (c *controller) newDaemonSetExpected(rawObj client.Object) *apps.DaemonSet
 		},
 		ObjectMeta: metav1.ObjectMeta{
 			Namespace: c.ns,
-			Name:      appLabel,
+			Name:      appLabel + "-" + devicePlugin.Name,
 			Labels: map[string]string{
 				"app": appLabel,
 			},
@@ -139,6 +139,7 @@ func (c *controller) newDaemonSetExpected(rawObj client.Object) *apps.DaemonSet
 // equal to the expected daemonset.
 func TestNewDaemonSetDLB(t *testing.T) {
 	plugin := &devicepluginv1.DlbDevicePlugin{}
+	plugin.Name = "testing"
 	c := &controller{}
 
 	expected := c.newDaemonSetExpected(plugin)