Skip to content

fix: add OVN-K and Multus validation to isolated-network CI test #228

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

fix: add OVN-K and Multus validation to isolated-network CI test #228

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
65 changes: 64 additions & 1 deletion .github/actions/build/action.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ runs:
# when the isolated network is enabled
if [ "${{ inputs.isolated-network }}" = "1" ]; then
for cmd in "ping -c1 -W10 8.8.8.8" "curl -I -m 10 quay.io" "curl -I -m 10 ghcr.io"; do
if sudo podman exec -i microshift-okd ${cmd} ; then
if sudo podman exec -i microshift-okd-1 ${cmd} ; then
echo "ERROR: Internet access is available in the isolated network container"
exit 1
fi
Expand All @@ -116,6 +116,69 @@ runs:
make run-ready
make run-healthy

# Validate OVN-K networking when enabled
if [ "${{ inputs.ovnk-networking }}" = "1" ]; then
echo "=== Validating OVN-K networking ==="
CONTAINER="microshift-okd-1"

echo "Checking OVN-K pods in openshift-ovn-kubernetes namespace..."
ovnk_pods=$(sudo podman exec -i "${CONTAINER}" \
kubectl get pods -n openshift-ovn-kubernetes \
--field-selector=status.phase=Running \
-o name 2>&1) || true
Comment on lines +126 to +128

@coderabbitai coderabbitai Bot Jul 22, 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

Require all expected pods to be Ready.

Filtering to status.phase=Running only proves that at least one pod is running. Pending, crashing, or unready OVN-K/Multus pods are excluded, so a partially broken deployment can pass. Validate the full expected pod set and require each pod’s Ready condition and IP.

Also applies to: 139-142, 161-163

🤖 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 @.github/actions/build/action.yaml around lines 126 - 128, Update the pod
validation commands in the build action, including the occurrences around the
existing kubectl checks, to inspect the complete expected OVN-K/Multus pod set
rather than filtering to Running pods. Require every expected pod to have a
Ready condition and a pod IP, and fail validation when any pod is pending,
crashing, unready, or missing its IP.

if [ -z "${ovnk_pods}" ]; then
echo "ERROR: No running OVN-K pods found in openshift-ovn-kubernetes namespace"
sudo podman exec -i "${CONTAINER}" kubectl get pods -n openshift-ovn-kubernetes -o wide || true
exit 1
fi
echo "OVN-K pods running:"
echo "${ovnk_pods}"

echo "Checking OVN-K pod IP assignments..."
pods_without_ip=$(sudo podman exec -i "${CONTAINER}" \
kubectl get pods -n openshift-ovn-kubernetes \
--field-selector=status.phase=Running \
-o jsonpath='{range .items[*]}{.metadata.name}{"\t"}{.status.podIP}{"\n"}{end}' 2>&1 \
| awk -F'\t' '$2 == "" {print $1}')
Comment on lines +125 to +142

@coderabbitai coderabbitai Bot Jul 22, 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

Fail closed on kubectl errors.

2>&1 plus || true mixes command failures into pod output; a non-empty error message can then be accepted as a successful pod listing. For Multus, the block can pass if pod inspection fails but the CRD lookup succeeds. Capture command status separately and parse output only after successful queries.

Also applies to: 160-163

🤖 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 @.github/actions/build/action.yaml around lines 125 - 142, Update the OVN-K
pod checks using ovnk_pods and pods_without_ip to capture each kubectl exit
status separately and only parse output after a successful query. Remove the
2>&1/|| true pattern so kubectl errors cannot be treated as pod data, and exit
nonzero with diagnostics when either inspection fails. Apply the same
fail-closed handling to the Multus query around the referenced later block,
including its CRD lookup.

if [ -n "${pods_without_ip}" ]; then
echo "ERROR: The following OVN-K pods have no IP address:"
echo "${pods_without_ip}"
sudo podman exec -i "${CONTAINER}" kubectl get pods -n openshift-ovn-kubernetes -o wide || true
exit 1
fi
echo "All OVN-K pods have IP addresses assigned"

echo "=== OVN-K networking validation passed ==="
fi

# Validate Multus when enabled
if [ "${{ inputs.with-multus }}" = "1" ]; then
echo "=== Validating Multus CNI ==="
CONTAINER="microshift-okd-1"

echo "Checking Multus pods in openshift-multus namespace..."
multus_pods=$(sudo podman exec -i "${CONTAINER}" \
kubectl get pods -n openshift-multus \
--field-selector=status.phase=Running \
-o name 2>&1) || true
if [ -z "${multus_pods}" ]; then
echo "ERROR: No running Multus pods found in openshift-multus namespace"
sudo podman exec -i "${CONTAINER}" kubectl get pods -n openshift-multus -o wide || true
exit 1
fi
echo "Multus pods running:"
echo "${multus_pods}"

if ! sudo podman exec -i "${CONTAINER}" \
kubectl get crd network-attachment-definitions.k8s.cni.cncf.io >/dev/null 2>&1; then
echo "ERROR: Multus NetworkAttachmentDefinition CRD not found"
exit 1
Comment on lines +172 to +175

