feat(sliceconfig): add machine-observable status conditions#369
feat(sliceconfig): add machine-observable status conditions#369Karman580 wants to merge 1 commit into
Conversation
Add Conditions []metav1.Condition to SliceConfigStatus so that provisioning state is observable via kubectl wait, FluxCD, ArgoCD, and Helm post-install hooks. Four conditions are set by ReconcileSliceConfig: - WorkerConfigsProvisioned: after WorkerSliceConfig creation (step 4) - GatewaysProvisioned: after WorkerSliceGateway creation (step 5) - VPNConfigured: after VpnKeyRotation CR creation (step 7) - Ready: aggregate; True when all above are True For no-network slices only WorkerConfigsProvisioned and Ready are set since gateways and VPN are not applicable to that deployment mode. A single util.UpdateStatus call is made at the end of each successful reconcile path. Error paths do not write status; the next successful reconcile sets conditions correctly (level-triggered semantics). A +kubebuilder:printcolumn marker surfaces READY in kubectl get output. Existing kubesliceEvents behaviour is unchanged (backward compatible). Signed-off-by: KARMAN SINGH TALWAR <karmansinghtalwar@KARMANs-MacBook-Pro.local>
mdryaan
left a comment
mdryaan
left a comment
There was a problem hiding this comment.
Hi @Karman580 i have review the PR, the logic is correct condition placement after each step, the NONET path setting only WorkerConfigsProvisioned and Ready, and the single UpdateStatus call at the end are all right.
Two things need fixing
-
make manifests generatewasn't run after editingsliceconfig_types.go. The CRD YAML is missing the conditions field and the READY printcolumn, andzz_generated.deepcopy.goleavesConditionsshallow-copied. Runmake manifests generateand commit both generated files. -
clientMock.On("Status").Return(clientMock)panics at runtime in all four tests that callutil.UpdateStatus— same root cause as PR #308. Needs afakeStatusWriterstruct, details in the inline comment.
| // Conditions represent the latest available observations of the SliceConfig's provisioning state. | ||
| // +listType=map | ||
| // +listMapKey=type | ||
| Conditions []metav1.Condition `json:"conditions,omitempty"` |
There was a problem hiding this comment.
make manifests generate wasn't run after editing this file. Two generated files are stale:
config/crd/bases/controller.kubeslice.io_sliceconfigs.yamlis missing theconditionsfield and the READY printcolumn —kubectl get sliceconfigwon't show READY andkubectl wait --for=condition=Readywon't work until the CRD schema is regenerated and applied.zz_generated.deepcopy.go—DeepCopyIntoforSliceConfigStatusstill only copiesKubesliceEvents. The newConditionsslice gets shallow-copied, so the original and the copy share the same underlying array after anyDeepCopy()call.
Run make manifests generate and commit both generated files.
| }).Once() | ||
| workerServiceImportMock.On("CreateMinimalWorkerServiceImport", ctx, sliceConfig.Spec.Clusters, requestObj.Namespace, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil).Once() | ||
| // status condition update at end of reconcile | ||
| clientMock.On("Status").Return(clientMock).Once() |
There was a problem hiding this comment.
Same panic issue as in PR #308. *utilMock.Client satisfies client.Client but not client.StatusWriter — in controller-runtime v0.20.4, StatusWriter is SubResourceWriter whose Update takes ...SubResourceUpdateOption, not ...client.UpdateOption. The type assertion inside Status() panics at runtime. All four tests hitting util.UpdateStatus will panic rather than fail cleanly. Needs a fakeStatusWriter struct the same way PR #308 does.
2 participants
Summary
Conditions []metav1.ConditiontoSliceConfigStatuswith+listType=map/+listMapKey=typemarkers for correct strategic-merge-patch behaviour.ReconcileSliceConfigsets four conditions after key provisioning steps and calls a singleutil.UpdateStatuson successful completion.+kubebuilder:printcolumnforREADYsokubectl get sliceconfigshows readiness inline.kubesliceEventsbehaviour is unchanged; this change is fully backward compatible.Motivation
SliceConfigStatuspreviously exposed onlykubesliceEvents[], an append-style audit log with no machine-queryable readiness semantics. A fully provisioned slice and one stalled at step 3 were externally indistinguishable. This blockedkubectl wait --for=condition=Ready, FluxCD/ArgoCD health checks, and Helm--waiton slice provisioning.Conditions added
WorkerConfigsProvisionedWorkerSliceConfigobjects createdGatewaysProvisionedWorkerSliceGatewaypairs createdVPNConfiguredVpnKeyRotationCR createdReadyTrueFor no-network slices, only
WorkerConfigsProvisionedandReadyare set (gateways and VPN are not applicable).Test plan
SliceConfig_ConditionsSetOnSuccessfulReconcile— all four conditions areTrueafter a complete network-slice reconcileSliceConfig_ConditionsSetOnNoNetSuccessfulReconcile— onlyWorkerConfigsProvisionedandReadyare set for NoNet slicesSliceConfig_ReadyConditionFalseWhenGatewayFails—Status()is never called when gateway creation returns an errorSliceConfigTestBedcases updated / continue to passMigration notes
conditionsisomitempty; existing objects without the field remain valid. No CRD migration needed. After merging, runmake manifests && make installto apply the updated CRD.Fixes #368