Skip to content

OTA-1814: docs(alerts): README updates for #2279 #2296

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.

Sign up for GitHub

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

Open
nbottari9 wants to merge 1 commit into
base: main
Choose a base branch
from
+17 −7
Open

OTA-1814: docs(alerts): README updates for #2279 #2296

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 17 additions & 7 deletions pkg/cli/admin/upgrade/recommend/examples/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,20 +1,30 @@
# Examples for `oc adm upgrade recommend`

Each example consists of inputs and outputs, matched by a common substring:
Each testcase is anchored by an `xxx-cv.yaml`, where `xxx` is a common substring that identifies the testcase. The `-cv.yaml` file describes the ClusterVersion object essential to the functionality of `oc adm upgrade recommend`

@hongkailiu hongkailiu Jul 2, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Each testcase is anchored by an `xxx-cv.yaml`, where `xxx` is a common substring that identifies the testcase. The `-cv.yaml` file describes the ClusterVersion object essential to the functionality of `oc adm upgrade recommend`
Each testcase is anchored by a `<test-case>-cv.yaml` file which describes the ClusterVersion object and is essential to the testing of `oc adm upgrade recommend`.


* `TESTCASE-cv.yaml` (input): ClusterVersion object (created by `oc get clusterversion version -o yaml`). Lists are also supported.
* `TESTCASE.output` (output): expected output of `oc adm upgrade recommend`.
* `TESTCASE.show-outdated-releases-output` (output): expected output of `oc adm upgrade recommend --show-outdated-releases`.
* `TESTCASE.version-<VERSION>-output` (output): expected output of `oc adm upgrade recommend --to <VERSION>`.
**INPUTS**
---

@hongkailiu hongkailiu Jul 2, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: We would not need a divider line here if we use ## INPUTS in Line 5.

* `TESTCASE-cv.yaml`: ClusterVersion object (required, created by `oc get clusterversion version -o yaml`). Lists are also supported.
* `TESTCASE-featuregate.yaml`: FeatureGate object (optional, created by `oc get featuregate cluster -o yaml`). Lists are NOT supported.
* `TESTCASE-infrastructure.yaml`: Infrastructure object (optional, created by `oc get infrastructure cluster -o yaml`). Lists are NOT supported
* `TESTCASE-alerts.json`: Alerts currently present in the cluster (optional, created by `OC_ENABLE_CMD_INSPECT_ALERTS=true oc adm inspect-alerts`)

@hongkailiu hongkailiu Jul 2, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting, Trevor did not use this command directly in 26ec958

Suggested change
* `TESTCASE-alerts.json`: Alerts currently present in the cluster (optional, created by `OC_ENABLE_CMD_INSPECT_ALERTS=true oc adm inspect-alerts`)
* `TESTCASE-alerts.json`: Running alerts in the cluster (optional, expected output of `OC_ENABLE_CMD_INSPECT_ALERTS=true oc adm inspect-alerts`)

The wording is from

$ OC_ENABLE_CMD_INSPECT_ALERTS=true oc adm inspect-alerts -h
Collect information about running alerts.

Usage:
  oc adm inspect-alerts [flags] [options]

Use "oc options" for a list of global command-line options (applies to all commands).


**OUTPUTS**
---
* `TESTCASE.output`: expected output of `oc adm upgrade recommend`.
* `TESTCASE.show-outdated-releases-output`: expected output of `oc adm upgrade recommend --show-outdated-releases`.
* `TESTCASE.version-<VERSION>-output`: expected output of `oc adm upgrade recommend --to <VERSION>`.

@coderabbitai coderabbitai Bot Jul 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Use --version, not --to.

The command contract in recommend.go exposes --version; --to is not a supported flag, so this example will send readers to the wrong CLI surface.

🤖 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 `@pkg/cli/admin/upgrade/recommend/examples/README.md` at line 16, The README
example uses the wrong CLI flag for the upgrade recommend command. Update the
example in the recommend examples README to reference the `--version` flag
exposed by `recommend.go` instead of `--to`, so the documented command matches
the actual `oc adm upgrade recommend` interface and the
`TESTCASE.version-<VERSION>-output` description stays aligned.


The `TestExamples` test in [`examples_test.go`](../examples_test.go) file above validates all examples.
When the testcase is executed with a non-empty `UPDATE` environmental variable, it will update the `TESTCASE.out` fixture:

**TESTING**
---
* When the testcase is executed with a non-empty `UPDATE` environmental variable, it will update the `TESTCASE.out` fixture:
```console
$ UPDATE=yes go test -v ./pkg/cli/admin/upgrade/recommend/...
```
Comment on lines +22 to 25

@coderabbitai coderabbitai Bot Jul 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Fix the fixture name in the update instructions.

examples_test.go derives output fixtures from *-cv.yaml, and the outputs in this README are named *.output / *.show-outdated-releases-output / *.version-<VERSION>-output; TESTCASE.out does not match that contract.

🧰 Tools
🪛 markdownlint-cli2 (0.22.1)

[warning] 24-24: Dollar signs used before commands without showing output

(MD014, commands-show-output)

🤖 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 `@pkg/cli/admin/upgrade/recommend/examples/README.md` around lines 22 - 25, The
update instructions reference the wrong fixture name; the examples in
examples_test.go derive output files from the testcase name, not TESTCASE.out.
Update the README text to match the existing fixture naming contract used by
examples_test.go and the related example outputs (for example, the *.output,
*.show-outdated-releases-output, and *.version-<VERSION>-output patterns) so the
documented UPDATE workflow points to the correct generated files.


You can also pass the inputs to the `oc adm upgrade recommend` directly:
* You can also pass the inputs to the `oc adm upgrade recommend` directly:

```console
$ oc adm upgrade recommend --mock-clusterversion=4.14.1-all-recommended-cv.yaml
Expand Down