-
Notifications
You must be signed in to change notification settings - Fork 527
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: Add version ldflags for building manager-api in Dockerfile #1393
Conversation
Signed-off-by: imjoey <[email protected]>
@imjoey thanks |
9d4a35c
to
5c8259e
Compare
Signed-off-by: imjoey <[email protected]>
5c8259e
to
2aa8dd8
Compare
Codecov Report
@@ Coverage Diff @@
## master #1393 +/- ##
==========================================
+ Coverage 65.73% 65.87% +0.13%
==========================================
Files 44 44
Lines 2948 2948
==========================================
+ Hits 1938 1942 +4
+ Misses 781 774 -7
- Partials 229 232 +3
Continue to review full report at Codecov.
|
@nic-chen thanks for the direction. An additional CI test triggered on pull request is added for docker testing. Any feedback is welcome. |
well done! thanks. |
Signed-off-by: imjoey [email protected]
Please answer these questions before submitting a pull request
Why submit this pull request?
Bugfix
New feature provided
Improve performance
Backport patches
Related issues
Fixes #1350 .
Bugfix
At previous manager-api build phase in Dockerfile, the ldflags of
github.com/apisix/manager-api/cmd.Version
andgithub.com/apisix/manager-api/cmd.GitHash
are missing. This lead to the noVersion
output when runningmanager-api
, as described in #1350 .Add missing
ldflags
in Dockerfile. For simplicity, this PR is going to reuse the existing built script./api/build.sh
to compile out the binary.In addition, this PR will also reduce the usage of
RUN
commands for smaller image size.FYI, since there is no
.githash
or.git/
within the artifact downloaded from Github , theGitHash
output is still missing.