Skip to content

feat: Implement health checks for Service resources#41

Open
hamzabouissi wants to merge 3 commits into
mainfrom
feat/gluekube_integration
Open

feat: Implement health checks for Service resources#41
hamzabouissi wants to merge 3 commits into
mainfrom
feat/gluekube_integration

Conversation

@hamzabouissi

@hamzabouissi hamzabouissi commented Oct 20, 2025

Copy link
Copy Markdown
Contributor

User description

Add custom health check logic for Services in Argo CD.


PR Type

Enhancement


Description

  • Add custom health check logic for Kubernetes Service resources

  • Evaluate service exposure via externalIPs or LoadBalancer type

  • Return appropriate health status based on provisioning state

  • Support LoadBalancer ingress detection for health assessment


Diagram Walkthrough

flowchart LR
  A["Service Resource"] --> B{"Has externalIPs?"}
  B -->|Yes| C["Status: Healthy"]
  B -->|No| D{"Type: LoadBalancer?"}
  D -->|Yes| E{"Ingress provisioned?"}
  E -->|Yes| F["Status: Healthy"]
  E -->|No| G["Status: Progressing"]
  D -->|No| H["Status: Healthy"]
Loading

File Walkthrough

Relevant files
Enhancement
argocd.yaml.tpl
Implement Service resource health check logic                       

argocd.yaml.tpl

  • Add Lua-based custom health check for Service resources in Argo CD
    configuration
  • Check for externalIPs and mark service as Healthy if present
  • Evaluate LoadBalancer type services and check ingress provisioning
    status
  • Return Progressing status when LoadBalancer is still being provisioned
+26/-0   

Add custom health check logic for Services in Argo CD.
@codiumai-pr-agent-free

codiumai-pr-agent-free Bot commented Oct 20, 2025

Copy link
Copy Markdown

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
- [ ] Create ticket/issue <!-- /create_ticket --create_ticket=true -->

</details></td></tr>
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
No custom compliance provided

Follow the guide to enable custom compliance check.

  • Update
Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@github-actions github-actions Bot added enhancement New feature or request minor and removed Review effort 2/5 labels Oct 20, 2025

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 adds custom health check logic for Kubernetes Service resources in Argo CD. The implementation evaluates Service health based on the service type and configuration, specifically handling LoadBalancer services and services with external IPs.

Key Changes:

  • Added Lua-based health check function for Service resources
  • Implemented health status evaluation for LoadBalancer type services
  • Added support for services with externalIPs configuration

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread argocd.yaml.tpl Outdated
Comment thread argocd.yaml.tpl Outdated
Comment thread argocd.yaml.tpl
@codiumai-pr-agent-free

codiumai-pr-agent-free Bot commented Oct 20, 2025

Copy link
Copy Markdown

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
Consider service endpoints for health

Improve the Service health check by verifying the existence of active endpoints.
A Service without endpoints is not functional and should not be considered
"Healthy".

Examples:

argocd.yaml.tpl [180-203]
    resource.customizations.health.Service: |
      hs = {}

      if obj.spec.externalIPs ~= nil and #obj.spec.externalIPs > 0 then
        hs.status = "Healthy"
        hs.message = "Service is exposed via externalIPs"
        return hs
      end

      if obj.spec.type == "LoadBalancer" then

 ... (clipped 14 lines)

Solution Walkthrough:

Before:

hs = {}
if obj.spec.externalIPs ~= nil then
  hs.status = "Healthy"
  return hs
end

if obj.spec.type == "LoadBalancer" then
  if obj.status.loadBalancer.ingress ~= nil then
    hs.status = "Healthy"
  else
    hs.status = "Progressing"
  end
else
  hs.status = "Healthy" -- Problem: Healthy even with no endpoints
end
return hs

After:

-- (Assuming a function to get related Endpoints resource exists)
endpoints = getResource("v1", "Endpoints", obj.metadata.name, obj.metadata.namespace)

if endpoints == nil or endpoints.subsets == nil or #endpoints.subsets == 0 then
  hs.status = "Progressing"
  hs.message = "Service has no active endpoints"
  return hs
end

-- Existing logic for LoadBalancer/externalIPs can follow
if obj.spec.type == "LoadBalancer" and (obj.status.loadBalancer.ingress == nil) then
  hs.status = "Progressing"
  hs.message = "Waiting for LoadBalancer to be provisioned"
  return hs
end

hs.status = "Healthy"
return hs
Suggestion importance[1-10]: 9

__

Why: This suggestion points out a critical flaw in the health check logic, as a Service without active endpoints is non-functional and reporting it as Healthy can mask serious application issues.

High
Possible issue
Prevent nil pointer dereference error
Suggestion Impact:The commit implemented exactly the suggested change by adding a nil check for obj.status before accessing obj.status.loadBalancer in line 14, which prevents potential nil pointer dereference errors

code diff:

-        if obj.status.loadBalancer ~= nil and obj.status.loadBalancer.ingress ~= nil and #obj.status.loadBalancer.ingress > 0 then
+        if obj.status ~= nil and obj.status.loadBalancer ~= nil and obj.status.loadBalancer.ingress ~= nil and #obj.status.loadBalancer.ingress > 0 then

Add a nil check for obj.status before accessing obj.status.loadBalancer to
prevent a runtime error in the Lua health check script.

argocd.yaml.tpl [190-197]

-if obj.status.loadBalancer ~= nil and obj.status.loadBalancer.ingress ~= nil and #obj.status.loadBalancer.ingress > 0 then
+if obj.status ~= nil and obj.status.loadBalancer ~= nil and obj.status.loadBalancer.ingress ~= nil and #obj.status.loadBalancer.ingress > 0 then
   hs.status = "Healthy"
   hs.message = "LoadBalancer has been provisioned"
 else
   -- If no ingress IP, the LoadBalancer is still being created.
   hs.status = "Progressing"
   hs.message = "Waiting for LoadBalancer to be provisioned"
 end

[Suggestion processed]

Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies a potential nil pointer dereference when accessing obj.status.loadBalancer on a newly created Service, which would cause the Lua health check to fail.

Medium
  • Update

hamzabouissi and others added 2 commits October 21, 2025 11:57
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@github-actions github-actions Bot added the patch label Oct 21, 2025
Comment thread argocd.yaml.tpl
hs.message = "Waiting for LoadBalancer to be provisioned"
end
else
hs.status = "Healthy"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Does this default to healthy?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants