Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions test/extended-priv/const.go
Original file line number Diff line number Diff line change
Expand Up @@ -202,4 +202,7 @@ var (
OSImageStreamRHEL9,
OSImageStreamRHEL10,
}

// MCDCrioReloadedRegexp is the regexp to match crio reload success message in MCD logs
MCDCrioReloadedRegexp = "crio.* reloaded successfully"
)
40 changes: 40 additions & 0 deletions test/extended-priv/mco.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,3 +152,43 @@ func createMcAndVerifyMCValue(oc *exutil.CLI, stepText, mcName string, node *Nod
o.Expect(podOut).Should(o.MatchRegexp(textToVerify.textToVerifyForNode))
logger.Infof("%s is verified in the machine config daemon!", stepText)
}

func getClusterVersion(oc *exutil.CLI) (string, error) {
clusterBuild, err := oc.AsAdmin().WithoutNamespace().Run("get").Args("clusterversion", "-o", "jsonpath={..desired.version}").Output()
if err != nil {
return "", err
}
splitValues := strings.Split(clusterBuild, ".")
return splitValues[0] + "." + splitValues[1], nil
}
Comment on lines +156 to +163
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Guard cluster version parsing before indexing.

splitValues[0] / splitValues[1] can panic when desired.version is empty or malformed.

Suggested fix
 func getClusterVersion(oc *exutil.CLI) (string, error) {
 	clusterBuild, err := oc.AsAdmin().WithoutNamespace().Run("get").Args("clusterversion", "-o", "jsonpath={..desired.version}").Output()
 	if err != nil {
 		return "", err
 	}
-	splitValues := strings.Split(clusterBuild, ".")
-	return splitValues[0] + "." + splitValues[1], nil
+	clusterBuild = strings.TrimSpace(clusterBuild)
+	splitValues := strings.Split(clusterBuild, ".")
+	if len(splitValues) < 2 {
+		return "", fmt.Errorf("unexpected cluster version format: %q", clusterBuild)
+	}
+	return splitValues[0] + "." + splitValues[1], nil
 }
🤖 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 `@test/extended-priv/mco.go` around lines 156 - 163, The getClusterVersion
function currently indexes splitValues[0] and [1] without validating the output
of oc.AsAdmin().WithoutNamespace().Run(...).Output(); guard against empty or
malformed desired.version by trimming clusterBuild, checking that clusterBuild
is not empty and that strings.Split(clusterBuild, ".") yields at least two
elements, and return a clear error (rather than panicking) when the version
cannot be parsed; update getClusterVersion to perform these checks before
composing and returning the "major.minor" string.


func skipTestIfClusterVersion(oc *exutil.CLI, operator, constraintVersion string) {
clusterVersion, err := getClusterVersion(oc)
o.Expect(err).NotTo(o.HaveOccurred())

if CompareVersions(clusterVersion, operator, constraintVersion) {
g.Skip(fmt.Sprintf("Test case skipped because current cluster version %s %s %s",
clusterVersion, operator, constraintVersion))
}
}

