CNF-11589: Embed commit SHA and version info into LCA binaries#6308
CNF-11589: Embed commit SHA and version info into LCA binaries#6308sebrandon1 wants to merge 1 commit into
Conversation
|
@sebrandon1: This pull request references CNF-11589 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 story 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. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 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 |
|
Warning Review limit reached
More reviews will be available in 44 minutes and 41 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThis change introduces version metadata injection into Go binaries during the build process. A new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 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)
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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Dockerfile`:
- Around line 11-12: Add BUILD_TIME as a build-time argument and stop generating
it inside the layer: declare ARG BUILD_TIME (alongside ARG GIT_COMMIT and ARG
VERSION) and replace any RUN-step logic that computes or writes a timestamp
during the build with usage of the BUILD_TIME arg so callers can supply a
deterministic value; also update the same pattern referenced in the later block
(the lines around the second occurrence) to use ARG BUILD_TIME instead of
generating a timestamp inside the image.
In `@Dockerfile.multiarch`:
- Around line 11-12: Add a new build arg ARG BUILD_TIME and use it where build
timestamp metadata is set so the timestamp comes from the build invocation
rather than a cacheable layer; update the ARG declarations (alongside ARG
GIT_COMMIT and ARG VERSION) and ensure any places that previously relied on a
generated timestamp (including the secondary block around the other ARGs at the
later section referenced) consume BUILD_TIME so multiarch rebuilds pick up
current build metadata.
In `@Makefile`:
- Around line 41-46: The docker-build target currently omits the version
metadata injection (VERSION, GIT_COMMIT, BUILD_TIME / LDFLAGS) so images are
built with default metadata; update the docker-build rule to forward the same
build information used by build and cli-build by adding the appropriate
--build-arg flags (VERSION, GIT_COMMIT, BUILD_TIME) or exporting LDFLAGS into
the docker build command so the Dockerfile can use them during the internal go
build; ensure the docker-build target mirrors the behavior of build and
cli-build and include the same LDFLAGS/args so binaries inside the image receive
the injected Version, GitCommit, and BuildTime values.
🪄 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: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: 00add334-f7eb-480c-8436-0db97d83e991
📒 Files selected for processing (6)
DockerfileDockerfile.multiarchMakefilelca-cli/cmd/root.gomain/main.goversion/version.go
5cf32f3 to
69b3f37
Compare
69b3f37 to
45a639f
Compare
Add a version package with Version, GitCommit, and BuildTime variables injected via -ldflags at build time. Both the manager and lca-cli binaries now report build provenance, making it straightforward to identify exactly which commit is running in production. - Add version/ package with String() formatter - Update Makefile build/cli-build targets with -ldflags - Forward VERSION, GIT_COMMIT, BUILD_TIME as --build-arg in docker-build - Update Dockerfile to accept and use all three build args - Wire lca-cli --version to print version string - Log version at manager startup
45a639f to
1658154
Compare
|
@sebrandon1: 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. |
Jira: CNF-11589
Summary
version/package withVersion,GitCommit, andBuildTimevariables injected via-ldflagsat build timebuildandcli-buildtargets to pass-ldflagswith version infoDockerfileandDockerfile.multiarchto injectVERSION,GIT_COMMIT, andBUILD_TIMEbuild args into allgo buildcommandslca-cli --versionto print version string (replaces unusedreleaseVersionvariable)managerstartupExample output:
Test plan
make build— manager binary compiles with ldflagsmake cli-build— lca-cli binary compiles with ldflagsbin/lca-cli --version— prints version, commit SHA, and build timemake golangci-lint— 0 issuesmake unittest— 0 failures