Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jhernand 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 |
WalkthroughThis change removes the Helm hook job that previously ran after install and upgrade to patch the Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~4 minutes Suggested reviewers
Poem
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error)
✅ Passed checks (10 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
jhernand
changed the title
|
@jhernand: 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. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@base/osac-aap`:
- Line 1: The osac-aap image tag in the kustomization configuration is out of
sync with the submodule commit pointer. Locate the osac-aap image entry with
newName set to ghcr.io/osac-project/osac-aap and update its newTag field from
sha-f130cf4 to sha-1ada6eb to match the current submodule commit. This ensures
the image tag and submodule pointer remain synchronized as required by the
coding guidelines.
🪄 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: osac-project/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: aeba05fb-6994-46b6-b80f-fa7ad812f93e
📒 Files selected for processing (5)
base/kustomization.yamlbase/osac-aapbase/osac-fulfillment-servicebase/osac-operatorcharts/osac/templates/hooks/patch-grpc-probes.yaml
💤 Files with no reviewable changes (1)
- charts/osac/templates/hooks/patch-grpc-probes.yaml
| @@ -1 +1 @@ | |||
| Subproject commit f130cf4504090609a70d9896aee18cdf56e2530e | |||
| Subproject commit 1ada6eb443c45d0848a91bd81a2222cd6334081f | |||
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python - <<'PY'
import re
import subprocess
import sys
commit = subprocess.check_output(
["git", "submodule", "status", "base/osac-aap"],
text=True,
).split()[0].lstrip(" +-")
expected = f"sha-{commit[:7]}"
with open("base/kustomization.yaml", encoding="utf-8") as fh:
lines = fh.read().splitlines()
in_images = False
entries = []
current = None
for lineno, line in enumerate(lines, 1):
if re.match(r"^\s*images:\s*$", line):
in_images = True
continue
if in_images and re.match(r"^\S", line) and not re.match(r"^\s*-", line):
break
if not in_images:
continue
name = re.match(r"^\s*-\s*name:\s*[\"']?([^\"']+)[\"']?\s*$", line)
if name:
if current:
entries.append(current)
current = {"name": name.group(1), "newTag": None, "lines": [f"{lineno}:{line}"]}
continue
if current:
current["lines"].append(f"{lineno}:{line}")
tag = re.match(r"^\s*newTag:\s*[\"']?([^\"']+)[\"']?\s*$", line)
if tag:
current["newTag"] = tag.group(1)
if current:
entries.append(current)
matches = [entry for entry in entries if entry["name"] == "osac-aap"]
if not matches:
print("No osac-aap image entry found in base/kustomization.yaml", file=sys.stderr)
sys.exit(1)
for entry in matches:
print("\n".join(entry["lines"]))
if entry["newTag"] == expected:
print(f"OK: osac-aap newTag matches {expected}")
sys.exit(0)
print(
f"Expected osac-aap newTag {expected}, found {[entry['newTag'] for entry in matches]}",
file=sys.stderr,
)
sys.exit(2)
PYRepository: osac-project/osac-installer
Length of output: 314
Update osac-aap image tag to match submodule commit.
The submodule pointer was moved to 1ada6eb but the image tag in base/kustomization.yaml (line 34) is still sha-f130cf4. Update it to sha-1ada6eb to maintain the submodule/image sync contract. Per the coding guidelines, "Image tags in base/kustomization.yaml must match the submodule commit SHAs... After updating a submodule pointer, update the corresponding image tag."
Current state
32:- name: osac-aap
33: newName: ghcr.io/osac-project/osac-aap
34: newTag: sha-f130cf4
🤖 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 `@base/osac-aap` at line 1, The osac-aap image tag in the kustomization
configuration is out of sync with the submodule commit pointer. Locate the
osac-aap image entry with newName set to ghcr.io/osac-project/osac-aap and
update its newTag field from sha-f130cf4 to sha-1ada6eb to match the current
submodule commit. This ensures the image tag and submodule pointer remain
synchronized as required by the coding guidelines.
Source: Coding guidelines
The fulfillment-service upstream Helm charts now include increased probe timeouts (`timeoutSeconds` and `failureThreshold`) directly, as per osac-project/fulfillment-service#743. This makes the post-install hook that patched the gRPC server deployment unnecessary, so remove it. Assisted-by: Cursor Signed-off-by: Juan Hernandez <juan.hernandez@redhat.com>
ef0eb46 to
3d27703
Compare
jhernand
changed the title
|
💀 CI Triage: Root cause: The publish-templates AAP job failed with a 403 Forbidden because Authorino enforced an unpatched AuthConfig after PR 313 removed the osac-patch-authconfig hook. Explanation: The boot step failed during the refresh phase when the Evidence:
Suggestion: Rebase the PR on the latest main branch (which includes PR 317) and retrigger the tests. Prow job | Build For deeper investigation, use the |
|
/retest |
3 participants
Summary
Removes the
patch-grpc-probespost-install/post-upgrade Helm hook,which is no longer needed since the fulfillment-service upstream charts
now include increased gRPC probe timeouts directly.
Reference: osac-project/fulfillment-service#743
Test plan
kustomize-build-all.shpasseshelm templaterenders without the removed hookSummary by CodeRabbit
Release Notes