func skipTestIfExtensionsAreUsed(oc *exutil.CLI) {
wMcp := NewMachineConfigPool(oc.AsAdmin(), MachineConfigPoolWorker)
mMcp := NewMachineConfigPool(oc.AsAdmin(), MachineConfigPoolMaster)

wCurrentMC, err := wMcp.GetConfiguredMachineConfig()
o.ExpectWithOffset(1, err).NotTo(o.HaveOccurred(), "Error trying to get the current MC configured in worker pool")

mCurrentMC, err := mMcp.GetConfiguredMachineConfig()
o.ExpectWithOffset(1, err).NotTo(o.HaveOccurred(), "Error trying to get the current MC configured in master pool")

wExtensions, err := wCurrentMC.Get(`{.spec.extensions}`)
o.ExpectWithOffset(1, err).NotTo(o.HaveOccurred(), "Error trying to get the extensions configured in MC: %s", wCurrentMC.GetName())

mExtensions, err := mCurrentMC.Get(`{.spec.extensions}`)
o.ExpectWithOffset(1, err).NotTo(o.HaveOccurred(), "Error trying to get the extensions configured in MC: %s", mCurrentMC.GetName())

if wExtensions != "[]" || mExtensions != "[]" {
g.Skip("Current cluster is using extensions. This test cannot be execute in a cluster using extensions")
}
Comment on lines +191 to +193
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Extension detection is too strict for raw jsonpath output.

Comparing to exactly "[]" is brittle ("", whitespace, <no value> can appear) and may skip tests incorrectly.

Suggested fix
-	if wExtensions != "[]" || mExtensions != "[]" {
+	wExt := strings.TrimSpace(wExtensions)
+	mExt := strings.TrimSpace(mExtensions)
+	hasWorkerExt := wExt != "" && wExt != "[]" && wExt != "<no value>"
+	hasMasterExt := mExt != "" && mExt != "[]" && mExt != "<no value>"
+	if hasWorkerExt || hasMasterExt {
 		g.Skip("Current cluster is using extensions. This test cannot be execute in a cluster using extensions")
 	}
🤖 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 `@test/extended-priv/mco.go` around lines 191 - 193, The current check compares
raw jsonpath strings wExtensions and mExtensions to the literal "[]" which is
brittle; instead normalize and reliably detect emptiness by trimming whitespace
and handling jsonpath's special outputs or by parsing the value as JSON into a
slice. Update the conditional around wExtensions and mExtensions to treat any of
the following as empty: empty string, trimmed "[]", "<no value>", or a
successfully-unmarshaled zero-length []interface{} (use encoding/json to
unmarshal and check len==0), and only call g.Skip when either side actually
contains non-empty extensions.

}
715 changes: 715 additions & 0 deletions test/extended-priv/mco_registry.go

Large diffs are not rendered by default.

31 changes: 31 additions & 0 deletions test/extended-priv/mco_units.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
g "github.com/onsi/ginkgo/v2"
o "github.com/onsi/gomega"
exutil "github.com/openshift/machine-config-operator/test/extended-priv/util"
bootstrap "github.com/openshift/machine-config-operator/test/extended-priv/util/bootstrap"
logger "github.com/openshift/machine-config-operator/test/extended-priv/util/logext"
)

Expand Down Expand Up @@ -359,4 +360,34 @@ var _ = g.Describe("[sig-mco][Suite:openshift/machine-config-operator/longdurati

behaviourValidatorRemove.Validate()
})

