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

fix(clusters): switch from drain to ineligible system #1102

Conversation

MasterPtato
Copy link
Contributor

@MasterPtato MasterPtato commented Aug 29, 2024

Fixes RVTEE-574

Manually tested

Changes

Copy link
Contributor

graphite-app bot commented Aug 29, 2024

Your org requires the Graphite merge queue for merging into main

You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link.

You can enable merging using labels in your Graphite merge queue settings.

Copy link

linear bot commented Aug 29, 2024

RVTEE-574

@MasterPtato MasterPtato force-pushed the 08-28-fix_clusters_remove_nomad_drain_complete_signal branch from 7213122 to 0b5bd70 Compare August 29, 2024 00:34
@MasterPtato MasterPtato force-pushed the 08-28-fix_clusters_remove_nomad_drain_complete_signal_split branch from a315490 to 612136a Compare August 29, 2024 00:35
@MasterPtato MasterPtato force-pushed the 08-28-fix_clusters_remove_nomad_drain_complete_signal branch from 0b5bd70 to f5e78f6 Compare August 29, 2024 18:34
@MasterPtato MasterPtato force-pushed the 08-28-fix_clusters_remove_nomad_drain_complete_signal_split branch from 612136a to f526f3a Compare August 29, 2024 18:35
Copy link
Member

@NathanFlurry NathanFlurry left a comment

Choose a reason for hiding this comment

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

nit: renaming all the job run workers in the same pr as other changes makes it much harder to review & find impactful changes

@@ -67,13 +67,11 @@ async fn undrain_node(ctx: &ActivityCtx, input: &UndrainNodeInput) -> GlobalResu
.await?;

if let Some(nomad_node_id) = nomad_node_id {
Copy link
Member

Choose a reason for hiding this comment

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

updating this activity on existing ndoes is going to cause a bug where they'll be stuck with the nomad drain enabled. probably best to just manually resolve this when we update.

Copy link
Contributor

graphite-app bot commented Sep 3, 2024

Merge activity

<!-- Please make sure there is an issue that this PR is correlated to. -->
Fixes RVTEE-574

**Manually tested**
## Changes

<!-- If there are frontend changes, please include screenshots. -->
@MasterPtato MasterPtato force-pushed the 08-28-fix_clusters_remove_nomad_drain_complete_signal branch from f5e78f6 to c117224 Compare September 3, 2024 23:01
@MasterPtato MasterPtato force-pushed the 08-28-fix_clusters_remove_nomad_drain_complete_signal_split branch from f526f3a to 09f5143 Compare September 3, 2024 23:02
Base automatically changed from 08-28-fix_clusters_remove_nomad_drain_complete_signal to main September 3, 2024 23:06
@graphite-app graphite-app bot merged commit 09f5143 into main Sep 3, 2024
5 of 10 checks passed
@graphite-app graphite-app bot deleted the 08-28-fix_clusters_remove_nomad_drain_complete_signal_split branch September 3, 2024 23:06
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.

2 participants