NE-2126: Migrating Ipfailover test cases to images repo - #245
NE-2126: Migrating Ipfailover test cases to images repo#245melvinjoseph86 wants to merge 1 commit into
Conversation
openshift-ci-robot
added
the
jira/valid-reference
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
|
@melvinjoseph86: This pull request references NE-2126 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the epic to target the "5.0.0" version, but no target version was set. DetailsIn response to this: Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
Skipping CI for Draft Pull Request. |
WalkthroughAdds a Go-based ipfailover OTE test extension with cluster utilities, six E2E scenarios, named suite registration, and Docker image packaging. Changesipfailover test extension
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant testExtension as test extension
participant ginkgoSuite as OpenShift Ginkgo suite
participant e2eTests as ipfailover E2E tests
participant cluster as OpenShift cluster
testExtension->>ginkgoSuite: build test specs
ginkgoSuite->>e2eTests: filter local e2e specs
testExtension->>e2eTests: register suites and lifecycle
e2eTests->>cluster: create ipfailover deployment
cluster-->>e2eTests: return pod state and logs
e2eTests-->>testExtension: report test results
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (4 errors, 4 warnings)
✅ Passed checks (7 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
/test tests-extension |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with 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.
Inline comments:
In `@ipfailover/keepalived/Dockerfile`:
- Line 4: Replace the broad COPY . . instruction in the Dockerfile with explicit
COPY directives for only the test module’s required files and directories,
ensuring unrelated files and secrets are excluded from the builder context.
- Around line 24-25: Update the final base-rhel9 stage in the Dockerfile to run
with a non-root USER, not only the builder stage. Verify keepalived still
functions with that identity; if elevated privileges are required, document the
exception and restrict execution to the minimum necessary capabilities.
- Around line 24-25: The final image lacks a healthcheck for the keepalived
service. Add a Docker HEALTHCHECK to the keepalived Dockerfile that verifies
both monitor.sh and the keepalived process are running successfully, using an
appropriate command and interval, timeout, retries, and startup grace period.
In `@ipfailover/keepalived/tests/e2e/util.go`:
- Around line 107-123: Update the temporary directory creation in
getImagePullSpecFromPayload to use owner-only permissions instead of 0755,
ensuring the extracted pull-secret and its parent directory are inaccessible to
other local users while preserving the existing extraction and cleanup flow.
- Around line 414-437: Update getSpecificPodLogs to remove the bash-based
exec.Command filtering and extra cat process. Read podLogs directly in Go, apply
the optional case-insensitive filter in memory, and return the matching log
content while preserving the existing error logging and return behavior.
In `@ipfailover/keepalived/tests/README.md`:
- Line 49: Update the fenced directory-tree code block in the README to specify
the text language, changing the untyped fence to a text fence while preserving
its contents.
In `@tests-extension/README.md`:
- Line 38: Update the documented CI command in the execution instructions to
include --max-concurrency=1 when invoking run-suite for ipfailover/all,
preserving serial test execution required by the hostNetwork-based tests.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 10b4db7b-9654-4cb0-94fa-6e7281579f0b
⛔ Files ignored due to path filters (1)
ipfailover/keepalived/tests/go.sumis excluded by!**/*.sum
📒 Files selected for processing (13)
ipfailover/keepalived/.dockerignoreipfailover/keepalived/Dockerfileipfailover/keepalived/tests/.gitignoreipfailover/keepalived/tests/Makefileipfailover/keepalived/tests/README.mdipfailover/keepalived/tests/cmd/main.goipfailover/keepalived/tests/e2e/ipfailover.goipfailover/keepalived/tests/e2e/testdata/router/ipfailover.yamlipfailover/keepalived/tests/e2e/util.goipfailover/keepalived/tests/go.modtests-extension/.gitignoretests-extension/Makefiletests-extension/README.md
💤 Files with no reviewable changes (1)
- ipfailover/keepalived/.dockerignore
|
@melvinjoseph86: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
1bed137 to
2c8ac73
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: melvinjoseph86 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
|
/test tests-extension |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with 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.
Inline comments:
In `@ipfailover/keepalived/Dockerfile`:
- Around line 8-9: Update the Dockerfile artifact packaging commands to
gzip-compress the built ipfailover-tests-ext executable with gzip rather than
creating a tar archive, and publish the resulting raw gzip artifact at the
registered /usr/bin/ipfailover-test-extension.tar.gz path while preserving the
expected executable build flow.
In `@ipfailover/keepalived/tests/e2e/testdata/router/ipfailover.yaml`:
- Around line 69-74: Add a readinessProbe alongside the existing livenessProbe
in the Deployment container configuration, using a check that confirms
keepalived initialization and VRRP/VIP readiness (or the VRRP controller port)
rather than merely checking that the process is running. Keep the existing
livenessProbe unchanged.
- Around line 28-69: Add a resources block to the openshift-ipfailover container
definition, specifying both CPU and memory requests and limits. Keep the
existing container configuration unchanged and use the repository’s established
resource values or conventions if available.
- Around line 14-27: Update the ipfailover e2e setup around the deployment
replica configuration and OPENSHIFT_HA_REPLICA_COUNT to derive the count from
eligible worker nodes rather than assuming two workers. Inspect
infrastructure.Status.ControlPlaneTopology before running these tests, and
explicitly skip SingleReplica, HyperShift, arbiter, and External topologies;
keep the node-role.kubernetes.io/worker selection aligned with the computed
placement.
- Around line 22-47: Harden the ipfailover test PodSpec by removing the invalid
PodSpec-level privileged setting, hostNetwork, and host-slash host-path mount
unless each is demonstrably required; retain only required host integrations.
For the remaining container configuration, remove unnecessary privileges and add
allowPrivilegeEscalation: false, runAsNonRoot: true, and readOnlyRootFilesystem:
true where supported, using only verified capabilities if host networking must
remain.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| tar -czvf ipfailover-test-extension.tar.gz ipfailover-tests-ext && \ | ||
| rm -f ipfailover-tests-ext |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Search OTE artifact registrations and extractors:"
rg -n --hidden --glob '!vendor/**' \
'ipfailover-test-extension|ipfailover-tests-ext|tests-ext\.gz|tar -x|gzip|decompress|extract' . || true
echo
echo "Inspect the changed packaging paths:"
nl -ba ipfailover/keepalived/Dockerfile | sed -n '1,35p'
nl -ba ipfailover/keepalived/tests/Makefile | sed -n '1,25p'Repository: openshift/images
Length of output: 50375
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Changed files summary:"
git diff --stat HEAD~1..HEAD 2>/dev/null || git diff --stat || true
echo
echo "Locate keepalived Dockerfile and Makefile:"
fd -a '^(Dockerfile|Makefile)$' ipfailover/keepalived 2>/dev/null || find ipfailover/keepalived -maxdepth 3 -type f \( -name Dockerfile -o -name Makefile \) | sort
echo
echo "Packaging files (no vendor, no huge log):"
rg -n --hidden --glob '!ipfailover/keepalived/tests/vendor/**' --glob '!vendor/**' \
'ipfailover-test-extension|ipfailover-tests-ext|tests-ext\.gz|tar -czv|cp.*test-extension|BUILD|Extension|extension' ipfailover || true
echo
echo "Relevant Dockerfile:"
nl -ba ipfailover/keepalived/Dockerfile | sed -n '1,80p'
echo
echo "Relevant Makefile:"
nl -ba ipfailover/keepalived/tests/Makefile | sed -n '1,120p'
echo
echo "Focused OTE string search:"
rg -n --hidden --glob '!ipfailover/keepalived/tests/vendor/**' \
'openshift/tests-extension|tests-extension|tests-ext\.gz|test-extension' . || trueRepository: openshift/images
Length of output: 50375
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Focused keepalived files without vendor/log:"
find ipfailover/keepalived -maxdepth 3 -type f \( -name Dockerfile -o -name Makefile \) -print | sort
echo
echo "Relevant Dockerfile excerpts:"
for f in $(find ipfailover/keepalived -maxdepth 3 -type f -name Dockerfile -print | sort); do
echo "--- $f"
nl -ba "$f" | sed -n '1,120p'
done
echo
echo "Relevant Makefile excerpts:"
for f in $(find ipfailover/keepalived -maxdepth 3 -type f -name Makefile -print | sort); do
echo "--- $f"
nl -ba "$f" | sed -n '1,160p'
done
echo
echo "Focused OTE strings excluding vendor directories/files:"
python3 - <<'PY'
import re, os
root='ipfailover/keepalived'
for dirpath, dirnames, filenames in os.walk(root):
dirnames[:] = [d for d in dirnames if d != 'vendor']
for name in filenames:
if name == 'vendor' or name.endswith(('.md','.png','.jpg')):
continue
path=os.path.join(dirpath, name)
try:
data=open(path, encoding='utf-8', errors='ignore').read()
except Exception:
continue
for i,line in enumerate(data.splitlines(),1):
if re.search(r'ipfailover-test-extension|ipfailover-tests-ext|tests-ext\.gz|test-extension|archive|tar |gzip -n|COPY --from=test-extension-builder|Extension|extension', line):
print(f'{path}:{i}:{line}')
PYRepository: openshift/images
Length of output: 378
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- ipfailover/keepalived/Dockerfile'
awk '{printf "%6d\t%s\n", NR, $0}' ipfailover/keepalived/Dockerfile | sed -n '1,120p'
printf '%s\n' '--- ipfailover/keepalived/tests/Makefile'
awk '{printf "%6d\t%s\n", NR, $0}' ipfailover/keepalived/tests/Makefile | sed -n '1,160p'
printf '%s\n' '--- focused OTE strings'
python3 - <<'PY'
import re, os
root='ipfailover/keepalived'
for dirpath, dirnames, filenames in os.walk(root):
dirnames[:] = [d for d in dirnames if d != 'vendor']
for name in filenames:
if name == 'vendor' or name.endswith(('.md','.png','.jpg','.gz')):
continue
path=os.path.join(dirpath, name)
line=1
try:
data=open(path, encoding='utf-8', errors='ignore').read()
except Exception:
continue
for raw in data.splitlines():
if re.search(r'ipfailover-test-extension|ipfailover-tests-ext|tests-ext\.gz|test-extension|archive|tar |gzip -n|COPY --from=test-extension-builder|Extension|extension', raw):
print(f'{path}:{line}:{raw}')
line += 1
PYRepository: openshift/images
Length of output: 6235
🌐 Web query:
openshift-eng openshift-tests-extension OTE gzip tests-ext.gz extractor
💡 Result:
The openshift-tests-extension (OTE) is a framework that allows OpenShift component teams to contribute end-to-end tests to the core OpenShift test suite without needing to merge code directly into the main openshift/origin repository [1][2]. Key technical aspects include: 1. OTE Protocol: A command-line protocol used by the core openshift-tests binary to discover and execute these component-specific test binaries [1]. 2. Binary Packaging: Component teams build their tests as standalone Go executables, which are then compressed using gzip (typically resulting in filenames ending in .gz) to reduce image size [1][3][4]. These compressed binaries are placed at well-known paths (commonly /usr/bin/) within the component's payload images [1]. 3. Discovery and Extraction: The OpenShift test framework discovers these binaries via a registration process in openshift/origin [1]. During execution, the test framework automatically extracts these binaries from their respective payload images (e.g., using oc image extract), decompresses them, makes them executable, and runs them as part of the broader test suite [1][5][6]. This modular approach allows for decentralized test management while maintaining centralized orchestration through the openshift-tests binary [6][2].
Citations:
- 1: https://github.com/openshift-eng/ai-helpers/blob/main/plugins/ci/skills/prow-job-analysis/references/test-extension-binaries.md
- 2: https://github.com/openshift-eng/openshift-tests-extension
- 3: openshift/oc@037dbde
- 4: openshift/cluster-kube-controller-manager-operator@907c32a
- 5: CNTRLPLANE-2167:Add oc-tests-ext to extension binaries origin#30653
- 6: https://mtulio.dev/playbooks/openshift/dev-extract-openshift-tests/
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Check local archive module API if available:"
go doc github.com/moby/go-archive Archive 2>&1 || true
echo
echo "Simulate Dockerfile archive behavior with a small ELF-like payload:"
python3 - <<'PY'
import tarfile, gzip, os, stat, tempfile, subprocess, shutil
payload_path = os.path.join(tempfile.mkdtemp(prefix="ote-payload-"), "ipfailover-tests-ext")
with open(payload_path, "wb") as f:
f.write(b"\x7fELF-tests-extension-payload")
with tempfile.TemporaryDirectory() as tmp:
os.chmod(payload_path, stat.S_IRWXU | stat.S_IRGRP | stat.S_IXGRP | stat.S_IROTH | stat.S_IXOTH)
tarball = os.path.join(tmp, "ipfailover-test-extension.tar.gz")
with tarfile.open(tarball, "w:gz") as tf:
tf.add(payload_path, "ipfailover-tests-ext")
with gzip.open(os.path.join(tmp, "ipfailover-tests-ext.gz"), "wb") as gz:
with open(payload_path, "rb") as src:
gz.write(src.read())
print("tarball size:", os.stat(tarball).st_size)
print("raw gzip size:", os.path.getsize(os.path.join(tmp, "ipfailover-tests-ext.gz")))
with subprocess.Popen(["sh", "-c", "tar -xzf /tmp/*.tar.gz 2>&1 || true"], stdin=subprocess.DEVNULL, cwd=tmp, stdout=subprocess.PIPE, stderr=subprocess.PIPE) as p:
stdout, stderr = p.communicate()
print("tar extraction returncode:", p.returncode)
print("tar extraction stdout:", stdout.decode(errors="replace"))
print("tar extraction stderr:", stderr.decode(errors="replace"))
PYRepository: openshift/images
Length of output: 620
🌐 Web query:
site:github.com/openshift/oc registry payload image extract gzip *.gz executable tests-ext.gz
💡 Result:
The artifact oc-tests-ext.gz is a gzip-compressed binary typically found in OpenShift CLI test images (e.g., cli-tests) [1][2][3]. Because it is stored in a compressed state to reduce image size, it cannot be executed directly from the container image [1][2]. To extract, decompress, and use the oc-tests-ext binary, follow these steps [1][2]: 1. Extract the file from the image: You can use docker cp or podman cp to copy the file from a container, or oc image extract to pull it directly from the registry [1][4]. Example using container runtime: docker create --name temp-container docker cp temp-container:/usr/bin/oc-tests-ext.gz. docker rm temp-container 2. Decompress the binary: gunzip oc-tests-ext.gz 3. Set executable permissions: chmod +x oc-tests-ext 4. Execute the binary:./oc-tests-ext Alternatively, you can perform these actions in a single pipeline: docker run --rm cat /usr/bin/oc-tests-ext.gz | gunzip > oc-tests-ext && chmod +x oc-tests-ext [1]
Citations:
- 1: CNTRLPLANE-2167:Integrate OpenShift Test Extension (OTE) into oc oc#2177
- 2: CNTRLPLANE-2167:Integrate OpenShift Test Extension (OTE) into oc oc#2176
- 3: openshift/oc@037dbde
- 4: Bug 1919032: Fix image extract from Root Directory oc#713
Package the OTE artifact as a gzip-compressed binary.
The Dockerfile copies a tar archive to /usr/bin/ipfailover-test-extension.tar.gz, but OTE payloads are raw gzip-compressed executables, typically named *-tests-ext.gz. Build ipfailover-tests-ext, compress it with raw gzip, and copy/publish that artifact at the registered /usr/bin path.
🤖 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 `@ipfailover/keepalived/Dockerfile` around lines 8 - 9, Update the Dockerfile
artifact packaging commands to gzip-compress the built ipfailover-tests-ext
executable with gzip rather than creating a tar archive, and publish the
resulting raw gzip artifact at the registered
/usr/bin/ipfailover-test-extension.tar.gz path while preserving the expected
executable build flow.
| replicas: 2 | ||
| selector: | ||
| matchLabels: | ||
| ipfailover: hello-openshift | ||
| template: | ||
| metadata: | ||
| labels: | ||
| ipfailover: hello-openshift | ||
| spec: | ||
| serviceAccountName: ipfailover | ||
| privileged: true | ||
| hostNetwork: true | ||
| nodeSelector: | ||
| node-role.kubernetes.io/worker: "" |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 4 --glob '*.go' \
'ControlPlaneTopology|Infrastructure|ipfailover\.yaml|node-role\.kubernetes\.io/worker|REPLICA_COUNT|replicas' \
ipfailover/keepalived/testsRepository: openshift/images
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'File outline:\n'
wc -l ipfailover/keepalived/tests/e2e/testdata/router/ipfailover.yaml
printf '\nRelevant manifest:\n'
cat -n ipfailover/keepalived/tests/e2e/testdata/router/ipfailover.yaml
printf '\nNon-vendor matches in ipfailover/keepalived/tests:\n'
rg -n -C 3 --glob '!vendor/**' \
'ControlPlaneTopology|Infra|infrastructure\.Status|HyperShift|SingleReplica|DualReplica|HighlyAvailableArbiter|External|node-role\.kubernetes\.io/worker|REPLICA_COUNT|replicas|ipfailover\.yaml' \
ipfailover/keepalived/tests || true
printf '\nList ipfailover/keepalived/tests top-level:\n'
fd . ipfailover/keepalived/tests -maxdepth 3 -t f | sed -n '1,200p'Repository: openshift/images
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Target file size:\n'
wc -l ipfailover/keepalived/tests/e2e/testdata/router/ipfailover.yaml 2>/dev/null || true
printf '\nRelevant non-vendor matches:\n'
rg -n -C 3 --glob '!ipfailover/keepalived/tests/vendor/**' \
'ControlPlaneTopology|Infra|infrastructure\.Status|HyperShift|SingleReplica|DualReplica|HighlyAvailableArbiter|External|node-role\.kubernetes\.io/worker|REPLICA_COUNT|replicas|ipfailover\.yaml' \
ipfailover/keepalived/tests || true
printf '\nNon-vendor Go file count/listing:\n'
git ls-files ipfailover/keepalived/tests | rg '\.go$|\.yaml$|\.yml$' | sed -n '1,200p'Repository: openshift/images
Length of output: 29536
Make replica placement topology-aware.
replicas: 2 plus the node-role.kubernetes.io/worker selector and OPENSHIFT_HA_REPLICA_COUNT=2 assume two worker nodes. The ipfailover e2e tests do not inspect infrastructure.Status.ControlPlaneTopology, so this can fail on SingleReplica clusters, HyperShift, arbiter, or External topologies. Set replicas and OPENSHIFT_HA_REPLICA_COUNT based on eligible worker nodes and skip unsupported topologies explicitly.
🤖 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 `@ipfailover/keepalived/tests/e2e/testdata/router/ipfailover.yaml` around lines
14 - 27, Update the ipfailover e2e setup around the deployment replica
configuration and OPENSHIFT_HA_REPLICA_COUNT to derive the count from eligible
worker nodes rather than assuming two workers. Inspect
infrastructure.Status.ControlPlaneTopology before running these tests, and
explicitly skip SingleReplica, HyperShift, arbiter, and External topologies;
keep the node-role.kubernetes.io/worker selection aligned with the computed
placement.
Source: Coding guidelines
| spec: | ||
| serviceAccountName: ipfailover | ||
| privileged: true | ||
| hostNetwork: true | ||
| nodeSelector: | ||
| node-role.kubernetes.io/worker: "" | ||
| containers: | ||
| - name: openshift-ipfailover | ||
| image: ${IMAGE} | ||
| ports: | ||
| - containerPort: 63000 | ||
| hostPort: 63000 | ||
| imagePullPolicy: IfNotPresent | ||
| securityContext: | ||
| privileged: true | ||
| volumeMounts: | ||
| - name: lib-modules | ||
| mountPath: /lib/modules | ||
| readOnly: true | ||
| - name: host-slash | ||
| mountPath: /host | ||
| readOnly: true | ||
| mountPropagation: HostToContainer | ||
| - name: etc-sysconfig | ||
| mountPath: /etc/sysconfig | ||
| readOnly: true |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
oc process -f ipfailover/keepalived/tests/e2e/testdata/router/ipfailover.yaml \
-p NAME=ipfailover \
-p NAMESPACE=default \
-p IMAGE=example.invalid/ipfailover:latest \
-p HAINTERFACE=eth0 |
oc apply --dry-run=server --field-validation=Strict -f -Repository: openshift/images
Length of output: 230
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'File stats:\n'
wc -l ipfailover/keepalived/tests/e2e/testdata/router/ipfailover.yaml 2>/dev/null || true
printf '\nFile contents:\n'
cat -n ipfailover/keepalived/tests/e2e/testdata/router/ipfailover.yaml
printf '\nRelevant manifests/tests references:\n'
rg -n "ipfailover.yaml|hostNetwork|privileged|allowPrivilegeEscalation|runAsNonRoot|readOnlyRootFilesystem|host-slash|lib-modules|ipfailover" ipfailover/keepalived/tests ipfailover/keepalived 2>/dev/null || true
printf '\nKubernetes API docs in vendored/deps? (no-op listing):\n'
git ls-files | rg -i 'kubernetes|schema|api|docs|templates|README|Makefile|Dockerfile' | head -200Repository: openshift/images
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
path = Path("ipfailover/keepalived/tests/e2e/testdata/router/ipfailover.yaml")
text = path.read_text()
lines = text.splitlines()
print("Target lines:")
for i, line in enumerate(lines, start=1):
if 1 <= i <= 100:
# Print lines that would contain hostNetwork/privileged/securityContext volume mount contexts.
if i in range(1, 90):
print(f"{i:3}: {line}")
print("\nPodSpec-level privileged occurrences:")
for i, line in enumerate(lines, start=1):
if re.match(r"^\s{0,6}{0,2}privileged:\s*true\s*$", line):
print(f"line {i}: {line.strip()}")
print("\nContainer securityContext context (lines 30-50):")
for i, line in enumerate(lines, start=1):
if 30 <= i <= 50:
print(f"{i:3}: {line}")
print("\nHost-path volumes below PodSpec:")
for i, line in enumerate(lines, start=1):
s = line.strip()
if s.startswith("hostPath:") or "/host" in s or "lib-modules" in s:
print(f"line {i}: {line.rstrip()}")
print("\nSecurity context hints:")
for field in ["allowPrivilegeEscalation:", "runAsNonRoot:", "readOnlyRootFilesystem:", "capabilities:", "hostNetwork:", "privileged:"]:
matches = [(idx+1, line.rstrip()) for idx, line in enumerate(lines) if field in line]
if matches:
print(field, "=>", matches)
PYRepository: openshift/images
Length of output: 3855
Remove the host-level privilege configuration.
Line 23 sets privileged on PodSpec, where it is not a valid field. Line 35 still runs the container as privileged. Combined with hostNetwork and the / host-path mount, a compromise of this container can modify the node. Remove privileged, hostNetwork, and the host-path mounts unless the test requires each one. If host networking is required, drop ALL capabilities and add only the verified capabilities. Set allowPrivilegeEscalation: false, runAsNonRoot: true, and readOnlyRootFilesystem: true where the image supports them.
🤖 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 `@ipfailover/keepalived/tests/e2e/testdata/router/ipfailover.yaml` around lines
22 - 47, Harden the ipfailover test PodSpec by removing the invalid
PodSpec-level privileged setting, hostNetwork, and host-slash host-path mount
unless each is demonstrably required; retain only required host integrations.
For the remaining container configuration, remove unnecessary privileges and add
allowPrivilegeEscalation: false, runAsNonRoot: true, and readOnlyRootFilesystem:
true where supported, using only verified capabilities if host networking must
remain.
Sources: Coding guidelines, Path instructions
| containers: | ||
| - name: openshift-ipfailover | ||
| image: ${IMAGE} | ||
| ports: | ||
| - containerPort: 63000 | ||
| hostPort: 63000 | ||
| imagePullPolicy: IfNotPresent | ||
| securityContext: | ||
| privileged: true | ||
| volumeMounts: | ||
| - name: lib-modules | ||
| mountPath: /lib/modules | ||
| readOnly: true | ||
| - name: host-slash | ||
| mountPath: /host | ||
| readOnly: true | ||
| mountPropagation: HostToContainer | ||
| - name: etc-sysconfig | ||
| mountPath: /etc/sysconfig | ||
| readOnly: true | ||
| env: | ||
| - name: OPENSHIFT_HA_CONFIG_NAME | ||
| value: "ipfailover" | ||
| - name: OPENSHIFT_HA_VIRTUAL_IPS | ||
| value: ${VIRTUALIPS} | ||
| - name: OPENSHIFT_HA_VIP_GROUPS | ||
| value: "10" | ||
| - name: OPENSHIFT_HA_NETWORK_INTERFACE | ||
| value: ${HAINTERFACE} | ||
| - name: OPENSHIFT_HA_MONITOR_PORT | ||
| value: ${MONITORPORT} | ||
| - name: OPENSHIFT_HA_VRRP_ID_OFFSET | ||
| value: ${VRRP_ID_OFFSET} | ||
| - name: OPENSHIFT_HA_REPLICA_COUNT | ||
| value: "2" | ||
| - name: OPENSHIFT_HA_NFTABLES_RULE | ||
| value: "INPUT" | ||
| - name: OPENSHIFT_HA_PREEMPTION | ||
| value: "nopreempt" | ||
| - name: OPENSHIFT_HA_CHECK_INTERVAL | ||
| value: "5" | ||
| livenessProbe: |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the referenced YAML and nearby container specs.
printf '== file exists and size ==\n'
wc -l ipfailover/keepalived/tests/e2e/testdata/router/ipfailover.yaml 2>/dev/null || true
printf '\n== relevant YAML lines ==\n'
sed -n '1,120p' ipfailover/keepalived/tests/e2e/testdata/router/ipfailover.yaml 2>/dev/null
printf '\n== resource declarations in referenced file ==\n'
rg -n '^\s*resources:|^\s*limits:|^\s*requests:' ipfailover/keepalived/tests/e2e/testdata/router/ipfailover.yaml || echo 'none found'Repository: openshift/images
Length of output: 2871
Add CPU and memory requests and limits.
openshift-ipfailover has no resources configuration. This Kubernetes manifest container should define CPU and memory requests and limits.
🤖 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 `@ipfailover/keepalived/tests/e2e/testdata/router/ipfailover.yaml` around lines
28 - 69, Add a resources block to the openshift-ipfailover container definition,
specifying both CPU and memory requests and limits. Keep the existing container
configuration unchanged and use the repository’s established resource values or
conventions if available.
Source: Path instructions
| livenessProbe: | ||
| initialDelaySeconds: 10 | ||
| exec: | ||
| command: | ||
| - pgrep | ||
| - keepalived |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="ipfailover/keepalived/tests/e2e/testdata/router/ipfailover.yaml"
if [ -f "$file" ]; then
echo "== file exists =="
wc -l "$file"
echo "== relevant sections =="
cat -n "$file" | sed -n '1,140p'
else
echo "missing: $file"
fi
echo "== locate keepalived readyness/liveness probes in manifests =="
rg -n "readinessProbe|livenessProbe|pgrep keepalived|keepalived" -S .Repository: openshift/images
Length of output: 50372
Add a readiness probe.
This Deployment only defines livenessProbe. Absent a readiness probe, Kubernetes considers the container ready as soon as it starts running. Add a readiness probe that checks keepalived initialization state, such as VRRP/VIP readiness or the VRRP controller port.
🤖 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 `@ipfailover/keepalived/tests/e2e/testdata/router/ipfailover.yaml` around lines
69 - 74, Add a readinessProbe alongside the existing livenessProbe in the
Deployment container configuration, using a check that confirms keepalived
initialization and VRRP/VIP readiness (or the VRRP controller port) rather than
merely checking that the process is running. Keep the existing livenessProbe
unchanged.
Source: Path instructions
3 participants
Summary by CodeRabbit
New Features
Documentation
Chores