[BUG]: the nasName parameter in the powerflex secret is now mandatory

[coulof]

Bug Description

Despite what is written here the nasName.

The driver loops on the log below when that value is undefined and then crashes.

This impacts existing customers who upgraded from the previous version where we didn't have NAS support.

Logs

time="2024-01-15T14:43:16Z" level=info msg="Probing all arrays. Number of arrays: 1"
time="2024-01-15T14:43:16Z" level=info msg="default array is set to array ID: ca27126f4274560f"
time="2024-01-15T14:43:16Z" level=info msg="ca27126f4274560f is the default array, skipping VolumePrefixToSystems map update. \n"
time="2024-01-15T14:43:16Z" level=info msg="array ca27126f4274560f probed successfully"
time="2024-01-15T14:43:16Z" level=info msg="configured ca27126f4274560f" allSystemNames= endpoint="https://knl3013.krj.gie" isDefault=true nasName=0xc0007fa630 password="********" skipCertificateValidation=true systemID=ca27126f4274560f user=admin
time="2024-01-15T14:43:16Z" level=info msg="/csi.v1.Node/NodeGetInfo: REP 0002: nasName value not found in secret, it is mandatory parameter for NFS volume operations, if not specified, provide the default value as 'none'"
time="2024-01-15T14:43:17Z" level=info msg="/csi.v1.Identity/GetPluginInfo: REQ 0003: XXX_NoUnkeyedLiteral={}, XXX_sizecache=0"
time="2024-01-15T14:43:17Z" level=info msg="/csi.v1.Identity/GetPluginInfo: REP 0003: Name=csi-vxflexos.dellemc.com, VendorVersion=2.8.0+dirty, Manifest=map[commit:4a8a0ab90c4d56ca0ed00a8dd0aabdd7b526423c formed:Wed, 20 Sep 2023 06:15:43 UTC semver:2.8.0+dirty url:http://github.com/dell/csi-vxflexos], XXX_NoUnkeyedLiteral={}, XXX_sizecache=0"
time="2024-01-15T14:43:18Z" level=info msg="/csi.v1.Node/NodeGetInfo: REQ 0004: XXX_NoUnkeyedLiteral={}, XXX_sizecache=0"

Screenshots

No response

Additional Environment Information

No response

Steps to Reproduce

Install the driver and don't set a nasName (just like the second example from the sample secret).

Expected Behavior

If no value is set by the user in the secret, default to none without crashing

CSM Driver(s)

CSI Driver for PowerFlex v2.8

Installation Type

CSM Operator 1.8

Container Storage Modules Enabled

No response

Container Orchestrator

OCP 4.13

Operating System

CoreOS

[shanmydell]

link:20340

[adarsh-dell]

Hi @coulof , I believe array configured in the secret.yaml are >=v4.0 and as per the code I can see that this is how it was designed from day 1, to decide and add the required nfs topology on csi-node for the array that support NFS i.e. >=4.0.

If nasName will not be present then do you want us to add nfs topology or not?
If not then I can go ahead and can change this logic but my understanding is that NFS should be enabled by default and this label/topology should get added over all the csi-nodes and adding them without having a nasName in the secret doesn't make sense. I would love to hear and understand your approach and use case how you wanted it to work and then we can discuss and finalize the new approach if needed.

Currently we will add the topology on the basis of nasName: https://github.com/dell/csi-powerflex/blob/d50d49eb865444fbb2b639511f6dde1b1ab49ac2/service/node.go#L753

https://github.com/dell/csi-powerflex/blob/d50d49eb865444fbb2b639511f6dde1b1ab49ac2/service/service.go#L510

[shanmydell]

@coulof : Thoughts on above suggestion please

[coulof]

@adarsh-dell, I don't want to change the topology logic.

The request is to make nasName not mandatory in the configuration and, if not defined, fall back to none.

The reasons are :

• NAS is not available for older PowerFlex versions we support (3.6)
• In case of a driver upgrade, former configuration did not have nasName and after upgrade it will crash because this parameter is missing (cf. logs above)
• Our own sample (2nd one) is missing that tag

Please tell me if I'm wrong and if that parameter isn't mandatory.

[adarsh-dell]

Hi @coulof , Let me work on this and get back to you ASAP with some data and conclusion.

Thanks

[adarsh-dell]

@coulof So customer was using 4.0 array with v2.7.0 of driver?

[adarsh-dell]

Hello @coulof ,

We've updated the check logic, and you can find this in CSM 1.10.

Closing this issue. Feel free to reopen if you have any further questions.

Best regards,
Adarsh

[mlitka-cs]

Docs need to be updated as it still states this is optional or required:false:
https://dell.github.io/csm-docs/docs/deployment/helm/drivers/installation/powerflex/

[adarsh-dell]

Hi @mlitka-cs , Can you please explain that what is confusing here?
As part of this github issue, we have made this field as an optional field so this is not a mandatory one any more.
User can install the driver even without configuring this value in secret.yaml, doesn't matter what is the version of backend storage array i.e. version that support NFS >=4.x.