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

build: include -version option for csi-addons executable #397

Merged
merged 3 commits into from
Jun 27, 2023

Conversation

nixpanic
Copy link
Collaborator

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.

Just like for the manager and sidecar executables, add -version as an
option to the csi-addons tool.

Updates: #385

nixpanic added 2 commits June 27, 2023 13:56
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]>
Just like for the manager and sidecar executables, add `-version` as an
option to the `csi-addons` tool.

Signed-off-by: Niels de Vos <[email protected]>
@mergify mergify bot requested review from Rakshith-R and yati1998 June 27, 2023 12:05

# Use distroless as minimal base image to package the sidecar binary
# Refer to https://github.com/GoogleContainerTools/distroless for more details
FROM gcr.io/distroless/static:latest
WORKDIR /
COPY --from=builder /workspace /usr/bin/
COPY --from=builder /workspace/go/src/github.com/csi-addons/kubernetes-csi-addons/bin/csi-addons-sidecar /usr/sbin/
COPY --from=builder /workspace/go/src/github.com/csi-addons/kubernetes-csi-addons/bin/csi-addons /usr/bin/
Copy link
Member

Choose a reason for hiding this comment

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

it should be copied to /usr/sbin?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

/usr/sbin is the path for executables that act like servers or are for admin users. csi-addons is a tool that can be used by any user to talk to the csi-driver. I though it would be more suitable in /usr/bin. The distinction between the paths is not very important to me, I am happy yo change either, both should be in the $PATH when an admin user logs in the pod.

@@ -22,7 +22,7 @@ spec:
runAsNonRoot: true
containers:
- command:
- /manager
- /csi-addons-manager
Copy link
Member

Choose a reason for hiding this comment

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

same change should also be made to CSV? might be missed to updated CSV with git push

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hmm, I don't think the command is included there. I can not find it in config/manifests/bases/clusterserviceversion.yaml.in.

Disable building with CGo support, so that only native Golang functions
are used. This makes it possible to build the executables in one
container, and run them in a distroless container.

Fore rebuilding all components with `-a` in the `Makefile`, this
prevents including artifacts that may have been built outside the
contains with CGo enabled.

Signed-off-by: Niels de Vos <[email protected]>
@nixpanic nixpanic requested a review from Madhu-1 June 27, 2023 13:31
@nixpanic nixpanic added this to the release-v0.6.0 milestone Jun 27, 2023
@@ -4,17 +4,17 @@ FROM quay.io/projectquay/golang:1.20 as builder
# Copy the contents of the repository
ADD . /workspace/go/src/github.com/csi-addons/kubernetes-csi-addons

ENV GOPATH=/workspace/go
ENV GOPATH=/workspace/go CGO_ENABLED=0
Copy link
Member

Choose a reason for hiding this comment

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

GOOS=linux seems to be dropped.
I hope it makes no difference

@mergify mergify bot merged commit fcc6e23 into csi-addons:main Jun 27, 2023
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