HYPERFLEET-708 - doc: add Time-based stability preconditions and update related adapter config example#78
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
WalkthroughThis change updates an adapter task example and docs to introduce a time-based stability precondition. The task captures currentTime, clusterName, and generation; calls the Kubernetes API with timeout/retry/backoff; derives two conditions (clusterNotReady and clusterReadyTTL) replacing the prior readyConditionStatus; and validates using clusterNotReady || clusterReadyTTL to allow execution when the cluster has been Ready for a configured TTL. Documentation adds a "Time-based stability preconditions" subsection with guidance, examples, and timestamp arithmetic recommendations. Sequence Diagram(s)sequenceDiagram
participant Operator
participant AdapterTask
participant K8sAPI
participant TimeSource
Operator->>AdapterTask: trigger precondition evaluation
AdapterTask->>TimeSource: capture now() as currentTime
AdapterTask->>K8sAPI: GET cluster status (with timeout, retry, backoff)
K8sAPI-->>AdapterTask: clusterStatus (conditions, lastTransitionTime, generation, name)
AdapterTask->>AdapterTask: derive clusterNotReady and clusterReadyTTL using currentTime and lastTransitionTime
AdapterTask->>AdapterTask: evaluate validationCheck = clusterNotReady || clusterReadyTTL
alt validationCheck true
AdapterTask->>Operator: allow resource execution (proceed)
else validationCheck false
AdapterTask->>Operator: block or retry later
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@charts/examples/kubernetes/adapter-task-config.yaml`:
- Around line 56-57: The precondition uses
timestamp(readyCondition.last_transition_time) without guarding for a missing
last_transition_time, which can throw; update the boolean expression around
readyCondition in the condition (the expression using readyCondition,
last_transition_time, timestamp, and currentTime) to explicitly handle a
null/absent last_transition_time (e.g., treat missing last_transition_time as
stale by adding readyCondition.last_transition_time == null || before doing
timestamp math) so timestamp() is only called when last_transition_time is
present.
In `@docs/adapter-authoring-guide.md`:
- Around line 383-401: The example expression uses
timestamp(readyCondition.last_transition_time) without guarding for missing
last_transition_time; update the precondition expression to short-circuit when
readyCondition or readyCondition.last_transition_time is absent (e.g. use
readyCondition == null || readyCondition.status != "True" ||
readyCondition.last_transition_time == null || (timestamp(currentTime) -
timestamp(readyCondition.last_transition_time)).getSeconds() >= 300) so
timestamp(...) is only called when last_transition_time exists; adjust the
capture/expression lines referencing readyCondition and currentTime accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7b200dbb-0c0e-48b6-abcb-7d87cb06ed77
📒 Files selected for processing (2)
charts/examples/kubernetes/adapter-task-config.yamldocs/adapter-authoring-guide.md
…te related adapter config example
86254860
force-pushed
the
HYPERFLEET-708
branch
from
0ffabea to
321f3ca
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
charts/examples/kubernetes/adapter-task-config.yaml (1)
47-58: MakeclusterReadyTTLreflect readiness, not just condition age.This turns
truewhenever the Ready condition is older than 300s, even if the current status is"False". Line 58 still behaves correctly becauseclusterNotReadyis ORed in, but the capture no longer matches its name/comments and is easy to reuse incorrectly later. Please mirror the same adjustment in the guide snippet so the documented pattern stays aligned.♻️ Suggested adjustment
- name: "clusterReadyTTL" expression: | - (timestamp(now()) - timestamp( - status.conditions.filter(c, c.type == "Ready").size() > 0 - ? status.conditions.filter(c, c.type == "Ready")[0].last_transition_time - : now() - )).getSeconds() >= 300 + status.conditions.filter(c, c.type == "Ready").size() > 0 + && status.conditions.filter(c, c.type == "Ready")[0].status == "True" + && (timestamp(now()) - timestamp( + status.conditions.filter(c, c.type == "Ready")[0].last_transition_time + )).getSeconds() >= 300As per coding guidelines, "-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@charts/examples/kubernetes/adapter-task-config.yaml` around lines 47 - 58, The clusterReadyTTL expression currently only checks the Ready condition age and can be true even when Ready.status == "False"; update the clusterReadyTTL expression to first select the Ready condition, ensure its status equals "True", then compute age >= 300s (i.e., require condition exists AND condition.status == "True" AND (timestamp(now()) - timestamp(condition.last_transition_time)).getSeconds() >= 300), and apply the same corrected snippet to the guide example so the documented pattern matches; references: clusterReadyTTL, validationCheck, clusterNotReady.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@charts/examples/kubernetes/adapter-task-config.yaml`:
- Around line 47-58: The clusterReadyTTL expression currently only checks the
Ready condition age and can be true even when Ready.status == "False"; update
the clusterReadyTTL expression to first select the Ready condition, ensure its
status equals "True", then compute age >= 300s (i.e., require condition exists
AND condition.status == "True" AND (timestamp(now()) -
timestamp(condition.last_transition_time)).getSeconds() >= 300), and apply the
same corrected snippet to the guide example so the documented pattern matches;
references: clusterReadyTTL, validationCheck, clusterNotReady.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cd9a6879-3ccc-45c0-b111-d623c2c18a2d
📒 Files selected for processing (2)
charts/examples/kubernetes/adapter-task-config.yamldocs/adapter-authoring-guide.md
1 participant
Summary by CodeRabbit
New Features
Documentation