chore: bump nfs-ganesha to v6.2 #364
Conversation
Longhorn 9614 Signed-off-by: Derek Su <[email protected]>
WalkthroughThe changes in this pull request involve updates to the Dockerfile for the NFS Ganesha server, including version upgrades for the base image, source code, and dependencies. The base image has been updated from version 15.5 to 15.6, and the NFS Ganesha source code has been upgraded from version 5_20240716 to 6_20241120. Additionally, the compiler versions have been specified, and the build configuration has been slightly modified by removing a specific flag. Changes
Assessment against linked issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
package/Dockerfile (2)
15-18: Consider pinning package versions for reproducible buildsWhile using gcc14 is good for consistency, consider pinning versions for all packages to ensure reproducible builds across different environments.
Example:
-RUN zypper -n install autoconf bison curl cmake doxygen make git gcc14 gcc14-c++ flex Mesa-libGL-devel libdbus-1-3 \ +RUN zypper -n install autoconf=2.69 bison=3.0.4 curl=7.79.1 cmake=3.17.0 doxygen=1.8.17 make=4.2.1 git=2.35.3 gcc14=14.0.0 gcc14-c++=14.0.0 \🧰 Tools
🪛 GitHub Check: CodeFactor
[warning] 15-15: package/Dockerfile#L15
Specify version withzypper install -y <package>=<version>. (DL3037)
[warning] 15-15: package/Dockerfile#L15
zypper cleanmissing after zypper use. (DL3036)
Line range hint
1-24: Consider documenting version upgrade impactsThis is a significant version upgrade that might affect:
- Protocol compatibility with existing NFS clients
- Performance characteristics
- Resource requirements
Consider:
- Adding upgrade notes to documentation
- Updating any version-specific configurations
- Planning for regression testing
Also applies to: 30-34
🧰 Tools
🪛 GitHub Check: CodeFactor
[warning] 15-15: package/Dockerfile#L15
Specify version withzypper install -y <package>=<version>. (DL3037)
[warning] 15-15: package/Dockerfile#L15
zypper cleanmissing after zypper use. (DL3036)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
package/Dockerfile(1 hunks)
🧰 Additional context used
🪛 GitHub Check: CodeFactor
package/Dockerfile
[warning] 15-15: package/Dockerfile#L15
Specify version with zypper install -y <package>=<version>. (DL3037)
[warning] 15-15: package/Dockerfile#L15
zypper clean missing after zypper use. (DL3036)
🔇 Additional comments (3)
package/Dockerfile (3)
12-12: LGTM: Consistent version update for base image and repository
The repository URL update matches the base image version (15.6), maintaining system compatibility.
20-24: Verify version compatibility between nfs-ganesha and ntirpc
The upgrade to nfs-ganesha v6_20241120 and ntirpc v6.0.1 looks correct. Let's verify the compatibility.
30-34: Verify impact of removed USE_NFS3 flag
The explicit compiler paths are good for build reproducibility. However, the removal of -DUSE_NFS3=OFF flag needs verification:
- Was this intentional?
- Does this mean NFSv3 support is now enabled by default?
✅ Verification successful
Removal of USE_NFS3 flag is consistent with intended NFSv4-only support
The removal of -DUSE_NFS3=OFF flag is intentional and aligns with the codebase's design. Evidence from the Dockerfile shows this is explicitly a NFSv4-only build:
- Comment states:
# build ganesha only supporting nfsv4 and vfs - The installed packages focus on NFSv4 support (nfs4-acl-tools)
- The codebase shows NFSv4-specific references
The flag removal doesn't enable NFSv3 support as the build is already configured for NFSv4-only operation through other mechanisms.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check default NFS3 configuration in CMakeLists.txt
rg -A 5 "OPTION.*USE_NFS3"
# Check if there are any related configuration changes in the new version
ast-grep --pattern 'OPTION.*USE_NFS3.*$_'
Length of output: 72
Script:
#!/bin/bash
# Let's check CMakeLists.txt files for any NFS3-related configuration
fd CMakeLists.txt --exec cat {} | grep -i "nfs3\|nfsv3"
# Search for any NFS3 or NFSv3 related strings in the codebase
rg -i "nfs3\|nfsv3" --type cmake
# Let's also check if there are any NFS protocol version related configurations
fd CMakeLists.txt --exec cat {} | grep -i "option.*nfs"
Length of output: 303
Script:
#!/bin/bash
# Let's check for any NFS configuration in CMake files without type restriction
rg -i "option.*nfs"
# Look for NFS protocol version configurations in any file
rg -i "nfs.*version|nfsv|nfs[2-4]"
# Check if there are any NFS-related build flags in the Dockerfile history
rg -i "use_nfs" package/Dockerfile
Length of output: 9273
Which issue(s) this PR fixes:
Issue longhorn/longhorn#9614
What this PR does / why we need it:
Special notes for your reviewer:
Additional documentation or context