FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fix: handle provider status default cases across providers. by garvitssoni · Pull Request #107 · brevdev/cloud · GitHub

/ cloud Public

fix: handle provider status default cases across providers. - #107

Draft
garvitssoni wants to merge 3 commits into
mainfrom
BREV-3225/deletion-stuck-at-deploying
Draft

garvitssoni wants to merge 3 commits into
mainfrom
BREV-3225/deletion-stuck-at-deploying

Conversation

garvitssoni commented Apr 7, 2026 •
edited
Loading

Copy link
Copy Markdown

Problem

Across several providers, unknown/transient status values were being coerced into concrete lifecycle states (commonly pending). During deletion this led to incorrect “Starting/Deploying” signals and contributed to UI status regressions.

Root cause

Default-case handling in provider status mappers treated unknown values as known states instead of surfacing “unknown.”

Fix

  • Standardize provider mappers to return “unknown” (empty/unset lifecycle) for unrecognized provider status values, rather than defaulting to pending/failed.

garvitssoni self-assigned this Apr 7, 2026
garvitssoni marked this pull request as ready for review April 7, 2026 15:13
garvitssoni requested a review from a team as a code owner April 7, 2026 15:13
Comment thread v1/providers/fluidstack/instance.go Outdated
lifecycleStatus = v1.LifecycleStatusFailed
default:
lifecycleStatus = v1.LifecycleStatusPending
lifecycleStatus = ""

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality
Suggested change
lifecycleStatus = ""
lifecycleStatus = v1.LifecycleStatusEmpty

Copy link
Copy Markdown
Contributor

How do we handle the empty status in dev-plane and the UI? In these cases, does it make more sense to assume the instance is terminated rather than returning an empty status?

garvitssoni force-pushed the BREV-3225/deletion-stuck-at-deploying branch from c0ff74d to 464b0c3 Compare April 10, 2026 08:10

Copy link
Copy Markdown
Author

How do we handle the empty status in dev-plane and the UI? In these cases, does it make more sense to assume the instance is terminated rather than returning an empty status?

Hi @stephahart,

On the UI side, an empty provider status is handled as a separate case and displayed as “Deleting.” This differs from “Terminated,” which is shown as “Stopped.” Combining these would present users with an inaccurate view of the instance state.

On the dev-plane side, we track when an instance last entered Running, Stopped, or Terminated using the ProviderStatusTransitions struct. Cloud providers may occasionally return “not found” errors, which can resemble termination—even though the instance may not really be terminated yet. To ensure data accuracy, we only set LastTerminatedAt when termination is explicitly confirmed. If the status is empty or unknown, we do not record any transition time.

In practice, an unknown status indicates “not yet confirmed,” prompting frequent polling (every 15 seconds). A confirmed Terminated state is treated as final—we record the timestamp and reduce polling to every 60 seconds.

If we default unknown statuses to Terminated, we risk recording an incorrect termination time, slowing down polling prematurely, and potentially missing instances that recover later.

Thanks!

drewmalin commented Apr 15, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

This is probably better than what we have, but it will only slightly shift the problem to flickering into and out of "unknown".

One thing to consider now is: when reacting to the new state, we preserve the current state if the new state is "unknown". Providers implement MergeInstanceForUpdate, but a typical implementation is:

func (c *LaunchpadClient) MergeInstanceForUpdate(_ v1.Instance, newInstance v1.Instance) v1.Instance {
	return newInstance
}

this could change to something like:

func (c *LaunchpadClient) MergeInstanceForUpdate(currentInstance v1.Instance, newInstance v1.Instance) v1.Instance {
	if newInstance.Status.LifecycleStatus == v1.LifecycleStatusEmpty {
		newInstance.Status.LifecycleStatus = currentInstance.Status.LifecycleStatus
	}
	return newInstance
}

which at least preserves the old status (so instead of running -> pending -> terminating, we'd actually see running -> terminating).

Comment thread v1/instance.go
LifecycleStatusTerminating LifecycleStatus = "terminating"
LifecycleStatusTerminated LifecycleStatus = "terminated"
LifecycleStatusFailed LifecycleStatus = "failed"
LifecycleStatusEmpty LifecycleStatus = ""

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

"Unknown" might be better

garvitssoni marked this pull request as draft May 27, 2026 05:44
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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


Back | FazBrowse Home | New Git URL