Skip to content

HIVE-3148: Hive test extension binary #31059

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

Closed
miyadav wants to merge 1 commit into from
Closed

HIVE-3148: Hive test extension binary #31059

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
19 changes: 17 additions & 2 deletions pkg/test/extensions/binary.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,10 @@ type TestBinary struct {
imageTag string
// The binary path to extract from the image
binaryPath string
// optional marks non-payload extensions whose image may not be present in
// the release payload. Extraction failures for optional binaries are logged
// as warnings instead of treated as fatal errors.
optional bool

// Cache the info after gathering it
info *Extension
Expand Down Expand Up @@ -203,8 +207,9 @@ type Image struct {
Version string `json:"version"`
}

// extensionBinaries is the registry of additional test binaries to use as extension tests. Members
// of the registry must be part of the release payload.
// extensionBinaries is the registry of additional test binaries to use as extension tests.
// Payload members must be part of the release payload. Entries with optional=true are
// non-payload extensions whose extraction failure is non-fatal.
var extensionBinaries = []TestBinary{
// Self reference for origin's own internal extension
{
Expand Down Expand Up @@ -336,6 +341,12 @@ var extensionBinaries = []TestBinary{
{
imageTag: "vsphere-csi-driver-operator",
binaryPath: "/usr/bin/vmware-vsphere-csi-driver-operator-tests-ext.gz",

// Non-payload extensions (optional — image may not be present in every release payload)
{
imageTag: "hive",
binaryPath: "/usr/bin/openshift-tests-extension.gz",
optional: true,
},
}

Expand Down Expand Up @@ -683,6 +694,10 @@ func ExtractAllTestBinaries(ctx context.Context, parallelism int) (func(), TestB

testBinary, err := externalBinaryProvider.ExtractBinaryFromReleaseImage(b.imageTag, b.binaryPath)
if err != nil {
if b.optional {
logrus.Warnf("Skipping optional extension %s (not in payload): %v", b.imageTag, err)
continue
}
Comment on lines +697 to +700
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 23, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Handle only “missing from payload” as skippable for optional binaries.

At Line 694, the current logic skips on any extraction error for optional binaries. That can mask real failures (auth/transport/path regressions) and silently drop coverage for callers that continue on success.

Proposed hardening 
-					if err != nil {
-						if b.optional {
-							logrus.Warnf("Skipping optional extension %s (not in payload): %v", b.imageTag, err)
-							continue
-						}
-						errCh <- err
-						continue
-					}
+					if err != nil {
+						if b.optional && isMissingFromPayloadError(err) {
+							logrus.Warnf("Skipping optional extension %s (%s): %v", b.imageTag, b.binaryPath, err)
+							continue
+						}
+						errCh <- fmt.Errorf("failed extracting extension %s (%s): %w", b.imageTag, b.binaryPath, err)
+						continue
+					}
func isMissingFromPayloadError(err error) bool {
	msg := strings.ToLower(err.Error())
	return strings.Contains(msg, "not found") ||
		strings.Contains(msg, "manifest unknown") ||
		strings.Contains(msg, "no such image")
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@pkg/test/extensions/binary.go` around lines 694 - 697, The current
optional-binary handling inside the extraction loop (where b.optional and
b.imageTag are used) skips on any error, which can hide real failures; change
the logic to call a helper isMissingFromPayloadError(err) (implement as per the
suggested string checks) and only warn+continue when that helper returns true;
for any other error, log it as an error/return it (don’t silently continue) so
auth/transport/path issues surface. Ensure the helper is used where the
extraction error is checked and maintain the existing log format for the skip
case.

errCh <- err
continue
}
Expand Down