Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ci: label make slurmcluster instances for cloud spend [CM-405] #9792

Merged
merged 4 commits into from
Aug 5, 2024

Conversation

ioga
Copy link
Contributor

@ioga ioga commented Aug 2, 2024

Ticket

CM-405

Description

  1. Add a flag for make slurmcluster pipeline to set labels on the VM it creates and its disk
  2. Use them in the CI

Also I've discovered that make slurmcluster job will stay pending when a background task to create a cluster failed, so I've fixed that. The job should now fail right away if that happens.

Also I went to war with trailing whitespace in the circle config.

Test Plan

Screen Shot 2024-08-02 at 17 05 56

Do the VMs spawned by CI have that label?

Test run: https://app.circleci.com/pipelines/github/determined-ai/determined/59112/workflows/bd2dfd65-498c-46be-a01f-908aefa48a05/jobs/2836426

Checklist

  • Changes have been manually QA'd
  • New features have been approved by the corresponding PM
  • User-facing API changes have the "User-facing API Change" label
  • Release notes have been added as a separate file under docs/release-notes/
    See Release Note for details.
  • Licenses have been included for new code which was copied and/or modified from any external code

@cla-bot cla-bot bot added the cla-signed label Aug 2, 2024
Copy link

netlify bot commented Aug 2, 2024

Deploy Preview for determined-ui canceled.

Name Link
🔨 Latest commit 1e8670e
🔍 Latest deploy log https://app.netlify.com/sites/determined-ui/deploys/66ad712b660d400008da16dc

Copy link

codecov bot commented Aug 2, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 54.05%. Comparing base (786f258) to head (1e8670e).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #9792   +/-   ##
=======================================
  Coverage   54.04%   54.05%           
=======================================
  Files        1260     1260           
  Lines      155574   155574           
  Branches     3503     3504    +1     
=======================================
+ Hits        84085    84089    +4     
+ Misses      71343    71339    -4     
  Partials      146      146           
Flag Coverage Δ
backend 44.90% <ø> (+<0.01%) ⬆️
harness 72.62% <ø> (ø)
web 53.23% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

see 3 files with indirect coverage changes

@ioga ioga changed the title wip: slurmcluster labels ci: label make slurmcluster instances [CM-405] Aug 2, 2024
@ioga ioga requested review from a team and corban-beaird and removed request for a team August 3, 2024 00:07
@ioga ioga marked this pull request as ready for review August 3, 2024 00:08
@ioga ioga requested a review from a team as a code owner August 3, 2024 00:08
@ioga ioga changed the title ci: label make slurmcluster instances [CM-405] ci: label make slurmcluster instances for cloud spend [CM-405] Aug 3, 2024
Copy link
Contributor

@corban-beaird corban-beaird left a comment

Choose a reason for hiding this comment

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

Looks solid!

@ioga ioga merged commit 80822eb into main Aug 5, 2024
88 of 110 checks passed
@ioga ioga deleted the slurmcluster-labels branch August 5, 2024 19:29
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