@coderabbitai coderabbitai Bot Jul 22, 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.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Dump diagnostics when the Multus CRD check fails.

This branch only prints the error and exits, while docs/isolated-network-ovnk-validation.md promises relevant diagnostics on failure. Include the CRD/API and namespace pod state before exiting.

🤖 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 @.github/actions/build/action.yaml around lines 172 - 175, Update the Multus
CRD failure branch in the build action to dump the relevant CRD/API state and
namespace pod state before exiting. Preserve the existing error message and exit
behavior, and use the existing container and Kubernetes command context rather
than adding unrelated diagnostics.

fi
echo "Multus CRD verified: network-attachment-definitions.k8s.cni.cncf.io"

echo "=== Multus CNI validation passed ==="
fi

for i in $(seq 2 ${{ inputs.node-count }}); do
make add-node
done
Expand Down
115 changes: 115 additions & 0 deletions docs/isolated-network-ovnk-validation.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
# Isolated Network OVN-K Test Validation

**Issue**: [#221](https://github.com/microshift-io/microshift/issues/221)

---

## Problem

The `isolated-network` CI job in `.github/workflows/builders.yaml` defines
three matrix entries:

| Name | ovnk-networking | with-multus |
|------|-----------------|-------------|
| kindnet | 0 | 0 |
| ovnk | 1 | 0 |
| ovnk-multus | 1 | 1 |

When `ovnk-networking: 1`, the image is built with `WITH_KINDNET=0`, which
installs the `microshift-networking` package (OVN-K) instead of
`microshift-kindnet`. However, the test verification step in
`.github/actions/build/action.yaml` only checks:

1. Internet is blocked (ping/curl fail inside the container)
2. `microshift.service` is running (`make run-ready`)
3. `greenboot-healthcheck` has exited (`make run-healthy`)

These checks are identical for all three matrix variants. The OVN-K and
OVN-K+Multus jobs pass as long as MicroShift starts — they do not validate
that OVN-K networking is actually functioning. A broken OVN-K configuration
would go undetected.

### Container name bug

The existing internet isolation check on line 108 of the build action
references a container named `microshift-okd`. The actual container created
by `cluster_manager.sh` is named `microshift-okd-1` (`NODE_BASE_NAME` is
`microshift-okd-` and the first node appends `1`). This check has been
silently passing because `podman exec` against a non-existent container
fails, and the test asserts that the commands fail.

---

## What changed

All changes are in `.github/actions/build/action.yaml`.

### Fix: container name

Changed `microshift-okd` to `microshift-okd-1` in the internet isolation
check so it runs against the actual container.

### Addition: OVN-K validation

When `ovnk-networking == 1`, after `make run-healthy` passes:

1. **Pod check**: Verifies that pods are running in the
`openshift-ovn-kubernetes` namespace. If no running pods are found, the
job fails and dumps pod state for debugging.

2. **IP assignment check**: Verifies that every running OVN-K pod has an IP
address assigned. Pods running without IPs would indicate a broken network
plane. If any pod lacks an IP, the job fails with the pod names listed.

### Addition: Multus validation

When `with-multus == 1`, after the OVN-K check:

1. **Pod check**: Verifies that pods are running in the `openshift-multus`
namespace.

2. **CRD check**: Verifies that the `network-attachment-definitions.k8s.cni.cncf.io`
Custom Resource Definition exists, confirming Multus installed its API
extension.

---

## Why this approach

**No test pod creation.** In isolated network mode, the container runs with
`--network none` and no internet access. Container images cannot be pulled,
so creating a test pod with an external image is not possible. The embedded
images are MicroShift components, not general-purpose test containers.

Instead, the validation checks the OVN-K pods themselves. After greenboot
passes (which validates core MicroShift workloads), OVN-K pods running with
assigned IPs is strong evidence that the network plane is functional. The
OVN-K pods depend on OpenVSwitch, the OVN databases, and the CNI plugin
chain — if any of those are broken, the pods will not reach Running state
with IPs.

**Inline bash, not a separate script.** The existing test step already
contains inline validation logic (the internet isolation check). Adding the
networking checks in the same style keeps the change minimal and
reviewable. A separate script would require additional plumbing (file copy
into the container or a new Makefile target) for a small amount of logic.

**No retry loops.** The checks run after `make run-healthy`, which polls
greenboot for up to 5 minutes. By that point, all expected workload pods
should be stable. Adding retry logic would mask real failures.

---

## Impact

- **kindnet variant**: Unaffected. The new blocks only execute when
`ovnk-networking` or `with-multus` inputs are `1`.
- **ovnk variant**: Now validates OVN-K pods are running with IPs.
- **ovnk-multus variant**: Now validates both OVN-K pods and Multus
deployment.
- **Other jobs**: Unaffected. The `ovnk-networking` and `with-multus` inputs
default to `0`.

On failure, the job dumps `kubectl get pods -o wide` for the relevant
namespace before exiting, providing immediate diagnostic context in the CI
logs.
Loading