-
Notifications
You must be signed in to change notification settings - Fork 245
feat: provide configurable cse timeout in seconds #7766
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
base: main
Are you sure you want to change the base?
Conversation
|
The latest Buf updates on your PR. Results from workflow Buf CI / buf (pull_request).
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds a configurable CSE (Custom Script Extension) timeout feature to allow users to override the default 15-minute timeout. This can be useful for debugging slow CSE scripts or handling scenarios that require longer execution times.
Changes:
- Added
CSETimeoutInMinutesconfiguration field with validation (max 360 minutes/6 hours) - Updated shell scripts to use the configurable timeout instead of hardcoded 15m
- Added protobuf field and parser logic to support the new configuration
Reviewed changes
Copilot reviewed 74 out of 134 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/agent/datamodel/types.go | Added CSETimeoutInMinutes field and GetCSETimeout() validation method |
| pkg/agent/datamodel/const.go | Added CSE timeout constants (default 15m, max 360m) |
| pkg/agent/baker.go | Added GetCSETimeout function to template function map |
| parts/linux/cloud-init/artifacts/cse_start.sh | Changed hardcoded 15m timeout to $CSE_TIMEOUT variable |
| parts/linux/cloud-init/artifacts/cse_cmd.sh | Added CSE_TIMEOUT variable from template |
| aks-node-controller/proto/aksnodeconfig/v1/config.proto | Added cse_timeout_in_minutes protobuf field |
| aks-node-controller/parser/parser.go | Added CSE_TIMEOUT to environment variable map |
| aks-node-controller/parser/helper.go | Added getCSETimeout() helper function with validation |
| aks-node-controller/Makefile | Updated to use linux/amd64 platform and changed branch reference |
| testdata/* | Updated binary test data files to reflect the new CSE timeout configuration |
a55136d to
6961bfa
Compare
6961bfa to
105904e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 74 out of 134 changed files in this pull request and generated 3 comments.
105904e to
b144e0a
Compare
b144e0a to
7d01661
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 75 out of 135 changed files in this pull request and generated 4 comments.
7d01661 to
95d9480
Compare
Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 75 out of 135 changed files in this pull request and generated 1 comment.
| if cseTimeout <= 0 || cseTimeout > MaxCSETimeout { | ||
| cseTimeout = DefaultCSETimeout | ||
| } | ||
| return fmt.Sprintf("%d", cseTimeout) |
Copilot
AI
Feb 3, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function should append 's' (seconds suffix) to the returned string for proper timeout command usage. The timeout command requires a duration with a unit suffix (e.g., "900s" for 900 seconds).
| return fmt.Sprintf("%d", cseTimeout) | |
| return fmt.Sprintf("%ds", cseTimeout) |
What this PR does / why we need it:
Sometimes this can be useful in debugging why certain CSE scripts are taking a long time, make this configurable through RP or external components
Which issue(s) this PR fixes:
Fixes #