g.It("[PolarionID:53960][OTP] No failed units in the bootstrap machine", func() {
skipTestIfSupportedPlatformNotMatched(oc, AWSPlatform, AzurePlatform)

failedUnitsCommand := "sudo systemctl list-units --failed --all"

// If no bootstrap is found, we skip the case.
// The test can only be executed in deployments that didn't remove the bootstrap machine
bs, err := bootstrap.GetBootstrap(oc)
if err != nil {
if _, notFound := err.(*bootstrap.InstanceNotFound); notFound {
g.Skip("skip test because bootstrap machine does not exist in the current cluster")
}
}
o.Expect(err).NotTo(o.HaveOccurred())

exutil.By("Verify that there is no failed units in the bootstrap machine")
// ssh client is a bit unstable, and it can return an empty string for no apparent reason every now and then.
// Hence we use 'Eventually' to verify the command to make the test robust.
o.Eventually(func() string {
logger.Infof("Executing command in bootstrap: %s", failedUnitsCommand)
failedUnits, err := bs.SSH.RunOutput(failedUnitsCommand)
logger.Infof("Command output:\n%s", failedUnits)
if err != nil {
logger.Errorf("Command Error:\n%s", err)
}
return failedUnits
}).Should(o.ContainSubstring("0 loaded units listed"),
Comment on lines +382 to +390
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

head -400 test/extended-priv/mco_units.go | tail -50

Repository: openshift/machine-config-operator

Length of output: 1941


🏁 Script executed:

sed -n '370,400p' test/extended-priv/mco_units.go

Repository: openshift/machine-config-operator

Length of output: 1124


🏁 Script executed:

rg -n "Eventually.*SSH|RunOutput.*Eventually" test/extended-priv/ -A 3 -B 1

Repository: openshift/machine-config-operator

Length of output: 59


🏁 Script executed:

rg -n "Eventually" test/extended-priv/mco_units.go | head -20

Repository: openshift/machine-config-operator

Length of output: 194


🏁 Script executed:

head -50 test/extended-priv/mco_units.go

Repository: openshift/machine-config-operator

Length of output: 1844


🏁 Script executed:

rg "Eventually.*\)\.Should|Eventually.*\)\.WithPolling" test/extended-priv/ -A 1 | head -40

Repository: openshift/machine-config-operator

Length of output: 3002


🏁 Script executed:

rg 'Eventually\(.*["\d]+[ms]' test/extended-priv/ -A 2 -B 1

Repository: openshift/machine-config-operator

Length of output: 50389


Set explicit timeout/polling for the SSH Eventually check.

The SSH client instability is acknowledged in the code comment, and relying on implicit Eventually defaults can cause test flakiness. Add explicit timeout and polling interval parameters consistent with other operations in the codebase.

Suggested fix
-		o.Eventually(func() string {
+		o.Eventually(func() string {
 			logger.Infof("Executing command in bootstrap: %s", failedUnitsCommand)
 			failedUnits, err := bs.SSH.RunOutput(failedUnitsCommand)
 			logger.Infof("Command output:\n%s", failedUnits)
 			if err != nil {
 				logger.Errorf("Command Error:\n%s", err)
 			}
 			return failedUnits
-		}).Should(o.ContainSubstring("0 loaded units listed"),
+		}, "3m", "15s").Should(o.ContainSubstring("0 loaded units listed"),
 			"There are failed units in the bootstrap machine")
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
o.Eventually(func() string {
logger.Infof("Executing command in bootstrap: %s", failedUnitsCommand)
failedUnits, err := bs.SSH.RunOutput(failedUnitsCommand)
logger.Infof("Command output:\n%s", failedUnits)
if err != nil {
logger.Errorf("Command Error:\n%s", err)
}
return failedUnits
}).Should(o.ContainSubstring("0 loaded units listed"),
o.Eventually(func() string {
logger.Infof("Executing command in bootstrap: %s", failedUnitsCommand)
failedUnits, err := bs.SSH.RunOutput(failedUnitsCommand)
logger.Infof("Command output:\n%s", failedUnits)
if err != nil {
logger.Errorf("Command Error:\n%s", err)
}
return failedUnits
}, "3m", "15s").Should(o.ContainSubstring("0 loaded units listed"),
"There are failed units in the bootstrap machine")
🤖 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 `@test/extended-priv/mco_units.go` around lines 382 - 390, The SSE Eventually
block that calls bs.SSH.RunOutput with failedUnitsCommand should supply explicit
timeout and polling interval parameters to reduce flakiness; modify the
o.Eventually invocation that wraps the anonymous func returning failedUnits (and
that ends with .Should(o.ContainSubstring("0 loaded units listed"))) to pass a
concrete timeout and polling duration consistent with other tests (e.g., same
durations used elsewhere) so the SSH call is retried for the specified period
with a fixed poll interval instead of relying on defaults.

"There are failed units in the bootstrap machine")
})
})
17 changes: 17 additions & 0 deletions test/extended-priv/testdata/files/add-image-tag-mirror-set.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
apiVersion: template.openshift.io/v1
kind: Template
metadata:
name: imaa-tag-mirror-set
objects:
- kind: ImageTagMirrorSet
apiVersion: config.openshift.io/v1
metadata:
name: "${NAME}"
spec:
imageTagMirrors:
- mirrors:
- mirror.example.com/redhat
source: registry.redhat.io/openshift4
mirrorSourcePolicy: AllowContactingSource
parameters:
- name: NAME
19 changes: 19 additions & 0 deletions test/extended-priv/testdata/files/repository-mirror.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
apiVersion: template.openshift.io/v1
kind: Template
metadata:
name: repository-mirror
objects:
- kind: ImageContentSourcePolicy
apiVersion: operator.openshift.io/v1alpha1
metadata:
name: "${NAME}"
spec:
repositoryDigestMirrors:
- mirrors:
- example.io/example/ubi-minimal
source: registry.access.redhat.com/ubi8/ubi-minimal
- mirrors:
- example.com/example/ubi-minimal
source: registry.access.redhat.com/ubi8/ubi-minimal
parameters:
- name: NAME
35 changes: 35 additions & 0 deletions test/extended-priv/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,11 @@ type ImageTagMirrorSet struct {
Template
}

