Skip to content

Conversation

@phlogistonjohn
Copy link
Collaborator

Add a mechanism for changing the CTDB port.

The samba wiki documents the way to have CTDB use a different port is
to edit /etc/services. While this seems a bit strange we are running
in a container so we won't side-effect anything else.

See: https://wiki.samba.org/index.php/Advanced_CTDB_configuration#CTDB_port

Signed-off-by: John Mulligan <[email protected]>
@phlogistonjohn phlogistonjohn marked this pull request as ready for review June 12, 2025 18:48
@phlogistonjohn
Copy link
Collaborator Author

Basic manual testing on a ceph cluster passed.

Copy link
Collaborator

@spuiuk spuiuk left a comment

Choose a reason for hiding this comment

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

Minor change requested.

But otherwise ACK.

Copy link
Collaborator

@avanthakkar avanthakkar left a comment

Choose a reason for hiding this comment

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

LGTM!
Just one thing I read in wiki
"Note: All nodes in the cluster must use the same port.", so was wondering if we need to explicitly check this condition?

@mergify mergify bot merged commit 2f89a38 into samba-in-kubernetes:master Jun 13, 2025
10 checks passed
Copy link
Collaborator

@anoopcs9 anoopcs9 left a comment

Choose a reason for hiding this comment

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

Just one thing I read in wiki
"Note: All nodes in the cluster must use the same port.", so was wondering if we need to explicitly check this condition?

Be it any configuration file (/etc/services in this case) this is implicitly taken care of as each node is acting based on the same instance config passed down from higher levels.

@phlogistonjohn phlogistonjohn deleted the jjm-ctdb-port branch June 17, 2025 23:48
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.

4 participants