Skip to content
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

feature: print runtime git commit #19

Merged
merged 1 commit into from
Jun 24, 2021

Conversation

hhyasdf
Copy link
Collaborator

@hhyasdf hhyasdf commented Jun 23, 2021

Pull Request Description

Describe what this PR does / why we need it

Print runtime commit id of Rama in rama-manager and rama-daemon logs. It helps us to find the correct commit id while debugging.

Does this pull request fix one issue?

NONE

Describe how you did it

Link a flag "main.gitVersion" while building binaries, and add logs in rama-daemon and rama-manager.

Describe how to verify it

Run rama-daemon and rama-manager pod, logs of "starting rama manager with git version: " will be printed at the first place. Executing command of "cat /rama/COMMIT_ID" in pod will show commit id too.

Special notes for reviews

@codecov-commenter
Copy link

Codecov Report

Merging #19 (26c3499) into main (0633357) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main      #19   +/-   ##
=======================================
  Coverage   13.21%   13.21%           
=======================================
  Files          22       22           
  Lines        1907     1907           
=======================================
  Hits          252      252           
  Misses       1606     1606           
  Partials       49       49           
Flag Coverage Δ
unittests 13.21% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0633357...26c3499. Read the comment docs.

Copy link
Collaborator

@mars1024 mars1024 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO, I don't think this will help us a lot, because

  1. log won't be kept for a long time, it rotates usually
  2. commit id is not useful compared with tag, which could be found in image url

@hhyasdf hhyasdf force-pushed the feature/print-runtime-git-commit branch from 26c3499 to e726907 Compare June 23, 2021 08:10
@hhyasdf
Copy link
Collaborator Author

hhyasdf commented Jun 23, 2021

IMO, I don't think this will help us a lot, because

  1. log won't be kept for a long time, it rotates usually
  2. commit id is not useful compared with tag, which could be found in image url

For the first problem, I add an /rama/VERSION file into the image for long time usage.

For the second problem, image tag sometimes is unreliable. This happens mostly in develop environment, developer sometime don't know the exact code version if they dose not use a official release or have changed the image's tag.

Dockerfile.amd64 Outdated Show resolved Hide resolved
Dockerfile.amd64 Outdated Show resolved Hide resolved
cmd/daemon/daemon.go Outdated Show resolved Hide resolved
@hhyasdf hhyasdf force-pushed the feature/print-runtime-git-commit branch from e726907 to 669cb54 Compare June 23, 2021 11:58
Copy link
Collaborator

@mars1024 mars1024 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm
/approve
thanks!

@mars1024 mars1024 merged commit ea59d4f into alibaba:main Jun 24, 2021
@hhyasdf hhyasdf deleted the feature/print-runtime-git-commit branch April 18, 2023 07:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants