Skip to content

feat(swarm): constrain core services to initial installation node#155

Open
Schreini wants to merge 1 commit intoDokploy:mainfrom
Schreini:feat/constrain-core-services
Open

feat(swarm): constrain core services to initial installation node#155
Schreini wants to merge 1 commit intoDokploy:mainfrom
Schreini:feat/constrain-core-services

Conversation

@Schreini
Copy link

@Schreini Schreini commented Mar 20, 2026

Pin the Dokploy management services (web, postgres, and redis) to the node where Dokploy was first deployed. This ensures data persistence and service stability by preventing the orchestrator's core components from migrating across the Swarm cluster.

This is my first PR to an open source project. Please tell me if something is wrong.

Greptile Summary

This PR changes the Docker Swarm placement constraints for the three core Dokploy services (postgres, redis, and the web app) from node.role==manager (any manager) to node.labels.role==dokploy-main (the specific node where Dokploy was installed), and introduces the corresponding docker node update --label-add step to stamp that label. The approach is sound and solves a real problem (volume-backed data stores migrating to other managers), but there are two concerns in the implementation:

  • Critical – no error guard on docker node update: If the label command fails for any reason (hostname/node-name mismatch, permission issue, etc.), the script continues, creates all three services with the node.labels.role==dokploy-main constraint, and exits successfully — but the services will stay in a perpetual "pending" state because no node carries the label. Adding || { echo "…" >&2; exit 1; } or a simple if ! check would catch this.
  • Suggestion – use the Docker node ID instead of $(hostname): $(docker info --format '{{.Swarm.NodeID}}') is the canonical, Docker-native way to reference the current node and avoids any potential hostname ↔ Swarm-node-name mismatch.

Confidence Score: 3/5

  • Safe to merge after adding an error guard around docker node update; without it a silent failure leaves all core services permanently unscheduled.
  • The conceptual change (label-based placement instead of role-based) is correct and well-motivated. The single blocking concern is the missing error check: a failure in docker node update is silently swallowed and the install completes with no warning, but Dokploy is completely non-functional. The unquoted $(hostname) and hostname-vs-node-ID issues are minor but worth addressing before this ships to users.
  • apps/website/public/install.sh — specifically lines 241-244 (the new docker node update block)

Last reviewed commit: "feat(swarm): constra..."

Greptile also left 2 inline comments on this PR.

(2/5) Greptile learns from your feedback when you react with thumbs up/down!

Pin the Dokploy management services (web, postgres, and redis) to the
node where Dokploy was first deployed. This ensures data persistence
and service stability by preventing the orchestrator's core components
from migrating across the Swarm cluster.
@Schreini Schreini requested a review from Siumauricio as a code owner March 20, 2026 20:10
Comment on lines +241 to +244
# Add label to current node so we can constrain our services to this node
docker node update \
--label-add role=dokploy-main \
$(hostname)
Copy link

Choose a reason for hiding this comment

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

P1 Missing error check on docker node update

If docker node update fails (e.g. because the Swarm node name doesn't match the system hostname), the script will continue and create all three services with the constraint node.labels.role==dokploy-main. Since no node will carry that label, every service will remain permanently in a pending/unschedulable state and Dokploy will never start — with no error message from the install script.

Add an explicit failure check so the script aborts early if labelling fails:

Suggested change
# Add label to current node so we can constrain our services to this node
docker node update \
--label-add role=dokploy-main \
$(hostname)
# Add label to current node so we can constrain our services to this node
docker node update \
--label-add role=dokploy-main \
"$(hostname)" || { echo "Error: Failed to label Swarm node '$(hostname)'. Aborting." >&2; exit 1; }

Comment on lines +241 to +244
# Add label to current node so we can constrain our services to this node
docker node update \
--label-add role=dokploy-main \
$(hostname)
Copy link

Choose a reason for hiding this comment

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

P2 Prefer node ID over hostname for reliability

docker node update accepts a node name, node ID, or unique ID prefix. The Swarm node name defaults to the hostname at swarm init time, but using $(docker info --format '{{.Swarm.NodeID}}') directly retrieves the canonical node ID from Docker itself, making the label step immune to any mismatch between the OS hostname and the registered Swarm node name.

Suggested change
# Add label to current node so we can constrain our services to this node
docker node update \
--label-add role=dokploy-main \
$(hostname)
NODE_ID=$(docker info --format '{{.Swarm.NodeID}}')
docker node update \
--label-add role=dokploy-main \
"$NODE_ID" || { echo "Error: Failed to label Swarm node. Aborting." >&2; exit 1; }

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.

1 participant