Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix nfs route bug #10304

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Conversation

iishitahere
Copy link

@iishitahere iishitahere commented Jan 30, 2025

Description

This PR fixes the issue #10163, where the secondary storage VM fails to add a static route to the NFS server, causing a No route to host error. The _storageIp was previously null, resulting in failed route addition (via null).

Changes Made

  • Improved _storageIp handling to prevent null values.
  • Added utility methods to validate NFS server reachability.
  • Enhanced error handling and logging.

Testing

  • Verified successful template uploads to secondary storage.
  • Checked route addition on the SSVM.
  • Ran unit tests (S3TemplateTest, etc.), all tests passed successfully.

@weizhouapache
Copy link
Member

thanks @iishitahere for the PR

I would suggest you keep this PR clean and remove all unnecessary changes
IMHO, this PR could be around 5 lines of code changes

@iishitahere
Copy link
Author

thanks @iishitahere for the PR

I would suggest you keep this PR clean and remove all unnecessary changes IMHO, this PR could be around 5 lines of code changes

Thank you for your feedback and for reviewing the PR. I sincerely appreciate your guidance.

I apologize for the unnecessary changes. I would like to keep the PR as clean and concise as possible. Could you kindly point out where exactly the code changes are needed so that I can work on it and ensure the PR is focused on resolving the issue effectively?

Thank you for your time and support. I look forward to your response.

Best regards,
Ishita Jaiswal

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants