Bug 1963899

+++ This bug was initially created as a clone of Bug #1933630 +++

Description of problem:
After adding iscsi disk on node, I get following disks, one disk in /dev/disk/by-id has the id like scsi-1LIO-ORG_disk01:60c21f72-174c-467b-a592-f9cd88ff01b2 as below:
sh-4.4# lsblk 
NAME    MAJ:MIN RM   SIZE RO TYPE MOUNTPOINT
sda       8:0    0     2G  0 disk 
sdb       8:16   0     2G  0 disk 
sdc       8:32   0     2G  0 disk 
sdd       8:48   0     2G  0 disk 

sh-4.4# ls /dev/sd*
/dev/sda  /dev/sdb  /dev/sdc  /dev/sdd

sh-4.4# ls -l /dev/disk/by-id/
total 0
lrwxrwxrwx. 1 root root 9 Mar  1 07:01 scsi-1LIO-ORG_disk01:60c21f72-174c-467b-a592-f9cd88ff01b2 -> ../../sdb

Then I created local volume set, but this volume failed to be provisioned with following error:
{"level":"info","ts":1614582558.1743612,"logger":"localvolumeset-symlink-controller","msg":"creating","Request.Namespace":"openshift-local-storage","Request.Name":"lvs","Device.Name":"sdb","pv.Name":"local-pv-2e4d7067"}
{"level":"error","ts":1614582558.1798408,"logger":"controller-runtime.controller","msg":"Reconciler error","controller":"localvolumeset-symlink-controller","request":"openshift-local-storage/lvs","error":"could not provision disk: PersistentVolume \"local-pv-2e4d7067\" is invalid: metadata.labels: Invalid value: \"scsi-1LIO-ORG_disk01:60c21f72-174c-467b-a592-f9cd88ff01b2\": a valid label must be an empty string or consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyValue',  or 'my_value',  or '12345', regex used for validation is '(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?')","stacktrace":"github.com/go-logr/zapr.(*zapLogger).Error\n\t/go/src/github.com/openshift/local-storage-operator/vendor/github.com/go-logr/zapr/zapr.go:128\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).reconcileHandler\n\t/go/src/github.com/openshift/local-storage-operator/vendor/sigs.k8s.io/controller-runtime/pkg/internal/controller/controller.go:258\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).processNextWorkItem\n\t/go/src/github.com/openshift/local-storage-operator/vendor/sigs.k8s.io/controller-runtime/pkg/internal/controller/controller.go:232\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).worker\n\t/go/src/github.com/openshift/local-storage-operator/vendor/sigs.k8s.io/controller-runtime/pkg/internal/controller/controller.go:211\nk8s.io/apimachinery/pkg/util/wait.JitterUntil.func1\n\t/go/src/github.com/openshift/local-storage-operator/vendor/k8s.io/apimachinery/pkg/util/wait/wait.go:152\nk8s.io/apimachinery/pkg/util/wait.JitterUntil\n\t/go/src/github.com/openshift/local-storage-operator/vendor/k8s.io/apimachinery/pkg/util/wait/wait.go:153\nk8s.io/apimachinery/pkg/util/wait.Until\n\t/go/src/github.com/openshift/local-storage-operator/vendor/k8s.io/apimachinery/pkg/util/wait/wait.go:88"}

Version-Release number of selected component (if applicable):
4.7.0

How reproducible:
Always in my test

Steps to Reproduce:
See Description:
1. Create iscsi_targe provisioner
2. Creare local volume set

Actual results:
iscsi disk could not be provisioned.

Expected results:
iscsi disk should be provisioned.


Master Log:

Node Log (of failed PODs):

PV Dump:
  
PVC Dump:

StorageClass Dump (if StorageClass used by PV/PVC):

Additional info:

--- Additional comment from Wei Duan on 2021-03-02 03:11:43 UTC ---

Add the local storage version:
oc get csv -A
NAMESPACE                              NAME                                           DISPLAY          VERSION                 REPLACES   PHASE
openshift-local-storage                local-storage-operator.4.7.0-202102110027.p0   Local Storage    4.7.0-202102110027.p0              Succeeded


And all the records from /dev/disk/by-id/
sh-4.4# ls -l /dev/disk/by-id/
total 0
lrwxrwxrwx. 1 root root 9 Mar  1 07:01 scsi-1LIO-ORG_disk01:60c21f72-174c-467b-a592-f9cd88ff01b2 -> ../../sdb
lrwxrwxrwx. 1 root root 9 Mar  1 07:01 scsi-1LIO-ORG_disk02:c0708dd2-606a-490d-863c-009bff119cfd -> ../../sdd
lrwxrwxrwx. 1 root root 9 Mar  1 07:01 scsi-3600140560c21f72174c467ba592f9cd8 -> ../../sdb
lrwxrwxrwx. 1 root root 9 Mar  1 07:01 scsi-36001405c0708dd2606a490d863c009bf -> ../../sdd
lrwxrwxrwx. 1 root root 9 Mar  1 07:01 scsi-SLIO-ORG_disk01_60c21f72-174c-467b-a592-f9cd88ff01b2 -> ../../sdb
lrwxrwxrwx. 1 root root 9 Mar  1 07:01 scsi-SLIO-ORG_disk02_c0708dd2-606a-490d-863c-009bff119cfd -> ../../sdd
lrwxrwxrwx. 1 root root 9 Mar  1 07:01 wwn-0x600140560c21f72174c467ba592f9cd8 -> ../../sdb
lrwxrwxrwx. 1 root root 9 Mar  1 07:01 wwn-0x6001405c0708dd2606a490d863c009bf -> ../../sdd

--- Additional comment from Wei Duan on 2021-03-02 03:12:23 UTC ---



--- Additional comment from Jan Safranek on 2021-03-02 15:41:45 UTC ---

Having /dev/disk/by-id names in PV labels is problematic, as they can have weird characters. We need either escape those (ugly!) or move them to annotations (but then you can't select them using a label selector - do we need this?)

--- Additional comment from Rohan CJ on 2021-04-23 06:11:27 UTC ---

Fixed in https://github.com/openshift/local-storage-operator/pull/219

--- Additional comment from Rohan CJ on 2021-04-23 06:14:33 UTC ---

To verify, check if PV labels with keys storage.openshift.com/device-name,storage.openshift.com/device-id move to annotations.

--- Additional comment from Chao Yang on 2021-04-26 10:22:57 UTC ---

oc get pv local-pv-8f6c0fad -o json | jq .metadata.annotations
{
  "storage.openshift.com/device-id": "scsi-1LIO-ORG_disk02:0c52d090-31ca-469d-9805-d743d467942d",
  "storage.openshift.com/device-name": "sdc"
}

local-storage-operator.4.8.0-202104231341.p0