ansible-scylla-monitoring: make start wrapper idempotent for systemd unit#540
Draft
cursor[bot] wants to merge 13 commits into
Draft
ansible-scylla-monitoring: make start wrapper idempotent for systemd unit#540cursor[bot] wants to merge 13 commits into
cursor[bot] wants to merge 13 commits into
Conversation
Check systemctl is-active scylla-server while polling nodetool during cluster health and replace-node UN waits, matching start_one_node.yml. Fixes scylladb/field-engineering#2109 Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Addresses Copilot review: systemctl is-active can return deactivating, unknown, etc. Fail for any state other than active/activating and include the actual state in the error message for diagnosis. Co-authored-by: Cursor <cursoragent@cursor.com>
* Fail fast when scylla-server stops during bootstrap health waits Check systemctl is-active scylla-server while polling nodetool during cluster health and replace-node UN waits, matching start_one_node.yml. Fixes scylladb/field-engineering#2109 Co-authored-by: Cursor <cursoragent@cursor.com> * ci: retrigger molecule after scylla-manager flake Co-authored-by: Cursor <cursoragent@cursor.com> * fix: allowlist active/activating states instead of blocklisting Addresses Copilot review: systemctl is-active can return deactivating, unknown, etc. Fail for any state other than active/activating and include the actual state in the error message for diagnosis. Co-authored-by: Cursor <cursoragent@cursor.com> * scylla-manager: automatically set Alternator credentials Newer Scylla Managers in combination with Alternator-enabled clusters, will attempt to retrieve the Alternator schema using Alternator credentials when performing a backup. In order to be able to perform a backup, we must set these credentials. Fixes https://scylladb.atlassian.net/browse/ANSROLES-17 Signed-off-by: Vincent Reniers <vincent.reniers@scylladb.com> Co-authored-by: Cursor <cursoragent@cursor.com> --------- Signed-off-by: Vincent Reniers <vincent.reniers@scylladb.com> Co-authored-by: manjots <manjot.singh@scylladb.com> Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: Manjot Singh <manjot@manjotsingh.net>
* Persist scylla-monitoring start/stop wrapper scripts on deploy.
Write /opt/scylla-monitoring/{start,stop}-monitoring.sh during monitoring
role deploy so operators can restart the stack without re-running ansible
or reconstructing start-all.sh arguments by hand.
Co-authored-by: Cursor <cursoragent@cursor.com>
* Address Copilot review on monitoring wrapper scripts.
Quote paths and credentials, use conditional sudo for stop, move CQL
secrets to a 0600 env file sourced at runtime, and restore env vars for
ansible-driven starts.
Co-authored-by: Cursor <cursoragent@cursor.com>
* ci: retrigger molecule workflow
Co-authored-by: Cursor <cursoragent@cursor.com>
---------
Co-authored-by: Cursor <cursoragent@cursor.com>
The assumption cannot be made that the hostname is DNS resolvable by all nodes in the cluster. It's more appropriate to require that the seed nodes are specified explicitly. Signed-off-by: Vincent Reniers <vincent.reniers@scylladb.com>
Removing the implicit scylla_seeds default meant the role would only surface an undefined-variable error deep into common.yml. Add an early explicit check so misconfiguration fails fast with a clear message instead. Also fix the commented example to use consistent groups['scylla'][0] bracket indexing and drop trailing whitespace. Co-authored-by: Cursor <cursoragent@cursor.com>
ansible-scylla-node: remove default scylla_seeds
* Persist iptables DROP rules via ansible-scylla-node variable Add iptables_drop_sources to ansible-scylla-node. When firewall_enabled is true, the role applies INPUT DROP rules for each listed source IP. Remove the ad-hoc iptables DROP task from replace_node.yml so blocking is owned by the role (and therefore by cluster vars), not by a one-off playbook side effect. Field Cloud replace jobs set iptables_drop_sources before replace and clear it after wiped nodes, then re-run ansible-scylla-node. * Address review: move iptables DROP into ansible-scylla-common Merge disable_firewall + iptables_drop_sources into handle_firewall.yml, gate DROP on not disable_firewall, drop block_replaced_node and stale comments. Co-authored-by: Cursor <cursoragent@cursor.com> * Clean up comments * ansible-scylla-node: bump API port wait timeout to 600s rockylinux9/10 Molecule hits Timeout when waiting for 127.0.0.1:10000 under loaded runners; same flaky class as prior readiness bumps. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Allow opt-in install of scylla-monitoring.service so the stack restarts after reboot via the existing start/stop wrappers. Co-authored-by: Cursor <cursoragent@cursor.com>
- Start the unit (state: started) so systemctl status reflects the running stack right after deploy, not just after reboot. - Use short systemd module name for consistency with the rest of docker.yml. - Add disable/remove cleanup when install_scylla_monitoring_service is false so flipping the flag off no longer leaves an enabled unit behind; running stack is left untouched. - Add reload systemd handler for unit removal. - Exercise the flag in molecule: enable it in converge and assert the unit is installed and enabled in verify. Co-authored-by: Cursor <cursoragent@cursor.com>
Security review flagged a local privilege escalation: the root-run systemd
unit executed a wrapper script owned by the (potentially non-root) deploy
user, who could edit it and have it run as root on boot.
- Run the unit as the deploy user by default (User=/Group={{ ansible_user_id }}),
matching the manual start path so root no longer executes a user-owned script.
- When run_docker_with_sudo is set, docker requires root and the unit runs as
root, so write the wrapper scripts root:root to keep them non-writable by the
deploy user.
Co-authored-by: Cursor <cursoragent@cursor.com>
…unit The 'Enable scylla-monitoring at boot' task sets state: started on the scylla-monitoring.service unit right after the stack was already started manually via the same start wrapper a few tasks earlier. Since the unit was freshly installed (and thus inactive from systemd's point of view), enabling it with state: started re-runs ExecStart (start-monitoring.sh -> start-all.sh) against containers that are already up, so start-all.sh fails with a container/name conflict and systemd reports 'control process exited with error code'. Make start-monitoring.sh.j2 tear down any already-running stack (via the already-idempotent stop wrapper) before starting, so it is safe to invoke back-to-back from the manual start task and then again from the systemd unit. Co-authored-by: Manjot Singh <ManjotS@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
CI (
build (ubuntu2404),build (rockylinux9),build (debian11), and others) fails onfeat/optional-monitoring-boot-service(PR #536) at theansible-scylla-monitoring : docker.yml: Enable scylla-monitoring at boottask:Root cause
Commit a547c62 ("address PR review on boot service") added
state: startedto the systemd task that enablesscylla-monitoring.service. That task runs afterdocker.yml: start scylla-monitoring, which already invoked the same start wrapper (start-monitoring.sh->start-all.sh) manually a few tasks earlier. Because the unit was just installed (and daemon-reloaded), systemd considers it inactive, sostate: startedre-runsExecStartagainst a stack that is already up, andstart-all.shfails on the container/name conflict, which systemd reports as "control process exited with error code".Fix
Make
start-monitoring.sh.j2tear down any already-running stack (via the existing, already-idempotent stop wrapper,... || true) before starting. This makes the start wrapper safe to invoke back-to-back — once from the manual deploy task and again from the systemd unit — without changing the intended behavior (systemctl statusstill reflects the running stack after deploy and after reboot).Branched from the tip of
feat/optional-monitoring-boot-service(004d9ac) so the fix can be reviewed/merged independently; opened againstdevper this automation's configuration.