chore: add helper workflow for operator upgrade tests#19415
Open
chore: add helper workflow for operator upgrade tests#19415
Conversation
|
Skipping CI for Draft Pull Request. |
Contributor
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- There is a leftover
import pdbandpdb.set_trace()instart_version()which will pause execution and break the GitHub workflow run; this should be removed before merge. - The usage/help text in
main()still refers torhacs_versions.py, which does not match the actual script filename (operator-upgrade-test-helper.py); update the message so users see the correct command. - In the workflow, the
workflow_dispatchinputversionis never used and the job always runs over a hard-coded matrix of versions; consider either wiring the input into the run or separating the fixed-matrix validation from the manual helper workflow so the behavior is clearer.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- There is a leftover `import pdb` and `pdb.set_trace()` in `start_version()` which will pause execution and break the GitHub workflow run; this should be removed before merge.
- The usage/help text in `main()` still refers to `rhacs_versions.py`, which does not match the actual script filename (`operator-upgrade-test-helper.py`); update the message so users see the correct command.
- In the workflow, the `workflow_dispatch` input `version` is never used and the job always runs over a hard-coded matrix of versions; consider either wiring the input into the run or separating the fixed-matrix validation from the manual helper workflow so the behavior is clearer.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
6b6e89a to
6d74b2f
Compare
Contributor
|
Images are ready for the commit at 4d878c4. To use with deploy scripts, first |
The operator upgrade tests require two separate OCP clusters that need to be compatible with the version of StackRox being tested and either the oldest version supported or two minor versions prior. Two separate pages need to be checked in order to fully get the information on what the start version should be and what OCP version to use. Instead of doing this by hand, this patch adds a python script and a GH workflow to simplify the process. The script will use Selenium to retrieve each page, find the relevant information from each with a combination of BeautifulSoup and regex, finally calculate the needed versions and output them to stdout. Assisted-by: Claude Sonnet 4.5 <noreply@anthropic.com>
6d74b2f to
4d878c4
Compare
Contributor
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- In
find_compatible_openshift, the branch wheretarget_ocp_versionis empty assumesstart_ocp_versionis non-empty (max(start_ocp_version)), which will raise if the matrix lacks entries for the start version; consider short-circuiting with a clearer error or fallback when either side has no compatibility data. - In
start_version,max([v for v in all_versions if v.major == version.major - 1])will fail if there are no entries for the previous major (e.g., early in a new major); you may want to explicitly detect this case and either fallback to the oldest supported version or emit a clear error instead of relying onValueErrorfrommax(). - The HTML/shadow-DOM parsing in
parse_supported_versionsandparse_all_versionsdepends on brittle regexes and specific strings likeproduct-lifecycle-Full Support; adding basic validation (e.g., ensuring expected tables/sections are present and failing with a clear message if not) will make the script more robust to minor page layout/content changes.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `find_compatible_openshift`, the branch where `target_ocp_version` is empty assumes `start_ocp_version` is non-empty (`max(start_ocp_version)`), which will raise if the matrix lacks entries for the start version; consider short-circuiting with a clearer error or fallback when either side has no compatibility data.
- In `start_version`, `max([v for v in all_versions if v.major == version.major - 1])` will fail if there are no entries for the previous major (e.g., early in a new major); you may want to explicitly detect this case and either fallback to the oldest supported version or emit a clear error instead of relying on `ValueError` from `max()`.
- The HTML/shadow-DOM parsing in `parse_supported_versions` and `parse_all_versions` depends on brittle regexes and specific strings like `product-lifecycle-Full Support`; adding basic validation (e.g., ensuring expected tables/sections are present and failing with a clear message if not) will make the script more robust to minor page layout/content changes.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #19415 +/- ##
=======================================
Coverage 49.70% 49.71%
=======================================
Files 2701 2701
Lines 203453 203453
=======================================
+ Hits 101134 101147 +13
+ Misses 94790 94781 -9
+ Partials 7529 7525 -4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Description
The operator upgrade tests require two separate OCP clusters that need to be compatible with the version of StackRox being tested and either the oldest version supported or two minor versions prior. Two separate pages need to be checked in order to fully get the information on what the start version should be and what OCP version to use.
Instead of doing this by hand, this patch adds a python script and a GH workflow to simplify the process. The script will use Selenium to retrieve each page, find the relevant information from each with a combination of BeautifulSoup and regex, finally calculate the needed versions and output them to stdout.
Assisted-by: Claude Sonnet 4.5 noreply@anthropic.com
User-facing documentation
Testing and quality
Automated testing
How I validated my change
Manually tried different versions of ACS and checked the result was correct.
Ran the GH workflow with a testing matrix and validated the versions returned: https://github.com/stackrox/stackrox/actions/runs/23054970395