Skip to content

fix: make health_checks optional in service deployment module#520

Merged
mbhandari31 merged 2 commits into
masterfrom
fix/service-health-checks-optional
Apr 8, 2026
Merged

fix: make health_checks optional in service deployment module#520
mbhandari31 merged 2 commits into
masterfrom
fix/service-health-checks-optional

Conversation

@srikxcipher

@srikxcipher srikxcipher commented Mar 15, 2026

Copy link
Copy Markdown
Contributor

Description

The health_checks is not required by the 3_utility/application/ helm chart template. The template guards probe generation with hasKey checks and uses default values throughout - missing health_checks simply results in no probes configured, which is a valid deployment state.

deployment.yaml line 198–199:
  {{- if ( hasKey .Values.spec.runtime "health_checks") }}
      {{- include "app-chart.liveness-readiness-check" . | nindent 8 }}

Note: The entire probe block is skipped if health_checks is absent. No probes = valid deployment.

Related issues

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Checklist

  • I have created feat/bugfix branch out of develop branch
  • Code passes linting/formatting checks
  • Changes to resources have been tested in our dev environments
  • I have made corresponding changes to the documentation

Testing

Reviewer instructions

Summary by CodeRabbit

  • Improvements
    • Health checks are now optional when configuring deployments and stateful services, allowing services to be deployed without mandatory health check specifications and providing greater flexibility in deployment scenarios.

health_checks is not required by the application/2.0 helm chart template.
The template guards probe generation with hasKey checks and uses default
values throughout - missing health_checks simply results in no probes
configured, which is a valid deployment state.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Mar 15, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 7b42b93f-8943-424f-b1f3-9ddb312dab10

📥 Commits

Reviewing files that changed from the base of the PR and between 2847764 and 8213b6f.

📒 Files selected for processing (1)
  • modules/service/statefulset/0.1/facets.yaml
💤 Files with no reviewable changes (1)
  • modules/service/statefulset/0.1/facets.yaml

Walkthrough

Removed health_checks from the spec.required lists in two facets schemas (deployment and statefulset). The health_checks field remains present in UI ordering but is no longer mandatory during spec validation.

Changes

Cohort / File(s) Summary
Service facets schemas
modules/service/deployment/0.1/facets.yaml, modules/service/statefulset/0.1/facets.yaml
Deleted health_checks from spec.required, making the field optional while keeping it in x-ui-order. No other schema structure or ordering changes.
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: making health_checks optional in the service deployment module, which aligns with the changeset modifications to both deployment and statefulset facets.yaml files.
Description check ✅ Passed The description includes a clear rationale explaining why health_checks should be optional, references the helm chart template behavior, and completes the required template sections including type of change and relevant checklist items.
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 unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/service-health-checks-optional
📝 Coding Plan
  • Generate coding plan for human review comments

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.

Tip

CodeRabbit can scan for known vulnerabilities in your dependencies using OSV Scanner.

OSV Scanner will automatically detect and report security vulnerabilities in your project's dependencies. No additional configuration is required.

@mbhandari31
mbhandari31 merged commit ceacb93 into master Apr 8, 2026
2 checks passed
@mbhandari31
mbhandari31 deleted the fix/service-health-checks-optional branch April 8, 2026 05:31
anujhydrabadi added a commit that referenced this pull request Jun 26, 2026
…ti-affinity optional (#555)

Two module-schema bugs that caused false-positive pre-flight validation
errors (raptor create release) while the platform/Deployer accepts the
specs fine.

mongo/k8s/0.2: the size schema declared `cpu`/`memory` (required), but the
v0.2 Terraform (facets-iac .../1_input_instance/mongodb/main.tf) reads
`cpu_requests`/`memory_requests` behind a `contains(keys, "cpu_requests")`
guard. The `cpu`/`memory` naming only belongs to the 0.3 TF. As a result a
blueprint following the 0.2 schema would silently get NO resource requests,
and a correct blueprint (using cpu_requests/memory_requests, as prod does)
fails validation with "spec.size.memory required field is missing". Rename
the request fields to cpu_requests/memory_requests so the schema matches the
v0.2 module and real usage; limit fields and their x-ui-compare refs updated
to match.

service/deployment/0.1: `enable_host_anti_affinity` was in the spec
`required` list with no default, but the shared app-chart helm template
guards it with `hasKey` and treats absence as "no anti-affinity". The field
is behaviorally optional, so requiring it rejects valid specs (e.g.
service/control-plane-ui). Remove it from `required` (kept in properties and
x-ui-order). Follows the precedent of #520 (health_checks made optional).

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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