Skip to content

Commit fa3c147

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 5d11801 commit fa3c147

2 files changed

Lines changed: 782 additions & 20 deletions

File tree

internal/controller/instance_controller.go

Lines changed: 183 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,34 @@ const (
8989
reasonNetworkFailedToCreate = "NetworkFailedToCreate"
9090
)
9191

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

881+
// resolveInstanceResources determines the vCPU and memory amounts to claim
882+
// for an instance. Explicit sizing always takes precedence over the instance
883+
// type catalog, so a workload that overrides container limits is accounted at
884+
// its actual resource footprint rather than the catalog baseline.
885+
//
886+
// Precedence order:
887+
// 1. Sandbox container Limits (sum across all containers) — all containers
888+
// must have both cpu and memory Limits for this path to succeed.
889+
// 2. Instance-level Resources.Requests — both cpu and memory must be present.
890+
// 3. instanceTypeCatalog lookup by instanceType — used for the common case
891+
// where a workload is sized only by instanceType with no explicit limits.
892+
//
893+
// Returns (0, 0, false) when none of the above yield a complete sizing, so
894+
// the caller falls back to claiming only the instance count.
853895
func resolveInstanceResources(instance *computev1alpha.Instance) (cpuMillicores int64, memMiB int64, resolved bool) {
854896
rt := instance.Spec.Runtime
897+
898+
// Path 1: explicit per-container Limits — most specific, wins if fully set.
855899
if rt.Sandbox != nil {
856900
var totalCPU resource.Quantity
857901
var totalMem resource.Quantity
@@ -870,18 +914,60 @@ func resolveInstanceResources(instance *computev1alpha.Instance) (cpuMillicores
870914
totalCPU.Add(cpu)
871915
totalMem.Add(mem)
872916
}
873-
if !allSet || len(rt.Sandbox.Containers) == 0 {
874-
return 0, 0, false
917+
if allSet && len(rt.Sandbox.Containers) > 0 {
918+
return totalCPU.MilliValue(), totalMem.Value() / (1024 * 1024), true
875919
}
876-
return totalCPU.MilliValue(), totalMem.Value() / (1024 * 1024), true
920+
// Containers exist but limits are incomplete — fall through so the
921+
// instance-level Requests and instanceType catalog paths can still
922+
// yield a sizing.
877923
}
878924

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

887973
// networkFailureChecker is a function that checks if a network creation failure
@@ -965,16 +1051,88 @@ func (r *InstanceReconciler) reconcileInstanceReadyCondition(
9651051
if programmedCondition == nil || programmedCondition.Status != metav1.ConditionTrue {
9661052
logger.Info("instance is not programmed", "instance", instance.Name)
9671053

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

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

9791137
return apimeta.SetStatusCondition(&instance.Status.Conditions, *readyCondition), nil
9801138
}
@@ -985,16 +1143,21 @@ func (r *InstanceReconciler) reconcileInstanceReadyCondition(
9851143
if availableCondition == nil || availableCondition.Status != metav1.ConditionTrue {
9861144
logger.Info("instance is not available", "instance", instance.Name)
9871145

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

994-
readyCondition.Message = "Instance is not available"
995-
if availableCondition != nil && availableCondition.Status != metav1.ConditionUnknown {
996-
readyCondition.Message = availableCondition.Message
997-
}
1158+
readyCondition.Status = readyStatus
1159+
readyCondition.Reason = readyReason
1160+
readyCondition.Message = readyMessage
9981161

9991162
return apimeta.SetStatusCondition(&instance.Status.Conditions, *readyCondition), nil
10001163
}

0 commit comments

Comments
 (0)