Skip to content

no-jira: no ttl long lived clusters#987

Merged
openshift-merge-bot[bot] merged 1 commit into
project-codeflare:mainfrom
kryanbeane:no-ttl-long-lived-clusters
Jan 30, 2026
Merged

no-jira: no ttl long lived clusters#987
openshift-merge-bot[bot] merged 1 commit into
project-codeflare:mainfrom
kryanbeane:no-ttl-long-lived-clusters

Conversation

@kryanbeane

@kryanbeane kryanbeane commented Jan 22, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Issue link

N/A

What changes have been made

No jira for this, came up from Cathal encountering this in a cluster. With long lived clusters, we shouldn't allow TTL field to be set as this is only for lifecycled clusters.

Verification steps

  • Run poetry build
  • Install custom build
%pip install ./codeflare_sdk-0.34.0-py3-none-any.whl --force-reinstall
  • Verify that you can't create a RayJob with TTL set when targetting an existing cluster
prod_job = RayJob(
    job_name='job',
    namespace='default',
    cluster_name='not-a-cluster',
    entrypoint="python nightly.py",
    ttl_seconds_after_finished=300,
)
  • The above should fail, you shouldn't need to run this on a cluster or anything, the object not being able to initialize and throwing an error should suffice

Checks

  • I've made sure the tests are passing.
  • Testing Strategy
    • Unit tests
    • Manual tests
    • Testing is not required for this change

@openshift-ci-robot

openshift-ci-robot commented Jan 22, 2026

Copy link
Copy Markdown
Collaborator

@kryanbeane: This pull request explicitly references no jira issue.

Details

In response to this:

Issue link

N/A

What changes have been made

No jira for this, came up from Cathal encountering this in a cluster. With long lived clusters, we shouldn't allow TTL field to be set as this is only for lifecycled clusters.

Verification steps

Checks

  • I've made sure the tests are passing.
  • Testing Strategy
    • Unit tests
    • Manual tests
    • Testing is not required for this change

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

@codecov

codecov Bot commented Jan 22, 2026 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.87%. Comparing base (4e4a181) to head (90ea08b).
⚠️ Report is 9 commits behind head on main.

Additional details and impacted files 
@@           Coverage Diff           @@
##             main     #987   +/-   ##
=======================================
  Coverage   95.87%   95.87%           
=======================================
  Files          22       22           
  Lines        2132     2134    +2     
=======================================
+ Hits         2044     2046    +2     
  Misses         88       88

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@laurafitzgerald

laurafitzgerald commented Jan 22, 2026

Copy link
Copy Markdown
Contributor

Changes look good. I haven't done the verification. Will leave that for someone else.

@kryanbeane kryanbeane added the test-guided-notebooks Run PR check to verify Guided notebooks label Jan 22, 2026
@kryanbeane kryanbeane force-pushed the no-ttl-long-lived-clusters branch from 710acaa to 90ea08b Compare January 22, 2026 11:26
@kryanbeane kryanbeane added test-ui-notebooks Run PR check to verify UI notebooks test-additional-notebooks labels Jan 22, 2026
@pawelpaszki

pawelpaszki commented Jan 30, 2026

Copy link
Copy Markdown
Contributor

Confirmed expected behaviour:

ValueError: ❌ Configuration Error: 'ttl_seconds_after_finished' cannot be set when targeting an existing cluster (via 'cluster_name').
TTL controls automatic cleanup of RayJob-managed clusters, which only applies when creating a new cluster via 'cluster_config'.
For existing clusters, the RayJob CR will remain after completion for inspection.

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Jan 30, 2026
@openshift-ci

openshift-ci Bot commented Jan 30, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: pawelpaszki

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 30, 2026
@openshift-merge-bot openshift-merge-bot Bot merged commit 035e935 into project-codeflare:main Jan 30, 2026
28 of 35 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pawelpaszki pawelpaszki pawelpaszki approved these changes

@CathalOConnorRH CathalOConnorRH Awaiting requested review from CathalOConnorRH

@szaher szaher Awaiting requested review from szaher

Assignees

@pawelpaszki pawelpaszki

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. jira/valid-reference lgtm Indicates that a PR is ready to be merged. test-additional-notebooks test-guided-notebooks Run PR check to verify Guided notebooks test-ui-notebooks Run PR check to verify UI notebooks

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@kryanbeane @openshift-ci-robot @laurafitzgerald @pawelpaszki