OCPBUGS-99649: Reinstall tzdata if /usr/share/zoneinfo is missing#246
Conversation
openshift-ci-robot
added
jira/valid-reference
|
@sdodson: This pull request references Jira Issue OCPBUGS-99649, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
The bug has been updated to refer to the pull request using the external bug tracker. 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. |
openshift-ci
Bot
added
the
needs-rebase
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: openshift/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (3)
Cache: Disabled due to data retention organization setting Knowledge base: Disabled due to data retention organization setting WalkthroughThe base, RHEL, and RHEL9 Dockerfiles conditionally ensure timezone data is installed. The RHEL9 Dockerfile also adds a builder stage that configures ChangesBase image updates
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant RHEL9Builder
participant UpdateCryptoPolicies
participant FinalRHEL9Image
RHEL9Builder->>UpdateCryptoPolicies: Apply DEFAULT:PQ policy on non-CentOS builds
UpdateCryptoPolicies-->>RHEL9Builder: Generate crypto-policy configuration
RHEL9Builder->>FinalRHEL9Image: Copy /etc/crypto-policies/
Suggested reviewers: 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: sdodson 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
|
@sdodson: This pull request references Jira Issue OCPBUGS-99649, which is valid. 3 validation(s) were run on this bug
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. |
25a070b to
e6f71a8
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@base/Dockerfile`:
- Line 17: Add a Dockerfile HEALTHCHECK for the final image, using an
appropriate command that verifies the container’s primary service is healthy and
returns a failure status when it is not. Place it with the final image
configuration alongside the existing runtime instructions.
- Line 17: Convert base/Dockerfile lines 17-17 and base/Dockerfile.rhel lines
20-20 to multi-stage builds with distinct builder and final stages. Install and
use build tooling only in the builder stages, then copy only the required
runtime artifacts into the final images so no build tools remain.
In `@base/Dockerfile.rhel9`:
- Around line 9-12: Add a HEALTHCHECK instruction to the final image definition
in Dockerfile.rhel9, after the base image and configuration copy setup, using a
command that verifies the container's primary service is responsive and returns
a nonzero status when unhealthy.
- Around line 9-12: Set an explicit non-root USER after privileged setup in
base/Dockerfile.rhel9 (lines 9-12), and add a non-root USER to the final image
in base/Dockerfile (line 17) and base/Dockerfile.rhel (line 20); ensure all
three images never run as root.
- Around line 9-12: Define the required HEALTHCHECK in each final image: add it
to base/Dockerfile.rhel9 near the final image setup, and add the equivalent
required health check to base/Dockerfile and base/Dockerfile.rhel at the
specified sites. Use a consistent valid health-check command across all three
Dockerfiles.
🪄 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: b628f943-569d-4a97-9113-41cca0d2fe7e
📒 Files selected for processing (3)
base/Dockerfilebase/Dockerfile.rhelbase/Dockerfile.rhel9
openshift-ci
Bot
removed
the
needs-rebase
ubi8-minimal and ubi9-minimal keep the tzdata package installed but strip /usr/share/zoneinfo from the image to save space. Detect that and reinstall tzdata to restore it. Note: RHEL10 minimal images do not ship tzdata by design, so this check should not be extended to re-add it there. Assuming anyone who has already moved to RHEL10 has tested successfully without it. OCPBUGS-99649
e6f71a8 to
906fa34
Compare
|
@coderabbitai please update the walkthrough as commits were removed |
|
✅ Action performedSummary regeneration triggered. |
|
@coderabbitai your summary and walk through are still out of sync with the current commits in this PR. |
|
✅ Action performedSummary regeneration triggered. |
|
/retest-required |
|
/override ci/prow/e2e-aws-upgrade |
|
@sdodson: Overrode contexts on behalf of sdodson: ci/prow/e2e-aws-upgrade 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 kubernetes-sigs/prow repository. |
|
@sdodson: 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. |
|
@sdodson: Jira Issue OCPBUGS-99649: All pull requests linked via external trackers have merged: Jira Issue OCPBUGS-99649 has been moved to the MODIFIED state. 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. |
|
Moving forward with this change unilaterally, need to be able to measure this success by Monday. Or more importantly prove that it didn't negatively impact anything or break the build. |
|
/cherry-pick release-4.22 release-4.21 release-4.20 |
|
@sdodson: new pull request created: #247 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 kubernetes-sigs/prow repository. |
3 participants
Summary
base/Dockerfile,base/Dockerfile.rhel, andbase/Dockerfile.rhel9, check for/usr/share/zoneinfoafter package install and reinstalltzdataif it's absent, falling back to a plain install ifreinstallfails (e.g. microdnf's stricter reinstall semantics, ortzdatamissing from the rpm db entirely).tzdatapackage installed but strip/usr/share/zoneinfofrom the image to save space. This detects that condition and reinstallstzdatato restore it.Note: RHEL10 minimal images do not ship
tzdataby design, so this check should not be extended to re-add it there. Assuming anyone who has already moved to RHEL10 has tested successfully without it.Test plan
/usr/share/zoneinfois presentFixes OCPBUGS-99649
Summary by CodeRabbit
/usr/share/zoneinfois available.