Skip to content

Commit 7aa3c7b

Browse files
scotwellsclaude
andcommitted
feat: surface instance blocking reasons and claim instanceType vCPU/memory
Two Instance-controller correctness changes: - Blocking-reason rollup: surface the most specific provider sub-condition (ImageUnavailable, InstanceCrashing, ConfigurationError, Provisioning) and its message onto the Instance Ready condition instead of a generic "Instance has not been programmed", so e.g. an image-pull failure reads as ImageUnavailable with the real message. Ranks the API reason constants in the blocking-reason priority. - Quota sizing: resolve vCPU/memory for instanceType-sized instances from a new instanceTypeCatalog (datumcloud/d1-standard-2 = 1 vCPU / 2 GiB) so the quota ResourceClaim requests vcpus + memory, not just instance count. Explicit container limits / instance requests still take precedence. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 476c402 commit 7aa3c7b

2 files changed

Lines changed: 787 additions & 20 deletions

File tree

internal/controller/instance_controller.go

Lines changed: 182 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,34 @@ const (
9696
reasonNetworkFailedToCreate = "NetworkFailedToCreate"
9797
)
9898

99+
// instanceTypeD1Standard2 is the platform instance type name for the
100+
// 1 vCPU / 2 GiB size used as the catalog baseline for quota accounting.
101+
const instanceTypeD1Standard2 = "datumcloud/d1-standard-2"
102+
103+
// instanceTypeResources holds the vCPU and memory for a named instance type.
104+
type instanceTypeResources struct {
105+
// CPUMillicores is the number of CPU millicores (1000 = 1 vCPU).
106+
CPUMillicores int64
107+
// MemoryMiB is the amount of RAM in mebibytes.
108+
MemoryMiB int64
109+
}
110+
111+
// instanceTypeCatalog maps platform instance type names to their resource
112+
// dimensions used for quota accounting when the instance spec carries only an
113+
// instanceType and no explicit container Limits or instance-level Requests.
114+
//
115+
// These are the platform-declared quota sizes for the instance type, not a
116+
// derivation of any infra provider's machine type. (infra-provider-gcp separately
117+
// maps datumcloud/d1-standard-2 to the GCP n2-standard-2 machine type for VM
118+
// provisioning; that mapping does not define the quota size here.) When new
119+
// instance types are added, add them here with their vCPU/memory values.
120+
var instanceTypeCatalog = map[string]instanceTypeResources{
121+
instanceTypeD1Standard2: {
122+
CPUMillicores: 1000, // 1 vCPU
123+
MemoryMiB: 2048, // 2 GiB
124+
},
125+
}
126+
99127
// Quota-pending requeue backoff. The instance controller is normally re-queued by
100128
// the ResourceClaim watch when a claim is granted, but that grant event lives on
101129
// the project control plane and can be missed (informer engagement races, watch
@@ -852,8 +880,24 @@ func (r *InstanceReconciler) classifyCreateError(
852880
}, fmt.Errorf("failed creating resource claim: %w", err)
853881
}
854882

