Skip to content

MULTIARCH-6014: PowerVS: Use PowerVS Stock Catalogue Images #10647

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
AshwinHIBM wants to merge 1 commit into
base: main
Choose a base branch
from
+105 −10
Open

MULTIARCH-6014: PowerVS: Use PowerVS Stock Catalogue Images #10647

Show file tree
Hide file tree
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
74 changes: 74 additions & 0 deletions pkg/asset/machines/powervs/images.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
package powervs

import (
"context"
"fmt"

"github.com/IBM-Cloud/power-go-client/clients/instance"
"github.com/IBM-Cloud/power-go-client/ibmpisession"
"github.com/IBM/go-sdk-core/v5/core"
"github.com/sirupsen/logrus"

powervsconfig "github.com/openshift/installer/pkg/asset/installconfig/powervs"
)

// GetBootImageFromWorkspace retrieves a boot image from the PowerVS workspace.
// If an active image is found in the workspace, it returns that image name.
// If no images are found, it returns a default name in the format "rhcos-{clusterID}".
func GetBootImageFromWorkspace(ctx context.Context, serviceInstanceGUID string, zone string, clusterID string) (string, error) {
// Create a new PowerVS client
client, err := powervsconfig.NewClient()
if err != nil {
return "", fmt.Errorf("failed to create PowerVS client: %w", err)
}

// Create authenticator
authenticator := &core.IamAuthenticator{
ApiKey: client.GetAPIKey(),
}

// Create PI session
piSession, err := ibmpisession.NewIBMPISession(&ibmpisession.IBMPIOptions{
Authenticator: authenticator,
UserAccount: client.BXCli.User.Account,
Zone: zone,
Debug: false,
})
if err != nil {
return "", fmt.Errorf("failed to create PowerVS session: %w", err)
}

// Create image client
imageClient := instance.NewIBMPIImageClient(ctx, piSession, serviceInstanceGUID)
if imageClient == nil {
return "", fmt.Errorf("failed to create PowerVS image client")
}

// Get all images from the workspace
images, err := imageClient.GetAll()
if err != nil {
return "", fmt.Errorf("failed to list images from PowerVS workspace: %w", err)
}
Comment thread
AshwinHIBM marked this conversation as resolved.

// If no images found in workspace, use default naming pattern
if images == nil || len(images.Images) == 0 {
defaultImage := fmt.Sprintf("rhcos-%s", clusterID)
logrus.Infof("No images found in PowerVS workspace, using default image name: %s", defaultImage)
return defaultImage, nil
}

// Find the first active image
for _, image := range images.Images {
if image.State != nil && *image.State == "active" && image.Name != nil {
logrus.Infof("Selected PowerVS boot image from workspace: %s", *image.Name)
return *image.Name, nil
}
Comment on lines +61 to +65

@coderabbitai coderabbitai Bot Jun 23, 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 | 🟠 Major | ⚡ Quick win

Selection predicate is too broad for boot image resolution.

At Line 61-64, the code returns the first active image regardless of OS/image intent. In workspaces with multiple active images, this can pick a non-RHCOS image and break node boot compatibility. Filter explicitly for the expected RHCOS image naming/metadata and require non-empty names.

🤖 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/asset/machines/powervs/images.go` around lines 61 - 65, The boot image
selection loop starting at line 61 with the range over images.Images returns the
first active image with a non-null name without verifying it is a RHCOS image.
This is too broad and can result in incompatible images being selected in
workspaces with multiple active images. Add additional filtering logic to the
condition checking image.State and image.Name to also validate that the image is
specifically a RHCOS image, either by checking the image name contains expected
RHCOS naming patterns or by validating image metadata. Only return the image
name when all conditions are satisfied: active state, non-empty name, and
confirmed RHCOS identity.

}

// If no active images found, use default naming pattern
defaultImage := fmt.Sprintf("rhcos-%s", clusterID)
logrus.Infof("No active images found in PowerVS workspace, using default image name: %s", defaultImage)
return defaultImage, nil
}
Comment thread
AshwinHIBM marked this conversation as resolved.

// Made with Bob
14 changes: 13 additions & 1 deletion pkg/asset/machines/powervs/machinesets.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,11 @@
package powervs

import (
"context"
"fmt"
"time"

"github.com/sirupsen/logrus"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"

Expand All @@ -25,7 +28,16 @@ func MachineSets(clusterID string, config *types.InstallConfig, pool *types.Mach
platform := config.Platform.PowerVS
mpool := pool.Platform.PowerVS
var network string
image := fmt.Sprintf("rhcos-%s", clusterID)

// Get the boot image from the PowerVS workspace, fallback to default if error occurs
ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute)
defer cancel()
image, err := GetBootImageFromWorkspace(ctx, config.PowerVS.ServiceInstanceGUID, config.PowerVS.Zone, clusterID)
if err != nil {
// Fallback to default image naming pattern
image = fmt.Sprintf("rhcos-%s", clusterID)
logrus.Warnf("Failed to get boot image from PowerVS workspace, using default: %s (error: %v)", image, err)
}

total := int32(0)
if pool.Replicas != nil {
Expand Down
27 changes: 18 additions & 9 deletions pkg/asset/machines/powervs/powervsmachines.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,11 @@
package powervs

import (
"context"
"fmt"
"time"

"github.com/sirupsen/logrus"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/utils/ptr"
Expand Down Expand Up @@ -37,10 +40,18 @@ func GenerateMachines(clusterID string, ic *types.InstallConfig, pool *types.Mac
powerVSMachine *capibm.IBMPowerVSMachine
dataSecret string
machine *capi.Machine
err error
)

// Note: This will be created later
image = fmt.Sprintf("rhcos-%s", clusterID)
// Get the boot image from the PowerVS workspace, fallback to default if error occurs
ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute)
defer cancel()
image, err = GetBootImageFromWorkspace(ctx, ic.PowerVS.ServiceInstanceGUID, ic.PowerVS.Zone, clusterID)
if err != nil {
// Fallback to default image naming pattern
image = fmt.Sprintf("rhcos-%s", clusterID)
logrus.Warnf("Failed to get boot image from PowerVS workspace, using default: %s (error: %v)", image, err)
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

if ic.PowerVS.ServiceInstanceGUID == "" {
serviceName := fmt.Sprintf("%s-power-iaas", clusterID)
Expand Down Expand Up @@ -112,13 +123,11 @@ func GenerateMachine(ic *types.InstallConfig, service capibm.IBMPowerVSResourceR
ServiceInstanceID: ic.PowerVS.ServiceInstanceGUID,
ServiceInstance: &service,
SSHKey: "",
ImageRef: &v1.LocalObjectReference{
Name: image,
},
SystemType: mpool.SysType,
ProcessorType: capibm.PowerVSProcessorType(mpool.ProcType),
Processors: mpool.Processors,
MemoryGiB: mpool.MemoryGiB,
Image: &capibm.IBMPowerVSResourceReference{Name: &image},
SystemType: mpool.SysType,
ProcessorType: capibm.PowerVSProcessorType(mpool.ProcType),
Processors: mpool.Processors,
MemoryGiB: mpool.MemoryGiB,
Comment thread
AshwinHIBM marked this conversation as resolved.
},
}
utils.SetMachineOSStreamLabels(machine, ic)
Expand Down