Skip to content

Conversation

@phlogistonjohn
Copy link
Collaborator

When the samba-container config-update --watch command is run on a CTDB enabled server we avoid updating the local config unless the node is the ctdb leader. However, the update function was returning the current config which the watch loop assigned to the previous var, meaning that the current and previous config would be equal on future trips around the loop. Therefore once the node became the leader it would not update the config even if it differed from the config in samba. This is a problem on start up because the loop initially runs before ctdb is ready and the node leadership is not known. Change it so that when the node is not a leader it returns None and "unsets" the previous config. This could lead to extra unneeded updates but should avoid missing updates that are needed.

When the `samba-container config-update --watch` command is run
on a CTDB enabled server we avoid updating the local config unless
the node is the ctdb leader. However, the update function was returning
the current config which the watch loop assigned to the previous
var, meaning that the current and previous config would be equal on
future trips around the loop. Therefore once the node became the leader
it would not update the config even if it differed from the config
in samba. This is a problem on start up because the loop initially
runs before ctdb is ready and the node leadership is not known.
Change it so that when the node is not a leader it returns None
and "unsets" the previous config. This could lead to extra unneeded
updates but should avoid missing updates that are needed.

Signed-off-by: John Mulligan <[email protected]>
@phlogistonjohn phlogistonjohn marked this pull request as ready for review May 7, 2025 17:32
@anoopcs9
Copy link
Collaborator

anoopcs9 commented May 9, 2025

What is the abnormality seen with the current code? Did we observe a cluster stuck in UNHEALTHY or some other undesired state?

@phlogistonjohn
Copy link
Collaborator Author

The observed issue was that on the Ceph SMB system there's a brief race condition where if you create shares while the ceph orchestration is still rolling out the first set of containers it will apply the "initial config" to smb but the config watch sidecar will not do anything. There's a few more details in the commit message.

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.

lgtm.

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!

@mergify mergify bot merged commit 3b0ac22 into samba-in-kubernetes:master May 15, 2025
9 checks passed
@phlogistonjohn phlogistonjohn deleted the jjm-no-cache-nonleader branch June 9, 2025 14:08
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.

3 participants