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

Adding version details to sidecar #385

Merged
merged 1 commit into from
Jun 27, 2023
Merged

Conversation

saranyareddy24
Copy link
Contributor

@saranyareddy24 saranyareddy24 commented Jun 25, 2023

Fixes #377
Fixes #378

The output will be printed as:
Version: v0.6.0
Git Commit: 43ebc4b
Go Version: go1.19.9
Compiler: gc
Platform: linux/amd64

@mergify mergify bot requested review from nixpanic, Rakshith-R and yati1998 June 25, 2023 16:41
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Show resolved Hide resolved
sidecar/internal/version/version.go Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
@nixpanic
Copy link
Collaborator

Many thanks for you contribution! I think it will be very helpful 👍

@nixpanic nixpanic added the enhancement New feature or request label Jun 26, 2023
@saranyareddy24 saranyareddy24 force-pushed the main branch 2 times, most recently from 523319c to 957a79d Compare June 26, 2023 08:57
@Madhu-1 Madhu-1 requested a review from nixpanic June 26, 2023 09:27
@saranyareddy24 saranyareddy24 force-pushed the main branch 2 times, most recently from 57407fe to c0c88b4 Compare June 26, 2023 09:37
Makefile Outdated Show resolved Hide resolved
nixpanic
nixpanic previously approved these changes Jun 26, 2023
@saranyareddy24
Copy link
Contributor Author

@Rakshith-R & @yati1998 can you please check and approve

Copy link
Member

@Rakshith-R Rakshith-R left a comment

Choose a reason for hiding this comment

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

Just a few small nits

sidecar/main.go Outdated Show resolved Hide resolved
sidecar/main.go Show resolved Hide resolved
@mergify mergify bot dismissed nixpanic’s stale review June 27, 2023 03:06

Pull request has been modified.

@saranyareddy24 saranyareddy24 force-pushed the main branch 2 times, most recently from 85c93a7 to adda110 Compare June 27, 2023 03:11
Rakshith-R
Rakshith-R previously approved these changes Jun 27, 2023
@Rakshith-R Rakshith-R requested review from Madhu-1 and nixpanic June 27, 2023 03:34
@saranyareddy24
Copy link
Contributor Author

can someone help me understand why build is failing.

@Rakshith-R
Copy link
Member

can someone help me understand why build is failing.

I think there is spacing issue in the import "time" pkg line.
Can you try running make test locally?
or copy the spacing from the above "flag" pkg line and paste it there.

https://github.com/csi-addons/kubernetes-csi-addons/actions/runs/5385350011/jobs/9774541855?pr=385

and after amending your commit, check if that change still exists with git show

Copy link
Member

@Madhu-1 Madhu-1 left a comment

Choose a reason for hiding this comment

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

small nit, lgtm

internal/version/version.go Outdated Show resolved Hide resolved
sidecar/main.go Outdated Show resolved Hide resolved
@mergify mergify bot dismissed Rakshith-R’s stale review June 27, 2023 06:39

Pull request has been modified.

@Madhu-1 Madhu-1 requested a review from Rakshith-R June 27, 2023 06:40
@Madhu-1
Copy link
Member

Madhu-1 commented Jun 27, 2023

@Mergifyio requeue

@mergify
Copy link

mergify bot commented Jun 27, 2023

requeue

✅ The queue state of this pull request has been cleaned. It can be re-embarked automatically

@Rakshith-R
Copy link
Member

@Mergifyio requeue

@mergify
Copy link

mergify bot commented Jun 27, 2023

requeue

✅ The queue state of this pull request has been cleaned. It can be re-embarked automatically

@Rakshith-R
Copy link
Member

Rakshith-R commented Jun 27, 2023

@saranyareddy24
https://github.com/csi-addons/kubernetes-csi-addons/pull/385/checks?check_run_id=14579557040

Can you please try logging into mergifyio
or rebase this pr on top of latest main branch?

This pull request cannot be embarked for merge
The merge queue pull request can't be updated
Details:

Unable to update: user saranyareddy24 is unknown.

Please make sure saranyareddy24 has logged in Mergify dashboard.

View more details on Mergify

@Madhu-1
Copy link
Member

Madhu-1 commented Jun 27, 2023

@Mergifyio rebase

@mergify
Copy link

mergify bot commented Jun 27, 2023

rebase

✅ Branch has been successfully rebased

@Rakshith-R
Copy link
Member

@Mergifyio rebase

@mergify
Copy link

mergify bot commented Jun 27, 2023

rebase

✅ Branch has been successfully rebased

@mergify mergify bot merged commit 3fd87a2 into csi-addons:main Jun 27, 2023
nixpanic added a commit to nixpanic/kubernetes-csi-addons that referenced this pull request Jun 27, 2023
The `Makefile` now contains logic to provide a version (git tag) and the
current git commit hash in a `-version` argument to executables. However
the container-images did not use `make`, and therefor the executables
would not have the version information set.

While updating the building of the executables in the container, the
names of the executables has been prefixed with `csi-addons-`, so that a
listing of running processes on a Kubernetes node does not show
`manager` for the CSI-Addons Operator, but `csi-addons-manager`.

Updates: csi-addons#385
Signed-off-by: Niels de Vos <[email protected]>
mergify bot pushed a commit that referenced this pull request Jun 27, 2023
The `Makefile` now contains logic to provide a version (git tag) and the
current git commit hash in a `-version` argument to executables. However
the container-images did not use `make`, and therefor the executables
would not have the version information set.

While updating the building of the executables in the container, the
names of the executables has been prefixed with `csi-addons-`, so that a
listing of running processes on a Kubernetes node does not show
`manager` for the CSI-Addons Operator, but `csi-addons-manager`.

Updates: #385
Signed-off-by: Niels de Vos <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add version flag for volume replication operator Add version flag for csi-addons-sidecar
4 participants