Fix fcpath

[mkumatag]

What type of PR is this?

/kind bug

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #95450

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

NONE

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

[mkumatag]

/retest

[mkumatag]

/assign @saad-ali @rootfs

[mkumatag]

/retest

[saad-ali]

I am not a FC expert, but the change looks reasonable to me.
/approve
/hold

@rootfs can you PTAL

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mkumatag, saad-ali

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• pkg/volume/fc/OWNERS [saad-ali]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[saad-ali]

/lgtm

Want to give @rootfs a chance to look at this as well. Will let him remove hold.
If he is not able to take a look, feel free to ping me again.

[saad-ali]

/triage accepted

[rootfs]

I don't have a FC setup to verify, but this could be due to a difference in udev rules. @mkimuram @mtanino can you check?

[rootfs]

this appears to be caused by different udev rules. I see RHEL and SuSE have different name conventions. So it is possible the paths could have different prefix.

[rootfs]

looks good, @mkimuram @mtanino any comment?

[mkumatag]

/retest

[mkimuram]

The name depends on udev rules, so it makes sense to add expressions that match to other naming conventions.
(I haven't tested with the actual environment, but as https://github.com/hreinecke/sg3_utils/blob/master/scripts/59-fc-wwpn-id.rules shows, distributions that uses sg3_utils seems to name with the convention in this PR.)

So, the overall change looks good to me.

I would like to confirm two things:

1. Should we care the name that contains "-fc-0x" but doesn't start with "pci-"?
   It was allowed by the original code, but will be denied by the new code.
   I'm not sure that there are such naming conventions used in a real environment, but would like to confirm that it is OK, just for sure.

2. Should we need to care a distribution that has both "pci-xxx-fc-0xYYYY" and " fc-0xYYYY"?
   In such a distribution, the path found first in the loop will be used.
   I believe that this won't make any issue for the device name is resolved from the symlink and the symlink should point to the same device, but I would like to make sure that it won't be an issue.

[mkimuram]

@rootfs

1. Should we care the name that contains "-fc-0x" but doesn't start with "pci-"?

I read through the discussion again and your comment "Unless there are other naming conventions, we only have /dev/disk/by-path/fc-0x and /dev/disk/by-path/pci-xxxx-fc-0x. The regex should just filter these cases." seems to say that we don't need to care about this case. I'm OK with it, if it is practically good.

Then, could you just double check "2. Should we need to care a distribution that has both "pci-xxx-fc-0xYYYY" and " fc-0xYYYY"?"?

[rootfs]

thanks @mkimuram
for the 2nd case when both paths exist (which should be considered poor configuration), then the first match is returned.

[rootfs]

/lgtm

[rootfs]

/hold cancel

[mkumatag]

Thanks @mkimuram @rootfs @saad-ali