// NewImageTagMirrorSet creates a new ImageTagMirrorSet struct
func NewImageTagMirrorSet(oc *exutil.CLI, name string, t Template) *ImageTagMirrorSet {
return &ImageTagMirrorSet{Resource: *NewResource(oc, "ImageTagMirrorSet", name), Template: t}
}

// TextToVerify is a helper struct to verify configurations using the `createMcAndVerifyMCValue` function
type TextToVerify struct {
textToVerifyForMC string
Expand Down Expand Up @@ -1312,3 +1317,33 @@ func skipTestIfOsIsNotCoreOs(oc *exutil.CLI) *Node {
}
return allCoreOs[0]
}

// GetDataFromConfigMap returns a map[string]string with the information of the ".data" section of a configmap
func GetDataFromConfigMap(oc *exutil.CLI, namespace, name string) (map[string]string, error) {
data := map[string]string{}
cm := NewNamespacedResource(oc.AsAdmin(), "ConfigMap", namespace, name)
dataJSON, err := cm.Get(`{.data}`)
if err != nil {
return nil, err
}

if dataJSON == "" {
return data, nil
}

if err := json.Unmarshal([]byte(dataJSON), &data); err != nil {
return nil, err
}

return data, nil
}

// GetImageRegistryCertificates returns a map with image registry certificates from the image-registry-ca configmap
func GetImageRegistryCertificates(oc *exutil.CLI) (map[string]string, error) {
return GetDataFromConfigMap(oc.AsAdmin(), "openshift-config-managed", "image-registry-ca")
}

// GetManagedMergedTrustedImageRegistryCertificates returns a map with the merged trusted image registry certificates
func GetManagedMergedTrustedImageRegistryCertificates(oc *exutil.CLI) (map[string]string, error) {
return GetDataFromConfigMap(oc.AsAdmin(), "openshift-config-managed", "merged-trusted-image-registry-ca")
}
75 changes: 75 additions & 0 deletions test/extended-priv/util/bootstrap/aws.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
package bootstrap

import (
"fmt"
"os"
"os/exec"
"strings"

util "github.com/openshift/machine-config-operator/test/extended-priv/util"
logger "github.com/openshift/machine-config-operator/test/extended-priv/util/logext"
)

const (
// EnvVarSSHCloudPrivAWSUser stores the environment variable for the AWS ssh user
EnvVarSSHCloudPrivAWSUser = "SSH_CLOUD_PRIV_AWS_USER"
)

// AWSBSInfoProvider implements interface BSInfoProvider for AWS
type AWSBSInfoProvider struct{}

