Skip to content
Merged
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
34 changes: 21 additions & 13 deletions test/extended/networking/network_segmentation.go
Original file line number Diff line number Diff line change
Expand Up @@ -1047,13 +1047,13 @@ var _ = Describe("[sig-network][OCPFeatureGate:NetworkSegmentation][Feature:User
g.Expect(json.Unmarshal([]byte(conditionsJSON), &actualConditions)).To(Succeed())
return normalizeConditions(actualConditions)
}, 5*time.Second, 1*time.Second).Should(SatisfyAny(
ConsistOf(metav1.Condition{
ContainElement(metav1.Condition{
Type: "NetworkReady",
Status: metav1.ConditionFalse,
Reason: "NetworkAttachmentDefinitionSyncError",
Message: expectedMessage,
}),
ConsistOf(metav1.Condition{
ContainElement(metav1.Condition{
Type: "NetworkCreated",
Status: metav1.ConditionFalse,
Reason: "NetworkAttachmentDefinitionSyncError",
Expand Down Expand Up @@ -1481,23 +1481,31 @@ func validateClusterUDNStatusReportsActiveNamespacesFunc(client dynamic.Interfac
return fmt.Errorf("expected at least one condition in %v", cUDN)
}

c := conditions[0]
if c.Type != "NetworkCreated" && c.Type != "NetworkReady" {
return fmt.Errorf("expected NetworkCreated/NetworkReady type in %v", c)
// Find NetworkCreated/NetworkReady condition among all conditions
var networkCond *metav1.Condition
for i := range conditions {
if conditions[i].Type == "NetworkCreated" || conditions[i].Type == "NetworkReady" {
networkCond = &conditions[i]
break
}
}
if c.Status != metav1.ConditionTrue {
return fmt.Errorf("expected True status in %v", c)
if networkCond == nil {
return fmt.Errorf("NetworkCreated/NetworkReady condition not found in conditions: %v", conditions)
}

if networkCond.Status != metav1.ConditionTrue {
return fmt.Errorf("expected True status in %v", networkCond)
}
if c.Reason != "NetworkAttachmentDefinitionCreated" && c.Reason != "NetworkAttachmentDefinitionReady" {
return fmt.Errorf("expected NetworkAttachmentDefinitionCreated/NetworkAttachmentDefinitionReady reason in %v", c)
if networkCond.Reason != "NetworkAttachmentDefinitionCreated" && networkCond.Reason != "NetworkAttachmentDefinitionReady" {
return fmt.Errorf("expected NetworkAttachmentDefinitionCreated/NetworkAttachmentDefinitionReady reason in %v", networkCond)
}
if !strings.Contains(c.Message, "NetworkAttachmentDefinition has been created in following namespaces:") {
return fmt.Errorf("expected \"NetworkAttachmentDefinition has been created in following namespaces:\" in %s", c.Message)
if !strings.Contains(networkCond.Message, "NetworkAttachmentDefinition has been created in following namespaces:") {
return fmt.Errorf("expected \"NetworkAttachmentDefinition has been created in following namespaces:\" in %s", networkCond.Message)
}

for _, ns := range expectedActiveNsNames {
if !strings.Contains(c.Message, ns) {
return fmt.Errorf("expected to find %q namespace in %s", ns, c.Message)
if !strings.Contains(networkCond.Message, ns) {
return fmt.Errorf("expected to find %q namespace in %s", ns, networkCond.Message)
Comment on lines +1484 to +1508
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

This still assumes the first network condition is the right one.

It no longer depends on conditions[0], but it still breaks on the first NetworkCreated/NetworkReady entry. If both are present and only a later one matches the expected status/message, the helper still fails even though the CR status is valid.

Suggested fix
-		// Find NetworkCreated/NetworkReady condition among all conditions
-		var networkCond *metav1.Condition
-		for i := range conditions {
-			if conditions[i].Type == "NetworkCreated" || conditions[i].Type == "NetworkReady" {
-				networkCond = &conditions[i]
-				break
-			}
-		}
-		if networkCond == nil {
-			return fmt.Errorf("NetworkCreated/NetworkReady condition not found in conditions: %v", conditions)
-		}
-
-		if networkCond.Status != metav1.ConditionTrue {
-			return fmt.Errorf("expected True status in %v", networkCond)
-		}
-		if networkCond.Reason != "NetworkAttachmentDefinitionCreated" && networkCond.Reason != "NetworkAttachmentDefinitionReady" {
-			return fmt.Errorf("expected NetworkAttachmentDefinitionCreated/NetworkAttachmentDefinitionReady reason in %v", networkCond)
-		}
-		if !strings.Contains(networkCond.Message, "NetworkAttachmentDefinition has been created in following namespaces:") {
-			return fmt.Errorf("expected \"NetworkAttachmentDefinition has been created in following namespaces:\" in %s", networkCond.Message)
-		}
-
-		for _, ns := range expectedActiveNsNames {
-			if !strings.Contains(networkCond.Message, ns) {
-				return fmt.Errorf("expected to find %q namespace in %s", ns, networkCond.Message)
-			}
-		}
-		return nil
+		for i := range conditions {
+			cond := conditions[i]
+			if cond.Type != "NetworkCreated" && cond.Type != "NetworkReady" {
+				continue
+			}
+			if cond.Status != metav1.ConditionTrue {
+				continue
+			}
+			if cond.Reason != "NetworkAttachmentDefinitionCreated" && cond.Reason != "NetworkAttachmentDefinitionReady" {
+				continue
+			}
+			if !strings.Contains(cond.Message, "NetworkAttachmentDefinition has been created in following namespaces:") {
+				continue
+			}
+
+			missingNamespace := false
+			for _, ns := range expectedActiveNsNames {
+				if !strings.Contains(cond.Message, ns) {
+					missingNamespace = true
+					break
+				}
+			}
+			if !missingNamespace {
+				return nil
+			}
+		}
+		return fmt.Errorf("no NetworkCreated/NetworkReady condition matched expected active namespaces: %v", conditions)
As per coding guidelines, "Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity."
📝 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
// Find NetworkCreated/NetworkReady condition among all conditions
var networkCond *metav1.Condition
for i := range conditions {
if conditions[i].Type == "NetworkCreated" || conditions[i].Type == "NetworkReady" {
networkCond = &conditions[i]
break
}
}
if c.Status != metav1.ConditionTrue {
return fmt.Errorf("expected True status in %v", c)
if networkCond == nil {
return fmt.Errorf("NetworkCreated/NetworkReady condition not found in conditions: %v", conditions)
}
if networkCond.Status != metav1.ConditionTrue {
return fmt.Errorf("expected True status in %v", networkCond)
}
if c.Reason != "NetworkAttachmentDefinitionCreated" && c.Reason != "NetworkAttachmentDefinitionReady" {
return fmt.Errorf("expected NetworkAttachmentDefinitionCreated/NetworkAttachmentDefinitionReady reason in %v", c)
if networkCond.Reason != "NetworkAttachmentDefinitionCreated" && networkCond.Reason != "NetworkAttachmentDefinitionReady" {
return fmt.Errorf("expected NetworkAttachmentDefinitionCreated/NetworkAttachmentDefinitionReady reason in %v", networkCond)
}
if !strings.Contains(c.Message, "NetworkAttachmentDefinition has been created in following namespaces:") {
return fmt.Errorf("expected \"NetworkAttachmentDefinition has been created in following namespaces:\" in %s", c.Message)
if !strings.Contains(networkCond.Message, "NetworkAttachmentDefinition has been created in following namespaces:") {
return fmt.Errorf("expected \"NetworkAttachmentDefinition has been created in following namespaces:\" in %s", networkCond.Message)
}
for _, ns := range expectedActiveNsNames {
if !strings.Contains(c.Message, ns) {
return fmt.Errorf("expected to find %q namespace in %s", ns, c.Message)
if !strings.Contains(networkCond.Message, ns) {
return fmt.Errorf("expected to find %q namespace in %s", ns, networkCond.Message)
for i := range conditions {
cond := conditions[i]
if cond.Type != "NetworkCreated" && cond.Type != "NetworkReady" {
continue
}
if cond.Status != metav1.ConditionTrue {
continue
}
if cond.Reason != "NetworkAttachmentDefinitionCreated" && cond.Reason != "NetworkAttachmentDefinitionReady" {
continue
}
if !strings.Contains(cond.Message, "NetworkAttachmentDefinition has been created in following namespaces:") {
continue
}
missingNamespace := false
for _, ns := range expectedActiveNsNames {
if !strings.Contains(cond.Message, ns) {
missingNamespace = true
break
}
}
if !missingNamespace {
return nil
}
}
return fmt.Errorf("no NetworkCreated/NetworkReady condition matched expected active namespaces: %v", conditions)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/extended/networking/network_segmentation.go` around lines 1484 - 1508,
The helper currently selects the first condition with Type
"NetworkCreated"/"NetworkReady" (networkCond) which can be incorrect if a later
one is the valid condition; update the logic to scan all entries in conditions
and pick the first condition that satisfies all required checks (Status ==
metav1.ConditionTrue, Reason is "NetworkAttachmentDefinitionCreated" or
"NetworkAttachmentDefinitionReady", Message contains
"NetworkAttachmentDefinition has been created in following namespaces:" and
contains every namespace from expectedActiveNsNames); only return success when
such a matching condition is found, otherwise return a clear error listing
conditions examined or that no matching network condition met all criteria.

}
}
return nil
Expand Down