-
Notifications
You must be signed in to change notification settings - Fork 14
HYPERFLEET-536 - feat: add condition subfield queries for selective Sentinel polling #71
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
rafabene
wants to merge
8
commits into
openshift-hyperfleet:main
Choose a base branch
from
rafabene:HYPERFLEET-536
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+1,310
−107
Open
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
b68d405
HYPERFLEET-536 - feat: add condition subfield queries for selective S…
rafabene 8f68cff
HYPERFLEET-536 - fix: address PR review findings
rafabene 0904a44
HYPERFLEET-536: address PR review findings and add subfield expressio…
rafabene 0f30823
HYPERFLEET-536: reset Ready=False atomically on spec change
rafabene 73282a4
HYPERFLEET-536: add NodePool condition subfield integration test
rafabene 9e30b38
HYPERFLEET-536: reject NOT on condition queries and add search max le…
rafabene 6a0b8c7
HYPERFLEET-536: trigger PR conflict recheck
rafabene 8346325
HYPERFLEET-536: update NOT operator docs and refine condition query l…
rafabene File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,46 @@ | ||
| package dao | ||
|
|
||
| import ( | ||
| "encoding/json" | ||
| "time" | ||
|
|
||
| "gorm.io/datatypes" | ||
|
|
||
| "github.com/openshift-hyperfleet/hyperfleet-api/pkg/api" | ||
| ) | ||
|
|
||
| // resetReadyConditionOnSpecChange flips Ready=False when the spec changes (generation incremented). | ||
| // This ensures Sentinel's selective querying immediately picks up the resource | ||
| // via the "not-ready resources" query on the next poll cycle. | ||
| // Available is intentionally left unchanged — it reflects last known good state at any generation. | ||
| func resetReadyConditionOnSpecChange(existingConditions datatypes.JSON, now time.Time) (datatypes.JSON, error) { | ||
| if len(existingConditions) == 0 { | ||
| return existingConditions, nil | ||
| } | ||
|
|
||
| var conditions []api.ResourceCondition | ||
| if err := json.Unmarshal(existingConditions, &conditions); err != nil { | ||
| return existingConditions, nil | ||
| } | ||
|
|
||
| changed := false | ||
| for i := range conditions { | ||
| if conditions[i].Type == api.ConditionTypeReady && conditions[i].Status == api.ConditionTrue { | ||
| conditions[i].Status = api.ConditionFalse | ||
| conditions[i].LastTransitionTime = now | ||
| conditions[i].LastUpdatedTime = now | ||
| reason := "SpecChanged" | ||
| message := "Spec updated, awaiting adapters to reconcile at new generation" | ||
| conditions[i].Reason = &reason | ||
| conditions[i].Message = &message | ||
| changed = true | ||
| break | ||
| } | ||
| } | ||
|
|
||
| if !changed { | ||
| return existingConditions, nil | ||
| } | ||
|
|
||
| return json.Marshal(conditions) | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,98 @@ | ||
| package dao | ||
|
|
||
| import ( | ||
| "encoding/json" | ||
| "testing" | ||
| "time" | ||
|
|
||
| . "github.com/onsi/gomega" | ||
| "github.com/openshift-hyperfleet/hyperfleet-api/pkg/api" | ||
| ) | ||
|
|
||
| func TestResetReadyConditionOnSpecChange(t *testing.T) { | ||
| now := time.Now() | ||
|
|
||
| tests := []struct { | ||
| name string | ||
| conditions []api.ResourceCondition | ||
| expectReadyFalse bool | ||
| expectReasonChange bool | ||
| }{ | ||
| { | ||
| name: "Ready=True is flipped to False", | ||
| conditions: []api.ResourceCondition{ | ||
| {Type: "Available", Status: api.ConditionTrue}, | ||
| {Type: "Ready", Status: api.ConditionTrue}, | ||
| }, | ||
| expectReadyFalse: true, | ||
| expectReasonChange: true, | ||
| }, | ||
| { | ||
| name: "Ready=False stays False", | ||
| conditions: []api.ResourceCondition{ | ||
| {Type: "Available", Status: api.ConditionTrue}, | ||
| {Type: "Ready", Status: api.ConditionFalse}, | ||
| }, | ||
| expectReadyFalse: true, | ||
| expectReasonChange: false, | ||
| }, | ||
| { | ||
| name: "Available is not changed", | ||
| conditions: []api.ResourceCondition{ | ||
| {Type: "Available", Status: api.ConditionTrue}, | ||
| {Type: "Ready", Status: api.ConditionTrue}, | ||
| }, | ||
| expectReadyFalse: true, | ||
| }, | ||
| } | ||
|
|
||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| RegisterTestingT(t) | ||
|
|
||
| input, err := json.Marshal(tt.conditions) | ||
| Expect(err).ToNot(HaveOccurred()) | ||
|
|
||
| result, err := resetReadyConditionOnSpecChange(input, now) | ||
| Expect(err).ToNot(HaveOccurred()) | ||
|
|
||
| var resultConditions []api.ResourceCondition | ||
| Expect(json.Unmarshal(result, &resultConditions)).To(Succeed()) | ||
|
|
||
| for _, cond := range resultConditions { | ||
| switch cond.Type { | ||
| case "Ready": | ||
| if tt.expectReadyFalse { | ||
| Expect(cond.Status).To(Equal(api.ConditionFalse)) | ||
| } | ||
| if tt.expectReasonChange { | ||
| Expect(cond.Reason).ToNot(BeNil()) | ||
| Expect(*cond.Reason).To(Equal("SpecChanged")) | ||
| Expect(cond.LastTransitionTime.Equal(now)).To(BeTrue()) | ||
| } | ||
| case "Available": | ||
| Expect(cond.Status).To(Equal(api.ConditionTrue)) | ||
| } | ||
| } | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| func TestResetReadyConditionOnSpecChange_EmptyConditions(t *testing.T) { | ||
| RegisterTestingT(t) | ||
| now := time.Now() | ||
|
|
||
| result, err := resetReadyConditionOnSpecChange(nil, now) | ||
| Expect(err).ToNot(HaveOccurred()) | ||
| Expect(result).To(BeNil()) | ||
| } | ||
|
|
||
| func TestResetReadyConditionOnSpecChange_InvalidJSON(t *testing.T) { | ||
| RegisterTestingT(t) | ||
| now := time.Now() | ||
|
|
||
| input := []byte(`not valid json`) | ||
| result, err := resetReadyConditionOnSpecChange(input, now) | ||
| Expect(err).ToNot(HaveOccurred()) | ||
| Expect(string(result)).To(Equal(string(input))) | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
76 changes: 76 additions & 0 deletions
76
pkg/db/migrations/202603100001_add_conditions_subfield_indexes.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,76 @@ | ||
| package migrations | ||
|
|
||
| import ( | ||
| "github.com/go-gormigrate/gormigrate/v2" | ||
| "gorm.io/gorm" | ||
| ) | ||
|
|
||
| // addConditionsSubfieldIndexes adds expression indexes for condition subfield queries. | ||
| // The primary use case is Sentinel polling for stale-ready resources: | ||
| // | ||
| // status.conditions.Ready.last_updated_time < '...' | ||
| // | ||
| // PostgreSQL requires all functions in expression indexes to be IMMUTABLE. | ||
| // The built-in CAST(text AS TIMESTAMPTZ) is STABLE (depends on timezone settings), | ||
| // so we create an IMMUTABLE wrapper. This is safe because our timestamps are always | ||
| // stored in RFC3339 format with explicit timezone offsets. | ||
| // | ||
| // LIMITATION: The query builder uses Squirrel, which treats '?' in jsonpath filter | ||
| // syntax ($[*] ? (...)) as bind placeholders, forcing parameterized queries that | ||
| // PostgreSQL's planner cannot match against these expression indexes. HYPERFLEET-736 | ||
| // evaluates generated columns or table normalization as a proper fix. | ||
| func addConditionsSubfieldIndexes() *gormigrate.Migration { | ||
| return &gormigrate.Migration{ | ||
| ID: "202603100001", | ||
| Migrate: func(tx *gorm.DB) error { | ||
| // Create IMMUTABLE wrapper for text-to-timestamptz conversion. | ||
| // Required because PostgreSQL's built-in cast is STABLE, not IMMUTABLE. | ||
| if err := tx.Exec(` | ||
| CREATE OR REPLACE FUNCTION immutable_timestamptz(text) | ||
| RETURNS TIMESTAMPTZ | ||
| LANGUAGE SQL IMMUTABLE STRICT | ||
| AS $$ SELECT $1::timestamptz $$; | ||
| `).Error; err != nil { | ||
| return err | ||
| } | ||
|
|
||
| // Expression index on clusters for Ready condition last_updated_time | ||
| if err := tx.Exec(` | ||
| CREATE INDEX IF NOT EXISTS idx_clusters_ready_last_updated_time | ||
| ON clusters USING BTREE (( | ||
| immutable_timestamptz( | ||
| jsonb_path_query_first(status_conditions, '$[*] ? (@.type == "Ready")') ->> 'last_updated_time' | ||
| ) | ||
| )); | ||
| `).Error; err != nil { | ||
| return err | ||
| } | ||
|
|
||
| // Expression index on node_pools for Ready condition last_updated_time | ||
| if err := tx.Exec(` | ||
| CREATE INDEX IF NOT EXISTS idx_node_pools_ready_last_updated_time | ||
| ON node_pools USING BTREE (( | ||
| immutable_timestamptz( | ||
| jsonb_path_query_first(status_conditions, '$[*] ? (@.type == "Ready")') ->> 'last_updated_time' | ||
| ) | ||
| )); | ||
| `).Error; err != nil { | ||
rafabene marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| return err | ||
| } | ||
|
|
||
| return nil | ||
| }, | ||
| Rollback: func(tx *gorm.DB) error { | ||
| if err := tx.Exec("DROP INDEX IF EXISTS idx_clusters_ready_last_updated_time;").Error; err != nil { | ||
| return err | ||
| } | ||
| if err := tx.Exec("DROP INDEX IF EXISTS idx_node_pools_ready_last_updated_time;").Error; err != nil { | ||
| return err | ||
| } | ||
| if err := tx.Exec("DROP FUNCTION IF EXISTS immutable_timestamptz(text);").Error; err != nil { | ||
| return err | ||
| } | ||
| return nil | ||
| }, | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.