Skip to content

test #10427

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
danilo-gemoli wants to merge 1 commit into from
Closed

test #10427

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
6 changes: 6 additions & 0 deletions pkg/asset/machines/aws/machines.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ type machineProviderInput struct {
}

// Machines returns a list of machines for a machinepool.
// func Machines(clusterID string, region string, subnets aws.SubnetsByZone, pool *types.MachinePool, role, userDataSecret string, userTags map[string]string, publicSubnet bool, osImageStream string) ([]machineapi.Machine, *machinev1.ControlPlaneMachineSet, error) {
func Machines(clusterID string, region string, subnets aws.SubnetsByZone, pool *types.MachinePool, role, userDataSecret string, userTags map[string]string, publicSubnet bool) ([]machineapi.Machine, *machinev1.ControlPlaneMachineSet, error) {
if poolPlatform := pool.Platform.Name(); poolPlatform != awstypes.Name {
return nil, nil, fmt.Errorf("non-AWS machine-pool: %q", poolPlatform)
Expand Down Expand Up @@ -98,6 +99,9 @@ func Machines(clusterID string, region string, subnets aws.SubnetsByZone, pool *
"machine.openshift.io/cluster-api-cluster": clusterID,
"machine.openshift.io/cluster-api-machine-role": role,
"machine.openshift.io/cluster-api-machine-type": role,
// TODO: understand if the label needs to be set here or if just setting it in
// the MachineSet's Spec.Template.ObjectMeta.Labels is enough.
// "machine.openshift.io/os-image-stream": osImageStream,
},
},
Spec: machineapi.MachineSpec{
Expand Down Expand Up @@ -177,6 +181,8 @@ func Machines(clusterID string, region string, subnets aws.SubnetsByZone, pool *
"machine.openshift.io/cluster-api-cluster": clusterID,
"machine.openshift.io/cluster-api-machine-role": role,
"machine.openshift.io/cluster-api-machine-type": role,
// TODO: see if we need these labels on CPMSs
// "machine.openshift.io/os-image-stream": osImageStream,
},
},
Spec: machineapi.MachineSpec{
Expand Down
11 changes: 8 additions & 3 deletions pkg/asset/machines/aws/machinesets.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ type MachineSetInput struct {
Role string
UserDataSecret string
Hosts map[string]icaws.Host
OSImageStream string
}

// MachineSets returns a list of machinesets for a machinepool.
Expand All @@ -48,7 +49,7 @@ func MachineSets(in *MachineSetInput) ([]*machineapi.MachineSet, error) {
replicas++
}

nodeLabels := make(map[string]string, 3)
machineSpecLabels := make(map[string]string, 3)
nodeTaints := []corev1.Taint{}
instanceType := mpool.InstanceType
publicSubnet := in.PublicSubnet
Expand All @@ -71,7 +72,7 @@ func MachineSets(in *MachineSetInput) ([]*machineapi.MachineSet, error) {
if zone.PreferredInstanceType != "" {
instanceType = zone.PreferredInstanceType
}
nodeLabels = map[string]string{
machineSpecLabels = map[string]string{
"node-role.kubernetes.io/edge": "",
"machine.openshift.io/zone-type": zone.Type,
"machine.openshift.io/zone-group": zone.GroupName,
Expand Down Expand Up @@ -112,13 +113,16 @@ func MachineSets(in *MachineSetInput) ([]*machineapi.MachineSet, error) {
return nil, errors.Wrap(err, "failed to create provider")
}

// TODO: use const for label
machineSpecLabels["machine.openshift.io/os-image-stream"] = in.OSImageStream

name := fmt.Sprintf("%s-%s-%s", in.ClusterID, in.Pool.Name, az)
spec := machineapi.MachineSpec{
ProviderSpec: machineapi.ProviderSpec{
Value: &runtime.RawExtension{Object: provider},
},
ObjectMeta: machineapi.ObjectMeta{
Labels: nodeLabels,
Labels: machineSpecLabels,
},
Taints: nodeTaints,
}
Expand Down Expand Up @@ -150,6 +154,7 @@ func MachineSets(in *MachineSetInput) ([]*machineapi.MachineSet, error) {
"machine.openshift.io/cluster-api-cluster": in.ClusterID,
"machine.openshift.io/cluster-api-machine-role": in.Role,
"machine.openshift.io/cluster-api-machine-type": in.Role,
"machine.openshift.io/os-image-stream": in.OSImageStream,
},
},
Spec: spec,
Expand Down
5 changes: 3 additions & 2 deletions pkg/asset/machines/master.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,7 @@ func (m *Master) Generate(ctx context.Context, dependencies asset.Parents) error
masterUserDataSecretName,
installConfig.Config.Platform.AWS.UserTags,
awstypes.IsPublicOnlySubnetsEnabled(),
// GetOSImageStream(ic),
)
if err != nil {
return errors.Wrap(err, "failed to create master machine objects")
Expand All @@ -285,7 +286,7 @@ func (m *Master) Generate(ctx context.Context, dependencies asset.Parents) error
mpool.Zones = azs
}
pool.Platform.GCP = &mpool
machines, controlPlaneMachineSet, err = gcp.Machines(clusterID.InfraID, ic, &pool, rhcosImage.ControlPlane, "master", masterUserDataSecretName)
machines, controlPlaneMachineSet, err = gcp.Machines(clusterID.InfraID, ic, &pool, rhcosImage.ControlPlane, "master", masterUserDataSecretName, GetOSImageStream(ic))
if err != nil {
return errors.Wrap(err, "failed to create master machine objects")
}
Expand Down Expand Up @@ -408,7 +409,7 @@ func (m *Master) Generate(ctx context.Context, dependencies asset.Parents) error
if err != nil {
return err
}
machines, controlPlaneMachineSet, err = azure.Machines(clusterID.InfraID, ic, &pool, rhcosImage.ControlPlane, "master", masterUserDataSecretName, capabilities, session)
machines, controlPlaneMachineSet, err = azure.Machines(clusterID.InfraID, ic, &pool, rhcosImage.ControlPlane, "master", masterUserDataSecretName, capabilities, session, GetOSImageStream(ic))
if err != nil {
return errors.Wrap(err, "failed to create master machine objects")
}
Expand Down
13 changes: 13 additions & 0 deletions pkg/asset/machines/util.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/openshift/installer/pkg/asset/installconfig"
"github.com/openshift/installer/pkg/asset/machines/machineconfig"
"github.com/openshift/installer/pkg/asset/machines/vsphere"
"github.com/openshift/installer/pkg/rhcos"
"github.com/openshift/installer/pkg/types"
azuretypes "github.com/openshift/installer/pkg/types/azure"
vspheretypes "github.com/openshift/installer/pkg/types/vsphere"
Expand All @@ -19,6 +20,9 @@ import (
const (
// VsphereScsiByPath defines the path format for vsphere disks being added.
VsphereScsiByPath = "/dev/disk/by-path/pci-0000:03:00.0-scsi-0:0:%d:0"

// OSImageStreamLabel is the label key used to identify the OS image stream for a machine.
OSImageStreamLabel = "machine.openshift.io/os-image-stream"
)

// NodeDiskSetup determines the path per disk type, and per platform and role, runs ForDiskSetup.
Expand Down Expand Up @@ -71,3 +75,12 @@ func NodeDiskSetup(installConfig *installconfig.InstallConfig, role string, disk
}
return nil, nil
}

// GetOSImageStream returns the OS image stream value from the install config, defaulting to the
// value of `DefaultOSImageStream` in as defined in pkg/rhcos/stream.go if not specified.
func GetOSImageStream(ic *types.InstallConfig) string {
if ic.OSImageStream == "" {
return string(rhcos.DefaultOSImageStream)
}
return string(ic.OSImageStream)
}
5 changes: 3 additions & 2 deletions pkg/asset/machines/worker.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -554,6 +554,7 @@ func (w *Worker) Generate(ctx context.Context, dependencies asset.Parents) error
Role: pool.Name,
UserDataSecret: workerUserDataSecretName,
Hosts: dHosts,
OSImageStream: GetOSImageStream(ic),
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Mar 24, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Verify MachineSets function signatures in azure and gcp packages

echo "=== Azure MachineSets signature ==="
ast-grep --pattern 'func MachineSets($$$) ($$$) {
  $$$
}' | grep -A2 "azure/machinesets.go" || rg -n "^func MachineSets" --type go -g '**/azure/machinesets.go'

echo "=== GCP MachineSets signature ==="
ast-grep --pattern 'func MachineSets($$$) ($$$) {
  $$$
}' | grep -A2 "gcp/machinesets.go" || rg -n "^func MachineSets" --type go -g '**/gcp/machinesets.go'

Repository: openshift/installer

Length of output: 14768

🏁 Script executed:

# Check the actual function calls in worker.go at the mentioned lines
echo "=== Worker.go lines 555-560 (around line 557 for AWS) ==="
sed -n '555,560p' pkg/asset/machines/worker.go

echo "=== Worker.go lines 625-635 (around line 629 for Azure) ==="
sed -n '625,635p' pkg/asset/machines/worker.go

echo "=== Worker.go lines 663-673 (around line 667 for GCP) ==="
sed -n '663,673p' pkg/asset/machines/worker.go

echo "=== Check MachineSetInput struct in aws package ==="
rg -n "type MachineSetInput" --type go -g '**/aws/**'

echo "=== Check GetOSImageStream function ==="
rg -n "GetOSImageStream" --type go -A 2

Repository: openshift/installer

Length of output: 3672

🏁 Script executed:

# Verify the exact function signatures to confirm argument count mismatch
echo "=== Azure MachineSets signature (parameters) ==="
sed -n '21p' pkg/asset/machines/azure/machinesets.go | sed 's/func MachineSets(//' | sed 's/) \(\[//'

echo "=== GCP MachineSets signature (parameters) ==="
sed -n '18p' pkg/asset/machines/gcp/machinesets.go | sed 's/func MachineSets(//' | sed 's/) \(\[//'

# Check MachineSetInput to confirm AWS accepts OSImageStream
echo "=== MachineSetInput struct ==="
sed -n '19,45p' pkg/asset/machines/aws/machinesets.go

Repository: openshift/installer

Length of output: 1202

🏁 Script executed:

# Get full Azure and GCP MachineSets signatures to check if osImageStream was added
echo "=== Full Azure MachineSets signature ==="
head -1 pkg/asset/machines/azure/machinesets.go | grep -A 1 "^package"
sed -n '21p' pkg/asset/machines/azure/machinesets.go

echo "=== Full GCP MachineSets signature ==="
sed -n '18p' pkg/asset/machines/gcp/machinesets.go

# Check if there are multiple versions or overloads
echo "=== All azure.MachineSets definitions ==="
rg "func MachineSets" pkg/asset/machines/azure/ --type go -A 1

echo "=== All gcp.MachineSets definitions ==="
rg "func MachineSets" pkg/asset/machines/gcp/ --type go -A 1

Repository: openshift/installer

Length of output: 1310

🏁 Script executed:

# Double-check the exact argument count being passed to confirm the mismatch
echo "=== Line 629 (Azure call) - count arguments ==="
sed -n '629p' pkg/asset/machines/worker.go | tr ',' '\n' | nl

echo "=== Line 667 (GCP call) - count arguments ==="
sed -n '667p' pkg/asset/machines/worker.go | tr ',' '\n' | nl

Repository: openshift/installer

Length of output: 257

🏁 Script executed:

# Check the exact argument count being passed
echo "=== Line 629 (Azure call) ==="
sed -n '629p' pkg/asset/machines/worker.go

echo "=== Line 667 (GCP call) ==="
sed -n '667p' pkg/asset/machines/worker.go

# Count commas to verify argument count
echo "=== Azure arguments (count commas + 1) ==="
sed -n '629p' pkg/asset/machines/worker.go | grep -o ',' | wc -l

echo "=== GCP arguments (count commas + 1) ==="
sed -n '667p' pkg/asset/machines/worker.go | grep -o ',' | wc -l

Repository: openshift/installer

Length of output: 538

Azure and GCP function signatures need updating to accept the OS image stream parameter.

The code at lines 629 and 667 passes GetOSImageStream(ic) to azure.MachineSets() and gcp.MachineSets(), but neither function signature has been updated to accept this parameter:

  • Azure (line 629): Passing 10 arguments; function accepts only 9
  • GCP (line 667): Passing 7 arguments; function accepts only 6

The AWS implementation is correct—MachineSetInput struct includes the OSImageStream field. Update the Azure and GCP MachineSets() function signatures to accept the OS image stream parameter, or adjust the calling code.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@pkg/asset/machines/worker.go` at line 557, The calls to
azure.MachineSets(...) and gcp.MachineSets(...) are supplying
GetOSImageStream(ic) but their function signatures still expect one fewer
argument; update the azure.MachineSets and gcp.MachineSets function signatures
(and any related MachineSetInput/parameter handling inside those functions) to
accept and propagate the OS image stream parameter (same type as used by AWS's
MachineSetInput.OSImageStream) so the callers at the spots using
GetOSImageStream(ic) compile and the OS image stream is used consistently across
MachineSets implementations.

})
if err != nil {
return errors.Wrap(err, "failed to create worker machine objects")
Expand Down Expand Up @@ -625,7 +626,7 @@ func (w *Worker) Generate(ctx context.Context, dependencies asset.Parents) error
return err
}

sets, err := azure.MachineSets(clusterID.InfraID, installConfig, &pool, rhcosImage.Compute, "worker", workerUserDataSecretName, capabilities, subnetZones, session)
sets, err := azure.MachineSets(clusterID.InfraID, installConfig, &pool, rhcosImage.Compute, "worker", workerUserDataSecretName, capabilities, subnetZones, session, GetOSImageStream(ic))
if err != nil {
return errors.Wrap(err, "failed to create worker machine objects")
}
Expand Down Expand Up @@ -663,7 +664,7 @@ func (w *Worker) Generate(ctx context.Context, dependencies asset.Parents) error
mpool.Zones = azs
}
pool.Platform.GCP = &mpool
sets, err := gcp.MachineSets(clusterID.InfraID, ic, &pool, rhcosImage.Compute, "worker", workerUserDataSecretName)
sets, err := gcp.MachineSets(clusterID.InfraID, ic, &pool, rhcosImage.Compute, "worker", workerUserDataSecretName, GetOSImageStream(ic))
if err != nil {
return errors.Wrap(err, "failed to create worker machine objects")
}
Expand Down