DRA: Fix the comments of the review

Signed-off-by: cyclinder <[email protected]>

charts/spiderpool/templates/configmap.yaml

@@ -21,7 +21,8 @@ data:
     enableKubevirtStaticIP: {{ .Values.ipam.enableKubevirtStaticIP }}
     enableSpiderSubnet: {{ .Values.ipam.enableSpiderSubnet }}
     enableDRA: {{ .Values.dra.enabled }}
-    cdiRoot: {{ .Values.dra.cdiRoot }}
+    cdiRootPath: {{ .Values.dra.cdiRootPath }}
+    draLibraryPath: {{ .Values.dra.libraryPath }}
     {{- if .Values.ipam.enableSpiderSubnet }}
     clusterSubnetDefaultFlexibleIPNumber: {{ .Values.ipam.subnetDefaultFlexibleIPNumber }}
     {{- else}}

charts/spiderpool/templates/daemonset.yaml

@@ -236,10 +236,6 @@ spec:
           value: {{ .Values.spiderpoolAgent.httpPort | quote }}
         - name: SPIDERPOOL_GOPS_LISTEN_PORT
           value: {{ .Values.spiderpoolAgent.debug.gopsPort | quote }}
-        {{- if and .Values.dra.enabled .Values.dra.libraryPath }}
-        - name: SPIDERPOOL_DRA_LIBRARY_PATH
-          value: {{ .Values.dra.libraryPath }}
-        {{- end }}
         {{- if .Values.multus.multusCNI.defaultCniCRName }}
         - name: MULTUS_CLUSTER_NETWORK
           value: {{ .Release.Namespace }}/{{ .Values.multus.multusCNI.defaultCniCRName }}
@@ -249,7 +245,9 @@ spec:
         {{- end }}
         {{- if or .Values.dra.enabled .Values.spiderpoolAgent.securityContext }}
         securityContext:
+          {{- if .Values.dra.enabled }}
           privileged: true
+          {{- end }}
         {{- with .Values.spiderpoolAgent.securityContext }}
         {{- toYaml . | nindent 8 }}
         {{- end }}

charts/spiderpool/values.yaml

@@ -266,9 +266,9 @@ multus:
 dra:
   ## @param dra.enabled to enable dra feature
   enabled: false
-  ## @param dra.cdiRoot the dir of cdi root
-  cdiRoot: "/var/run/cdi"
-  ## @param dra.libraryPath
+  ## @param dra.cdiRootPath the dir of cdi root
+  cdiRootPath: "/var/run/cdi"
+  ## @param dra.libraryPath the dir path of the library
   libraryPath: ""
 
 ## @section plugins parameters

cmd/spiderpool-agent/cmd/config.go

