diff --git a/CHANGELOG.md b/CHANGELOG.md index bc17754..b479e73 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - Dependabot version updates for Go modules and GitHub Actions (`.github/dependabot.yml`), running weekly on Mondays - Scheduled workflow (`.github/workflows/update-tool-versions.yaml`) that runs `scripts/update-tool-versions.sh` every Monday and opens a PR when `ZARF_VERSION` or `UDS_VERSION` change upstream; fills the gap Dependabot leaves since its Docker ecosystem only tracks `FROM` lines, not `ARG` values +### Added +- `TestDeriveConditions_ReconcilingReason`: covers all six phases to verify the `Reconciling` reason is correct for terminal vs in-progress states +- `TestDeriveConditions_DeterministicOrder`: asserts two `DeriveConditions` calls with identical input produce identical condition order + +### Fixed +- `Reconciling` condition `Reason` was always `Progressing` even when `Status=False` (terminal); now correctly reflects the outcome — `Succeeded` when `PhaseCompleted`, `Failed` when `PhaseFailed` +- Per-operation conditions in the status slice had non-deterministic order (map iteration); `OperationConditionTypes` is now an ordered slice so conditions appear in stable order (`build → create → publish → deploy`) + +### Changed +- `ConditionReasonSuspended` comment updated to clearly mark it as a forward declaration for a future `spec.suspend` feature +- `conditionsToUnstructured` now logs via `klog.ErrorS` when a condition cannot be converted rather than silently dropping it ## [0.12.0] - 2026-05-29 diff --git a/chart/forge/crds/forge.dev_udsbundlejobs.yaml b/chart/forge/crds/forge.dev_udsbundlejobs.yaml index 07af783..b94e3e8 100644 --- a/chart/forge/crds/forge.dev_udsbundlejobs.yaml +++ b/chart/forge/crds/forge.dev_udsbundlejobs.yaml @@ -1986,6 +1986,69 @@ spec: status: description: UDSBundleJobStatus defines the observed state of a UDSBundleJob properties: + conditions: + description: |- + Conditions is the standard Kubernetes condition array for this job. + The Ready condition reflects the overall outcome; per-operation conditions + (CreateSucceeded, PublishSucceeded, DeploySucceeded) track individual steps. + items: + description: Condition contains details for one aspect of the current + state of this API Resource. + properties: + lastTransitionTime: + description: |- + lastTransitionTime is the last time the condition transitioned from one status to another. + This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable. + format: date-time + type: string + message: + description: |- + message is a human readable message indicating details about the transition. + This may be an empty string. + maxLength: 32768 + type: string + observedGeneration: + description: |- + observedGeneration represents the .metadata.generation that the condition was set based upon. + For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date + with respect to the current state of the instance. + format: int64 + minimum: 0 + type: integer + reason: + description: |- + reason contains a programmatic identifier indicating the reason for the condition's last transition. + Producers of specific condition types may define expected values and meanings for this field, + and whether the values are considered a guaranteed API. + The value should be a CamelCase string. + This field may not be empty. + maxLength: 1024 + minLength: 1 + pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ + type: string + status: + description: status of the condition, one of True, False, Unknown. + enum: + - "True" + - "False" + - Unknown + type: string + type: + description: type of condition in CamelCase or in foo.example.com/CamelCase. + maxLength: 316 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ + type: string + required: + - lastTransitionTime + - message + - reason + - status + - type + type: object + type: array + x-kubernetes-list-map-keys: + - type + x-kubernetes-list-type: map createStatus: description: CreateStatus tracks the create operation status properties: diff --git a/chart/forge/crds/forge.dev_zarfpackagejobs.yaml b/chart/forge/crds/forge.dev_zarfpackagejobs.yaml index a0cd907..40ab9df 100644 --- a/chart/forge/crds/forge.dev_zarfpackagejobs.yaml +++ b/chart/forge/crds/forge.dev_zarfpackagejobs.yaml @@ -1996,6 +1996,69 @@ spec: format: date-time type: string type: object + conditions: + description: |- + Conditions is the standard Kubernetes condition array for this job. + The Ready condition reflects the overall outcome; per-operation conditions + (BuildSucceeded, PublishSucceeded, DeploySucceeded) track individual steps. + items: + description: Condition contains details for one aspect of the current + state of this API Resource. + properties: + lastTransitionTime: + description: |- + lastTransitionTime is the last time the condition transitioned from one status to another. + This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable. + format: date-time + type: string + message: + description: |- + message is a human readable message indicating details about the transition. + This may be an empty string. + maxLength: 32768 + type: string + observedGeneration: + description: |- + observedGeneration represents the .metadata.generation that the condition was set based upon. + For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date + with respect to the current state of the instance. + format: int64 + minimum: 0 + type: integer + reason: + description: |- + reason contains a programmatic identifier indicating the reason for the condition's last transition. + Producers of specific condition types may define expected values and meanings for this field, + and whether the values are considered a guaranteed API. + The value should be a CamelCase string. + This field may not be empty. + maxLength: 1024 + minLength: 1 + pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ + type: string + status: + description: status of the condition, one of True, False, Unknown. + enum: + - "True" + - "False" + - Unknown + type: string + type: + description: type of condition in CamelCase or in foo.example.com/CamelCase. + maxLength: 316 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ + type: string + required: + - lastTransitionTime + - message + - reason + - status + - type + type: object + type: array + x-kubernetes-list-map-keys: + - type + x-kubernetes-list-type: map deployStatus: description: DeployStatus contains deploy operation status properties: diff --git a/pkg/apis/uds/v1alpha3/types.go b/pkg/apis/uds/v1alpha3/types.go index 221b164..b686c3a 100644 --- a/pkg/apis/uds/v1alpha3/types.go +++ b/pkg/apis/uds/v1alpha3/types.go @@ -594,6 +594,14 @@ type UDSBundleJobStatus struct { // +optional Message string `json:"message,omitempty"` + // Conditions is the standard Kubernetes condition array for this job. + // The Ready condition reflects the overall outcome; per-operation conditions + // (CreateSucceeded, PublishSucceeded, DeploySucceeded) track individual steps. + // +optional + // +listType=map + // +listMapKey=type + Conditions []metav1.Condition `json:"conditions,omitempty"` + // CreateStatus tracks the create operation status // +optional CreateStatus *OperationStatus `json:"createStatus,omitempty"` diff --git a/pkg/apis/uds/v1alpha3/zz_generated.deepcopy.go b/pkg/apis/uds/v1alpha3/zz_generated.deepcopy.go index d373408..3116f2d 100644 --- a/pkg/apis/uds/v1alpha3/zz_generated.deepcopy.go +++ b/pkg/apis/uds/v1alpha3/zz_generated.deepcopy.go @@ -7,6 +7,7 @@ package v1alpha3 import ( "github.com/kylegalloway/forge/pkg/apis/common" v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" ) @@ -613,6 +614,13 @@ func (in *UDSBundleJobSpec) DeepCopy() *UDSBundleJobSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *UDSBundleJobStatus) DeepCopyInto(out *UDSBundleJobStatus) { *out = *in + if in.Conditions != nil { + in, out := &in.Conditions, &out.Conditions + *out = make([]metav1.Condition, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } if in.CreateStatus != nil { in, out := &in.CreateStatus, &out.CreateStatus *out = new(OperationStatus) diff --git a/pkg/apis/zarf/v1alpha3/types.go b/pkg/apis/zarf/v1alpha3/types.go index 317bcc2..a388a1d 100644 --- a/pkg/apis/zarf/v1alpha3/types.go +++ b/pkg/apis/zarf/v1alpha3/types.go @@ -582,6 +582,14 @@ type ZarfPackageJobStatus struct { // +optional Message string `json:"message,omitempty"` + // Conditions is the standard Kubernetes condition array for this job. + // The Ready condition reflects the overall outcome; per-operation conditions + // (BuildSucceeded, PublishSucceeded, DeploySucceeded) track individual steps. + // +optional + // +listType=map + // +listMapKey=type + Conditions []metav1.Condition `json:"conditions,omitempty"` + // BuildStatus contains build operation status // +optional BuildStatus *OperationStatus `json:"buildStatus,omitempty"` diff --git a/pkg/apis/zarf/v1alpha3/zz_generated.deepcopy.go b/pkg/apis/zarf/v1alpha3/zz_generated.deepcopy.go index 3b3a0ee..9473466 100644 --- a/pkg/apis/zarf/v1alpha3/zz_generated.deepcopy.go +++ b/pkg/apis/zarf/v1alpha3/zz_generated.deepcopy.go @@ -7,6 +7,7 @@ package v1alpha3 import ( "github.com/kylegalloway/forge/pkg/apis/common" v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" ) @@ -584,6 +585,13 @@ func (in *ZarfPackageJobSpec) DeepCopy() *ZarfPackageJobSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ZarfPackageJobStatus) DeepCopyInto(out *ZarfPackageJobStatus) { *out = *in + if in.Conditions != nil { + in, out := &in.Conditions, &out.Conditions + *out = make([]metav1.Condition, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } if in.BuildStatus != nil { in, out := &in.BuildStatus, &out.BuildStatus *out = new(OperationStatus) diff --git a/pkg/constants/conditions.go b/pkg/constants/conditions.go new file mode 100644 index 0000000..721614f --- /dev/null +++ b/pkg/constants/conditions.go @@ -0,0 +1,48 @@ +package constants + +const ( + // ConditionTypeReady indicates the overall job completed successfully. + // True=succeeded, False=failed terminally, Unknown=in progress. + ConditionTypeReady = "Ready" + + // ConditionTypeReconciling indicates the controller is actively working on the job. + ConditionTypeReconciling = "Reconciling" + + // ConditionTypeBuildSucceeded reflects the build operation outcome. + ConditionTypeBuildSucceeded = "BuildSucceeded" + + // ConditionTypeCreateSucceeded reflects the UDS create operation outcome. + ConditionTypeCreateSucceeded = "CreateSucceeded" + + // ConditionTypePublishSucceeded reflects the publish operation outcome. + ConditionTypePublishSucceeded = "PublishSucceeded" + + // ConditionTypeDeploySucceeded reflects the deploy operation outcome. + ConditionTypeDeploySucceeded = "DeploySucceeded" +) + +const ( + // ConditionReasonSucceeded is used when an operation or job completed successfully. + ConditionReasonSucceeded = "Succeeded" + // ConditionReasonFailed is used when an operation or job terminated with an error. + ConditionReasonFailed = "Failed" + // ConditionReasonProgressing is used while an operation or job is in flight. + ConditionReasonProgressing = "Progressing" + // ConditionReasonSuspended is reserved for when spec.suspend is added. + // Not yet wired; defined here so the constant is stable when that feature lands. + ConditionReasonSuspended = "Suspended" +) + +// OperationConditionTypes maps OperationStatus JSON field names to condition types. +// Keys must match the json tags on the status types exactly — renaming a status +// field requires updating this list in sync. +// Slice order determines the order conditions appear in the status output. +var OperationConditionTypes = []struct { + Field string + CondType string +}{ + {"buildStatus", ConditionTypeBuildSucceeded}, + {"createStatus", ConditionTypeCreateSucceeded}, + {"publishStatus", ConditionTypePublishSucceeded}, + {"deployStatus", ConditionTypeDeploySucceeded}, +} diff --git a/pkg/controller/conditions.go b/pkg/controller/conditions.go new file mode 100644 index 0000000..a8cce1f --- /dev/null +++ b/pkg/controller/conditions.go @@ -0,0 +1,148 @@ +package controller + +import ( + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + klog "k8s.io/klog/v2" + + "github.com/kylegalloway/forge/pkg/constants" +) + +// DeriveConditions projects the phase state machine into a []metav1.Condition slice. +// existing is the prior conditions slice, used to preserve LastTransitionTime when +// a condition's status has not changed. +func DeriveConditions(phase string, status map[string]interface{}, existing []metav1.Condition, generation int64) []metav1.Condition { + now := metav1.Now() + conditions := make([]metav1.Condition, 0, 6) + + conditions = setCondition(conditions, existing, readyCondition(phase, generation, now)) + conditions = setCondition(conditions, existing, reconcilingCondition(phase, generation, now)) + + for _, op := range constants.OperationConditionTypes { + opStatus, ok := status[op.Field].(map[string]interface{}) + if !ok { + continue + } + opPhase, ok := opStatus[constants.StatusKeyState].(string) + if !ok || opPhase == "" { + continue + } + conditions = setCondition(conditions, existing, operationCondition(op.CondType, opPhase, generation, now)) + } + + return conditions +} + +func readyCondition(phase string, generation int64, now metav1.Time) metav1.Condition { + c := metav1.Condition{ + Type: constants.ConditionTypeReady, + ObservedGeneration: generation, + LastTransitionTime: now, + } + switch phase { + case constants.PhaseCompleted: + c.Status = metav1.ConditionTrue + c.Reason = constants.ConditionReasonSucceeded + case constants.PhaseFailed: + c.Status = metav1.ConditionFalse + c.Reason = constants.ConditionReasonFailed + default: + c.Status = metav1.ConditionUnknown + c.Reason = constants.ConditionReasonProgressing + } + return c +} + +func reconcilingCondition(phase string, generation int64, now metav1.Time) metav1.Condition { + status := metav1.ConditionTrue + reason := constants.ConditionReasonProgressing + switch phase { + case constants.PhaseCompleted: + status = metav1.ConditionFalse + reason = constants.ConditionReasonSucceeded + case constants.PhaseFailed: + status = metav1.ConditionFalse + reason = constants.ConditionReasonFailed + } + return metav1.Condition{ + Type: constants.ConditionTypeReconciling, + Status: status, + Reason: reason, + ObservedGeneration: generation, + LastTransitionTime: now, + } +} + +func operationCondition(condType, phase string, generation int64, now metav1.Time) metav1.Condition { + c := metav1.Condition{ + Type: condType, + ObservedGeneration: generation, + LastTransitionTime: now, + } + switch phase { + case constants.PhaseCompleted: + c.Status = metav1.ConditionTrue + c.Reason = constants.ConditionReasonSucceeded + case constants.PhaseFailed: + c.Status = metav1.ConditionFalse + c.Reason = constants.ConditionReasonFailed + default: + c.Status = metav1.ConditionUnknown + c.Reason = constants.ConditionReasonProgressing + } + return c +} + +// setCondition appends or replaces a condition, preserving LastTransitionTime +// when the status is unchanged. +func setCondition(conditions []metav1.Condition, existing []metav1.Condition, c metav1.Condition) []metav1.Condition { + for _, e := range existing { + if e.Type == c.Type && e.Status == c.Status { + c.LastTransitionTime = e.LastTransitionTime + break + } + } + for i, cur := range conditions { + if cur.Type == c.Type { + conditions[i] = c + return conditions + } + } + return append(conditions, c) +} + +// conditionsToUnstructured converts a []metav1.Condition to the []interface{} +// form required for unstructured status writes. +func conditionsToUnstructured(conditions []metav1.Condition) []interface{} { + out := make([]interface{}, 0, len(conditions)) + for i := range conditions { + m, err := runtime.DefaultUnstructuredConverter.ToUnstructured(&conditions[i]) + if err != nil { + klog.ErrorS(err, "failed to convert condition to unstructured", "type", conditions[i].Type) + continue + } + out = append(out, m) + } + return out +} + +// conditionsFromUnstructured reads the conditions slice from a raw status map. +func conditionsFromUnstructured(raw interface{}) []metav1.Condition { + list, ok := raw.([]interface{}) + if !ok { + return nil + } + conditions := make([]metav1.Condition, 0, len(list)) + for _, item := range list { + m, ok := item.(map[string]interface{}) + if !ok { + continue + } + var c metav1.Condition + if err := runtime.DefaultUnstructuredConverter.FromUnstructured(m, &c); err != nil { + continue + } + conditions = append(conditions, c) + } + return conditions +} diff --git a/pkg/controller/conditions_test.go b/pkg/controller/conditions_test.go new file mode 100644 index 0000000..c399980 --- /dev/null +++ b/pkg/controller/conditions_test.go @@ -0,0 +1,198 @@ +package controller + +import ( + "testing" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/kylegalloway/forge/pkg/constants" +) + +func TestDeriveConditions_Ready(t *testing.T) { + cases := []struct { + phase string + wantReadyStatus metav1.ConditionStatus + wantReconciling metav1.ConditionStatus + }{ + {constants.PhaseCompleted, metav1.ConditionTrue, metav1.ConditionFalse}, + {constants.PhaseFailed, metav1.ConditionFalse, metav1.ConditionFalse}, + {constants.PhaseRunning, metav1.ConditionUnknown, metav1.ConditionTrue}, + {constants.PhasePending, metav1.ConditionUnknown, metav1.ConditionTrue}, + {constants.PhaseRetrying, metav1.ConditionUnknown, metav1.ConditionTrue}, + {constants.PhaseQueued, metav1.ConditionUnknown, metav1.ConditionTrue}, + } + for _, tc := range cases { + t.Run(tc.phase, func(t *testing.T) { + conds := DeriveConditions(tc.phase, map[string]interface{}{}, nil, 1) + ready := findCondition(conds, constants.ConditionTypeReady) + if ready == nil { + t.Fatal("Ready condition missing") + } + if ready.Status != tc.wantReadyStatus { + t.Errorf("Ready status: got %s, want %s", ready.Status, tc.wantReadyStatus) + } + reconciling := findCondition(conds, constants.ConditionTypeReconciling) + if reconciling == nil { + t.Fatal("Reconciling condition missing") + } + if reconciling.Status != tc.wantReconciling { + t.Errorf("Reconciling status: got %s, want %s", reconciling.Status, tc.wantReconciling) + } + }) + } +} + +func TestDeriveConditions_OperationConditions(t *testing.T) { + status := map[string]interface{}{ + "buildStatus": map[string]interface{}{ + constants.StatusKeyState: constants.PhaseCompleted, + }, + "publishStatus": map[string]interface{}{ + constants.StatusKeyState: constants.PhaseRunning, + }, + } + conds := DeriveConditions(constants.PhaseRunning, status, nil, 1) + + build := findCondition(conds, constants.ConditionTypeBuildSucceeded) + if build == nil { + t.Fatal("BuildSucceeded condition missing") + } + if build.Status != metav1.ConditionTrue { + t.Errorf("BuildSucceeded: got %s, want True", build.Status) + } + + publish := findCondition(conds, constants.ConditionTypePublishSucceeded) + if publish == nil { + t.Fatal("PublishSucceeded condition missing") + } + if publish.Status != metav1.ConditionUnknown { + t.Errorf("PublishSucceeded: got %s, want Unknown", publish.Status) + } + + // deployStatus not in status map — condition should not appear + deploy := findCondition(conds, constants.ConditionTypeDeploySucceeded) + if deploy != nil { + t.Error("DeploySucceeded should not be present when deployStatus is absent") + } +} + +func TestDeriveConditions_PreservesLastTransitionTime(t *testing.T) { + past := metav1.NewTime(metav1.Now().Add(-60e9)) + existing := []metav1.Condition{ + { + Type: constants.ConditionTypeReady, + Status: metav1.ConditionUnknown, + LastTransitionTime: past, + Reason: constants.ConditionReasonProgressing, + }, + } + // Same status (Unknown → Unknown): LastTransitionTime must be preserved. + conds := DeriveConditions(constants.PhaseRunning, map[string]interface{}{}, existing, 1) + ready := findCondition(conds, constants.ConditionTypeReady) + if ready == nil { + t.Fatal("Ready condition missing") + } + if !ready.LastTransitionTime.Equal(&past) { + t.Errorf("LastTransitionTime changed when status was unchanged: got %v, want %v", ready.LastTransitionTime, past) + } +} + +func TestDeriveConditions_TransitionsLastTransitionTime(t *testing.T) { + past := metav1.NewTime(metav1.Now().Add(-60e9)) + existing := []metav1.Condition{ + { + Type: constants.ConditionTypeReady, + Status: metav1.ConditionUnknown, + LastTransitionTime: past, + Reason: constants.ConditionReasonProgressing, + }, + } + // Status changes (Unknown → True): LastTransitionTime must be updated. + conds := DeriveConditions(constants.PhaseCompleted, map[string]interface{}{}, existing, 1) + ready := findCondition(conds, constants.ConditionTypeReady) + if ready == nil { + t.Fatal("Ready condition missing") + } + if ready.LastTransitionTime.Equal(&past) { + t.Error("LastTransitionTime should have been updated on status transition") + } +} + +func TestConditionsRoundtrip(t *testing.T) { + original := []metav1.Condition{ + { + Type: constants.ConditionTypeReady, + Status: metav1.ConditionTrue, + Reason: constants.ConditionReasonSucceeded, + ObservedGeneration: 3, + LastTransitionTime: metav1.Now(), + }, + } + roundtripped := conditionsFromUnstructured(conditionsToUnstructured(original)) + if len(roundtripped) != 1 { + t.Fatalf("expected 1 condition after roundtrip, got %d", len(roundtripped)) + } + if roundtripped[0].Type != original[0].Type { + t.Errorf("Type: got %s, want %s", roundtripped[0].Type, original[0].Type) + } + if roundtripped[0].Status != original[0].Status { + t.Errorf("Status: got %s, want %s", roundtripped[0].Status, original[0].Status) + } + if roundtripped[0].ObservedGeneration != original[0].ObservedGeneration { + t.Errorf("ObservedGeneration: got %d, want %d", roundtripped[0].ObservedGeneration, original[0].ObservedGeneration) + } +} + +func TestDeriveConditions_ReconcilingReason(t *testing.T) { + cases := []struct { + phase string + wantReason string + }{ + {constants.PhaseCompleted, constants.ConditionReasonSucceeded}, + {constants.PhaseFailed, constants.ConditionReasonFailed}, + {constants.PhaseRunning, constants.ConditionReasonProgressing}, + {constants.PhasePending, constants.ConditionReasonProgressing}, + {constants.PhaseRetrying, constants.ConditionReasonProgressing}, + {constants.PhaseQueued, constants.ConditionReasonProgressing}, + } + for _, tc := range cases { + t.Run(tc.phase, func(t *testing.T) { + conds := DeriveConditions(tc.phase, map[string]interface{}{}, nil, 1) + reconciling := findCondition(conds, constants.ConditionTypeReconciling) + if reconciling == nil { + t.Fatal("Reconciling condition missing") + } + if reconciling.Reason != tc.wantReason { + t.Errorf("Reconciling reason: got %s, want %s", reconciling.Reason, tc.wantReason) + } + }) + } +} + +func TestDeriveConditions_DeterministicOrder(t *testing.T) { + status := map[string]interface{}{ + "buildStatus": map[string]interface{}{constants.StatusKeyState: constants.PhaseCompleted}, + "publishStatus": map[string]interface{}{constants.StatusKeyState: constants.PhaseRunning}, + "deployStatus": map[string]interface{}{constants.StatusKeyState: constants.PhaseFailed}, + } + first := DeriveConditions(constants.PhaseRunning, status, nil, 1) + second := DeriveConditions(constants.PhaseRunning, status, nil, 1) + + if len(first) != len(second) { + t.Fatalf("non-deterministic length: %d vs %d", len(first), len(second)) + } + for i := range first { + if first[i].Type != second[i].Type { + t.Errorf("position %d: first=%s second=%s — order is non-deterministic", i, first[i].Type, second[i].Type) + } + } +} + +func findCondition(conditions []metav1.Condition, condType string) *metav1.Condition { + for i := range conditions { + if conditions[i].Type == condType { + return &conditions[i] + } + } + return nil +} diff --git a/pkg/controller/generic_controller.go b/pkg/controller/generic_controller.go index 8f63a79..dff8493 100644 --- a/pkg/controller/generic_controller.go +++ b/pkg/controller/generic_controller.go @@ -644,28 +644,39 @@ func (ctrl *GenericController[T]) updateResourceStatus(ctx context.Context, obj return ctrl.updateStatus(ctx, obj, phase, message, nil) } -// updateStatus updates the resource status +// updateStatus updates the resource status. +// It merges into the existing status (preserving prior operation fields across +// chained actions) and derives standard conditions from the resulting phase. func (ctrl *GenericController[T]) updateStatus(ctx context.Context, obj *unstructured.Unstructured, phase, message string, operationStatus map[string]interface{}) error { name := obj.GetName() namespace := obj.GetNamespace() - // Build status object - status := map[string]interface{}{ - "phase": phase, - "message": message, - "lastUpdateTime": metav1.Now().Format(time.RFC3339), - "observedGeneration": obj.GetGeneration(), + // Start from existing status so chained actions don't lose prior operation fields. + existing, _, err := unstructured.NestedMap(obj.Object, "status") + if err != nil { + return fmt.Errorf("failed to read existing status: %w", err) + } + if existing == nil { + existing = map[string]interface{}{} } - // Add operation-specific status if provided + existingConditions := conditionsFromUnstructured(existing["conditions"]) + + existing["phase"] = phase + existing["message"] = message + existing["lastUpdateTime"] = metav1.Now().Format(time.RFC3339) + existing["observedGeneration"] = obj.GetGeneration() + for key, value := range operationStatus { - status[key] = value + existing[key] = value } - // Update status subresource - obj.Object["status"] = status + conditions := DeriveConditions(phase, existing, existingConditions, obj.GetGeneration()) + existing["conditions"] = conditionsToUnstructured(conditions) + + obj.Object["status"] = existing - _, err := ctrl.dynamicClient.Resource(ctrl.resourceGVR).Namespace(namespace).UpdateStatus(ctx, obj, metav1.UpdateOptions{}) + _, err = ctrl.dynamicClient.Resource(ctrl.resourceGVR).Namespace(namespace).UpdateStatus(ctx, obj, metav1.UpdateOptions{}) if err != nil { if errors.IsNotFound(err) { klog.V(4).InfoS("Resource not found during status update", "name", name, "namespace", namespace)