Skip to content

fix: correct default healthcheck probe logic and enable probes by default#20

Open
AdheipSingh wants to merge 1 commit into
apache:masterfrom
deep-bi:fix/default-healthcheck-probes
Open

fix: correct default healthcheck probe logic and enable probes by default#20
AdheipSingh wants to merge 1 commit into
apache:masterfrom
deep-bi:fix/default-healthcheck-probes

Conversation

@AdheipSingh
Copy link
Copy Markdown
Member

@AdheipSingh AdheipSingh commented May 14, 2026

Fixes #XXXX.

Description

Summary

  • Fix inverted inequality checks in setDefaultProbe that incorrectly applied readiness paths to startup probes
  • Replace != conditions with explicit equality checks using probe type constants
  • Change DefaultProbes from bool to *bool — nil (field omitted) defaults to true, explicit false preserved for backward compatibility

Test plan

  • Verified historical probes: liveness /status/health, readiness /druid/historical/v1/readiness, startup /druid/historical/v1/readiness
  • Verified broker probes: liveness /status/health, readiness /druid/broker/v1/readiness, startup /druid/broker/v1/readiness
  • Verified defaultProbes: false removes all default probes (backward compat)
  • Verified defaultProbes: true restores all default probes

This PR has:

  • been tested on a real K8S cluster to ensure creation of a brand new Druid cluster works.
  • been tested for backward compatibility on a real K*S cluster by applying the changes introduced here on an existing Druid cluster. If there are any backward incompatible changes then they have been noted in the PR description.
  • added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
  • added documentation for new or modified features or behaviors.

Key changed/added files in this PR
  • MyFoo
  • OurBar
  • TheirBaz

@AdheipSingh AdheipSingh force-pushed the fix/default-healthcheck-probes branch from ee31246 to ab7f3fc Compare May 14, 2026 21:06
@pjain1
Copy link
Copy Markdown
Member

pjain1 commented May 15, 2026

Codex review

  1. Medium - apis/druid/v1alpha1/druid_types.go:276: DefaultProbes changed to *bool, but generated artifacts are not updated. make generate adds the missing deep-copy block for the bool pointer; without it, DruidSpec.DeepCopy() aliases the pointer instead of copying it. make
     manifests also changes config/crd/... and chart/crds/..., and make api-docs updates the API docs.
  2. Medium - apis/druid/v1alpha1/druid_types.go:273: the field comment says DefaultProbes defaults to true, but the PR removes +kubebuilder:default:=true. Once manifests are regenerated, the CRD no longer advertises or applies that default. I’d keep the CRD default marker on the
     *bool field and still keep the nil check in the controller for in-memory/un-defaulted objects.

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.

3 participants