TRT-2618: resource watch env var support - #83149
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@neisw: This pull request references TRT-2618 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set. DetailsIn response to this: Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
WalkthroughThe resource-watch observer now controls event collection and resource monitoring through environment variables. The nightly 5.0 and 5.1 GCP OVN real-time test configurations enable the relevant controls. ChangesResource observation controls
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant TestConfig
participant ResourceWatchObserver
participant ResourceWatchCommands
participant run-resourcewatch
participant run-monitor
TestConfig->>ResourceWatchObserver: set resource observation variables
ResourceWatchObserver->>ResourceWatchCommands: provide enabled controls
ResourceWatchCommands->>run-resourcewatch: use --enable-events when enabled
ResourceWatchCommands->>run-monitor: start when monitoring is enabled
🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: neisw 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 |
openshift-ci
Bot
added
the
approved
|
[REHEARSALNOTIFIER]
A total of 2940 jobs have been affected by this change. The above listing is non-exhaustive and limited to 25 jobs. A full list of affected jobs can be found here Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, 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: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@ci-operator/step-registry/observers/resource-watch/observers-resource-watch-commands.sh`:
- Line 84: Quote STORE_PATH in the run-monitor invocation and the cleanup
commands so its value is passed as a single literal argument, preserving
whitespace and preventing glob expansion; update each command use consistently.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 6bb73eb9-c488-4c18-9dad-ae5a79d42502
📒 Files selected for processing (4)
ci-operator/config/openshift/release/openshift-release-main__nightly-5.0.yamlci-operator/config/openshift/release/openshift-release-main__nightly-5.1.yamlci-operator/step-registry/observers/resource-watch/observers-resource-watch-commands.shci-operator/step-registry/observers/resource-watch/observers-resource-watch-observer.yaml
|
|
||
| if [[ "${RESOURCE_MONITOR_ENABLED:-false}" == "true" ]]; then | ||
| DISABLED_MONITOR_TESTS="apiserver-new-disruption-invariant,disruption-summary-serializer,incluster-disruption-serializer,pod-network-avalibility" | ||
| openshift-tests run-monitor ${MONITOR_ARGS:-} --artifact-dir $STORE_PATH --disable-monitor=${DISABLED_MONITOR_TESTS} > "${ARTIFACT_DIR}/run-monitor.log" 2>&1 & |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
file="$(fd -i -t f 'observers-resource-watch-commands\.sh$' . | head -n 1)"
printf '%s\n' "FILE=$file"
wc -l "$file"
cat -n "$file"
printf '\n--- lifecycle references ---\n'
rg -n 'run-monitor|STORE_PATH|wait|kill|ARTIFACT_DIR|DISABLED_MONITOR_TESTS|MONITOR_ARGS' "$file"Repository: openshift/release
Length of output: 5081
🏁 Script executed:
#!/bin/bash
set -euo pipefail
probe() {
local label="$1"
local store_path="$2"
STORE_PATH="$store_path"
MONITOR_ARGS=""
DISABLED_MONITOR_TESTS="disabled"
openshift-tests() {
printf '%s argc=%s\n' "$label" "$#"
printf ' <%s>\n' "$@"
}
export -f openshift-tests
bash -c '
openshift-tests run-monitor ${MONITOR_ARGS:-} --artifact-dir $STORE_PATH --disable-monitor=${DISABLED_MONITOR_TESTS} > /dev/null
' 2>/dev/null || true
}
probe "whitespace-unquoted" "/tmp/path with spaces"
probe "glob-unquoted" "/tmp/*"
printf '\nquoted controls\n'
probe_quoted() {
local label="$1"
local store_path="$2"
STORE_PATH="$store_path"
MONITOR_ARGS=""
DISABLED_MONITOR_TESTS="disabled"
openshift-tests() {
printf '%s argc=%s\n' "$label" "$#"
printf ' <%s>\n' "$@"
}
openshift-tests run-monitor ${MONITOR_ARGS:-} --artifact-dir "$STORE_PATH" --disable-monitor="${DISABLED_MONITOR_TESTS}" > /dev/null
}
probe_quoted "whitespace-quoted" "/tmp/path with spaces"
probe_quoted "glob-quoted" "/tmp/*"
printf '\nSTORE_PATH occurrences\n'
rg -n '\$STORE_PATH|STORE_PATH' ci-operator/step-registry/observers/resource-watch/observers-resource-watch-commands.shRepository: openshift/release
Length of output: 825
🏁 Script executed:
#!/bin/bash
set -euo pipefail
openshift-tests() {
printf '%s argc=%s\n' "$1" "$#" >&2
shift
printf ' <%s>\n' "$@" >&2
}
run_unquoted() {
local label="$1"
STORE_PATH="$2"
MONITOR_ARGS=""
DISABLED_MONITOR_TESTS="disabled"
printf '%s STORE_PATH=%q\n' "$label" "$STORE_PATH"
openshift-tests run-monitor ${MONITOR_ARGS:-} --artifact-dir $STORE_PATH --disable-monitor=${DISABLED_MONITOR_TESTS} > /dev/null
}
run_quoted() {
local label="$1"
STORE_PATH="$2"
MONITOR_ARGS=""
DISABLED_MONITOR_TESTS="disabled"
printf '%s STORE_PATH=%q\n' "$label" "$STORE_PATH"
openshift-tests run-monitor ${MONITOR_ARGS:-} --artifact-dir "$STORE_PATH" --disable-monitor="${DISABLED_MONITOR_TESTS}" > /dev/null
}
run_unquoted "whitespace-unquoted" "/tmp/path with spaces"
run_unquoted "glob-unquoted" "/tmp/*"
run_quoted "whitespace-quoted" "/tmp/path with spaces"
run_quoted "glob-quoted" "/tmp/*"
printf '\nSTORE_PATH occurrences:\n'
rg -n '\$STORE_PATH|STORE_PATH' ci-operator/step-registry/observers/resource-watch/observers-resource-watch-commands.shRepository: openshift/release
Length of output: 1904
Quote STORE_PATH in all command uses.
If STORE_PATH contains whitespace or glob characters, shell expansion changes the arguments. Quote it in run-monitor and in the cleanup commands at lines 31–32.
🧰 Tools
🪛 Shellcheck (0.11.0)
[info] 84-84: Double quote to prevent globbing and word splitting.
(SC2086)
[info] 84-84: Double quote to prevent globbing and word splitting.
(SC2086)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@ci-operator/step-registry/observers/resource-watch/observers-resource-watch-commands.sh`
at line 84, Quote STORE_PATH in the run-monitor invocation and the cleanup
commands so its value is passed as a single literal argument, preserving
whitespace and preventing glob expansion; update each command use consistently.
Source: Linters/SAST tools
|
/pj-rehearse periodic-ci-openshift-release-main-nightly-4.22-e2e-gcp-ovn-rt /pj-rehearse periodic-ci-openshift-release-main-nightly-5.0-e2e-gcp-ovn-rt /pj-rehearse periodic-ci-openshift-release-main-nightly-5.1-e2e-gcp-ovn-rt periodic-ci-openshift-release-main-nightly-5.0-e2e-aws-ovn-upgrade-fips |
|
@neisw: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
@neisw: job(s): /pj-rehearse, /pj-rehearse either don't exist or were not found to be affected, and cannot be rehearsed |
|
/pj-rehearse periodic-ci-openshift-release-main-nightly-4.22-e2e-gcp-ovn-rt |
|
@neisw: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
@neisw: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
2 participants
Summary by CodeRabbit
e2e-gcp-ovn-rtjobs to enable resource watching and resource event collection.--enable-eventsonly when event collection is enabled and starts resource monitoring only whenRESOURCE_MONITOR_ENABLEDis enabled.