Skip to content

Create k8s_failed_cronjobs.py - #49

Merged
kkellerlbl merged 7 commits into
masterfrom
DEVOPS-2524
Sep 2, 2026
Merged

Create k8s_failed_cronjobs.py#49
kkellerlbl merged 7 commits into
masterfrom
DEVOPS-2524

Conversation

@kkellerlbl

Copy link
Copy Markdown
Member

Initial commit of script checking Jobs spawned by CronJobs. Suitable for use as an MRPE check.

Initial commit of script checking Jobs spawned by CronJobs.

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

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.py to 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) > 0 is 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 status remains 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.

Comment thread lakehouse/k8s_failed_cronjobs.py Outdated
kkellerlbl and others added 2 commits September 1, 2026 14:50
fix output if status is unknown
Unverified

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

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.

🔵 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_children is 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 checks SuccessCriteriaMet, but many clusters report Job success via the Complete condition, 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_children branch 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 status can get stuck at UNKNOWN (3) if the first CronJob processed has no child Jobs (so cronjobstatus stays 3). In that case, later CronJobs with valid statuses will never update status because the cronjobstatus > status check fails when status is 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

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.

🟡 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_children contains 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

Comment thread lakehouse/k8s_failed_cronjobs.py Outdated
Comment thread lakehouse/k8s_failed_cronjobs.py Outdated
Comment thread lakehouse/k8s_failed_cronjobs.py Outdated
kkellerlbl and others added 3 commits September 2, 2026 12:27
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>

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.

🟡 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, batch is undefined and the next block raises UnboundLocalError; 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 cronjobs or jobs unset 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_time with metadata.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

Comment thread lakehouse/k8s_failed_cronjobs.py Outdated
suggested by copilot

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@kkellerlbl
kkellerlbl requested review from bio-boris and removed request for bio-boris September 2, 2026 19:52
@kkellerlbl kkellerlbl self-assigned this Sep 2, 2026
@bio-boris
bio-boris requested a balanced review from Copilot September 2, 2026 20:12

@bio-boris bio-boris 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.

LGTM

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.

🟡 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 status object yet. Accessing j.status.start_time then raises AttributeError while 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 batch unassigned, so a client-initialization failure becomes an UnboundLocalError rather than an MRPE UNKNOWN result. Return status 3 here, and avoid intercepting process-control exceptions with a bare except.
    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 AttributeError for 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

Comment on lines +48 to +49
except:
print("can't get CronJobs from k8s")
@kkellerlbl
kkellerlbl merged commit 846b56e into master Sep 2, 2026
3 checks passed
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