// GetIPs returns the IPs of the bootstrap machine if this machine exists in AWS
func (a AWSBSInfoProvider) GetIPs(oc *util.CLI) (*Ips, error) {
infraName, err := oc.WithoutNamespace().AsAdmin().Run("get").Args("infrastructure", "cluster", "-o=jsonpath={.status.infrastructureName}").Output()
if err != nil {
logger.Errorf("Could not get bootstrap's IP in AWS. Unable to get infrastructure's name. Error: %s", err)
return nil, err
}

bootstrapName := infraName + "-bootstrap"

publicIP, err := awsCLIGetInstancePublicIP(bootstrapName)
if err != nil {
return nil, &InstanceNotFound{bootstrapName}
}
if publicIP == "" || publicIP == "None" {
logger.Infof("Bootstrap instance '%s' has no public IP or is terminated", bootstrapName)
return nil, &InstanceNotFound{bootstrapName}
}

privateIP, _ := awsCLIGetInstancePrivateIP(bootstrapName)
logger.Infof("Bootstrap instance '%s': publicIP=%s, privateIP=%s", bootstrapName, publicIP, privateIP)
return &Ips{PublicIPAddress: publicIP, PrivateIPAddress: privateIP}, nil
}

// GetSSHUser returns the user needed to connect to the bootstrap machine via ssh
func (a AWSBSInfoProvider) GetSSHUser() string {
if user, exists := os.LookupEnv(EnvVarSSHCloudPrivAWSUser); exists {
return user
}
return DefaultSSHUser
}

func awsCLIGetInstancePublicIP(instanceName string) (string, error) {
out, err := exec.Command("aws", "ec2", "describe-instances",
"--filters", fmt.Sprintf("Name=tag:Name,Values=%s", instanceName),
"--query", "Reservations[0].Instances[0].PublicIpAddress",
"--output", "text",
).Output()
Comment on lines +54 to +58
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

AWS CLI query assumes instance exists at array position [0].

The JMESPath query Reservations[0].Instances[0].PublicIpAddress will return null or fail if no reservations or instances exist, rather than returning an error. When AWS CLI outputs null, it's converted to the string "null" in text format, which could be confused with a valid value.

Consider adding error handling to distinguish between "instance not found" (empty results) and "instance exists but has no public IP" (empty/None value).

🔍 Suggested improvement for robustness
 func awsCLIGetInstancePublicIP(instanceName string) (string, error) {
 	out, err := exec.Command("aws", "ec2", "describe-instances",
 		"--filters", fmt.Sprintf("Name=tag:Name,Values=%s", instanceName),
 		"--query", "Reservations[0].Instances[0].PublicIpAddress",
 		"--output", "text",
 	).Output()
 	if err != nil {
 		return "", err
 	}
-	return strings.TrimSpace(string(out)), nil
+	result := strings.TrimSpace(string(out))
+	if result == "null" {
+		return "", fmt.Errorf("no instance found with name %s", instanceName)
+	}
+	return result, nil
 }
🤖 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 `@test/extended-priv/util/bootstrap/aws.go` around lines 54 - 58, The AWS CLI
call using exec.Command(..., "describe-instances", ..., "--query",
"Reservations[0].Instances[0].PublicIpAddress", "--output", "text") can return
the literal "null" (or empty string) which is ambiguous; change the call to
request JSON output (use "--output", "json") and parse the returned JSON (or, if
you keep text output, explicitly check the raw stdout for "null" or an empty
string) after the exec.Command call that uses instanceName, then return distinct
errors like "instance not found" when Reservations/Instances are missing and "no
public IP" when the instance exists but PublicIpAddress is null/empty; update
the code around the exec.Command(...) invocation to implement this check and
adjust the returned value/error accordingly.

if err != nil {
return "", err
}
return strings.TrimSpace(string(out)), nil
}
Comment on lines +53 to +63
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Add timeout and context to AWS CLI command execution.

The exec.Command calls have no timeout, which could cause tests to hang indefinitely if the AWS CLI is unresponsive or network connectivity is poor. This is especially problematic in CI environments.

