Skip to content

Ignore case difference for host name when X_CSI_IG_MODIFY_HOSTNAME is enabled#392

Merged
santhoshatdell merged 5 commits into
mainfrom
aq/case-check
Dec 20, 2024
Merged

Ignore case difference for host name when X_CSI_IG_MODIFY_HOSTNAME is enabled#392
santhoshatdell merged 5 commits into
mainfrom
aq/case-check

Conversation

@santhoshatdell

@santhoshatdell santhoshatdell commented Dec 19, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

Description

Ignored case difference for host name when X_CSI_IG_MODIFY_HOSTNAME is enabled. So, existing host with different case will be considered instead of modifying it which fails.

GitHub Issues

List the GitHub issues impacted by this PR:

GitHub Issue #
https://github.com/dell/csm/issues/1650

Checklist:

  • Have you run format,vet & lint checks against your submission?
  • Have you made sure that the code compiles?
  • Did you run the unit tests successfully?
  • Have you maintained at least 90% code coverage?
  • Have you commented your code, particularly in hard-to-understand areas
  • Have you done corresponding changes to the documentation
  • Did you run tests in a real Kubernetes cluster?
  • Backward compatibility is not broken

How Has This Been Tested?

The issue was reproduced on my setup and the node pod crashed.
Then with the fix on the same setup, the node pod didn't crash.

time="2024-12-19T21:49:29Z" level=info msg="**************************nodeHostSetup executing...*******************************"
time="2024-12-19T21:49:29Z" level=info msg="Waiting for 10 seconds"
time="2024-12-19T21:49:39Z" level=info msg="valid FC initiators: []"
time="2024-12-19T21:49:39Z" level=info msg="No existing NVMe hosts; new hosts will be created for all discovered initiators"
time="2024-12-19T21:49:39Z" level=info msg="valid NVMeTCP initiators: []"
time="2024-12-19T21:49:39Z" level=info msg="Checking initiator OR-1C:000:iqn.1994-05.com.redhat:00001 against host csi-node-SL-worker-1-0bx8wdgbjidld"
time="2024-12-19T21:49:39Z" level=info msg="Valid initiator: OR-1C:000:iqn.1994-05.com.redhat:00001"
time="2024-12-19T21:49:39Z" level=info msg="valid (existing) iSCSI initiators (must be manually created): [iqn.1994-05.com.redhat:00001]"
time="2024-12-19T21:49:39Z" level=info msg="ISCSI Daemon is active"
time="2024-12-19T21:49:39Z" level=info msg="setting up array 000000000001 for Iscsi, host name: csi-node-SL-worker-1-0bx8wdgbjidld masking view ID: csi-mv-SL-worker-1-0bx8wdgbjidld"
time="2024-12-19T21:49:39Z" level=debug msg="Processing Iscsi Host array: 000000000001, nodeName: csi-node-SL-worker-1-0bx8wdgbjidld, initiators: [iqn.1994-05.com.redhat:00001]"
time="2024-12-19T21:49:39Z" level=info msg="GetHostById returned: &{csi-node-sl-WORKER-1-0bx8wdgbjidld 0 1 0 false false   iSCSI [iqn.1994-05.com.redhat:00001] [] [] 0 0}, <nil>"

Able to provision with that worker node, no issues observed.

cert-csi basic test suites have passed. Output attached to ticket KRV-30759.

@santhoshatdell santhoshatdell changed the title Support case sensitivity for host name when X_CSI_IG_MODIFY_HOSTNAME is enabled Ignore case difference for host name when X_CSI_IG_MODIFY_HOSTNAME is enabled Dec 19, 2024
@santhoshatdell santhoshatdell marked this pull request as ready for review December 20, 2024 16:53
jooseppi-luna
jooseppi-luna approved these changes Dec 20, 2024
View reviewed changes

@jooseppi-luna jooseppi-luna left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@santhoshatdell santhoshatdell merged commit 20addb1 into main Dec 20, 2024
@santhoshatdell santhoshatdell deleted the aq/case-check branch December 20, 2024 20:17
abhi16394 pushed a commit that referenced this pull request Jan 3, 2025
@santhoshatdell @abhi16394
Ignore case difference for host name when X_CSI_IG_MODIFY_HOSTNAME is…
65bc838 
… enabled (#392)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@abhi16394 abhi16394 Awaiting requested review from abhi16394

@adarsh-dell adarsh-dell Awaiting requested review from adarsh-dell

@AkshaySainiDell AkshaySainiDell Awaiting requested review from AkshaySainiDell

@bpjain2004 bpjain2004 Awaiting requested review from bpjain2004

@donatwork donatwork Awaiting requested review from donatwork

@tdawe tdawe Awaiting requested review from tdawe tdawe is a code owner

@Sahiba-Gupta Sahiba-Gupta Awaiting requested review from Sahiba-Gupta

2 more reviewers

@jooseppi-luna jooseppi-luna jooseppi-luna approved these changes

@aqu-dell aqu-dell aqu-dell approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@santhoshatdell @jooseppi-luna @aqu-dell