Skip to content

[installer] Bandage broken conditionals#4126

Merged
imnasnainaec merged 1 commit intomasterfrom
installer-string-bool-arg
Jan 30, 2026
Merged

[installer] Bandage broken conditionals#4126
imnasnainaec merged 1 commit intomasterfrom
installer-string-bool-arg

Conversation

@imnasnainaec
Copy link
Collaborator

@imnasnainaec imnasnainaec commented Jan 30, 2026

Fixes #4125


This change is Reviewable

Summary by CodeRabbit

  • Chores
    • Enhanced Kubernetes installation logging to provide better visibility into the deployment process.
    • Updated deployment configuration settings for improved compatibility during installation.

✏️ Tip: You can customize this high-level summary in your review settings.

@imnasnainaec imnasnainaec self-assigned this Jan 30, 2026
@imnasnainaec imnasnainaec added the 🟥High High-priority PR: please review this asap! label Jan 30, 2026
@coderabbitai
Copy link

coderabbitai bot commented Jan 30, 2026

📝 Walkthrough

Walkthrough

The installer script was updated to fix how Ansible handles string-to-boolean conversions in variable assignments. An environment variable was added to allow broken conditionals, and logging behavior was modified to unconditionally print the Kubernetes user information.

Changes

Cohort / File(s) Summary
Kubernetes Installer Script
deploy/scripts/install-combine.sh
Added ANSIBLE_ALLOW_BROKEN_CONDITIONALS=True environment variable before Ansible playbook execution to handle string boolean values. Replaced conditional debug logging with unconditional log output for the Kubernetes user.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

Suggested labels

bug

Suggested reviewers

  • jasonleenaylor

Poem

🐰 Ansible's booleans gave us quite a fright,
Strings masquerading as true/false in sight!
With conditionals broken, a flag to the rescue,
Our installer now works like it's supposed to!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main change: addressing broken conditionals in the installer by introducing a workaround for Ansible's handling of string-to-boolean conversion.
Linked Issues check ✅ Passed The PR successfully addresses issue #4125 by setting ANSIBLE_ALLOW_BROKEN_CONDITIONALS=True to enable automatic string-to-boolean conversion in Ansible conditionals.
Out of Scope Changes check ✅ Passed All changes are directly related to fixing the string-to-boolean conversion issue: the unconditional log print improves visibility, and the environment variable setting solves the root problem described in issue #4125.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch installer-string-bool-arg

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@imnasnainaec imnasnainaec marked this pull request as ready for review January 30, 2026 00:37
@codecov
Copy link

codecov bot commented Jan 30, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.73%. Comparing base (db43ae0) to head (113d3a8).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #4126   +/-   ##
=======================================
  Coverage   74.73%   74.73%           
=======================================
  Files         296      296           
  Lines       10958    10958           
  Branches     1369     1369           
=======================================
  Hits         8190     8190           
  Misses       2373     2373           
  Partials      395      395           
Flag Coverage Δ
backend 86.54% <ø> (ø)
frontend 65.67% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@deploy/scripts/install-combine.sh`:
- Around line 84-93: The script currently exports
ANSIBLE_ALLOW_BROKEN_CONDITIONALS globally which leaks the env var; instead,
scope it to the single ansible-playbook invocation by removing the export and
prepending ANSIBLE_ALLOW_BROKEN_CONDITIONALS=True only to the ansible-playbook
command that calls playbook_desktop_setup.yml (the line with ansible-playbook
... ${EXTRA_VARS} $(((DEBUG == 1)) && echo "-vv")). Keep the rest of the logic
(K8S_USER, EXTRA_VARS, airgap check, DEBUG handling) unchanged so only the
ansible-playbook process sees the env var.

@imnasnainaec imnasnainaec merged commit a16e634 into master Jan 30, 2026
24 checks passed
@imnasnainaec imnasnainaec deleted the installer-string-bool-arg branch January 30, 2026 00:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bash deployment 🟥High High-priority PR: please review this asap!

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[installer] String arguments not automatically converted to bool

1 participant