NO-JIRA: Rename cilium directory to v1.15.1 - #7888
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci-robot
added
the
jira/valid-reference
|
@wewang58: This pull request explicitly references no jira issue. 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. |
WalkthroughUpdated Cilium references to v1.15.1 (docs and manifests), pinned OLM images to digests and removed one env var, increased Cilium e2e timeouts, and applied extensive HyperShift API/aggregated-docs updates (new/modified fields, validation, and platform specs). Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
openshift-ci
Bot
added
do-not-merge/work-in-progress
|
Skipping CI for Draft Pull Request. |
wewang58
changed the title
openshift-ci
Bot
added
area/testing
|
When the pr is merged, will update cilium version to 1.18.6 in openshift/release#75520 too |
jparrill
left a comment
jparrill
left a comment
There was a problem hiding this comment.
Thanks for the PR, I've seen some things that I need to drop here due to I cannot do it in the Files changed tab:
- cluster-network-06-cilium-00002-cilium-olm-deployment.yaml:55 still references the v1.14.5 image:
image: registry.connect.redhat.com/isovalent/cilium-olm:7750223f8f7aba7be99f428c46e6cc91ff9d8145-v1.14.5 - This should reference a v1.18.6 image tag.
- All RELATED_IMAGE_* environment variables (lines 33-54 in the deployment YAML) still point to the old Cilium v1.14.5 image digests.
For a genuine version bump to v1.18.6, these digests should correspond to the 1.18.6 release. 4. Documentation references remain at v1.14.5 in:
- docs/content/reference/aggregated-docs.md (3 occurrences)
- docs/content/how-to/sdn/other-sdn-providers.md:143
|
@jparrill Thanks reviewing, I found no manifests: https://github.com/isovalent/olm-for-cilium/tree/main/manifests for v1.18.6, latest is v1.15.1, Update version to v1.15.1 now. |
677a36d to
36d511e
Compare
wewang58
changed the title
openshift-ci
Bot
added
the
area/documentation
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
test/e2e/util/cilium.go (1)
42-43: Significant timeout increases warrant justification.The default timeout increased 3x (10m → 30m) and the long timeout increased 2.25x (20m → 45m). While these may be necessary for the new Cilium version, such significant increases could mask genuine deployment failures or performance regressions.
Consider adding a comment explaining why these longer timeouts are needed for v1.15.1, or verify that the original timeouts were insufficient through CI logs.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/e2e/util/cilium.go` around lines 42 - 43, The timeout constants ciliumDefaultTimeout and ciliumLongTimeout were raised significantly; update the declaration to include a brief comment explaining why v1.15.1 requires 30m/45m (e.g., known slower startup, CI flakiness, or documented upstream change) and cite the evidence (links or CI log IDs) or, if no justification exists, revert to previous values and add a TODO to re-evaluate after collecting CI failure logs; ensure the comment mentions the specific symbols ciliumDefaultTimeout and ciliumLongTimeout so future reviewers can find the rationale.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/content/how-to/sdn/other-sdn-providers.md`:
- Line 143: Update the PR metadata and all textual references from v1.18.6 to
v1.15.1: change the PR title and PR description to state v1.15.1, and update any
occurrences of the version string "v1.18.6" in the documentation (including the
sentence that currently reads "In this scenario we are using the Cilium version
v1.15.1..." if inconsistent) as well as any manifest URLs referencing the
isovalent/olm-for-cilium repo so they point to the v1.15.1 tag (confirm links
return HTTP 200). Ensure the PR description and any commit messages consistently
mention v1.15.1 and remove or replace any leftover v1.18.6 references.
In `@docs/content/reference/aggregated-docs.md`:
- Around line 2807-2810: Update the Cilium version pins in the source docs file
docs/content/how-to/sdn/other-sdn-providers.md (not the generated
docs/content/reference/aggregated-docs.md) to replace v1.15.1 with the
PR-intended v1.18.6 and update the upstream manifest URLs to point at the
authoritative cilium/cilium manifests for v1.18.6 (ensure you replace
isovalent/olm-for-cilium links that 404), then regenerate the aggregated docs so
the changes propagate to docs/content/reference/aggregated-docs.md (also check
and update the other affected aggregated-docs occurrences mentioned in the
comment).
- Line 2783: Update the Cilium reference from v1.15.1 to v1.18.6 in the Cilium
section (current text containing "v1.15.1" and the sentence "last one at the
time of this writing"), and update any shell/snippet version pinning that uses
"1.15.1" to "1.18.6" in docs/content/how-to/sdn/other-sdn-providers.md so the
change flows into aggregated-docs.md; also remove or reword "last one at the
time of this writing" to a version-neutral phrase (or explicitly state v1.18.6)
and, before committing, confirm that isovalent/olm-for-cilium publishes v1.18.6
upstream.
---
Nitpick comments:
In `@test/e2e/util/cilium.go`:
- Around line 42-43: The timeout constants ciliumDefaultTimeout and
ciliumLongTimeout were raised significantly; update the declaration to include a
brief comment explaining why v1.15.1 requires 30m/45m (e.g., known slower
startup, CI flakiness, or documented upstream change) and cite the evidence
(links or CI log IDs) or, if no justification exists, revert to previous values
and add a TODO to re-evaluate after collecting CI failure logs; ensure the
comment mentions the specific symbols ciliumDefaultTimeout and ciliumLongTimeout
so future reviewers can find the rationale.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e2aa797a-2929-45d5-8324-7c4b398f7c3b
📒 Files selected for processing (17)
docs/content/how-to/sdn/other-sdn-providers.mddocs/content/reference/aggregated-docs.mdtest/e2e/assets/cilium/v1.15.1/cluster-network-03-cilium-ciliumconfigs-crd.yamltest/e2e/assets/cilium/v1.15.1/cluster-network-06-cilium-00000-cilium-namespace.yamltest/e2e/assets/cilium/v1.15.1/cluster-network-06-cilium-00001-cilium-olm-serviceaccount.yamltest/e2e/assets/cilium/v1.15.1/cluster-network-06-cilium-00002-cilium-olm-deployment.yamltest/e2e/assets/cilium/v1.15.1/cluster-network-06-cilium-00003-cilium-olm-service.yamltest/e2e/assets/cilium/v1.15.1/cluster-network-06-cilium-00004-cilium-olm-leader-election-role.yamltest/e2e/assets/cilium/v1.15.1/cluster-network-06-cilium-00005-cilium-olm-role.yamltest/e2e/assets/cilium/v1.15.1/cluster-network-06-cilium-00006-leader-election-rolebinding.yamltest/e2e/assets/cilium/v1.15.1/cluster-network-06-cilium-00007-cilium-olm-rolebinding.yamltest/e2e/assets/cilium/v1.15.1/cluster-network-06-cilium-00008-cilium-cilium-olm-clusterrole.yamltest/e2e/assets/cilium/v1.15.1/cluster-network-06-cilium-00009-cilium-cilium-clusterrole.yamltest/e2e/assets/cilium/v1.15.1/cluster-network-06-cilium-00010-cilium-cilium-olm-clusterrolebinding.yamltest/e2e/assets/cilium/v1.15.1/cluster-network-06-cilium-00011-cilium-cilium-clusterrolebinding.yamltest/e2e/assets/cilium/v1.15.1/connectivity-check.yamltest/e2e/util/cilium.go
| version="1.15.1" | ||
| oc apply -f https://raw.githubusercontent.com/isovalent/olm-for-cilium/main/manifests/cilium.v${version}/cluster-network-03-cilium-ciliumconfigs-crd.yaml | ||
| oc apply -f https://raw.githubusercontent.com/isovalent/olm-for-cilium/main/manifests/cilium.v${version}/cluster-network-06-cilium-00000-cilium-namespace.yaml | ||
| oc apply -f https://raw.githubusercontent.com/isovalent/olm-for-cilium/main/manifests/cilium.v${version}/cluster-network-06-cilium-00001-cilium-olm-serviceaccount.yaml |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Checking stale version pins in docs..."
rg -n 'v1\.15\.1|version="1\.15\.1"|last one at the time of this writing'
echo
echo "Checking whether upstream OLM manifests exist for v1.18.6..."
curl -fsSI \
'https://raw.githubusercontent.com/isovalent/olm-for-cilium/main/manifests/cilium.v1.18.6/cluster-network-03-cilium-ciliumconfigs-crd.yaml' \
|| true
echo
echo "Checking whether upstream connectivity-check exists for v1.18.6..."
curl -fsSI \
'https://raw.githubusercontent.com/cilium/cilium/v1.18.6/examples/kubernetes/connectivity-check/connectivity-check.yaml' \
|| trueRepository: openshift/hypershift
Length of output: 7001
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Checking if aggregated-docs.md is generated ==="
head -30 docs/content/reference/aggregated-docs.md
echo
echo "=== Looking for source doc generation scripts ==="
fd -t f "\.py|\.sh|Makefile" | xargs grep -l "aggregated-docs" 2>/dev/null || echo "No generation scripts found in search"
echo
echo "=== Checking other-sdn-providers.md source ==="
head -30 docs/content/how-to/sdn/other-sdn-providers.md
echo
echo "=== Finding Cilium version references in non-vendor source ==="
rg -t md 'cilium.*1\.15\.1|v1\.15\.1.*cilium' docs/ || echo "No additional references found"Repository: openshift/hypershift
Length of output: 3567
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Checking current Cilium versions available from isovalent/olm-for-cilium ==="
curl -s https://api.github.com/repos/isovalent/olm-for-cilium/contents/manifests \
| jq -r '.[] | select(.type=="dir") | .name' | grep -o 'v[0-9.]*' | sort -V | tail -5
echo
echo "=== Checking what versions are available in cilium/cilium main branch ==="
curl -s https://api.github.com/repos/cilium/cilium/branches \
| jq -r '.[] | .name' | grep -E '^v[0-9]+\.[0-9]+\.[0-9]+$' | sort -V | tail -5
echo
echo "=== Checking docs for version update guidance ==="
rg -A 5 -B 5 'version.*update|update.*version' docs/content/how-to/sdn/other-sdn-providers.md | head -40Repository: openshift/hypershift
Length of output: 226
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== List cilium manifest directories in isovalent/olm-for-cilium main ==="
curl -s 'https://api.github.com/repos/isovalent/olm-for-cilium/contents/manifests?ref=main' \
2>/dev/null | grep '"name"' | grep -o 'cilium\.[^"]*' | sort -V | tail -10 || echo "API call failed"
echo
echo "=== Check if source doc has any guidance on updating versions ==="
cat docs/content/how-to/sdn/other-sdn-providers.md | head -100 | tail -50
echo
echo "=== Verify that 1.18.6 is the version mentioned in PR/release context ==="
rg -i '1\.18\.6|latest.*cilium|cilium.*latest' docs/ | head -10
echo
echo "=== Check if docs suggest using dynamic/latest approach ==="
rg -i 'latest|current|stable' docs/content/how-to/sdn/other-sdn-providers.md | head -10Repository: openshift/hypershift
Length of output: 3244
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Examining the full Cilium section in other-sdn-providers.md ==="
sed -n '143,500p' docs/content/how-to/sdn/other-sdn-providers.md | head -120
echo
echo "=== Checking if there are any TODOs or FIXMEs regarding versions ==="
rg -i 'todo|fixme|update.*version|stale' docs/content/how-to/sdn/other-sdn-providers.md
echo
echo "=== Direct check: does v1.15.1 exist in isovalent/olm-for-cilium? ==="
curl -fsSI 'https://raw.githubusercontent.com/isovalent/olm-for-cilium/main/manifests/cilium.v1.15.1/cluster-network-03-cilium-ciliumconfigs-crd.yaml' | head -1
echo
echo "=== Check what versions ARE available in isovalent/olm-for-cilium ==="
for v in 1.14.0 1.15.0 1.15.1 1.16.0 1.17.0 1.18.0; do
status=$(curl -fsSo /dev/null -w "%{http_code}" "https://raw.githubusercontent.com/isovalent/olm-for-cilium/main/manifests/cilium.v${v}/cluster-network-03-cilium-ciliumconfigs-crd.yaml")
echo "v${v}: HTTP $status"
doneRepository: openshift/hypershift
Length of output: 5170
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Check what versions ARE available in isovalent/olm-for-cilium ==="
for v in 1.14.0 1.14.5 1.15.0 1.15.1 1.16.0 1.17.0 1.18.0 1.18.6; do
status=$(curl -fsSo /dev/null -w "%{http_code}" "https://raw.githubusercontent.com/isovalent/olm-for-cilium/main/manifests/cilium.v${v}/cluster-network-03-cilium-ciliumconfigs-crd.yaml" 2>&1)
echo "v${v}: HTTP $status"
done
echo
echo "=== Check if cilium v1.15.1 is actually available ==="
curl -fsSI 'https://raw.githubusercontent.com/isovalent/olm-for-cilium/main/manifests/cilium.v1.15.1/cluster-network-03-cilium-ciliumconfigs-crd.yaml' 2>&1 | head -5
echo
echo "=== Summary of review findings ==="
echo "1. v1.15.1 pins found in aggregated-docs.md and other-sdn-providers.md: CONFIRMED"
echo "2. aggregated-docs.md is generated file: CONFIRMED"
echo "3. v1.18.6 OLM manifests do NOT exist (404): CONFIRMED"
echo "4. v1.15.1 is the current pin in source docs: CONFIRMED"
echo "5. Testing if v1.15.1 itself is accessible..."Repository: openshift/hypershift
Length of output: 134
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Check source doc directly to understand context ==="
cat docs/content/how-to/sdn/other-sdn-providers.md | grep -A 2 -B 2 "1.15.1"
echo
echo "=== List all files with 1.15.1 references (non-vendor) ==="
rg -l 'v1\.15\.1|1\.15\.1' --type md docs/
echo
echo "=== Check Cilium project releases to find available versions ==="
curl -s https://api.github.com/repos/cilium/cilium/releases | python3 << 'EOF'
import json, sys
try:
data = json.load(sys.stdin)
versions = [r['tag_name'] for r in data if not r['prerelease'] and not r['draft']][:15]
for v in sorted(versions, reverse=True):
print(v)
except:
print("Failed to parse")
EOFRepository: openshift/hypershift
Length of output: 1283
Update Cilium version pins from v1.15.1 to match the version shipped by this PR (v1.18.6)
The documentation currently pins Cilium to v1.15.1, which will install an outdated release instead of the version this PR intends to ship. This is risky given that the PR addresses failures in older OLM flows (1.14.5).
Additionally, the upstream isovalent/olm-for-cilium repository does not publish OLM manifests for v1.18.6 (returns HTTP 404), so you'll need to point to the authoritative manifests available in the upstream repository. The Cilium connectivity-check examples do exist for v1.18.6 in cilium/cilium.
Since docs/content/reference/aggregated-docs.md is a generated file, update the source documentation at docs/content/how-to/sdn/other-sdn-providers.md and regenerate to avoid reintroducing the stale pins.
Also applies to: docs/content/reference/aggregated-docs.md lines 3062, 6846, 7101, 21798, 22053 and docs/content/how-to/sdn/other-sdn-providers.md lines 167, 422
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/content/reference/aggregated-docs.md` around lines 2807 - 2810, Update
the Cilium version pins in the source docs file
docs/content/how-to/sdn/other-sdn-providers.md (not the generated
docs/content/reference/aggregated-docs.md) to replace v1.15.1 with the
PR-intended v1.18.6 and update the upstream manifest URLs to point at the
authoritative cilium/cilium manifests for v1.18.6 (ensure you replace
isovalent/olm-for-cilium links that 404), then regenerate the aggregated docs so
the changes propagate to docs/content/reference/aggregated-docs.md (also check
and update the other affected aggregated-docs occurrences mentioned in the
comment).
|
/test e2e-azure-self-managed |
36d511e to
37b9de9
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/e2e/util/cilium.go (1)
42-43: Scope the longer wait to install-only paths.Line 42 and Line 43 are also used by the post-install readiness checks later in this file, so this change makes unrelated failures take 2-3x longer to surface. Since the regression described in the PR is on operator install, I'd keep the existing defaults and introduce a dedicated install timeout for the waits at Line 215 and Line 323 instead.
♻️ Proposed change
- ciliumDefaultTimeout = 30 * time.Minute - ciliumLongTimeout = 45 * time.Minute + ciliumInstallTimeout = 30 * time.Minute + ciliumDefaultTimeout = 10 * time.Minute + ciliumLongTimeout = 20 * time.MinuteUse
ciliumInstallTimeoutfor the install-specific waits, and leave the existing readiness/connectivity timeouts unchanged.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/e2e/util/cilium.go` around lines 42 - 43, Add a new constant ciliumInstallTimeout (e.g. 45 * time.Minute) and use it for the install-specific wait calls instead of reusing ciliumLongTimeout; leave ciliumDefaultTimeout and ciliumLongTimeout unchanged so post-install readiness/connectivity checks still use the original defaults. Locate the two install-specific wait calls referenced in the review (the install-phase waits) and swap their timeout argument to ciliumInstallTimeout while keeping all other timeout usages (reads of ciliumDefaultTimeout and ciliumLongTimeout) as-is.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@test/e2e/util/cilium.go`:
- Around line 42-43: Add a new constant ciliumInstallTimeout (e.g. 45 *
time.Minute) and use it for the install-specific wait calls instead of reusing
ciliumLongTimeout; leave ciliumDefaultTimeout and ciliumLongTimeout unchanged so
post-install readiness/connectivity checks still use the original defaults.
Locate the two install-specific wait calls referenced in the review (the
install-phase waits) and swap their timeout argument to ciliumInstallTimeout
while keeping all other timeout usages (reads of ciliumDefaultTimeout and
ciliumLongTimeout) as-is.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 765180b9-94ed-49a5-a2c2-7e62e9b10ab7
📒 Files selected for processing (17)
docs/content/how-to/sdn/other-sdn-providers.mddocs/content/reference/aggregated-docs.mdtest/e2e/assets/cilium/v1.15.1/cluster-network-03-cilium-ciliumconfigs-crd.yamltest/e2e/assets/cilium/v1.15.1/cluster-network-06-cilium-00000-cilium-namespace.yamltest/e2e/assets/cilium/v1.15.1/cluster-network-06-cilium-00001-cilium-olm-serviceaccount.yamltest/e2e/assets/cilium/v1.15.1/cluster-network-06-cilium-00002-cilium-olm-deployment.yamltest/e2e/assets/cilium/v1.15.1/cluster-network-06-cilium-00003-cilium-olm-service.yamltest/e2e/assets/cilium/v1.15.1/cluster-network-06-cilium-00004-cilium-olm-leader-election-role.yamltest/e2e/assets/cilium/v1.15.1/cluster-network-06-cilium-00005-cilium-olm-role.yamltest/e2e/assets/cilium/v1.15.1/cluster-network-06-cilium-00006-leader-election-rolebinding.yamltest/e2e/assets/cilium/v1.15.1/cluster-network-06-cilium-00007-cilium-olm-rolebinding.yamltest/e2e/assets/cilium/v1.15.1/cluster-network-06-cilium-00008-cilium-cilium-olm-clusterrole.yamltest/e2e/assets/cilium/v1.15.1/cluster-network-06-cilium-00009-cilium-cilium-clusterrole.yamltest/e2e/assets/cilium/v1.15.1/cluster-network-06-cilium-00010-cilium-cilium-olm-clusterrolebinding.yamltest/e2e/assets/cilium/v1.15.1/cluster-network-06-cilium-00011-cilium-cilium-clusterrolebinding.yamltest/e2e/assets/cilium/v1.15.1/connectivity-check.yamltest/e2e/util/cilium.go
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/content/how-to/sdn/other-sdn-providers.md
|
TestCiliumConnectivity is passed in debug job |
openshift-ci
Bot
removed
the
do-not-merge/work-in-progress
37b9de9 to
6f8e812
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/e2e/util/cilium.go (1)
42-43: Scope the longer timeout to install-specific waits.These constants are reused by the post-install checks too, so a broken run can now spend 30–45 minutes polling in several later phases. A separate install timeout would keep the 1.15.1 rollout tolerant without stretching unrelated failure paths.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/e2e/util/cilium.go` around lines 42 - 43, The long timeout constants ciliumDefaultTimeout and ciliumLongTimeout are too broadly applied; introduce a new constant (e.g., ciliumInstallTimeout) with the longer duration (45 * time.Minute) and use it only for install-specific waits (referencing install-related functions/variables that currently use ciliumLongTimeout such as the installer/poller calls in test/e2e/util/cilium.go), while leaving post-install checks to continue using ciliumDefaultTimeout; update the call sites in the install flow to use ciliumInstallTimeout and keep post-install polling untouched.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@test/e2e/util/cilium.go`:
- Around line 42-43: The long timeout constants ciliumDefaultTimeout and
ciliumLongTimeout are too broadly applied; introduce a new constant (e.g.,
ciliumInstallTimeout) with the longer duration (45 * time.Minute) and use it
only for install-specific waits (referencing install-related functions/variables
that currently use ciliumLongTimeout such as the installer/poller calls in
test/e2e/util/cilium.go), while leaving post-install checks to continue using
ciliumDefaultTimeout; update the call sites in the install flow to use
ciliumInstallTimeout and keep post-install polling untouched.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c3ab0c98-5083-4fef-a153-7fc7c6b9d916
📒 Files selected for processing (17)
docs/content/how-to/sdn/other-sdn-providers.mddocs/content/reference/aggregated-docs.mdtest/e2e/assets/cilium/v1.15.1/cluster-network-03-cilium-ciliumconfigs-crd.yamltest/e2e/assets/cilium/v1.15.1/cluster-network-06-cilium-00000-cilium-namespace.yamltest/e2e/assets/cilium/v1.15.1/cluster-network-06-cilium-00001-cilium-olm-serviceaccount.yamltest/e2e/assets/cilium/v1.15.1/cluster-network-06-cilium-00002-cilium-olm-deployment.yamltest/e2e/assets/cilium/v1.15.1/cluster-network-06-cilium-00003-cilium-olm-service.yamltest/e2e/assets/cilium/v1.15.1/cluster-network-06-cilium-00004-cilium-olm-leader-election-role.yamltest/e2e/assets/cilium/v1.15.1/cluster-network-06-cilium-00005-cilium-olm-role.yamltest/e2e/assets/cilium/v1.15.1/cluster-network-06-cilium-00006-leader-election-rolebinding.yamltest/e2e/assets/cilium/v1.15.1/cluster-network-06-cilium-00007-cilium-olm-rolebinding.yamltest/e2e/assets/cilium/v1.15.1/cluster-network-06-cilium-00008-cilium-cilium-olm-clusterrole.yamltest/e2e/assets/cilium/v1.15.1/cluster-network-06-cilium-00009-cilium-cilium-clusterrole.yamltest/e2e/assets/cilium/v1.15.1/cluster-network-06-cilium-00010-cilium-cilium-olm-clusterrolebinding.yamltest/e2e/assets/cilium/v1.15.1/cluster-network-06-cilium-00011-cilium-cilium-clusterrolebinding.yamltest/e2e/assets/cilium/v1.15.1/connectivity-check.yamltest/e2e/util/cilium.go
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/content/how-to/sdn/other-sdn-providers.md
This commit updates the cilium version from 1.14.5 to 1.15.1 in the documentation and e2e test assets. It also increases the default and long timeouts in the cilium e2e util.
6f8e812 to
dc7b179
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
docs/content/how-to/sdn/other-sdn-providers.md (1)
143-143: Clarify what “last one” refers to.This reads as if
v1.15.1were the latest Cilium release. I’d tighten it to something like “the latest version available in the referenced Isovalent OLM manifests” so the doc is less ambiguous.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/content/how-to/sdn/other-sdn-providers.md` at line 143, Update the sentence that currently reads "In this scenario we are using the Cilium version v1.15.1 which is the last one at the time of this writing." to explicitly state what "last one" refers to; e.g., say "v1.15.1, the latest version available in the referenced Isovalent OLM manifests" (or similar) so readers understand the version is relative to the OLM manifests rather than implying it is the overall latest Cilium release; modify the line containing the version reference to replace the ambiguous phrase with this clarified wording.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/content/reference/aggregated-docs.md`:
- Line 2783: The sentence "Cilium version v1.15.1 which is the last one at the
time of this writing" is time-sensitive and should be removed from the generated
doc; update the source document that generates
docs/content/reference/aggregated-docs.md to replace that phrase with a neutral
statement such as "Cilium version v1.15.1 (tested)" or a note to "verify the
latest supported manifest upstream", then regenerate the aggregated docs so the
change applies to the instances referenced (the occurrences containing "Cilium
version v1.15.1" and the exact phrase "last one at the time of this writing").
---
Nitpick comments:
In `@docs/content/how-to/sdn/other-sdn-providers.md`:
- Line 143: Update the sentence that currently reads "In this scenario we are
using the Cilium version v1.15.1 which is the last one at the time of this
writing." to explicitly state what "last one" refers to; e.g., say "v1.15.1, the
latest version available in the referenced Isovalent OLM manifests" (or similar)
so readers understand the version is relative to the OLM manifests rather than
implying it is the overall latest Cilium release; modify the line containing the
version reference to replace the ambiguous phrase with this clarified wording.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b2f6967d-0eb7-48ed-b91b-5ee3c981fb66
📒 Files selected for processing (17)
docs/content/how-to/sdn/other-sdn-providers.mddocs/content/reference/aggregated-docs.mdtest/e2e/assets/cilium/v1.15.1/cluster-network-03-cilium-ciliumconfigs-crd.yamltest/e2e/assets/cilium/v1.15.1/cluster-network-06-cilium-00000-cilium-namespace.yamltest/e2e/assets/cilium/v1.15.1/cluster-network-06-cilium-00001-cilium-olm-serviceaccount.yamltest/e2e/assets/cilium/v1.15.1/cluster-network-06-cilium-00002-cilium-olm-deployment.yamltest/e2e/assets/cilium/v1.15.1/cluster-network-06-cilium-00003-cilium-olm-service.yamltest/e2e/assets/cilium/v1.15.1/cluster-network-06-cilium-00004-cilium-olm-leader-election-role.yamltest/e2e/assets/cilium/v1.15.1/cluster-network-06-cilium-00005-cilium-olm-role.yamltest/e2e/assets/cilium/v1.15.1/cluster-network-06-cilium-00006-leader-election-rolebinding.yamltest/e2e/assets/cilium/v1.15.1/cluster-network-06-cilium-00007-cilium-olm-rolebinding.yamltest/e2e/assets/cilium/v1.15.1/cluster-network-06-cilium-00008-cilium-cilium-olm-clusterrole.yamltest/e2e/assets/cilium/v1.15.1/cluster-network-06-cilium-00009-cilium-cilium-clusterrole.yamltest/e2e/assets/cilium/v1.15.1/cluster-network-06-cilium-00010-cilium-cilium-olm-clusterrolebinding.yamltest/e2e/assets/cilium/v1.15.1/cluster-network-06-cilium-00011-cilium-cilium-clusterrolebinding.yamltest/e2e/assets/cilium/v1.15.1/connectivity-check.yamltest/e2e/util/cilium.go
🚧 Files skipped from review as they are similar to previous changes (1)
- test/e2e/util/cilium.go
| ### Deployment | ||
|
|
||
| In this scenario we are using the Cilium version v1.14.5 which is the last one at the time of this writing. The steps followed rely on the docs by Cilium project to deploy Cilium on OpenShift. | ||
| In this scenario we are using the Cilium version v1.15.1 which is the last one at the time of this writing. The steps followed rely on the docs by Cilium project to deploy Cilium on OpenShift. |
There was a problem hiding this comment.
Drop the “last one at the time of this writing” wording.
That sentence is inherently time-sensitive and will go stale again. Prefer either a neutral statement about the tested version or guidance to verify the latest supported manifest version upstream. Since this file is generated, please fix the source doc and regenerate.
Also applies to: 6822-6822, 21774-21774
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/content/reference/aggregated-docs.md` at line 2783, The sentence "Cilium
version v1.15.1 which is the last one at the time of this writing" is
time-sensitive and should be removed from the generated doc; update the source
document that generates docs/content/reference/aggregated-docs.md to replace
that phrase with a neutral statement such as "Cilium version v1.15.1 (tested)"
or a note to "verify the latest supported manifest upstream", then regenerate
the aggregated docs so the change applies to the instances referenced (the
occurrences containing "Cilium version v1.15.1" and the exact phrase "last one
at the time of this writing").
|
/retest |
|
Seems failed e2e-azure-self-managed job is not related to my pr |
|
/retest |
|
/lgtm Where can we see the passing tests? |
|
Scheduling tests matching the |
@mgencur Here's result log in my debug pr, when the pr is merged, it can be runned in clilium ci |
|
@wewang58 Thanks. In the debug PR, there are a number of tests failing. Is the resolution tracked somewhere else? What is the next step with those errors? |
Some failure are not related to the cilium, some e2e should update
@mgencur some e2e should be updated when is runned in cilium cni, I will create related bugs to track, and fix these e2e later, just like: https://redhat.atlassian.net/browse/OCPBUGS-65957 |
|
/retest-required |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jparrill, wewang58 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
|
/verified by @wewang58 |
openshift-ci-robot
added
the
verified
|
@wewang58: This PR has been marked as verified by 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. |
|
@wewang58: all tests passed! 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. |
4 participants
What this PR does / why we need it:
e2e failed with installation for cilium olm with version 1.14.5, Updated to 1.15.1
Which issue(s) this PR fixes:
Fixes cilium-olm operator was trying to deploy Cilium, but the cluster's API server was publishing a broken OpenAPI schema for OLM types, causing the operator's internal validation to fail
"""
"level": "ERROR",
"ts": "2026-03-04T03:55:53.549Z",
"msg": "Release failed",
"error": "failed to install release: unable to build kubernetes objects from release manifest:
SchemaError(com.github.operator-framework.operator-lifecycle-manager.pkg.package-server.apis.operators.v1.CSVDescription.apiservicedefinitions): unknown model in reference:
"github.com
1operator-framework1api1pkg1operators~1v1alpha1.APIServiceDefinitions"""""
Special notes for your reviewer:
Checklist:
Summary by CodeRabbit
Documentation
Chores