OCPBUGS-83863: Remove RHEL 8 binary build, strip debug info#3149
OCPBUGS-83863: Remove RHEL 8 binary build, strip debug info#3149sdodson wants to merge 2 commits into
Conversation
|
@sdodson: This pull request references Jira Issue OCPBUGS-83870, 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. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
WalkthroughRemoved the RHEL‑8 build stage and RHEL‑8 artifact copies from the Dockerfile. The builder now adds LDFLAGS="-w -s" to built binaries. The build script unconditionally appends ${LDFLAGS} to -ldflags for native and Windows builds; final image no longer installs RHEL‑8-specific shims or ovnkube-trace. ChangesDockerfile and build script
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@sdodson: This pull request references Jira Issue OCPBUGS-83870, 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. |
|
@sdodson: This pull request references Jira Issue OCPBUGS-83868, which is valid. 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. |
|
@sdodson: This pull request references Jira Issue OCPBUGS-83863, which is valid. 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. |
|
/hold |
Drop OS detection and version-specific directory probing from both cnibincopy scripts (multus.yaml and 008-script-lib.yaml) entirely. multus.yaml: - Consolidate RHEL8_SOURCE_DIRECTORY, RHEL9_SOURCE_DIRECTORY, and DEFAULT_SOURCE_DIRECTORY env vars into a single SOURCE_DIRECTORY - Remove os-release host volume mount from all cnibincopy init containers and the multus DaemonSet - Binaries are now copied directly from the default paths (e.g. /usr/src/multus-cni/bin/, /bondcni/, /usr/src/plugins/bin/) 008-script-lib.yaml (OVN): - Remove os-release sourcing and RHEL version case statements - Copy ovn-k8s-cni-overlay directly from /usr/libexec/cni/ This unblocks removing rhel8 build stages from upstream images (openshift/ovn-kubernetes#3149, openshift/multus-cni#285). By the time version-specific paths would be needed again (RHEL 11+), all in-cluster components will use native FIPS, making this logic permanently unnecessary. rh-pre-commit.version: 2.4.0 rh-pre-commit.check-secrets: ENABLED
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: sdodson The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
…ectories The version-specific CNI directories (rhel8/, rhel9/) are no longer needed since cluster-network-operator#2967 removed the OS detection logic and now copies ovn-k8s-cni-overlay directly. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> rh-pre-commit.version: 2.4.0 rh-pre-commit.check-secrets: ENABLED
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> rh-pre-commit.version: 2.4.0 rh-pre-commit.check-secrets: ENABLED
|
/testwith openshift/ovn-kubernetes/master/e2e-aws-ovn openshift/bond-cni#113 openshift/route-override-cni#66 openshift/egress-router-cni#100 openshift/containernetworking-plugins#228 openshift/whereabouts-cni#405 openshift/multus-cni#285 |
|
@sdodson, |
|
@sdodson, |
|
/testwith openshift/ovn-kubernetes/e2e-aws-ovn openshift/bond-cni#113 openshift/route-override-cni#66 openshift/egress-router-cni#100 openshift/containernetworking-plugins#228 openshift/whereabouts-cni#405 openshift/multus-cni#285 |
|
@sdodson, |
|
/testwith openshift/ovn-kubernetes/main/e2e-aws-ovn openshift/bond-cni#113 openshift/route-override-cni#66 openshift/egress-router-cni#100 openshift/containernetworking-plugins#228 openshift/whereabouts-cni#405 openshift/multus-cni#285 |
|
/testwith openshift/ovn-kubernetes/main/e2e-aws-ovn-serial openshift/bond-cni#113 openshift/route-override-cni#66 openshift/egress-router-cni#100 openshift/containernetworking-plugins#228 openshift/whereabouts-cni#405 openshift/multus-cni#285 |
|
/testwith openshift/ovn-kubernetes/main/5.0-upgrade-from-stable-4.22-e2e-aws-ovn-upgrade openshift/bond-cni#113 openshift/route-override-cni#66 openshift/egress-router-cni#100 openshift/containernetworking-plugins#228 openshift/whereabouts-cni#405 openshift/multus-cni#285 |
|
@sdodson, |
Drop OS detection and version-specific directory probing from both cnibincopy scripts (multus.yaml and 008-script-lib.yaml) entirely. multus.yaml: - Consolidate RHEL8_SOURCE_DIRECTORY, RHEL9_SOURCE_DIRECTORY, and DEFAULT_SOURCE_DIRECTORY env vars into a single SOURCE_DIRECTORY - Remove os-release host volume mount from all cnibincopy init containers and the multus DaemonSet - Binaries are now copied directly from the default paths (e.g. /usr/src/multus-cni/bin/, /bondcni/, /usr/src/plugins/bin/) 008-script-lib.yaml (OVN): - Remove os-release sourcing and RHEL version case statements - Copy ovn-k8s-cni-overlay directly from /usr/libexec/cni/ This unblocks removing rhel8 build stages from upstream images (openshift/ovn-kubernetes#3149, openshift/multus-cni#285). By the time version-specific paths would be needed again (RHEL 11+), all in-cluster components will use native FIPS, making this logic permanently unnecessary. rh-pre-commit.version: 2.4.0 rh-pre-commit.check-secrets: ENABLED
Drop OS detection and version-specific directory probing from both cnibincopy scripts (multus.yaml and 008-script-lib.yaml) entirely. multus.yaml: - Consolidate RHEL8_SOURCE_DIRECTORY, RHEL9_SOURCE_DIRECTORY, and DEFAULT_SOURCE_DIRECTORY env vars into a single SOURCE_DIRECTORY - Remove os-release host volume mount from all cnibincopy init containers and the multus DaemonSet - Binaries are now copied directly from the default paths (e.g. /usr/src/multus-cni/bin/, /bondcni/, /usr/src/plugins/bin/) 008-script-lib.yaml (OVN): - Remove os-release sourcing and RHEL version case statements - Copy ovn-k8s-cni-overlay directly from /usr/libexec/cni/ This unblocks removing rhel8 build stages from upstream images (openshift/ovn-kubernetes#3149, openshift/multus-cni#285). By the time version-specific paths would be needed again (RHEL 11+), all in-cluster components will use native FIPS, making this logic permanently unnecessary. rh-pre-commit.version: 2.4.0 rh-pre-commit.check-secrets: ENABLED
|
/hold cancel |
|
/retest-required |
|
Ahh, all those failures are leftovers about 4.22. Maybe this will clear them out of the summary report. |
|
/test e2e-aws-ovn-edge-zones |
|
@sdodson: 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. |
|
/test e2e-aws-ovn-edge-zones |
| -X ${OVN_KUBE_GO_PACKAGE}/pkg/config.BuildDate=${BUILD_DATE} \ | ||
| -X k8s.io/client-go/pkg/version.gitVersion=${K8S_CLIENT_VERSION} \ | ||
| `if [ "$binbase" != "ovnkube" ]; then echo ${LDFLAGS}; fi`" \ | ||
| ${LDFLAGS}" \ |
There was a problem hiding this comment.
This commit description doesn't match its content, debug/symbol stripping actually happened in the previous commit. Mind squashing the two, and adding a short rationale for stripping symbols from all binaries?
Remove the rhel8 build stage, apply LDFLAGS to all binaries, and strip debug symbols.
The rhel9/rhel10 version-specific subdirectories are no longer needed
since openshift/cluster-network-operator#2967 removed the OS detection
logic and now copies ovn-k8s-cni-overlay directly from /usr/libexec/cni/.
Summary by CodeRabbit