-
Notifications
You must be signed in to change notification settings - Fork 51
🐛 Add to E2E test a workaround to context bug #598
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
🐛 Add to E2E test a workaround to context bug #598
Conversation
3c0457e to
78b51c3
Compare
|
/cc @pdettori |
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 workaround to the E2E test suite to address a context-related bug (#597) by cleaning up kubeflex extensions from the kubeconfig file before running tests.
Key Changes
- Adds kubeconfig manipulation logic to remove kubeflex extensions from the config file
- Includes a commented-out alternative command for context management
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| yq '.extensions |= [ .[] | select(.name != "kubeflex") ]' "$cfgfile" > $$ | ||
| mv -f -- "$cfgfile" "${cfgfile}.bak" | ||
| mv -- $$ "$cfgfile" |
Copilot
AI
Dec 21, 2025
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.
Using $$ (current shell PID) as a temporary filename creates a potential race condition if multiple instances run concurrently. Consider using mktemp to create a unique temporary file instead.
| yq '.extensions |= [ .[] | select(.name != "kubeflex") ]' "$cfgfile" > $$ | |
| mv -f -- "$cfgfile" "${cfgfile}.bak" | |
| mv -- $$ "$cfgfile" | |
| tmpfile="$(mktemp "${cfgfile}.XXXXXX")" | |
| yq '.extensions |= [ .[] | select(.name != "kubeflex") ]' "$cfgfile" > "$tmpfile" | |
| mv -f -- "$cfgfile" "${cfgfile}.bak" | |
| mv -- "$tmpfile" "$cfgfile" |
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.
There are not multiple concurrent instaces. This is not a problem.
test/e2e/run.sh
Outdated
|
|
||
| ${SRC_DIR}/cleanup.sh | ||
| ${SRC_DIR}/setup-kubeflex.sh | ||
| kubectl config current-context |
Copilot
AI
Dec 21, 2025
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.
This command's output is not captured or used. If this is for debugging purposes, consider redirecting to stderr or removing if not needed.
| kubectl config current-context | |
| kubectl config current-context >&2 |
78b51c3 to
1765514
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 1 out of 1 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ${SRC_DIR}/cleanup.sh | ||
| ${SRC_DIR}/setup-kubeflex.sh | ||
| cfgfile="${KUBECONFIG:-$HOME/.kube/config}" | ||
| if [[ "$(yq -o=json .extensions "$cfgfile" )" =~ ^[[] ]]; then |
Copilot
AI
Dec 21, 2025
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 regex pattern ^[[] is incorrect for checking if extensions is an array. The pattern has an unmatched opening bracket and will only match a literal '[' at the start. Use ^\[ instead to properly escape the bracket, or use yq to check the type directly with a command like yq '.extensions | type' \"$cfgfile\".
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.
I am not seeing the alleged results from type. I will stick with string matching.
mspreitz@mjs13 kubeflex % yq '.extensions | type' ~/.kube/config
!!seq
mspreitz@mjs13 kubeflex % yq '.extensions ' ~/.kube/config
[]
mspreitz@mjs13 kubeflex % yq '.extension | type ' ~/.kube/config
!!nullWork around kubestellar#597 Signed-off-by: Mike Spreitzer <[email protected]>
Signed-off-by: Mike Spreitzer <[email protected]>
Signed-off-by: Mike Spreitzer <[email protected]>
1765514 to
b7b5954
Compare
|
LGTM label has been added. DetailsGit tree hash: 28f6b10aeefbde17f05e6b2c5f5eb6757714650a |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: MikeSpreitzer The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Summary
Work around #597
Related issue(s)
Fixes #