883+
// resolveInstanceResources determines the vCPU and memory amounts to claim
884+
// for an instance. Explicit sizing always takes precedence over the instance
885+
// type catalog, so a workload that overrides container limits is accounted at
886+
// its actual resource footprint rather than the catalog baseline.
887+
//
888+
// Precedence order:
889+
// 1. Sandbox container Limits (sum across all containers) — all containers
890+
// must have both cpu and memory Limits for this path to succeed.
891+
// 2. Instance-level Resources.Requests — both cpu and memory must be present.
892+
// 3. instanceTypeCatalog lookup by instanceType — used for the common case
893+
// where a workload is sized only by instanceType with no explicit limits.
894+
//
895+
// Returns (0, 0, false) when none of the above yield a complete sizing, so
896+
// the caller falls back to claiming only the instance count.
855897
func resolveInstanceResources(instance *computev1alpha.Instance) (cpuMillicores int64, memMiB int64, resolved bool) {
856898
rt := instance.Spec.Runtime
899+
900+
// Path 1: explicit per-container Limits — most specific, wins if fully set.
857901
if rt.Sandbox != nil {
858902
var totalCPU resource.Quantity
859903
var totalMem resource.Quantity
@@ -872,18 +916,59 @@ func resolveInstanceResources(instance *computev1alpha.Instance) (cpuMillicores
872916
totalCPU.Add(cpu)
873917
totalMem.Add(mem)
874918
}
875-
if !allSet || len(rt.Sandbox.Containers) == 0 {
876-
return 0, 0, false
919+
if allSet && len(rt.Sandbox.Containers) > 0 {
920+
return totalCPU.MilliValue(), totalMem.Value() / (1024 * 1024), true
877921
}
878-
return totalCPU.MilliValue(), totalMem.Value() / (1024 * 1024), true
922+
// Containers exist but limits are incomplete — fall through to catalog
923+
// rather than returning false, because instanceType is still set.
879924
}
880925

926+
// Path 2: instance-level resource requests.
881927
cpu, hasCPU := rt.Resources.Requests[corev1.ResourceCPU]
882928
mem, hasMem := rt.Resources.Requests[corev1.ResourceMemory]
883-
if !hasCPU || !hasMem {
884-
return 0, 0, false
929+
if hasCPU && hasMem {
930+
return cpu.MilliValue(), mem.Value() / (1024 * 1024), true
931+
}
932+
933+
// Path 3: instanceType catalog — handles the typical production case where
934+
// instanceType is the only sizing signal and no explicit limits are set.
935+
if rt.Resources.InstanceType != "" {
936+
if spec, ok := instanceTypeCatalog[rt.Resources.InstanceType]; ok {
937+
return spec.CPUMillicores, spec.MemoryMiB, true
938+
}
939+
}
940+
941+
return 0, 0, false
942+
}
943+
944+
// instanceBlockingReasonPriority ranks Instance blocking reasons so the most
945+
// specific, user-actionable cause wins when several conditions are unsatisfied.
946+
// Higher numbers are more specific. Reasons absent from the table rank 0.
947+
//
948+
// 0 - unknown/default
949+
// 1 - Provisioning (transient runtime startup)
950+
// 3 - PendingQuota (operator action may be needed)
951+
// 5 - ImageUnavailable / InstanceCrashing / ConfigurationError
952+
// (hard runtime error, user-actionable)
953+
// 7 - NetworkFailedToCreate (hard infra error)
954+
func instanceBlockingReasonPriority(reason string) int {
955+
switch reason {
956+
case computev1alpha.InstanceReadyReasonProvisioning:
957+
return 1
958+
case computev1alpha.InstanceProgrammedReasonPendingQuota:
959+
return 3
960+
case computev1alpha.InstanceReadyReasonImageUnavailable,
961+
computev1alpha.InstanceReadyReasonInstanceCrashing,
962+
computev1alpha.InstanceReadyReasonConfigurationError:
963+
// Hard runtime errors are user-actionable (wrong image, crashing app, bad
964+
// config) and rank highest among non-infra reasons so they are not buried
965+
// under transient startup/quota reasons.
966+
return 5
967+
case reasonNetworkFailedToCreate:
968+
return 7
969+
default:
970+
return 0
885971
}
886-
return cpu.MilliValue(), mem.Value() / (1024 * 1024), true
887972
}
888973

889974
// networkFailureChecker is a function that checks if a network creation failure
@@ -967,16 +1052,88 @@ func (r *InstanceReconciler) reconcileInstanceReadyCondition(
9671052
if programmedCondition == nil || programmedCondition.Status != metav1.ConditionTrue {
9681053
logger.Info("instance is not programmed", "instance", instance.Name)
9691054

970-
readyCondition.Status = metav1.ConditionFalse
971-
readyCondition.Reason = computev1alpha.InstanceProgrammedReasonPendingProgramming
972-
if programmedCondition != nil && programmedCondition.Reason != pendingReason {
973-
readyCondition.Reason = programmedCondition.Reason
1055+
// Surface the most specific provider sub-condition rather than a generic
1056+
// "Instance has not been programmed". A provider reason like
1057+
// ImageUnavailable (set on the Available condition while Programmed is
1058+
// still Unknown) must surface on Ready with its actionable message.
1059+
//
1060+
// Two tiers are tracked:
1061+
// - bestKnown: the best candidate from the priority table (ranked 1-7).
1062+
// - fallback: the Programmed condition's own reason/message when it has
1063+
// one but it is not in the priority table (e.g. a provider
1064+
// writes a custom Programmed reason otherwise unknown to
1065+
// this controller). Preserves Programmed.Reason → Ready.Reason
1066+
// pass-through behavior.
1067+
type candidate struct {
1068+
status metav1.ConditionStatus
1069+
reason string
1070+
message string
1071+
priority int
1072+
}
1073+
1074+
// Generic default — used only when nothing better is found.
1075+
fallbackCandidate := candidate{
1076+
status: metav1.ConditionFalse,
1077+
reason: computev1alpha.InstanceProgrammedReasonPendingProgramming,
1078+
message: msgNotProgrammed,
1079+
priority: -1,
1080+
}
1081+
// Promote the Programmed condition's own reason as a fallback when it is
1082+
// more specific than PendingProgramming/Pending but not in the priority
1083+
// table. Preserves pass-through for provider-written Programmed reasons.
1084+
if programmedCondition != nil && programmedCondition.Reason != pendingReason &&
1085+
programmedCondition.Reason != computev1alpha.InstanceProgrammedReasonPendingProgramming {
1086+
fallbackCandidate = candidate{
1087+
status: programmedCondition.Status,
1088+
reason: programmedCondition.Reason,
1089+
message: programmedCondition.Message,
1090+
priority: 0,
1091+
}
1092+
}
1093+
1094+
best := fallbackCandidate
1095+
consider := func(status metav1.ConditionStatus, reason, message string) {
1096+
// A generic "Pending" reason carries no actionable signal; skip it so
1097+
// it cannot displace an already-set specific reason from the provider.
1098+
if reason == pendingReason {
1099+
return
1100+
}
1101+
p := instanceBlockingReasonPriority(reason)
1102+
if p > best.priority {
1103+
best = candidate{status: status, reason: reason, message: message, priority: p}
1104+
}
9741105
}
9751106

976-
readyCondition.Message = msgNotProgrammed
977-
if programmedCondition != nil && programmedCondition.Status != metav1.ConditionUnknown {
978-
readyCondition.Message = programmedCondition.Message
1107+
// Sub-conditions set by the provider (e.g. Available=Unknown/ImageUnavailable)
1108+
// may be more specific than the Programmed condition. Consult each one so
1109+
// the highest-priority reason wins, regardless of which condition carries it.
1110+
for _, cond := range instance.Status.Conditions {
1111+
if cond.Status == metav1.ConditionTrue {
1112+
// Satisfied conditions are not blocking; skip them.
1113+
continue
1114+
}
1115+
switch cond.Type {
1116+
case computev1alpha.InstanceProgrammed,
1117+
computev1alpha.InstanceReady,
1118+
computev1alpha.InstanceQuotaGranted:
1119+
// InstanceProgrammed is handled below; InstanceReady is being set
1120+
// now. InstanceQuotaGranted is a gate-level signal evaluated before
1121+
// this branch is reached — including it here would let a transient
1122+
// PendingEvaluation reason displace the generic not-programmed
1123+
// fallback when no provider sub-condition is set yet.
1124+
continue
1125+
}
1126+
consider(cond.Status, cond.Reason, cond.Message)
9791127
}
1128+
// Also let the Programmed condition itself compete through the priority table
1129+
// in case it carries a known reason (e.g. PendingQuota).
1130+
if programmedCondition != nil {
1131+
consider(programmedCondition.Status, programmedCondition.Reason, programmedCondition.Message)
1132+
}
1133+
1134+
readyCondition.Status = best.status
1135+
readyCondition.Reason = best.reason
1136+
readyCondition.Message = best.message
9801137

9811138
return apimeta.SetStatusCondition(&instance.Status.Conditions, *readyCondition), nil
9821139
}
@@ -987,16 +1144,21 @@ func (r *InstanceReconciler) reconcileInstanceReadyCondition(
9871144
if availableCondition == nil || availableCondition.Status != metav1.ConditionTrue {
9881145
logger.Info("instance is not available", "instance", instance.Name)
9891146

990-
readyCondition.Status = metav1.ConditionFalse
991-
readyCondition.Reason = pendingReason
1147+
// Propagate the Available condition's reason and message directly —
1148+
// including when the status is Unknown — so provider-set reasons like
1149+
// ImageUnavailable surface on Ready rather than a generic message.
1150+
readyStatus := metav1.ConditionFalse
1151+
readyReason := pendingReason
1152+
readyMessage := "Instance is not available"
9921153
if availableCondition != nil && availableCondition.Reason != pendingReason {
993-
readyCondition.Reason = availableCondition.Reason
1154+
readyStatus = availableCondition.Status
1155+
readyReason = availableCondition.Reason
1156+
readyMessage = availableCondition.Message
9941157
}
9951158

996-
readyCondition.Message = "Instance is not available"
997-
if availableCondition != nil && availableCondition.Status != metav1.ConditionUnknown {
998-
readyCondition.Message = availableCondition.Message
999-
}
1159+
readyCondition.Status = readyStatus
1160+
readyCondition.Reason = readyReason
1161+
readyCondition.Message = readyMessage
10001162

10011163
return apimeta.SetStatusCondition(&instance.Status.Conditions, *readyCondition), nil
10021164
}

0 commit comments

Comments
 (0)