Skip to content

Limit pre-validation retries, add CLI improvements and Docker log dir#244

Open
devanshjainms wants to merge 1 commit intoAzure:development-feb-2026from
devanshjainms:remove-retries
Open

Limit pre-validation retries, add CLI improvements and Docker log dir#244
devanshjainms wants to merge 1 commit intoAzure:development-feb-2026from
devanshjainms:remove-retries

Conversation

@devanshjainms
Copy link
Copy Markdown
Contributor

@devanshjainms devanshjainms commented Apr 27, 2026

Summary

Multiple small improvements: pre-validation retry limits, CLI enhancements, and Docker log directory setup.

Changes

Pre-validation retries (DB HA & SCS HA)

  • pre-validations-db.yml: Changed retries from {{ default_retries }} (75) to hardcoded 2
  • pre-validations-scs.yml: Added until, retries: 2, and delay to cluster status check (previously had no retry logic)

CLI improvements (sap_automation_qa.sh)

  • Added ANSIBLE_CHECK_MODE support — when set to true, passes --syntax-check to ansible-playbook
  • Auto-override TEST_TYPE to SAPFunctionalTests when --test_groups is specified (prevents misconfiguration)

Docker (Dockerfile)

  • Create /app/data/logs/service directory with proper ownership for service log storage

Testing

  • All 748 unit tests pass with 85.23% coverage (above 85% threshold)

@devanshjainms devanshjainms requested a review from a team as a code owner April 27, 2026 18:21
@devanshjainms devanshjainms requested review from mkdeegan and removed request for a team April 27, 2026 18:21
Pre-validation:
- Reduce cluster status check retries from default_retries (75) to 2
  in DB HA and SCS HA pre-validation tasks
- Add retry logic to SCS pre-validation (previously had none)

CLI (sap_automation_qa.sh):
- Add ANSIBLE_CHECK_MODE support for syntax-check mode
- Auto-override TEST_TYPE to SAPFunctionalTests when --test_groups is set

Docker:
- Create /app/data/logs/service directory in Dockerfile for service logs

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@devanshjainms devanshjainms changed the title Limit pre-validation cluster status retries to 2 Limit pre-validation retries, add CLI improvements and Docker log dir Apr 27, 2026
@devanshjainms devanshjainms requested a review from Copilot April 28, 2026 02:31
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR makes small operational and UX improvements across the HA Ansible roles, the CLI wrapper script, and the container image—primarily to reduce long pre-validation waits, improve CLI ergonomics, and ensure a service log directory exists in Docker.

Changes:

  • Limit/introduce retry behavior for SCS/DB pre-validation cluster status checks.
  • Add CLI behavior: optional Ansible syntax-check mode and auto-adjust TEST_TYPE when --test_groups is used.
  • Ensure /app/data/logs/service exists in the Docker image with correct ownership.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
src/roles/misc/tasks/pre-validations-scs.yml Adds until/retries/delay to SCS cluster status pre-validation.
src/roles/misc/tasks/pre-validations-db.yml Reduces DB HA cluster status pre-validation retries from configurable default to a hardcoded value.
scripts/sap_automation_qa.sh Adds env-driven Ansible syntax-check mode and auto-overrides TEST_TYPE when filtering by test groups.
deploy/Dockerfile Creates a persistent service log directory under /app/data/logs/service with app user ownership.

register: cluster_status_pre
until: cluster_status_pre.ascs_node | default('') != "" and
cluster_status_pre.ers_node | default('') != ""
retries: 2
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

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

retries: 2 hardcodes the pre-validation wait window and bypasses the existing default_retries configuration (defined in src/vars/input-api.yaml). This can make SCS pre-validations flaky on slower clusters and removes the ability to tune retries per environment. Consider introducing a dedicated variable (e.g., pre_validation_retries defaulting to 2) and using that here so the limit is still configurable when needed.

Suggested change
retries: 2
retries: "{{ pre_validation_retries | default(2) }}"

Copilot uses AI. Check for mistakes.
until: cluster_status_pre.primary_node != "" and
cluster_status_pre.secondary_node != ""
retries: "{{ default_retries }}"
retries: 2
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

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

Changing cluster pre-validation from retries: {{ default_retries }} (75 by default) to retries: 2 significantly reduces the time allowed for the cluster to reach a stable state and removes configurability that other HA tasks still rely on. To keep the new shorter default without risking unnecessary test skips, consider using a separate variable (default 2) that can be overridden in input config when environments need longer stabilization.

Suggested change
retries: 2
retries: "{{ pre_validation_cluster_status_retries | default(2) }}"

Copilot uses AI. Check for mistakes.
Comment on lines +446 to +447
if [[ "${ANSIBLE_CHECK_MODE:-}" == "true" ]]; then
command+=" --syntax-check"
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

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

The new ANSIBLE_CHECK_MODE switch enables --syntax-check, which is different from Ansible's actual “check mode” (--check). The env var name is likely to mislead users into thinking this runs a dry-run against hosts. Consider renaming the flag to something like ANSIBLE_SYNTAX_CHECK (or, if the intent really is check mode, pass --check instead of --syntax-check).

Suggested change
if [[ "${ANSIBLE_CHECK_MODE:-}" == "true" ]]; then
command+=" --syntax-check"
if [[ "${ANSIBLE_SYNTAX_CHECK:-}" == "true" ]]; then
command+=" --syntax-check"
log "INFO" "Syntax-check mode enabled (ANSIBLE_SYNTAX_CHECK=true)"
elif [[ "${ANSIBLE_CHECK_MODE:-}" == "true" ]]; then
command+=" --syntax-check"
log "WARNING" \
"ANSIBLE_CHECK_MODE is deprecated for syntax validation; use " \
"ANSIBLE_SYNTAX_CHECK=true instead"

Copilot uses AI. Check for mistakes.
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.

2 participants