MGMT-22939: Fix cache key to get mustgather image#9940
Open
pastequo wants to merge 1 commit intoopenshift:masterfrom
Open
MGMT-22939: Fix cache key to get mustgather image#9940pastequo wants to merge 1 commit intoopenshift:masterfrom
pastequo wants to merge 1 commit intoopenshift:masterfrom
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@pastequo: This pull request references MGMT-22939 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 bug to target the "4.22.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. |
WalkthroughSwitches must-gather cache keys to use the full OpenShift version string, updates related tests, adds CPUArchitecture to MustGatherImage, changes several webhook validation signatures to accept context and old/new objects, simplifies webhook suite tests, adds a Makefile test target, and adjusts api module dependencies. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Comment |
openshift-ci
bot
added
the
size/XS
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: pastequo 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
|
@pastequo: This pull request references MGMT-22939 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 bug to target the "4.22.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. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/versions/common.go (1)
94-107:⚠️ Potential issue | 🟠 MajorFix cache key format mismatch between static config and runtime lookup.
The cache key change at line 94 from major.minor to full version introduces a silent data loss bug. Static configurations populate
mustGatherVersionsusing major.minor keys viagetVersionKey()(seeagentserviceconfig_controller.go:2428, which returnsfmt.Sprintf("%d.%d", v.Segments()[0], v.Segments()[1])), but the runtime lookup now uses the full version (e.g.,"4.20-x86_64"vs"4.20.10-x86_64"). When the full-version key is not found, lines 99–101 silently create an emptyMustGatherVersioninstead of checking the pre-populated major.minor key, causing operator must-gather images (cnv, odf, lso) to vanish from responses without any error.Either update
getVersionKey()to return the full version, or add fallback logic ingetMustGatherImagesto check the major.minor key if the full-version key misses.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/versions/common.go` around lines 94 - 107, The cacheKey used in getMustGatherImages now includes the full version (cacheKey := fmt.Sprintf("%s-%s", openshiftVersion, cpuArchitecture)) but static config is populated with major.minor keys from getVersionKey(), causing silent misses and overwriting pre-populated entries; update getMustGatherImages to, after computing the full-version cacheKey and before creating an empty MustGatherVersion, attempt a fallback lookup using the major.minor key returned by getVersionKey() (compose fallbackKey := fmt.Sprintf("%s-%s", getVersionKey(openshiftVersion), cpuArchitecture)) and use that entry if present, otherwise create the new MustGatherVersion in mustGatherVersions; reference mustGatherVersions, cacheKey, getVersionKey(), getMustGatherImages, MustGatherVersions, and MustGatherVersion when making the change.
🧹 Nitpick comments (1)
internal/versions/common_test.go (1)
289-318: Consider adding a test case that directly validates the originally reported bug: two distinct patch versions must not collide in the cache.The updated
ocpVersion = "4.8.0-fc.1"exercises the full-version key format, but there is no test that callsgetMustGatherImageswith two different patch versions of the same minor (e.g.,"4.8.5"then"4.8.10") and asserts each gets its ownocpmust-gather image. Such a test would directly pin the regression described in the PR.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/versions/common_test.go` around lines 289 - 318, Add a new test variant in the "caching" It block that calls getMustGatherImages twice with the same minor but two different patch versions (for example set ocpVersion to "4.8.5" for the first call and "4.8.10" for the second), mock the release client's GetMustGatherImage expectation for each distinct full version (mockRelease.EXPECT().GetMustGatherImage(..., "release_4.8.5", ...) and then for "release_4.8.10"), and assert both calls return without error and that each returned images.ocp must-gather entry is the expected, distinct value (using verifyOcpVersion or direct assertions) to ensure cache keys in getMustGatherImages do not collide across different patch versions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@internal/versions/common.go`:
- Around line 94-107: The cacheKey used in getMustGatherImages now includes the
full version (cacheKey := fmt.Sprintf("%s-%s", openshiftVersion,
cpuArchitecture)) but static config is populated with major.minor keys from
getVersionKey(), causing silent misses and overwriting pre-populated entries;
update getMustGatherImages to, after computing the full-version cacheKey and
before creating an empty MustGatherVersion, attempt a fallback lookup using the
major.minor key returned by getVersionKey() (compose fallbackKey :=
fmt.Sprintf("%s-%s", getVersionKey(openshiftVersion), cpuArchitecture)) and use
that entry if present, otherwise create the new MustGatherVersion in
mustGatherVersions; reference mustGatherVersions, cacheKey, getVersionKey(),
getMustGatherImages, MustGatherVersions, and MustGatherVersion when making the
change.
---
Nitpick comments:
In `@internal/versions/common_test.go`:
- Around line 289-318: Add a new test variant in the "caching" It block that
calls getMustGatherImages twice with the same minor but two different patch
versions (for example set ocpVersion to "4.8.5" for the first call and "4.8.10"
for the second), mock the release client's GetMustGatherImage expectation for
each distinct full version (mockRelease.EXPECT().GetMustGatherImage(...,
"release_4.8.5", ...) and then for "release_4.8.10"), and assert both calls
return without error and that each returned images.ocp must-gather entry is the
expected, distinct value (using verifyOcpVersion or direct assertions) to ensure
cache keys in getMustGatherImages do not collide across different patch
versions.
ℹ️ Review info
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (2)
internal/versions/common.gointernal/versions/common_test.go
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #9940 +/- ##
==========================================
+ Coverage 44.06% 44.10% +0.04%
==========================================
Files 414 415 +1
Lines 72210 72294 +84
==========================================
+ Hits 31819 31888 +69
- Misses 37513 37521 +8
- Partials 2878 2885 +7
🚀 New features to boost your workflow:
|
Contributor
Author
|
̶I̶ ̶d̶o̶n̶'̶t̶ ̶t̶h̶i̶n̶k̶ ̶t̶h̶e̶ ̶c̶o̶d̶e̶r̶a̶b̶b̶i̶t̶ ̶c̶o̶m̶m̶e̶n̶t̶ ̶i̶s̶ ̶r̶e̶l̶e̶v̶a̶n̶t̶ But I will have a problem with that https://gitlab.cee.redhat.com/service/app-interface/-/blob/master/data/services/assisted-installer/cicd/saas.yaml?ref_type=heads#L74 |
openshift-ci
bot
added
the
do-not-merge/work-in-progress
pastequo
force-pushed
the
fix/change-cache-key-for-must-gather-img
branch
from
753ea6a to
8ccd34d
Compare
openshift-ci
bot
added
size/XXL
openshift-ci
bot
added
api-review
pastequo
commented
Feb 27, 2026
| // is to be associated with. | ||
| OpenshiftVersion string `json:"openshiftVersion"` | ||
| // CPUArchitecture is the CPU architecture of the image (x86_64/arm64/multi/etc). | ||
| // +optional |
Contributor
Author
There was a problem hiding this comment.
I didn't make it required to be backward compatible, even if I'm not convinced it makes a lot of sense
pastequo
commented
Feb 27, 2026
| run-unit-test: | ||
| SKIP_UT_DB=1 $(MAKE) _unit_test TIMEOUT=30m TEST="$(or $(TEST),$(shell go list ./... | grep -v subsystem))" | ||
|
|
||
| run-unit-test-api: |
Contributor
Author
There was a problem hiding this comment.
adding this, since run-unit-test is never replaying the tests in api/
|
@pastequo: This pull request references MGMT-22939 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 bug to target the "4.22.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. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Makefile (1)
560-562: Consider aligning test output format with other targets.The new target uses plain
go testwhilerun-unit-testusesgotestsumwith coverage and reporting flags. This works but produces different output formats. If CI relies on consistent coverage/junit artifacts, consider using similar flags:♻️ Optional: Add coverage and reporting
run-unit-test-api: - cd api/ && go test ./... + cd api/ && gotestsum --format=$(GO_TEST_FORMAT) -- -count=1 -cover ./...🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Makefile` around lines 560 - 562, The run-unit-test-api Makefile target currently runs plain "go test" and should be changed to match the format and artifacts produced by run-unit-test: replace the plain test invocation with gotestsum and the same flags used by run-unit-test (e.g., --junitfile, --raw-command or -- -coverprofile and -covermode as appropriate) so CI gets consistent coverage and junit outputs; update the run-unit-test-api target to call gotestsum for the api package and generate the same coverage and reporting files as run-unit-test.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@Makefile`:
- Around line 560-562: The run-unit-test-api Makefile target currently runs
plain "go test" and should be changed to match the format and artifacts produced
by run-unit-test: replace the plain test invocation with gotestsum and the same
flags used by run-unit-test (e.g., --junitfile, --raw-command or --
-coverprofile and -covermode as appropriate) so CI gets consistent coverage and
junit outputs; update the run-unit-test-api target to call gotestsum for the api
package and generate the same coverage and reporting files as run-unit-test.
ℹ️ Review info
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
⛔ Files ignored due to path filters (162)
api/vendor/github.com/go-logr/zapr/.gitignoreis excluded by!**/vendor/**api/vendor/github.com/go-logr/zapr/.golangci.yamlis excluded by!**/vendor/**api/vendor/github.com/go-logr/zapr/LICENSEis excluded by!**/vendor/**api/vendor/github.com/go-logr/zapr/README.mdis excluded by!**/vendor/**api/vendor/github.com/go-logr/zapr/slogzapr.gois excluded by!**/vendor/**api/vendor/github.com/go-logr/zapr/zapr.gois excluded by!**/vendor/**api/vendor/github.com/go-logr/zapr/zapr_noslog.gois excluded by!**/vendor/**api/vendor/github.com/go-logr/zapr/zapr_slog.gois excluded by!**/vendor/**api/vendor/go.uber.org/multierr/.codecov.ymlis excluded by!**/vendor/**api/vendor/go.uber.org/multierr/.gitignoreis excluded by!**/vendor/**api/vendor/go.uber.org/multierr/CHANGELOG.mdis excluded by!**/vendor/**api/vendor/go.uber.org/multierr/LICENSE.txtis excluded by!**/vendor/**api/vendor/go.uber.org/multierr/Makefileis excluded by!**/vendor/**api/vendor/go.uber.org/multierr/README.mdis excluded by!**/vendor/**api/vendor/go.uber.org/multierr/error.gois excluded by!**/vendor/**api/vendor/go.uber.org/multierr/error_post_go120.gois excluded by!**/vendor/**api/vendor/go.uber.org/multierr/error_pre_go120.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/.codecov.ymlis excluded by!**/vendor/**api/vendor/go.uber.org/zap/.gitignoreis excluded by!**/vendor/**api/vendor/go.uber.org/zap/.golangci.ymlis excluded by!**/vendor/**api/vendor/go.uber.org/zap/.readme.tmplis excluded by!**/vendor/**api/vendor/go.uber.org/zap/CHANGELOG.mdis excluded by!**/vendor/**api/vendor/go.uber.org/zap/CODE_OF_CONDUCT.mdis excluded by!**/vendor/**api/vendor/go.uber.org/zap/CONTRIBUTING.mdis excluded by!**/vendor/**api/vendor/go.uber.org/zap/FAQ.mdis excluded by!**/vendor/**api/vendor/go.uber.org/zap/LICENSE.txtis excluded by!**/vendor/**api/vendor/go.uber.org/zap/Makefileis excluded by!**/vendor/**api/vendor/go.uber.org/zap/README.mdis excluded by!**/vendor/**api/vendor/go.uber.org/zap/array.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/buffer/buffer.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/buffer/pool.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/checklicense.shis excluded by!**/vendor/**api/vendor/go.uber.org/zap/config.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/doc.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/encoder.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/error.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/field.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/flag.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/glide.yamlis excluded by!**/vendor/**api/vendor/go.uber.org/zap/global.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/http_handler.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/internal/bufferpool/bufferpool.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/internal/color/color.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/internal/exit/exit.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/internal/level_enabler.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/internal/pool/pool.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/internal/stacktrace/stack.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/level.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/logger.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/options.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/sink.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/sugar.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/time.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/writer.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/zapcore/buffered_write_syncer.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/zapcore/clock.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/zapcore/console_encoder.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/zapcore/core.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/zapcore/doc.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/zapcore/encoder.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/zapcore/entry.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/zapcore/error.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/zapcore/field.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/zapcore/hook.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/zapcore/increase_level.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/zapcore/json_encoder.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/zapcore/lazy_with.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/zapcore/level.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/zapcore/level_strings.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/zapcore/marshaler.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/zapcore/memory_encoder.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/zapcore/reflected_encoder.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/zapcore/sampler.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/zapcore/tee.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/zapcore/write_syncer.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/.import-restrictionsis excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/conversion.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/deepcopy.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/defaults.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/doc.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/generated.protois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/marshal.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/register.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/types.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/types_jsonschema.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/zz_generated.conversion.gois excluded by!**/vendor/**,!**/zz_generated*api/vendor/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/zz_generated.deepcopy.gois excluded by!**/vendor/**,!**/zz_generated*api/vendor/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/zz_generated.defaults.gois excluded by!**/vendor/**,!**/zz_generated*api/vendor/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/zz_generated.prerelease-lifecycle.gois excluded by!**/vendor/**,!**/zz_generated*api/vendor/k8s.io/apiextensions-apiserver/pkg/client/applyconfiguration/apiextensions/v1/customresourcecolumndefinition.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/applyconfiguration/apiextensions/v1/customresourceconversion.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/applyconfiguration/apiextensions/v1/customresourcedefinition.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/applyconfiguration/apiextensions/v1/customresourcedefinitioncondition.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/applyconfiguration/apiextensions/v1/customresourcedefinitionnames.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/applyconfiguration/apiextensions/v1/customresourcedefinitionspec.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/applyconfiguration/apiextensions/v1/customresourcedefinitionstatus.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/applyconfiguration/apiextensions/v1/customresourcedefinitionversion.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/applyconfiguration/apiextensions/v1/customresourcesubresources.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/applyconfiguration/apiextensions/v1/customresourcesubresourcescale.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/applyconfiguration/apiextensions/v1/customresourcevalidation.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/applyconfiguration/apiextensions/v1/externaldocumentation.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/applyconfiguration/apiextensions/v1/jsonschemaprops.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/applyconfiguration/apiextensions/v1/servicereference.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/applyconfiguration/apiextensions/v1/validationrule.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/applyconfiguration/apiextensions/v1/webhookclientconfig.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/applyconfiguration/apiextensions/v1/webhookconversion.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/applyconfiguration/apiextensions/v1beta1/customresourcecolumndefinition.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/applyconfiguration/apiextensions/v1beta1/customresourceconversion.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/applyconfiguration/apiextensions/v1beta1/customresourcedefinition.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/applyconfiguration/apiextensions/v1beta1/customresourcedefinitioncondition.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/applyconfiguration/apiextensions/v1beta1/customresourcedefinitionnames.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/applyconfiguration/apiextensions/v1beta1/customresourcedefinitionspec.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/applyconfiguration/apiextensions/v1beta1/customresourcedefinitionstatus.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/applyconfiguration/apiextensions/v1beta1/customresourcedefinitionversion.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/applyconfiguration/apiextensions/v1beta1/customresourcesubresources.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/applyconfiguration/apiextensions/v1beta1/customresourcesubresourcescale.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/applyconfiguration/apiextensions/v1beta1/customresourcevalidation.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/applyconfiguration/apiextensions/v1beta1/externaldocumentation.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/applyconfiguration/apiextensions/v1beta1/jsonschemaprops.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/applyconfiguration/apiextensions/v1beta1/servicereference.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/applyconfiguration/apiextensions/v1beta1/validationrule.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/applyconfiguration/apiextensions/v1beta1/webhookclientconfig.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/clientset.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/scheme/doc.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/scheme/register.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/typed/apiextensions/v1/apiextensions_client.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/typed/apiextensions/v1/customresourcedefinition.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/typed/apiextensions/v1/doc.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/typed/apiextensions/v1/generated_expansion.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/typed/apiextensions/v1beta1/apiextensions_client.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/typed/apiextensions/v1beta1/customresourcedefinition.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/typed/apiextensions/v1beta1/doc.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/typed/apiextensions/v1beta1/generated_expansion.gois excluded by!**/vendor/**api/vendor/k8s.io/client-go/util/retry/OWNERSis excluded by!**/vendor/**api/vendor/k8s.io/client-go/util/retry/util.gois excluded by!**/vendor/**api/vendor/modules.txtis excluded by!**/vendor/**api/vendor/sigs.k8s.io/controller-runtime/pkg/envtest/crd.gois excluded by!**/vendor/**api/vendor/sigs.k8s.io/controller-runtime/pkg/envtest/doc.gois excluded by!**/vendor/**api/vendor/sigs.k8s.io/controller-runtime/pkg/envtest/helper.gois excluded by!**/vendor/**api/vendor/sigs.k8s.io/controller-runtime/pkg/envtest/server.gois excluded by!**/vendor/**api/vendor/sigs.k8s.io/controller-runtime/pkg/envtest/webhook.gois excluded by!**/vendor/**api/vendor/sigs.k8s.io/controller-runtime/pkg/internal/flock/doc.gois excluded by!**/vendor/**api/vendor/sigs.k8s.io/controller-runtime/pkg/internal/flock/errors.gois excluded by!**/vendor/**api/vendor/sigs.k8s.io/controller-runtime/pkg/internal/flock/flock_other.gois excluded by!**/vendor/**api/vendor/sigs.k8s.io/controller-runtime/pkg/internal/flock/flock_unix.gois excluded by!**/vendor/**api/vendor/sigs.k8s.io/controller-runtime/pkg/internal/testing/addr/manager.gois excluded by!**/vendor/**api/vendor/sigs.k8s.io/controller-runtime/pkg/internal/testing/certs/tinyca.gois excluded by!**/vendor/**api/vendor/sigs.k8s.io/controller-runtime/pkg/internal/testing/controlplane/apiserver.gois excluded by!**/vendor/**api/vendor/sigs.k8s.io/controller-runtime/pkg/internal/testing/controlplane/auth.gois excluded by!**/vendor/**api/vendor/sigs.k8s.io/controller-runtime/pkg/internal/testing/controlplane/etcd.gois excluded by!**/vendor/**api/vendor/sigs.k8s.io/controller-runtime/pkg/internal/testing/controlplane/kubectl.gois excluded by!**/vendor/**api/vendor/sigs.k8s.io/controller-runtime/pkg/internal/testing/controlplane/plane.gois excluded by!**/vendor/**api/vendor/sigs.k8s.io/controller-runtime/pkg/internal/testing/process/arguments.gois excluded by!**/vendor/**api/vendor/sigs.k8s.io/controller-runtime/pkg/internal/testing/process/bin_path_finder.gois excluded by!**/vendor/**api/vendor/sigs.k8s.io/controller-runtime/pkg/internal/testing/process/procattr_other.gois excluded by!**/vendor/**api/vendor/sigs.k8s.io/controller-runtime/pkg/internal/testing/process/procattr_unix.gois excluded by!**/vendor/**api/vendor/sigs.k8s.io/controller-runtime/pkg/internal/testing/process/process.gois excluded by!**/vendor/**api/vendor/sigs.k8s.io/controller-runtime/pkg/log/zap/flags.gois excluded by!**/vendor/**api/vendor/sigs.k8s.io/controller-runtime/pkg/log/zap/kube_helpers.gois excluded by!**/vendor/**api/vendor/sigs.k8s.io/controller-runtime/pkg/log/zap/zap.gois excluded by!**/vendor/**vendor/github.com/openshift/assisted-service/api/v1beta1/agentserviceconfig_types.gois excluded by!**/vendor/**,!vendor/**
📒 Files selected for processing (23)
Makefileapi/go.modapi/v1beta1/agent_webhook_test.goapi/v1beta1/agentclassification_webhook_test.goapi/v1beta1/agentserviceconfig_types.goapi/v1beta1/infraenv_webhook_test.goapi/v1beta1/webhook_suite_test.gocmd/main.goconfig/crd/bases/agent-install.openshift.io_agentserviceconfigs.yamlconfig/crd/bases/agent-install.openshift.io_hypershiftagentserviceconfigs.yamlconfig/crd/resources.yamldeploy/olm-catalog/manifests/agent-install.openshift.io_agentserviceconfigs.yamldeploy/olm-catalog/manifests/agent-install.openshift.io_hypershiftagentserviceconfigs.yamlinternal/controller/controllers/agentserviceconfig_controller.gointernal/controller/controllers/agentserviceconfig_controller_test.gointernal/versions/api_test.gointernal/versions/cache.gointernal/versions/cache_test.gointernal/versions/common.gointernal/versions/common_test.gointernal/versions/kube_api_versions.gointernal/versions/rest_api_versions.gointernal/versions/rest_api_versions_test.go
💤 Files with no reviewable changes (1)
- api/v1beta1/webhook_suite_test.go
pastequo
commented
Feb 27, 2026
| } | ||
| } | ||
|
|
||
| func (c MustGatherVersionCache) GetMustGatherVersion(openshiftVersion, cpuArchitecture string) (MustGatherVersion, error) { |
Contributor
Author
There was a problem hiding this comment.
We talked about it with @CrystalChun and she made the very valid remark that with the current config that we have on saas, the error from the ticket can still happen for the version in the config. It's a bigger problem on saas where the user can't edit this config.
So maybe we should remove the fallback logic (and the configuration on saas), but then we will lose the capability to define 1 mustgather image for all patches of a minor.
pastequo
commented
Feb 27, 2026
| } | ||
|
|
||
| // Return the merged result from cache (includes pre-populated entries like cnv, odf, lso) | ||
| ret, err = mustGatherVersionCache.GetMustGatherVersion(openshiftVersion, cpuArchitecture) |
Contributor
Author
There was a problem hiding this comment.
I changed a bit the behavior in this case, we used to return only the ocp version. But practically, on saas at least, we are not putting config except for 4.8, 4.9 and 4.10
pastequo
force-pushed
the
fix/change-cache-key-for-must-gather-img
branch
from
8ccd34d to
52e58b8
Compare
|
@pastequo: This pull request references MGMT-22939 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 bug to target the "4.22.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. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Makefile (1)
560-562: Consider wiring into CI and aligning with existing test infrastructure.The new target works, but it differs from
run-unit-testin several ways:
- Uses raw
go testinstead ofgotestsum(no standardized output format).- No coverage reporting (
-coverprofile).- No JUnit XML generation for CI visibility.
- Not called by
ci-unit-test, so API tests may still be skipped in CI pipelines.If API tests should run in CI, consider adding it to
ci-unit-test:ci-unit-test: ./hack/start_db.sh $(MAKE) run-unit-test + $(MAKE) run-unit-test-apiAlso, adding
.PHONYis conventional for targets without file outputs:+.PHONY: run-unit-test-api run-unit-test-api: cd api/ && go test ./...🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Makefile` around lines 560 - 562, The new Makefile target run-unit-test-api should be aligned with existing test targets: change it to invoke gotestsum (as run-unit-test does) so output is standardized, add coverage generation via -coverprofile and produce JUnit XML for CI, mark run-unit-test-api as .PHONY, and include/run it from the ci-unit-test target so API tests run in pipelines; update references to run-unit-test (for behavior examples) and use gotestsum flags consistent with other targets to match reporting and coverage conventions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@Makefile`:
- Around line 560-562: The new Makefile target run-unit-test-api should be
aligned with existing test targets: change it to invoke gotestsum (as
run-unit-test does) so output is standardized, add coverage generation via
-coverprofile and produce JUnit XML for CI, mark run-unit-test-api as .PHONY,
and include/run it from the ci-unit-test target so API tests run in pipelines;
update references to run-unit-test (for behavior examples) and use gotestsum
flags consistent with other targets to match reporting and coverage conventions.
ℹ️ Review info
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
⛔ Files ignored due to path filters (162)
api/vendor/github.com/go-logr/zapr/.gitignoreis excluded by!**/vendor/**api/vendor/github.com/go-logr/zapr/.golangci.yamlis excluded by!**/vendor/**api/vendor/github.com/go-logr/zapr/LICENSEis excluded by!**/vendor/**api/vendor/github.com/go-logr/zapr/README.mdis excluded by!**/vendor/**api/vendor/github.com/go-logr/zapr/slogzapr.gois excluded by!**/vendor/**api/vendor/github.com/go-logr/zapr/zapr.gois excluded by!**/vendor/**api/vendor/github.com/go-logr/zapr/zapr_noslog.gois excluded by!**/vendor/**api/vendor/github.com/go-logr/zapr/zapr_slog.gois excluded by!**/vendor/**api/vendor/go.uber.org/multierr/.codecov.ymlis excluded by!**/vendor/**api/vendor/go.uber.org/multierr/.gitignoreis excluded by!**/vendor/**api/vendor/go.uber.org/multierr/CHANGELOG.mdis excluded by!**/vendor/**api/vendor/go.uber.org/multierr/LICENSE.txtis excluded by!**/vendor/**api/vendor/go.uber.org/multierr/Makefileis excluded by!**/vendor/**api/vendor/go.uber.org/multierr/README.mdis excluded by!**/vendor/**api/vendor/go.uber.org/multierr/error.gois excluded by!**/vendor/**api/vendor/go.uber.org/multierr/error_post_go120.gois excluded by!**/vendor/**api/vendor/go.uber.org/multierr/error_pre_go120.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/.codecov.ymlis excluded by!**/vendor/**api/vendor/go.uber.org/zap/.gitignoreis excluded by!**/vendor/**api/vendor/go.uber.org/zap/.golangci.ymlis excluded by!**/vendor/**api/vendor/go.uber.org/zap/.readme.tmplis excluded by!**/vendor/**api/vendor/go.uber.org/zap/CHANGELOG.mdis excluded by!**/vendor/**api/vendor/go.uber.org/zap/CODE_OF_CONDUCT.mdis excluded by!**/vendor/**api/vendor/go.uber.org/zap/CONTRIBUTING.mdis excluded by!**/vendor/**api/vendor/go.uber.org/zap/FAQ.mdis excluded by!**/vendor/**api/vendor/go.uber.org/zap/LICENSE.txtis excluded by!**/vendor/**api/vendor/go.uber.org/zap/Makefileis excluded by!**/vendor/**api/vendor/go.uber.org/zap/README.mdis excluded by!**/vendor/**api/vendor/go.uber.org/zap/array.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/buffer/buffer.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/buffer/pool.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/checklicense.shis excluded by!**/vendor/**api/vendor/go.uber.org/zap/config.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/doc.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/encoder.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/error.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/field.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/flag.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/glide.yamlis excluded by!**/vendor/**api/vendor/go.uber.org/zap/global.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/http_handler.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/internal/bufferpool/bufferpool.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/internal/color/color.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/internal/exit/exit.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/internal/level_enabler.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/internal/pool/pool.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/internal/stacktrace/stack.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/level.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/logger.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/options.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/sink.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/sugar.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/time.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/writer.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/zapcore/buffered_write_syncer.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/zapcore/clock.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/zapcore/console_encoder.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/zapcore/core.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/zapcore/doc.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/zapcore/encoder.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/zapcore/entry.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/zapcore/error.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/zapcore/field.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/zapcore/hook.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/zapcore/increase_level.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/zapcore/json_encoder.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/zapcore/lazy_with.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/zapcore/level.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/zapcore/level_strings.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/zapcore/marshaler.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/zapcore/memory_encoder.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/zapcore/reflected_encoder.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/zapcore/sampler.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/zapcore/tee.gois excluded by!**/vendor/**api/vendor/go.uber.org/zap/zapcore/write_syncer.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/.import-restrictionsis excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/conversion.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/deepcopy.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/defaults.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/doc.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/generated.pb.gois excluded by!**/*.pb.go,!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/generated.protois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/marshal.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/register.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/types.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/types_jsonschema.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/zz_generated.conversion.gois excluded by!**/vendor/**,!**/zz_generated*api/vendor/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/zz_generated.deepcopy.gois excluded by!**/vendor/**,!**/zz_generated*api/vendor/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/zz_generated.defaults.gois excluded by!**/vendor/**,!**/zz_generated*api/vendor/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/zz_generated.prerelease-lifecycle.gois excluded by!**/vendor/**,!**/zz_generated*api/vendor/k8s.io/apiextensions-apiserver/pkg/client/applyconfiguration/apiextensions/v1/customresourcecolumndefinition.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/applyconfiguration/apiextensions/v1/customresourceconversion.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/applyconfiguration/apiextensions/v1/customresourcedefinition.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/applyconfiguration/apiextensions/v1/customresourcedefinitioncondition.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/applyconfiguration/apiextensions/v1/customresourcedefinitionnames.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/applyconfiguration/apiextensions/v1/customresourcedefinitionspec.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/applyconfiguration/apiextensions/v1/customresourcedefinitionstatus.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/applyconfiguration/apiextensions/v1/customresourcedefinitionversion.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/applyconfiguration/apiextensions/v1/customresourcesubresources.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/applyconfiguration/apiextensions/v1/customresourcesubresourcescale.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/applyconfiguration/apiextensions/v1/customresourcevalidation.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/applyconfiguration/apiextensions/v1/externaldocumentation.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/applyconfiguration/apiextensions/v1/jsonschemaprops.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/applyconfiguration/apiextensions/v1/servicereference.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/applyconfiguration/apiextensions/v1/validationrule.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/applyconfiguration/apiextensions/v1/webhookclientconfig.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/applyconfiguration/apiextensions/v1/webhookconversion.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/applyconfiguration/apiextensions/v1beta1/customresourcecolumndefinition.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/applyconfiguration/apiextensions/v1beta1/customresourceconversion.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/applyconfiguration/apiextensions/v1beta1/customresourcedefinition.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/applyconfiguration/apiextensions/v1beta1/customresourcedefinitioncondition.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/applyconfiguration/apiextensions/v1beta1/customresourcedefinitionnames.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/applyconfiguration/apiextensions/v1beta1/customresourcedefinitionspec.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/applyconfiguration/apiextensions/v1beta1/customresourcedefinitionstatus.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/applyconfiguration/apiextensions/v1beta1/customresourcedefinitionversion.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/applyconfiguration/apiextensions/v1beta1/customresourcesubresources.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/applyconfiguration/apiextensions/v1beta1/customresourcesubresourcescale.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/applyconfiguration/apiextensions/v1beta1/customresourcevalidation.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/applyconfiguration/apiextensions/v1beta1/externaldocumentation.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/applyconfiguration/apiextensions/v1beta1/jsonschemaprops.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/applyconfiguration/apiextensions/v1beta1/servicereference.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/applyconfiguration/apiextensions/v1beta1/validationrule.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/applyconfiguration/apiextensions/v1beta1/webhookclientconfig.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/clientset.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/scheme/doc.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/scheme/register.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/typed/apiextensions/v1/apiextensions_client.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/typed/apiextensions/v1/customresourcedefinition.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/typed/apiextensions/v1/doc.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/typed/apiextensions/v1/generated_expansion.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/typed/apiextensions/v1beta1/apiextensions_client.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/typed/apiextensions/v1beta1/customresourcedefinition.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/typed/apiextensions/v1beta1/doc.gois excluded by!**/vendor/**api/vendor/k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/typed/apiextensions/v1beta1/generated_expansion.gois excluded by!**/vendor/**api/vendor/k8s.io/client-go/util/retry/OWNERSis excluded by!**/vendor/**api/vendor/k8s.io/client-go/util/retry/util.gois excluded by!**/vendor/**api/vendor/modules.txtis excluded by!**/vendor/**api/vendor/sigs.k8s.io/controller-runtime/pkg/envtest/crd.gois excluded by!**/vendor/**api/vendor/sigs.k8s.io/controller-runtime/pkg/envtest/doc.gois excluded by!**/vendor/**api/vendor/sigs.k8s.io/controller-runtime/pkg/envtest/helper.gois excluded by!**/vendor/**api/vendor/sigs.k8s.io/controller-runtime/pkg/envtest/server.gois excluded by!**/vendor/**api/vendor/sigs.k8s.io/controller-runtime/pkg/envtest/webhook.gois excluded by!**/vendor/**api/vendor/sigs.k8s.io/controller-runtime/pkg/internal/flock/doc.gois excluded by!**/vendor/**api/vendor/sigs.k8s.io/controller-runtime/pkg/internal/flock/errors.gois excluded by!**/vendor/**api/vendor/sigs.k8s.io/controller-runtime/pkg/internal/flock/flock_other.gois excluded by!**/vendor/**api/vendor/sigs.k8s.io/controller-runtime/pkg/internal/flock/flock_unix.gois excluded by!**/vendor/**api/vendor/sigs.k8s.io/controller-runtime/pkg/internal/testing/addr/manager.gois excluded by!**/vendor/**api/vendor/sigs.k8s.io/controller-runtime/pkg/internal/testing/certs/tinyca.gois excluded by!**/vendor/**api/vendor/sigs.k8s.io/controller-runtime/pkg/internal/testing/controlplane/apiserver.gois excluded by!**/vendor/**api/vendor/sigs.k8s.io/controller-runtime/pkg/internal/testing/controlplane/auth.gois excluded by!**/vendor/**api/vendor/sigs.k8s.io/controller-runtime/pkg/internal/testing/controlplane/etcd.gois excluded by!**/vendor/**api/vendor/sigs.k8s.io/controller-runtime/pkg/internal/testing/controlplane/kubectl.gois excluded by!**/vendor/**api/vendor/sigs.k8s.io/controller-runtime/pkg/internal/testing/controlplane/plane.gois excluded by!**/vendor/**api/vendor/sigs.k8s.io/controller-runtime/pkg/internal/testing/process/arguments.gois excluded by!**/vendor/**api/vendor/sigs.k8s.io/controller-runtime/pkg/internal/testing/process/bin_path_finder.gois excluded by!**/vendor/**api/vendor/sigs.k8s.io/controller-runtime/pkg/internal/testing/process/procattr_other.gois excluded by!**/vendor/**api/vendor/sigs.k8s.io/controller-runtime/pkg/internal/testing/process/procattr_unix.gois excluded by!**/vendor/**api/vendor/sigs.k8s.io/controller-runtime/pkg/internal/testing/process/process.gois excluded by!**/vendor/**api/vendor/sigs.k8s.io/controller-runtime/pkg/log/zap/flags.gois excluded by!**/vendor/**api/vendor/sigs.k8s.io/controller-runtime/pkg/log/zap/kube_helpers.gois excluded by!**/vendor/**api/vendor/sigs.k8s.io/controller-runtime/pkg/log/zap/zap.gois excluded by!**/vendor/**vendor/github.com/openshift/assisted-service/api/v1beta1/agentserviceconfig_types.gois excluded by!**/vendor/**,!vendor/**
📒 Files selected for processing (23)
Makefileapi/go.modapi/v1beta1/agent_webhook_test.goapi/v1beta1/agentclassification_webhook_test.goapi/v1beta1/agentserviceconfig_types.goapi/v1beta1/infraenv_webhook_test.goapi/v1beta1/webhook_suite_test.gocmd/main.goconfig/crd/bases/agent-install.openshift.io_agentserviceconfigs.yamlconfig/crd/bases/agent-install.openshift.io_hypershiftagentserviceconfigs.yamlconfig/crd/resources.yamldeploy/olm-catalog/manifests/agent-install.openshift.io_agentserviceconfigs.yamldeploy/olm-catalog/manifests/agent-install.openshift.io_hypershiftagentserviceconfigs.yamlinternal/controller/controllers/agentserviceconfig_controller.gointernal/controller/controllers/agentserviceconfig_controller_test.gointernal/versions/api_test.gointernal/versions/cache.gointernal/versions/cache_test.gointernal/versions/common.gointernal/versions/common_test.gointernal/versions/kube_api_versions.gointernal/versions/rest_api_versions.gointernal/versions/rest_api_versions_test.go
💤 Files with no reviewable changes (1)
- api/v1beta1/webhook_suite_test.go
🚧 Files skipped from review as they are similar to previous changes (3)
- api/go.mod
- api/v1beta1/agentserviceconfig_types.go
- api/v1beta1/agent_webhook_test.go
|
@pastequo: all tests passed! 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. |
Contributor
Author
|
/retest |
CrystalChun
reviewed
Mar 4, 2026
| // OpenshiftVersion is the Major.Minor version of OpenShift that this image | ||
| // is to be associated with. | ||
| OpenshiftVersion string `json:"openshiftVersion"` | ||
| // CPUArchitecture is the CPU architecture of the image (x86_64/arm64/multi/etc). |
Contributor
There was a problem hiding this comment.
Would it make sense to have this field match the cpu_architecture field in the swagger.yaml
Lines 5845 to 5851 in 628d368
Contributor
Author
There was a problem hiding this comment.
I hesitated and decided to mimic the behavior of the CPUArchitecture in the other CR at the top of the file https://github.com/openshift/assisted-service/blob/v2.50.2/api/v1beta1/agentserviceconfig_types.go#L39-L41
I don't have a strong opinion on this, the enum make sense, being consistent also. As you prefer
CrystalChun
reviewed
Mar 4, 2026
Contributor
There was a problem hiding this comment.
Is the CPU Architecture normalized anywhere? Should it be normalized?
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
3 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.
If the cache was initially populated with 4.20.10, then following calls will use this must gather image even if it's not the same patch version
List all the issues related to this PR
What environments does this code impact?
How was this code tested?
Checklist
docs, README, etc)Reviewers Checklist
Summary by CodeRabbit
Refactor
New Features
Tests / Chores