@@ -67,8 +67,6 @@ var envInfo = []envConf{
 	{"SPIDERPOOL_WAIT_SUBNET_POOL_MAX_RETRIES", "25", false, nil, nil, &agentContext.Cfg.WaitSubnetPoolMaxRetries},
 
 	{"MULTUS_CLUSTER_NETWORK", "", false, &agentContext.Cfg.MultusClusterNetwork, nil, nil},
-
-	{"SPIDERPOOL_DRA_LIBRARY_PATH", "", false, &agentContext.Cfg.LibraryPath, nil, nil},
 }
 
 type Config struct {
@@ -92,7 +90,6 @@ type Config struct {
 	MetricHttpPort   string
 	GopsListenPort   string
 	PyroscopeAddress string
-	LibraryPath      string
 
 	IPPoolMaxAllocatedIPs    int
 	WaitSubnetPoolTime       int
@@ -101,15 +98,16 @@ type Config struct {
 	MultusClusterNetwork string
 
 	// configmap
-	IpamUnixSocketPath                string `yaml:"ipamUnixSocketPath"`
-	EnableIPv4                        bool   `yaml:"enableIPv4"`
-	EnableIPv6                        bool   `yaml:"enableIPv6"`
-	EnableStatefulSet                 bool   `yaml:"enableStatefulSet"`
-	EnableKubevirtStaticIP            bool   `yaml:"enableKubevirtStaticIP"`
-	EnableSpiderSubnet                bool   `yaml:"enableSpiderSubnet"`
-	EnableDRA                         bool   `yaml:"enableDRA"`
-	DRACDIRoot                        string `yaml:"cdiRoot"`
-	ClusterSubnetDefaultFlexibleIPNum int    `yaml:"clusterSubnetDefaultFlexibleIPNumber"`
+	IpamUnixSocketPath                string   `yaml:"ipamUnixSocketPath"`
+	EnableIPv4                        bool     `yaml:"enableIPv4"`
+	EnableIPv6                        bool     `yaml:"enableIPv6"`
+	EnableStatefulSet                 bool     `yaml:"enableStatefulSet"`
+	EnableKubevirtStaticIP            bool     `yaml:"enableKubevirtStaticIP"`
+	EnableSpiderSubnet                bool     `yaml:"enableSpiderSubnet"`
+	EnableDRA                         bool     `yaml:"enableDRA"`
+	DraCdiRootPath                    string   `yaml:"cdiRootPath"`
+	DraLibraryPath                    []string `yaml:"draLibraryPath"`
+	ClusterSubnetDefaultFlexibleIPNum int      `yaml:"clusterSubnetDefaultFlexibleIPNumber"`
 }
 
 type AgentContext struct {

cmd/spiderpool-agent/cmd/daemon.go

@@ -234,9 +234,9 @@ func DaemonMain() {
 	}
 	agentContext.unixClient = spiderpoolAgentAPI
 
-	if agentContext.Cfg.EnableDRA && agentContext.Cfg.LibraryPath != "" {
+	if agentContext.Cfg.EnableDRA {
 		logger.Info("Begin to start dra-plugin Server")
-		agentContext.DraPlugin, err = draplugin.StartDRAPlugin(logger, agentContext.Cfg.DRACDIRoot, agentContext.Cfg.LibraryPath)
+		agentContext.DraPlugin, err = draplugin.StartDRAPlugin(logger, agentContext.Cfg.DraCdiRootPath, agentContext.Cfg.DraLibraryPath)
 		if err != nil {
 			logger.Fatal(err.Error())
 		}

pkg/dra/dra-controller/controller.go

@@ -55,7 +55,7 @@ func StartController(ctx context.Context,
 				}
 			}()
 
-			informerFactory.Start(ctx.Done())
+			informerFactory.Start(innerCtx.Done())
 			controller.Run(1)
 		}
 	}()

pkg/dra/dra-plugin/cdi.go

@@ -81,14 +81,15 @@ func (cdi *CDIHandler) GetClaimDevices(claimUID string) []string {
 	return devices
 }
 
+// CreateClaimSpecFile create CDI file for the claim
 func (cdi *CDIHandler) CreateClaimSpecFile(claimUID string, scp *v2beta1.SpiderClaimParameter) error {
 	cdiSpec := cdispec.Spec{
-		// TODO(@cyclinder): should be make it to configurable?
+		// TODO(@cyclinder): should make it to configurable?
 		Version: cdiapi.CurrentVersion,
 		Kind:    cdi.cdiKind(),
 		Devices: []cdispec.Device{{
 			Name:           claimUID,
-			ContainerEdits: cdi.getContaineEdits(claimUID, scp.Spec.Rdma),
+			ContainerEdits: cdi.getContaineEdits(claimUID, scp.Spec.RdmaAcc),
 		}},
 	}
 
@@ -122,8 +123,7 @@ func (cdi *CDIHandler) DeleteClaimSpecFile(claimUID string) error {
 }
 
 // nolint: all
-func (cdi *CDIHandler) getContaineEdits(claim string, rdma bool) cdispec.ContainerEdits {
-	soName := path.Base(cdi.so)
+func (cdi *CDIHandler) getContaineEdits(claim string, rdmaAcc bool) cdispec.ContainerEdits {
 	ce := cdispec.ContainerEdits{
 		// why do we need this?
 		// a device MUST be have at lease a ContainerEdits, so if rdma is false:
@@ -134,7 +134,8 @@ func (cdi *CDIHandler) getContaineEdits(claim string, rdma bool) cdispec.Contain
 		},
 	}
 
-	if rdma {
+	if rdmaAcc {
+		soName := path.Base(cdi.so)
 		ce.Env = append(ce.Env, fmt.Sprintf("LD_PRELOAD=%s", soName))
 		ce.Mounts = []*cdispec.Mount{
 			{

pkg/dra/dra-plugin/driver.go

@@ -39,6 +39,10 @@ func NewDriver(logger *zap.Logger, cdiRoot string, so string) (*driver, error) {
 	}, nil
 }
 
+// NodePrepareResources prepares several ResourceClaims
+// for use on the node. If an error is returned, the
+// response is ignored. Failures for individual claims
+// can be reported inside NodePrepareResourcesResponse.
 func (d *driver) NodePrepareResources(ctx context.Context, req *drapbv1.NodePrepareResourcesRequest) (*drapbv1.NodePrepareResourcesResponse, error) {
 	d.logger.Info("NodePrepareResource is called")
 	preparedResources := &drapbv1.NodePrepareResourcesResponse{Claims: map[string]*drapbv1.NodePrepareResourceResponse{}}
@@ -95,6 +99,7 @@ func (d *driver) prepare(ctx context.Context, claim *drapbv1.Claim) ([]string, e
 			return err
 		}
 
+		// prepare CDI file for the claim
 		prepared, err = d.State.Prepare(ctx, claim.Uid, scp)
 		if err != nil {
 			return err
@@ -114,6 +119,8 @@ func (d *driver) isPrepared(ctx context.Context, claimUID string) (bool, []strin
 	return false, nil, nil
 }
 
+// NodeUnprepareResources is the opposite of NodePrepareResources.
+// The same error handling rules apply
 func (d *driver) NodeUnprepareResources(ctx context.Context, req *drapbv1.NodeUnprepareResourcesRequest) (*drapbv1.NodeUnprepareResourcesResponse, error) {
 	d.logger.Info("NodeUnprepareResources is called")
 	response := make(map[string]*drapbv1.NodeUnprepareResourceResponse, len(req.Claims))

pkg/dra/dra-plugin/plugin.go

@@ -11,7 +11,7 @@ import (
 	"k8s.io/dynamic-resource-allocation/kubeletplugin"
 )
 
-func StartDRAPlugin(logger *zap.Logger, cdiRoot, so string) (kubeletplugin.DRAPlugin, error) {
+func StartDRAPlugin(logger *zap.Logger, cdiRoot string, so string) (kubeletplugin.DRAPlugin, error) {
 	err := os.MkdirAll(constant.DRADriverPluginPath, 0750)
 	if err != nil {
 		return nil, err

pkg/k8s/apis/spiderpool.spidernet.io/v2beta1/spiderclaimparameters_types.go

@@ -12,7 +12,7 @@ import (
 type ClaimParameterSpec struct {
 	// +kubebuilder:validation:Optional
 	// +kubebuilder:default=false
-	Rdma bool `json:"rdma,omitempty"`
+	RdmaAcc bool `json:"rdma,omitempty"`
 
 	// +kubebuilder:validation:Optional
 	MultusNames []string `json:"multusNames,omitempty"`