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

mount.glusterfs: Increase the max characters allow in the hostnames of glusterservers #4331

Open
wants to merge 1 commit into
base: devel
Choose a base branch
from

Conversation

spotzero
Copy link

@spotzero spotzero commented Apr 4, 2024

The mount scripts validate hostnames by checking that they are less than 64 characters long. Host names on many cloud environments are generally much longer than this, but are still valid.

This change increases the hostname length to allow for these valid hostname.

@gluster-ant
Copy link
Collaborator

Can one of the admins verify this patch?

2 similar comments
@gluster-ant
Copy link
Collaborator

Can one of the admins verify this patch?

@gluster-ant
Copy link
Collaborator

Can one of the admins verify this patch?

@aravindavk
Copy link
Member

/run regression

Copy link
Member

@aravindavk aravindavk left a comment

Choose a reason for hiding this comment

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

Looks good. Please sign the commit using git commit --amend -s

@gluster-ant
Copy link
Collaborator

0 test(s) failed

1 test(s) generated core
./tests/bugs/glusterd/mgmt-handshake-and-volume-sync-post-glusterd-restart.t

2 test(s) needed retry
./tests/00-geo-rep/georep-basic-dr-rsync.t
./tests/basic/afr/ta-shd.t
https://build.gluster.org/job/gh_centos7-regression/3392/

…f glusterservers

Signed-off-by: David Pascoe-Deslauriers <[email protected]>
@spotzero
Copy link
Author

spotzero commented Apr 5, 2024

@aravindavk I've amended the commit to include the sign-off. Thanks!

Copy link
Member

@aravindavk aravindavk left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -35,7 +35,7 @@ _init ()
LOG_DEBUG=DEBUG;
LOG_TRACE=TRACE;

HOST_NAME_MAX=64;
HOST_NAME_MAX=255;
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to change at ctx.c(https://github.com/gluster/glusterfs/blob/517d75e611322dd5757d5d9231e98356273c15f9/libglusterfs/src/ctx.c#L53C5-L53C42) also. After this change maximum hostname length would be 255 chars but ctx->hostname still would be sysconf(_SC_HOST_NAME_MAX) and on my system the value is 64.

Copy link
Author

Choose a reason for hiding this comment

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

I did some quick research to try and understand what's going on.

From what I've read, it looks like HOST_NAME_MAX (_SC_HOST_NAME_MAX) does not refer to the FQDN, just the hostname.

The numbers do (sort of) line up with https://www.rfc-editor.org/rfc/rfc1035, which limits the FQDN to 255 bytes, but any labels to a max of 63 bytes. But looks like rfc1035 labels are 63 bytes + null and linux hostnames can be 64 bytes + null, so maybe I'm interpreting things wrong.

Pragmatically, with the glusters setup where we ran into issues, we only ran into issues with the mount commands and with this change the mounts work correctly. Checking gluster, the peers are connected via FQDNs longer than 64 characters and it's working correctly.

So I'm not sure how the MAX length in ctx plays in with all of the rest unfortunately.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think you are correct, we don;t need to change it.

@mohit84
Copy link
Contributor

mohit84 commented Apr 11, 2024

/run regression

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

Successfully merging this pull request may close these issues.

None yet

4 participants