From bbed31f54e2fa999483e7aaeecde5ef115a32cc5 Mon Sep 17 00:00:00 2001 From: Dmitrii Andreev Date: Mon, 29 Jun 2026 16:48:38 -0500 Subject: [PATCH 1/7] HYPERFLEET-1152 - feat: add resource_conditions table migration --- .../202606290001_add_resource_conditions.go | 38 +++++++++++++++++++ pkg/db/migrations/migration_structs.go | 1 + 2 files changed, 39 insertions(+) create mode 100644 pkg/db/migrations/202606290001_add_resource_conditions.go diff --git a/pkg/db/migrations/202606290001_add_resource_conditions.go b/pkg/db/migrations/202606290001_add_resource_conditions.go new file mode 100644 index 00000000..9291cc6e --- /dev/null +++ b/pkg/db/migrations/202606290001_add_resource_conditions.go @@ -0,0 +1,38 @@ +package migrations + +import ( + "github.com/go-gormigrate/gormigrate/v2" + "gorm.io/gorm" +) + +func addResourceConditions() *gormigrate.Migration { + return &gormigrate.Migration{ + ID: "202606290001", + Migrate: func(tx *gorm.DB) error { + if err := tx.Exec(`CREATE TABLE IF NOT EXISTS resource_conditions ( + resource_id VARCHAR(255) NOT NULL, + type VARCHAR(100) NOT NULL, + status VARCHAR(10) NOT NULL, + reason TEXT, + message TEXT, + observed_generation INTEGER NOT NULL DEFAULT 0, + created_time TIMESTAMPTZ NOT NULL DEFAULT NOW(), + last_updated_time TIMESTAMPTZ NOT NULL DEFAULT NOW(), + last_transition_time TIMESTAMPTZ NOT NULL DEFAULT NOW(), + PRIMARY KEY (resource_id, type), + FOREIGN KEY (resource_id) REFERENCES resources(id) ON DELETE CASCADE + );`).Error; err != nil { + return err + } + + if err := tx.Exec( + "CREATE INDEX IF NOT EXISTS idx_resource_conditions_resource_id " + + "ON resource_conditions (resource_id);", + ).Error; err != nil { + return err + } + + return nil + }, + } +} diff --git a/pkg/db/migrations/migration_structs.go b/pkg/db/migrations/migration_structs.go index 97b71618..1ae15820 100755 --- a/pkg/db/migrations/migration_structs.go +++ b/pkg/db/migrations/migration_structs.go @@ -37,6 +37,7 @@ var MigrationList = []*gormigrate.Migration{ addDeletedTimeIndexes(), addResources(), removeReadyCondition(), + addResourceConditions(), } // Model represents the base model struct. All entities will have this struct embedded. From 6ce4fbd9d0eab5e58d1ab0e418c30114ded58e1d Mon Sep 17 00:00:00 2001 From: Dmitrii Andreev Date: Mon, 29 Jun 2026 16:50:47 -0500 Subject: [PATCH 2/7] HYPERFLEET-1152 - feat: add ResourceCondition GORM model and Resource association --- pkg/api/resource.go | 27 ++++++++++++++------------- pkg/api/status_types.go | 28 +++++++++++++++++----------- 2 files changed, 31 insertions(+), 24 deletions(-) diff --git a/pkg/api/resource.go b/pkg/api/resource.go index 6dbf5837..75c42da7 100644 --- a/pkg/api/resource.go +++ b/pkg/api/resource.go @@ -16,20 +16,21 @@ import ( // differentiated by the Kind field. Existing Cluster and NodePool types // are NOT migrated to this model. type Resource struct { + OwnerID *string `json:"owner_id,omitempty" gorm:"size:255"` + OwnerHref *string `json:"owner_href,omitempty" gorm:"size:500"` + OwnerKind *string `json:"owner_kind,omitempty" gorm:"size:100"` + DeletedBy *string `json:"deleted_by,omitempty" gorm:"size:255"` + DeletedTime *time.Time `json:"deleted_time,omitempty"` Meta - Kind string `json:"kind" gorm:"size:100;not null"` - Name string `json:"name" gorm:"size:100;not null"` - Href string `json:"href,omitempty" gorm:"size:500"` - CreatedBy string `json:"created_by" gorm:"size:255;not null"` - UpdatedBy string `json:"updated_by" gorm:"size:255;not null"` - DeletedBy *string `json:"deleted_by,omitempty" gorm:"size:255"` - DeletedTime *time.Time `json:"deleted_time,omitempty"` - OwnerID *string `json:"owner_id,omitempty" gorm:"size:255"` - OwnerKind *string `json:"owner_kind,omitempty" gorm:"size:100"` - OwnerHref *string `json:"owner_href,omitempty" gorm:"size:500"` - Spec datatypes.JSON `json:"spec" gorm:"type:jsonb;not null"` - Labels datatypes.JSON `json:"labels,omitempty" gorm:"type:jsonb"` - Generation int32 `json:"generation" gorm:"default:1;not null"` + Kind string `json:"kind" gorm:"size:100;not null"` + Name string `json:"name" gorm:"size:100;not null"` + Href string `json:"href,omitempty" gorm:"size:500"` + CreatedBy string `json:"created_by" gorm:"size:255;not null"` + UpdatedBy string `json:"updated_by" gorm:"size:255;not null"` + Labels datatypes.JSON `json:"labels,omitempty" gorm:"type:jsonb"` + Spec datatypes.JSON `json:"spec" gorm:"type:jsonb;not null"` + Conditions []ResourceCondition `json:"-" gorm:"foreignKey:ResourceID;references:ID"` + Generation int32 `json:"generation" gorm:"default:1;not null"` } type ResourcePatch struct { diff --git a/pkg/api/status_types.go b/pkg/api/status_types.go index ec678a0c..30c6eb1a 100644 --- a/pkg/api/status_types.go +++ b/pkg/api/status_types.go @@ -50,18 +50,24 @@ const ( ResourceConditionTypeLastKnownReconciled = "LastKnownReconciled" ) -// ResourceCondition represents a condition of a resource -// Domain equivalent of openapi.ResourceCondition -// JSON tags match database JSONB structure +// ResourceCondition represents a condition of a resource. +// Dual-use: GORM model for the resource_conditions table (generic resources) +// and JSON-deserializable struct for JSONB columns (clusters/node pools). +// ResourceID is excluded from JSON (json:"-") to preserve JSONB backward compat. type ResourceCondition struct { - CreatedTime time.Time `json:"created_time"` - LastUpdatedTime time.Time `json:"last_updated_time"` - LastTransitionTime time.Time `json:"last_transition_time"` - Reason *string `json:"reason,omitempty"` - Message *string `json:"message,omitempty"` - Type string `json:"type"` - Status ResourceConditionStatus `json:"status"` - ObservedGeneration int32 `json:"observed_generation"` + CreatedTime time.Time `json:"created_time" gorm:"not null"` + LastUpdatedTime time.Time `json:"last_updated_time" gorm:"not null"` + LastTransitionTime time.Time `json:"last_transition_time" gorm:"not null"` + Reason *string `json:"reason,omitempty" gorm:"type:text"` + Message *string `json:"message,omitempty" gorm:"type:text"` + ResourceID string `json:"-" gorm:"primaryKey;size:255;not null"` + Type string `json:"type" gorm:"primaryKey;size:100;not null"` + Status ResourceConditionStatus `json:"status" gorm:"size:10;not null"` + ObservedGeneration int32 `json:"observed_generation" gorm:"default:0;not null"` +} + +func (ResourceCondition) TableName() string { + return "resource_conditions" } // AdapterCondition represents a condition of an adapter From ba986b87a5b3c89ceef6a07350a034c3ae72b687 Mon Sep 17 00:00:00 2001 From: Dmitrii Andreev Date: Mon, 29 Jun 2026 16:52:14 -0500 Subject: [PATCH 3/7] HYPERFLEET-1152 - feat: add ResourceConditionDao with UpdateConditions and preload --- pkg/dao/resource.go | 5 +-- pkg/dao/resource_condition.go | 66 +++++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 2 deletions(-) create mode 100644 pkg/dao/resource_condition.go diff --git a/pkg/dao/resource.go b/pkg/dao/resource.go index 52edd22e..d0c69a25 100644 --- a/pkg/dao/resource.go +++ b/pkg/dao/resource.go @@ -36,7 +36,7 @@ func NewResourceDao(sessionFactory db.SessionFactory) ResourceDao { func (d *sqlResourceDao) Get(ctx context.Context, kind, id string) (*api.Resource, error) { g2 := d.sessionFactory.New(ctx) var resource api.Resource - if err := g2.Take(&resource, "kind = ? AND id = ?", kind, id).Error; err != nil { + if err := g2.Preload("Conditions").Take(&resource, "kind = ? AND id = ?", kind, id).Error; err != nil { return nil, err } return &resource, nil @@ -55,7 +55,8 @@ func (d *sqlResourceDao) GetForUpdate(ctx context.Context, kind, id string) (*ap func (d *sqlResourceDao) GetByOwner(ctx context.Context, kind, id, ownerID string) (*api.Resource, error) { g2 := d.sessionFactory.New(ctx) var resource api.Resource - if err := g2.Take(&resource, "kind = ? AND id = ? AND owner_id = ?", kind, id, ownerID).Error; err != nil { + if err := g2.Preload("Conditions"). + Take(&resource, "kind = ? AND id = ? AND owner_id = ?", kind, id, ownerID).Error; err != nil { return nil, err } return &resource, nil diff --git a/pkg/dao/resource_condition.go b/pkg/dao/resource_condition.go new file mode 100644 index 00000000..c14d30b2 --- /dev/null +++ b/pkg/dao/resource_condition.go @@ -0,0 +1,66 @@ +package dao + +import ( + "context" + + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/api" + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/db" +) + +type ResourceConditionDao interface { + // UpdateConditions must be called within a write transaction — it performs + // delete+insert which is not atomic without one. Per ADR-0008, this is called + // from the adapter status upsert path (PUT), which has a transaction from + // TransactionMiddleware. + UpdateConditions(ctx context.Context, resourceID string, conditions []api.ResourceCondition) error +} + +var _ ResourceConditionDao = &sqlResourceConditionDao{} + +type sqlResourceConditionDao struct { + sessionFactory db.SessionFactory +} + +func NewResourceConditionDao(sessionFactory db.SessionFactory) ResourceConditionDao { + return &sqlResourceConditionDao{sessionFactory: sessionFactory} +} + +func (d *sqlResourceConditionDao) UpdateConditions( + ctx context.Context, resourceID string, conditions []api.ResourceCondition, +) error { + g2 := d.sessionFactory.New(ctx) + + var existing []api.ResourceCondition + if err := g2.Where("resource_id = ?", resourceID).Find(&existing).Error; err != nil { + return err + } + + prevByType := make(map[string]api.ResourceCondition, len(existing)) + for _, c := range existing { + prevByType[c.Type] = c + } + + for i := range conditions { + conditions[i].ResourceID = resourceID + if prev, ok := prevByType[conditions[i].Type]; ok { + if prev.Status == conditions[i].Status { + conditions[i].LastTransitionTime = prev.LastTransitionTime + } + conditions[i].CreatedTime = prev.CreatedTime + } + } + + if err := g2.Where("resource_id = ?", resourceID).Delete(&api.ResourceCondition{}).Error; err != nil { + db.MarkForRollback(ctx, err) + return err + } + + if len(conditions) > 0 { + if err := g2.Create(&conditions).Error; err != nil { + db.MarkForRollback(ctx, err) + return err + } + } + + return nil +} From 0c037c25c57704c4e6a080feee73eb9fc7aa5823 Mon Sep 17 00:00:00 2001 From: Dmitrii Andreev Date: Mon, 29 Jun 2026 16:53:08 -0500 Subject: [PATCH 4/7] HYPERFLEET-1152 - feat: surface resource conditions in presenter --- pkg/api/presenters/resource.go | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/pkg/api/presenters/resource.go b/pkg/api/presenters/resource.go index 888e9593..307b6bc5 100644 --- a/pkg/api/presenters/resource.go +++ b/pkg/api/presenters/resource.go @@ -71,7 +71,7 @@ func PresentResource(r *api.Resource) openapi.Resource { UpdatedBy: openapi_types.Email(r.UpdatedBy), DeletedTime: r.DeletedTime, Status: openapi.ResourceStatus{ - Conditions: []openapi.ResourceCondition{}, + Conditions: presentResourceConditions(r.Conditions), }, } @@ -109,6 +109,26 @@ func PresentResourceList(resources api.ResourceList, paging *api.PagingMeta) ope } } +func presentResourceConditions(conditions []api.ResourceCondition) []openapi.ResourceCondition { + if len(conditions) == 0 { + return []openapi.ResourceCondition{} + } + result := make([]openapi.ResourceCondition, len(conditions)) + for i, c := range conditions { + result[i] = openapi.ResourceCondition{ + Type: c.Type, + Status: openapi.ResourceConditionStatus(c.Status), + Reason: c.Reason, + Message: c.Message, + ObservedGeneration: c.ObservedGeneration, + CreatedTime: c.CreatedTime, + LastUpdatedTime: c.LastUpdatedTime, + LastTransitionTime: c.LastTransitionTime, + } + } + return result +} + func marshalLabels(labels *map[string]string) (datatypes.JSON, error) { if labels == nil { return datatypes.JSON("{}"), nil From 5a17a41ef072ba2f97cf5996f30fae4fc65f2ec0 Mon Sep 17 00:00:00 2001 From: Dmitrii Andreev Date: Mon, 29 Jun 2026 16:54:36 -0500 Subject: [PATCH 5/7] HYPERFLEET-1152 - test: add resource conditions integration and unit tests --- pkg/api/presenters/resource_test.go | 62 +++++ test/integration/resource_conditions_test.go | 236 +++++++++++++++++++ 2 files changed, 298 insertions(+) create mode 100644 test/integration/resource_conditions_test.go diff --git a/pkg/api/presenters/resource_test.go b/pkg/api/presenters/resource_test.go index eb2e2ae9..6b1ddda3 100644 --- a/pkg/api/presenters/resource_test.go +++ b/pkg/api/presenters/resource_test.go @@ -155,6 +155,68 @@ func TestPresentResource_EmptySpec(t *testing.T) { Expect(resp.Spec).To(BeEmpty()) } +func TestPresentResource_WithConditions(t *testing.T) { + RegisterTestingT(t) + + now := time.Now().Truncate(time.Microsecond) + reason := "AllAdaptersReporting" + message := "All adapters are available" + resource := &api.Resource{ + Meta: api.Meta{ID: "cond-id", CreatedTime: now, UpdatedTime: now}, + Kind: "Channel", + Name: "test", + Spec: datatypes.JSON(`{}`), + CreatedBy: "user@test.com", + UpdatedBy: "user@test.com", + Conditions: []api.ResourceCondition{ + { + Type: "Available", + Status: api.ConditionTrue, + Reason: &reason, + Message: &message, + ObservedGeneration: 3, + CreatedTime: now, + LastUpdatedTime: now, + LastTransitionTime: now, + }, + }, + } + + resp := PresentResource(resource) + Expect(resp.Status.Conditions).To(HaveLen(1)) + + cond := resp.Status.Conditions[0] + Expect(cond.Type).To(Equal("Available")) + Expect(cond.Status).To(Equal(openapi.ResourceConditionStatusTrue)) + Expect(*cond.Reason).To(Equal("AllAdaptersReporting")) + Expect(*cond.Message).To(Equal("All adapters are available")) + Expect(cond.ObservedGeneration).To(Equal(int32(3))) + Expect(cond.CreatedTime).To(BeTemporally("==", now)) + Expect(cond.LastTransitionTime).To(BeTemporally("==", now)) +} + +func TestPresentResource_WithEmptyConditions(t *testing.T) { + RegisterTestingT(t) + + now := time.Now() + resource := &api.Resource{ + Meta: api.Meta{ID: "empty-cond-id", CreatedTime: now, UpdatedTime: now}, + Kind: "Channel", + Name: "test", + Spec: datatypes.JSON(`{}`), + CreatedBy: "user@test.com", + UpdatedBy: "user@test.com", + Conditions: []api.ResourceCondition{}, + } + + resp := PresentResource(resource) + body, err := json.Marshal(resp) + Expect(err).NotTo(HaveOccurred()) + Expect(resp.Status.Conditions).NotTo(BeNil()) + Expect(resp.Status.Conditions).To(BeEmpty()) + Expect(string(body)).To(ContainSubstring(`"status":{"conditions":[]}`)) +} + func TestPresentResourceList(t *testing.T) { RegisterTestingT(t) diff --git a/test/integration/resource_conditions_test.go b/test/integration/resource_conditions_test.go new file mode 100644 index 00000000..f4411f11 --- /dev/null +++ b/test/integration/resource_conditions_test.go @@ -0,0 +1,236 @@ +package integration + +import ( + "context" + "testing" + "time" + + . "github.com/onsi/gomega" + + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/api" + "github.com/openshift-hyperfleet/hyperfleet-api/pkg/dao" +) + +func TestResourceConditions_UpdateAndPreload(t *testing.T) { + RegisterTestingT(t) + + svc, h := setupResourceTest(t) + ctx := context.Background() + + channel, svcErr := svc.Create(ctx, "Channel", newChannelResource("cond-test-update")) + Expect(svcErr).To(BeNil()) + + condDao := dao.NewResourceConditionDao(h.DBFactory) + resourceDao := dao.NewResourceDao(h.DBFactory) + + now := time.Now().UTC().Truncate(time.Microsecond) + conditions := []api.ResourceCondition{ + { + Type: "Available", + Status: api.ConditionTrue, + ObservedGeneration: 1, + CreatedTime: now, + LastUpdatedTime: now, + LastTransitionTime: now, + }, + { + Type: "Reconciled", + Status: api.ConditionFalse, + ObservedGeneration: 1, + CreatedTime: now, + LastUpdatedTime: now, + LastTransitionTime: now, + }, + } + + err := condDao.UpdateConditions(ctx, channel.ID, conditions) + Expect(err).ToNot(HaveOccurred()) + + fetched, err := resourceDao.Get(ctx, "Channel", channel.ID) + Expect(err).ToNot(HaveOccurred()) + Expect(fetched.Conditions).To(HaveLen(2)) + + condByType := make(map[string]api.ResourceCondition) + for _, c := range fetched.Conditions { + condByType[c.Type] = c + } + + Expect(condByType["Available"].Status).To(Equal(api.ConditionTrue)) + Expect(condByType["Reconciled"].Status).To(Equal(api.ConditionFalse)) +} + +func TestResourceConditions_LastTransitionTimePreserved(t *testing.T) { + RegisterTestingT(t) + + svc, h := setupResourceTest(t) + ctx := context.Background() + + channel, svcErr := svc.Create(ctx, "Channel", newChannelResource("cond-test-transition")) + Expect(svcErr).To(BeNil()) + + condDao := dao.NewResourceConditionDao(h.DBFactory) + resourceDao := dao.NewResourceDao(h.DBFactory) + + originalTime := time.Date(2025, 1, 1, 0, 0, 0, 0, time.UTC) + conditions := []api.ResourceCondition{ + { + Type: "Available", + Status: api.ConditionTrue, + ObservedGeneration: 1, + CreatedTime: originalTime, + LastUpdatedTime: originalTime, + LastTransitionTime: originalTime, + }, + } + + err := condDao.UpdateConditions(ctx, channel.ID, conditions) + Expect(err).ToNot(HaveOccurred()) + + // Same status — LastTransitionTime must be preserved + laterTime := time.Date(2025, 6, 1, 0, 0, 0, 0, time.UTC) + conditions2 := []api.ResourceCondition{ + { + Type: "Available", + Status: api.ConditionTrue, + ObservedGeneration: 2, + CreatedTime: laterTime, + LastUpdatedTime: laterTime, + LastTransitionTime: laterTime, + }, + } + + err = condDao.UpdateConditions(ctx, channel.ID, conditions2) + Expect(err).ToNot(HaveOccurred()) + + fetched, err := resourceDao.Get(ctx, "Channel", channel.ID) + Expect(err).ToNot(HaveOccurred()) + Expect(fetched.Conditions).To(HaveLen(1)) + Expect(fetched.Conditions[0].LastTransitionTime).To( + BeTemporally("==", originalTime), + "LastTransitionTime should be preserved when status unchanged", + ) + + // Status changes — LastTransitionTime must update + transitionTime := time.Date(2025, 7, 1, 0, 0, 0, 0, time.UTC) + conditions3 := []api.ResourceCondition{ + { + Type: "Available", + Status: api.ConditionFalse, + ObservedGeneration: 3, + CreatedTime: transitionTime, + LastUpdatedTime: transitionTime, + LastTransitionTime: transitionTime, + }, + } + + err = condDao.UpdateConditions(ctx, channel.ID, conditions3) + Expect(err).ToNot(HaveOccurred()) + + fetched, err = resourceDao.Get(ctx, "Channel", channel.ID) + Expect(err).ToNot(HaveOccurred()) + Expect(fetched.Conditions[0].LastTransitionTime).To( + BeTemporally("==", transitionTime), + "LastTransitionTime should update when status changes", + ) +} + +func TestResourceConditions_AtomicReplace(t *testing.T) { + RegisterTestingT(t) + + svc, h := setupResourceTest(t) + ctx := context.Background() + + channel, svcErr := svc.Create(ctx, "Channel", newChannelResource("cond-test-replace")) + Expect(svcErr).To(BeNil()) + + condDao := dao.NewResourceConditionDao(h.DBFactory) + resourceDao := dao.NewResourceDao(h.DBFactory) + + now := time.Now().UTC().Truncate(time.Microsecond) + + // Insert two conditions + err := condDao.UpdateConditions(ctx, channel.ID, []api.ResourceCondition{ + {Type: "Available", Status: api.ConditionTrue, ObservedGeneration: 1, + CreatedTime: now, LastUpdatedTime: now, LastTransitionTime: now}, + {Type: "Reconciled", Status: api.ConditionFalse, ObservedGeneration: 1, + CreatedTime: now, LastUpdatedTime: now, LastTransitionTime: now}, + }) + Expect(err).ToNot(HaveOccurred()) + + // Replace with one condition — old one must be gone + err = condDao.UpdateConditions(ctx, channel.ID, []api.ResourceCondition{ + {Type: "Available", Status: api.ConditionTrue, ObservedGeneration: 2, + CreatedTime: now, LastUpdatedTime: now, LastTransitionTime: now}, + }) + Expect(err).ToNot(HaveOccurred()) + + fetched, err := resourceDao.Get(ctx, "Channel", channel.ID) + Expect(err).ToNot(HaveOccurred()) + Expect(fetched.Conditions).To(HaveLen(1)) + Expect(fetched.Conditions[0].Type).To(Equal("Available")) +} + +func TestResourceConditions_SaveDoesNotTouchConditions(t *testing.T) { + RegisterTestingT(t) + + svc, h := setupResourceTest(t) + ctx := context.Background() + + channel, svcErr := svc.Create(ctx, "Channel", newChannelResource("cond-test-save")) + Expect(svcErr).To(BeNil()) + + condDao := dao.NewResourceConditionDao(h.DBFactory) + resourceDao := dao.NewResourceDao(h.DBFactory) + + now := time.Now().UTC().Truncate(time.Microsecond) + err := condDao.UpdateConditions(ctx, channel.ID, []api.ResourceCondition{ + {Type: "Available", Status: api.ConditionTrue, ObservedGeneration: 1, + CreatedTime: now, LastUpdatedTime: now, LastTransitionTime: now}, + }) + Expect(err).ToNot(HaveOccurred()) + + // Modify spec via Save — conditions must remain + channel.Spec = []byte(`{"is_default": true, "enabled_regex": ".*"}`) + err = resourceDao.Save(ctx, channel) + Expect(err).ToNot(HaveOccurred()) + + fetched, err := resourceDao.Get(ctx, "Channel", channel.ID) + Expect(err).ToNot(HaveOccurred()) + Expect(fetched.Conditions).To(HaveLen(1)) + Expect(fetched.Conditions[0].Type).To(Equal("Available")) +} + +func TestResourceConditions_CreatedTimePreserved(t *testing.T) { + RegisterTestingT(t) + + svc, h := setupResourceTest(t) + ctx := context.Background() + + channel, svcErr := svc.Create(ctx, "Channel", newChannelResource("cond-test-created")) + Expect(svcErr).To(BeNil()) + + condDao := dao.NewResourceConditionDao(h.DBFactory) + resourceDao := dao.NewResourceDao(h.DBFactory) + + originalCreated := time.Date(2025, 1, 15, 12, 0, 0, 0, time.UTC) + err := condDao.UpdateConditions(ctx, channel.ID, []api.ResourceCondition{ + {Type: "Available", Status: api.ConditionTrue, ObservedGeneration: 1, + CreatedTime: originalCreated, LastUpdatedTime: originalCreated, LastTransitionTime: originalCreated}, + }) + Expect(err).ToNot(HaveOccurred()) + + // Second update — CreatedTime must be preserved from first call + laterTime := time.Date(2025, 6, 1, 0, 0, 0, 0, time.UTC) + err = condDao.UpdateConditions(ctx, channel.ID, []api.ResourceCondition{ + {Type: "Available", Status: api.ConditionFalse, ObservedGeneration: 2, + CreatedTime: laterTime, LastUpdatedTime: laterTime, LastTransitionTime: laterTime}, + }) + Expect(err).ToNot(HaveOccurred()) + + fetched, err := resourceDao.Get(ctx, "Channel", channel.ID) + Expect(err).ToNot(HaveOccurred()) + Expect(fetched.Conditions[0].CreatedTime).To( + BeTemporally("==", originalCreated), + "CreatedTime should be preserved from the first UpdateConditions call", + ) +} From 9620c0f900527e13d017df49e95f700b4d4e6ebe Mon Sep 17 00:00:00 2001 From: Dmitrii Andreev Date: Mon, 29 Jun 2026 17:11:08 -0500 Subject: [PATCH 6/7] HYPERFLEET-1152 - test: add GetByOwner preload and empty-slice tests --- test/integration/resource_conditions_test.go | 60 ++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/test/integration/resource_conditions_test.go b/test/integration/resource_conditions_test.go index f4411f11..c51c4b42 100644 --- a/test/integration/resource_conditions_test.go +++ b/test/integration/resource_conditions_test.go @@ -198,6 +198,66 @@ func TestResourceConditions_SaveDoesNotTouchConditions(t *testing.T) { Expect(err).ToNot(HaveOccurred()) Expect(fetched.Conditions).To(HaveLen(1)) Expect(fetched.Conditions[0].Type).To(Equal("Available")) + Expect(fetched.Conditions[0].Status).To(Equal(api.ConditionTrue)) + Expect(fetched.Conditions[0].ObservedGeneration).To(Equal(int32(1))) + Expect(fetched.Conditions[0].CreatedTime).To(BeTemporally("~", now, time.Second)) +} + +func TestResourceConditions_GetByOwnerPreload(t *testing.T) { + RegisterTestingT(t) + + svc, h := setupResourceTest(t) + ctx := context.Background() + + channel, svcErr := svc.Create(ctx, "Channel", newChannelResource("cond-test-owner-preload")) + Expect(svcErr).To(BeNil()) + + version, svcErr := svc.Create(ctx, "Version", newVersionResource("v1", channel.ID)) + Expect(svcErr).To(BeNil()) + + condDao := dao.NewResourceConditionDao(h.DBFactory) + resourceDao := dao.NewResourceDao(h.DBFactory) + + now := time.Now().UTC().Truncate(time.Microsecond) + err := condDao.UpdateConditions(ctx, version.ID, []api.ResourceCondition{ + {Type: "Available", Status: api.ConditionTrue, ObservedGeneration: 1, + CreatedTime: now, LastUpdatedTime: now, LastTransitionTime: now}, + }) + Expect(err).ToNot(HaveOccurred()) + + fetched, err := resourceDao.GetByOwner(ctx, "Version", version.ID, channel.ID) + Expect(err).ToNot(HaveOccurred()) + Expect(fetched.Conditions).To(HaveLen(1)) + Expect(fetched.Conditions[0].Type).To(Equal("Available")) + Expect(fetched.Conditions[0].Status).To(Equal(api.ConditionTrue)) +} + +func TestResourceConditions_ClearWithEmptySlice(t *testing.T) { + RegisterTestingT(t) + + svc, h := setupResourceTest(t) + ctx := context.Background() + + channel, svcErr := svc.Create(ctx, "Channel", newChannelResource("cond-test-clear")) + Expect(svcErr).To(BeNil()) + + condDao := dao.NewResourceConditionDao(h.DBFactory) + resourceDao := dao.NewResourceDao(h.DBFactory) + + now := time.Now().UTC().Truncate(time.Microsecond) + err := condDao.UpdateConditions(ctx, channel.ID, []api.ResourceCondition{ + {Type: "Available", Status: api.ConditionTrue, ObservedGeneration: 1, + CreatedTime: now, LastUpdatedTime: now, LastTransitionTime: now}, + }) + Expect(err).ToNot(HaveOccurred()) + + // Clear all conditions with empty slice + err = condDao.UpdateConditions(ctx, channel.ID, []api.ResourceCondition{}) + Expect(err).ToNot(HaveOccurred()) + + fetched, err := resourceDao.Get(ctx, "Channel", channel.ID) + Expect(err).ToNot(HaveOccurred()) + Expect(fetched.Conditions).To(BeEmpty()) } func TestResourceConditions_CreatedTimePreserved(t *testing.T) { From 2976ac6d72f44570ffcd84370738bd3b257722d3 Mon Sep 17 00:00:00 2001 From: Dmitrii Andreev Date: Tue, 30 Jun 2026 10:36:51 -0500 Subject: [PATCH 7/7] HYPERFLEET-1152 - fix: add MarkForRollback on Find error, drop redundant index --- pkg/api/status_types.go | 7 ++--- pkg/dao/resource_condition.go | 28 ++++++++++++++++--- .../202606290001_add_resource_conditions.go | 7 ----- 3 files changed, 27 insertions(+), 15 deletions(-) diff --git a/pkg/api/status_types.go b/pkg/api/status_types.go index 30c6eb1a..af69cdb7 100644 --- a/pkg/api/status_types.go +++ b/pkg/api/status_types.go @@ -50,10 +50,9 @@ const ( ResourceConditionTypeLastKnownReconciled = "LastKnownReconciled" ) -// ResourceCondition represents a condition of a resource. -// Dual-use: GORM model for the resource_conditions table (generic resources) -// and JSON-deserializable struct for JSONB columns (clusters/node pools). -// ResourceID is excluded from JSON (json:"-") to preserve JSONB backward compat. +// ResourceCondition is the GORM model for the resource_conditions table and +// the domain type for JSONB deserialization in clusters/node pools. +// ResourceID is excluded from JSON to preserve JSONB backward compat. type ResourceCondition struct { CreatedTime time.Time `json:"created_time" gorm:"not null"` LastUpdatedTime time.Time `json:"last_updated_time" gorm:"not null"` diff --git a/pkg/dao/resource_condition.go b/pkg/dao/resource_condition.go index c14d30b2..5cf6d9e3 100644 --- a/pkg/dao/resource_condition.go +++ b/pkg/dao/resource_condition.go @@ -2,16 +2,25 @@ package dao import ( "context" + "time" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/api" "github.com/openshift-hyperfleet/hyperfleet-api/pkg/db" ) type ResourceConditionDao interface { - // UpdateConditions must be called within a write transaction — it performs - // delete+insert which is not atomic without one. Per ADR-0008, this is called - // from the adapter status upsert path (PUT), which has a transaction from - // TransactionMiddleware. + // UpdateConditions atomically replaces all condition rows for a resource. + // + // Prerequisites: + // - Write transaction (delete+insert is not atomic without one). + // - Caller holds a row lock on the parent resource (e.g. via GetForUpdate). + // Without it, concurrent callers read stale state and the loser's + // timestamp preservation computes against a pre-delete snapshot. + // + // Timestamp contract: for existing condition types, CreatedTime is always + // preserved and LastTransitionTime is preserved when status is unchanged. + // For new condition types, both are used as-is from the input — caller + // must populate them. Zero timestamps are defaulted to now. UpdateConditions(ctx context.Context, resourceID string, conditions []api.ResourceCondition) error } @@ -32,6 +41,7 @@ func (d *sqlResourceConditionDao) UpdateConditions( var existing []api.ResourceCondition if err := g2.Where("resource_id = ?", resourceID).Find(&existing).Error; err != nil { + db.MarkForRollback(ctx, err) return err } @@ -40,6 +50,7 @@ func (d *sqlResourceConditionDao) UpdateConditions( prevByType[c.Type] = c } + now := time.Now().UTC().Truncate(time.Microsecond) for i := range conditions { conditions[i].ResourceID = resourceID if prev, ok := prevByType[conditions[i].Type]; ok { @@ -48,6 +59,15 @@ func (d *sqlResourceConditionDao) UpdateConditions( } conditions[i].CreatedTime = prev.CreatedTime } + if conditions[i].CreatedTime.IsZero() { + conditions[i].CreatedTime = now + } + if conditions[i].LastTransitionTime.IsZero() { + conditions[i].LastTransitionTime = now + } + if conditions[i].LastUpdatedTime.IsZero() { + conditions[i].LastUpdatedTime = now + } } if err := g2.Where("resource_id = ?", resourceID).Delete(&api.ResourceCondition{}).Error; err != nil { diff --git a/pkg/db/migrations/202606290001_add_resource_conditions.go b/pkg/db/migrations/202606290001_add_resource_conditions.go index 9291cc6e..16763fcb 100644 --- a/pkg/db/migrations/202606290001_add_resource_conditions.go +++ b/pkg/db/migrations/202606290001_add_resource_conditions.go @@ -25,13 +25,6 @@ func addResourceConditions() *gormigrate.Migration { return err } - if err := tx.Exec( - "CREATE INDEX IF NOT EXISTS idx_resource_conditions_resource_id " + - "ON resource_conditions (resource_id);", - ).Error; err != nil { - return err - } - return nil }, }