⏱️ Proposed fix to add timeout context
+import (
+	"context"
+	"time"
+)
+
 func awsCLIGetInstancePublicIP(instanceName string) (string, error) {
+	ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
+	defer cancel()
+	
-	out, err := exec.Command("aws", "ec2", "describe-instances",
+	out, err := exec.CommandContext(ctx, "aws", "ec2", "describe-instances",
 		"--filters", fmt.Sprintf("Name=tag:Name,Values=%s", instanceName),
 		"--query", "Reservations[0].Instances[0].PublicIpAddress",
 		"--output", "text",
 	).Output()

Apply the same pattern to awsCLIGetInstancePrivateIP.

🤖 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 `@test/extended-priv/util/bootstrap/aws.go` around lines 53 - 63, The aws CLI
invocation in awsCLIGetInstancePublicIP (and similarly
awsCLIGetInstancePrivateIP) lacks a timeout; wrap the command with a
context.WithTimeout and use exec.CommandContext to ensure the call is cancelled
if it exceeds a reasonable duration (e.g., 30s), call cancel() in a defer,
capture and return the context/timeout error alongside the command output error,
and trim the output as before—update both functions to follow this pattern so
tests/CI cannot hang on an unresponsive AWS CLI.


func awsCLIGetInstancePrivateIP(instanceName string) (string, error) {
out, err := exec.Command("aws", "ec2", "describe-instances",
"--filters", fmt.Sprintf("Name=tag:Name,Values=%s", instanceName),
"--query", "Reservations[0].Instances[0].PrivateIpAddress",
"--output", "text",
).Output()
if err != nil {
return "", err
}
return strings.TrimSpace(string(out)), nil
}
28 changes: 28 additions & 0 deletions test/extended-priv/util/bootstrap/azure.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package bootstrap

import (
util "github.com/openshift/machine-config-operator/test/extended-priv/util"
logger "github.com/openshift/machine-config-operator/test/extended-priv/util/logext"
)

const (
// EnvVarSSHCloudPrivAzureUser stores the environment variable for the Azure ssh user
EnvVarSSHCloudPrivAzureUser = "SSH_CLOUD_PRIV_AZURE_USER"
)

// AzureBSInfoProvider implements interface BSInfoProvider for Azure
type AzureBSInfoProvider struct{}

// GetIPs returns the IPs of the bootstrap machine if this machine exists in Azure.
// Azure bootstrap discovery is not yet implemented; returns InstanceNotFound so the test skips.
func (a AzureBSInfoProvider) GetIPs(oc *util.CLI) (*Ips, error) {
infraName, _ := oc.WithoutNamespace().AsAdmin().Run("get").Args("infrastructure", "cluster", "-o=jsonpath={.status.infrastructureName}").Output()
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Handle the error from the oc command.

The error returned by .Output() is silently ignored. If the oc get infrastructure command fails, infraName will be an empty string, causing bootstrapName to become "-bootstrap" instead of the correct value. This masks command failures and could lead to confusing error messages downstream.

🛡️ Proposed fix to handle the error
-	infraName, _ := oc.WithoutNamespace().AsAdmin().Run("get").Args("infrastructure", "cluster", "-o=jsonpath={.status.infrastructureName}").Output()
+	infraName, err := oc.WithoutNamespace().AsAdmin().Run("get").Args("infrastructure", "cluster", "-o=jsonpath={.status.infrastructureName}").Output()
+	if err != nil {
+		logger.Errorf("Could not get infrastructure name for Azure bootstrap discovery. Error: %s", err)
+		return nil, err
+	}
 	bootstrapName := infraName + "-bootstrap"
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
infraName, _ := oc.WithoutNamespace().AsAdmin().Run("get").Args("infrastructure", "cluster", "-o=jsonpath={.status.infrastructureName}").Output()
infraName, err := oc.WithoutNamespace().AsAdmin().Run("get").Args("infrastructure", "cluster", "-o=jsonpath={.status.infrastructureName}").Output()
if err != nil {
logger.Errorf("Could not get infrastructure name for Azure bootstrap discovery. Error: %s", err)
return nil, err
}
bootstrapName := infraName + "-bootstrap"
🤖 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 `@test/extended-priv/util/bootstrap/azure.go` at line 19, The call to
oc.WithoutNamespace().AsAdmin().Run("get").Args(...).Output() ignores the
returned error and may leave infraName empty; change the assignment to capture
the error (e.g., infraName, err :=
oc.WithoutNamespace().AsAdmin().Run("get").Args(...).Output()), check err !=
nil, and handle it appropriately (return the error or fail the test / log and
exit) so that failures from the oc command are surfaced instead of producing an
incorrect bootstrapName; ensure subsequent uses of infraName (where
bootstrapName is formed) only proceed when the call succeeded.

bootstrapName := infraName + "-bootstrap"
logger.Infof("Azure bootstrap discovery is not yet implemented, skipping bootstrap machine: %s", bootstrapName)
return nil, &InstanceNotFound{bootstrapName}
}

// GetSSHUser returns the user needed to connect to the bootstrap machine via ssh
func (a AzureBSInfoProvider) GetSSHUser() string {
return DefaultSSHUser
}
81 changes: 81 additions & 0 deletions test/extended-priv/util/bootstrap/bootstrap.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
package bootstrap

import (
"fmt"

util "github.com/openshift/machine-config-operator/test/extended-priv/util"
logger "github.com/openshift/machine-config-operator/test/extended-priv/util/logext"
)

const (
// EnvVarSSHCloudPrivKey stores the environment variable for the ssh private key path
EnvVarSSHCloudPrivKey = "SSH_CLOUD_PRIV_KEY"
// DefaultSSHUser is the default ssh user for bootstrap machines
DefaultSSHUser = "core"
)

// InstanceNotFound reports an error because the bootstrap instance is not found. It can be used to skip the test case.
type InstanceNotFound struct{ InstanceName string }

// Error implements the error interface
func (inferr *InstanceNotFound) Error() string {
return fmt.Sprintf("Instance %s has 'terminated' status", inferr.InstanceName)
}

// BSInfoProvider any struct implementing this interface can be used to create a Bootstrap object.
type BSInfoProvider interface {
GetIPs(*util.CLI) (*Ips, error)
GetSSHUser() string
}

// Bootstrap contains the functionality regarding the bootstrap machine
type Bootstrap struct {
SSH util.SshClient
IPs Ips
}

// Ips struct to store the public and the private IPs of the bootstrap machine
type Ips struct {
PrivateIPAddress string
PublicIPAddress string
}

// GetBootstrap returns a bootstrap struct pointing to the bootstrap machine if it exists
func GetBootstrap(oc *util.CLI) (*Bootstrap, error) {
bsInfoProvider, err := GetBSInfoProvider(oc)
if err != nil {
return nil, err
}

bootstrapIPs, err := bsInfoProvider.GetIPs(oc.AsAdmin())
if err != nil {
return nil, err
}

user := bsInfoProvider.GetSSHUser()
return buildBootstrap(user, *bootstrapIPs, 22), nil
}

// GetBSInfoProvider returns a struct implementing BSInfoProvider for the current platform
func GetBSInfoProvider(oc *util.CLI) (BSInfoProvider, error) {
platform := util.CheckPlatform(oc)
switch platform {
case "aws":
return AWSBSInfoProvider{}, nil
case "azure":
return AzureBSInfoProvider{}, nil
default:
return nil, fmt.Errorf("platform not supported. Cannot get bootstrap information for platform: %s", platform)
}
}

func buildBootstrap(user string, bootstrapIPs Ips, port int) *Bootstrap {
privateKey := util.GetSSHPrivateKey()
publicIP := bootstrapIPs.PublicIPAddress
logger.Infof("Creating bootstrap with ip '%s', user: '%s', private key: '%s', port '%d'",
publicIP, user, privateKey, port)
return &Bootstrap{
SSH: util.SshClient{User: user, Host: publicIP, Port: port, PrivateKey: privateKey},
IPs: bootstrapIPs,
}
}
Loading