Create k8s_failed_cronjobs.py - #49
Conversation
Initial commit of script checking Jobs spawned by CronJobs.
There was a problem hiding this comment.
Pull request overview
Adds a new Python MRPE/Nagios-style check that queries Kubernetes for CronJobs and their child Jobs (via ownerReferences) and reports failures via exit status.
Changes:
- Introduces
lakehouse/k8s_failed_cronjobs.pyto enumerate CronJobs/Jobs cluster-wide and compute an overall status. - Emits human-readable summary output intended for MRPE consumption.
Suppressed comments (3)
lakehouse/k8s_failed_cronjobs.py:72
len(failed_children) > 0is a global list across all CronJobs, so a failure in one CronJob can incorrectly downgrade a different CronJob to WARNING as soon as it has a successful Job. Track failures per CronJob when deciding whether to return WARNING vs OK for that CronJob.
elif any(c.type == "SuccessCriteriaMet" and c.status == "True" for c in conditions):
if (len(failed_children) > 0):
cronjobstatus=1
lakehouse/k8s_failed_cronjobs.py:87
- The script can print "all CronJobs OK" while returning exit status 3 (UNKNOWN), e.g. when there are no CronJobs or no child Jobs and
statusremains 3. Also, the "failed CronJobs" message is currently listing failed Job names. Make the output consistent with the exit code and label the list accurately.
if failed_children:
print(f"failed CronJobs: {', '.join(failed_children)} ; CronJobs checked: {', '.join(cronjob_names)}")
else:
# print(str(status) + f" ok {ns}/{name} ({len(children)} job(s))")
print(f"all CronJobs OK, CronJobs checked: {', '.join(cronjob_names)}")
lakehouse/k8s_failed_cronjobs.py:93
- Unhandled Kubernetes API/config exceptions will currently bubble up as a Python traceback, which is noisy for MRPE/local checks and may result in a non-Nagios exit status. Catch exceptions at the top-level and exit with UNKNOWN (3) and a single-line message.
if __name__ == "__main__":
sys.exit(main())
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
fix output if status is unknown
Unverified Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🔵 Needs a closer look
The current status calculation/output has correctness issues (global failure tracking affecting per-CronJob results and misleading messaging), and the cluster-wide Job listing may be too expensive for a monitoring check.
Review details
Suppressed comments (5)
Previously missed (1) — in code that hasn't changed since the last review.
lakehouse/k8s_failed_cronjobs.py:27
- Unclosed parenthesis in the comment.
This issue also appears in the following locations of the same file:
- line 38
- line 85
- line 90
lakehouse/k8s_failed_cronjobs.py:77
failed_childrenis global across all CronJobs, but it is used to decide per-CronJob status (line 78). After one CronJob has a failure, later CronJobs with only successful Jobs can incorrectly be marked WARNING. Also, success detection only checksSuccessCriteriaMet, but many clusters report Job success via theCompletecondition, so CronJobs may remain UNKNOWN even when successful.
if any(c.type == "Failed" and c.status == "True" for c in conditions):
failed_children.append(job.metadata.name)
cronjobstatus=2
# explicitly look for success
elif any(c.type == "SuccessCriteriaMet" and c.status == "True" for c in conditions):
lakehouse/k8s_failed_cronjobs.py:96
- The output always goes down the
failed_childrenbranch when any Job has ever failed, even when the overall state is WARNING (exit 1). The message also says "failed CronJobs" but the list contains Job names, which is confusing for operators.
if failed_children:
print(f"failed CronJobs: {', '.join(failed_children)} ; CronJobs checked: {', '.join(cronjob_names)}")
elif status == 0:
# print(str(status) + f" ok {ns}/{name} ({len(children)} job(s))")
print(f"all CronJobs OK, CronJobs checked: {', '.join(cronjob_names)}")
else:
print(f"status of CronJobs unknown, CronJobs checked: {', '.join(cronjob_names)}")
lakehouse/k8s_failed_cronjobs.py:40
- Listing all Jobs cluster-wide (
list_job_for_all_namespaces) can be very expensive on clusters with many historical Jobs, and may cause this MRPE/Nagios check to time out or overload the API server. Since CronJobs are already listed, you can restrict Job listing to just the namespaces that contain CronJobs.
cronjobs = batch.list_cron_job_for_all_namespaces().items
jobs = batch.list_job_for_all_namespaces().items
lakehouse/k8s_failed_cronjobs.py:88
- Global
statuscan get stuck at UNKNOWN (3) if the first CronJob processed has no child Jobs (socronjobstatusstays 3). In that case, later CronJobs with valid statuses will never updatestatusbecause thecronjobstatus > statuscheck fails whenstatusis 3.
if (status == 3):
status=cronjobstatus
if (cronjobstatus > status and cronjobstatus != 3):
status=cronjobstatus
- Files reviewed: 1/1 changed files
- Comments generated: 0 new
- Review effort level: Lite
There was a problem hiding this comment.
🟡 Changes recommended
Successful Jobs are misclassified on older clusters, and Kubernetes API failures produce an incorrect warning status.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
lakehouse/k8s_failed_cronjobs.py:91
failed_childrencontains child Job names, not CronJob names, so this diagnostic misidentifies the listed resources. Label them as failed Jobs (or change the collected values to owning CronJob identifiers).
print(f"failed CronJobs: {', '.join(failed_children)} ; CronJobs checked: {', '.join(cronjob_names)}")
- Files reviewed: 1/1 changed files
- Comments generated: 3
- Review effort level: Balanced
suggested by Copilot Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
add try blocks to k8s API calls
suggested by Copilot Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
Error paths can crash with incorrect exit codes, and mixed timestamps can misidentify the latest Job.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (3)
lakehouse/k8s_failed_cronjobs.py:39
- This handler prints an error but continues, so if API client construction fails,
batchis undefined and the next block raisesUnboundLocalError; the resulting exit code is 1 rather than UNKNOWN. Return immediately with status 3 after reporting the exception.
except:
print("can't connect to k8s")
lakehouse/k8s_failed_cronjobs.py:45
- After either list call fails, execution continues with
cronjobsorjobsunset and crashes when those variables are iterated. This is a common operational failure (for example, missing RBAC permission), so report it and return UNKNOWN immediately.
except:
print("can't get CronJobs from k8s")
lakehouse/k8s_failed_cronjobs.py:73
- The sort mixes
status.start_timewithmetadata.creation_timestamp, so overlapping Jobs can be ordered incorrectly: an older Job that starts late may appear newer than a later-created Job. Since the status rules depend on which Job the CronJob spawned most recently, use the creation timestamp consistently.
key=lambda j: (
(j.status.start_time or j.metadata.creation_timestamp).timestamp()
if (j.status.start_time or j.metadata.creation_timestamp)
else 0.0
),
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Balanced
suggested by copilot Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
Error paths and Jobs without status objects can cause unhandled exceptions instead of valid MRPE results.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (3)
Previously missed (1) — in code that hasn't changed since the last review.
lakehouse/k8s_failed_cronjobs.py:77
- A newly created Kubernetes Job may not have a
statusobject yet. Accessingj.status.start_timethen raisesAttributeErrorwhile sorting, preventing the check from reporting any result; treat a missing start time as absent and fall back to the creation timestamp.
This issue also appears on line 85 of the same file.
lakehouse/k8s_failed_cronjobs.py:43
- This exception path continues into the next block with
batchunassigned, so a client-initialization failure becomes anUnboundLocalErrorrather than an MRPE UNKNOWN result. Return status 3 here, and avoid intercepting process-control exceptions with a bareexcept.
except:
print("can't connect to k8s")
lakehouse/k8s_failed_cronjobs.py:85
- Even after sorting handles a Job with no status, this access still raises
AttributeErrorfor that valid newly created Job state. Default missing status/conditions to an empty list so an in-progress Job does not crash the entire check.
conditions = job.status.conditions or []
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Balanced
| except: | ||
| print("can't get CronJobs from k8s") |
Initial commit of script checking Jobs spawned by CronJobs. Suitable for use as an MRPE check.