Skip to content

cleanup(vpa): remove go vendoring #7572

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Dec 9, 2024
Merged

cleanup(vpa): remove go vendoring #7572

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0b6e7be
cleanup(vpa): remove vendor directory
davidspek Dec 5, 2024
41c6eee
cleanup(vpa): add gitignore for vendor
davidspek Dec 5, 2024
f3a50a7
cleanup: tidy go mod
davidspek Dec 5, 2024
99e2f7f
fix(vpa): update dockerfiles to work without vendor
davidspek Dec 5, 2024
82908da
fix: undo change to nonroot distroless
davidspek Dec 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
The diff you're trying to view is too large. We only load the first 3000 changed files.
1 change: 1 addition & 0 deletions vertical-pod-autoscaler/.gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
# vendor directories
vendor/
e2e/vendor/
91 changes: 32 additions & 59 deletions vertical-pod-autoscaler/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,62 +72,35 @@ require (
sigs.k8s.io/yaml v1.3.0 // indirect
)

replace k8s.io/api => k8s.io/api v0.28.3

replace k8s.io/apiextensions-apiserver => k8s.io/apiextensions-apiserver v0.28.3

replace k8s.io/apimachinery => k8s.io/apimachinery v0.28.3

replace k8s.io/apiserver => k8s.io/apiserver v0.28.3

replace k8s.io/cli-runtime => k8s.io/cli-runtime v0.28.3

replace k8s.io/client-go => k8s.io/client-go v0.28.3

replace k8s.io/cloud-provider => k8s.io/cloud-provider v0.28.3

replace k8s.io/cluster-bootstrap => k8s.io/cluster-bootstrap v0.28.3

replace k8s.io/code-generator => k8s.io/code-generator v0.28.3

replace k8s.io/component-base => k8s.io/component-base v0.28.3

replace k8s.io/component-helpers => k8s.io/component-helpers v0.28.3

replace k8s.io/controller-manager => k8s.io/controller-manager v0.28.3

replace k8s.io/cri-api => k8s.io/cri-api v0.28.3

replace k8s.io/csi-translation-lib => k8s.io/csi-translation-lib v0.28.3

replace k8s.io/dynamic-resource-allocation => k8s.io/dynamic-resource-allocation v0.28.3

replace k8s.io/kms => k8s.io/kms v0.28.3

replace k8s.io/kube-aggregator => k8s.io/kube-aggregator v0.28.3

replace k8s.io/kube-controller-manager => k8s.io/kube-controller-manager v0.28.3

replace k8s.io/kube-proxy => k8s.io/kube-proxy v0.28.3

replace k8s.io/kube-scheduler => k8s.io/kube-scheduler v0.28.3

replace k8s.io/kubectl => k8s.io/kubectl v0.28.3

replace k8s.io/kubelet => k8s.io/kubelet v0.28.3

replace k8s.io/legacy-cloud-providers => k8s.io/legacy-cloud-providers v0.28.3

replace k8s.io/metrics => k8s.io/metrics v0.28.3

replace k8s.io/mount-utils => k8s.io/mount-utils v0.28.3

replace k8s.io/pod-security-admission => k8s.io/pod-security-admission v0.28.3

replace k8s.io/sample-apiserver => k8s.io/sample-apiserver v0.28.3

replace k8s.io/sample-cli-plugin => k8s.io/sample-cli-plugin v0.28.3

replace k8s.io/sample-controller => k8s.io/sample-controller v0.28.3

replace k8s.io/endpointslice => k8s.io/endpointslice v0.28.3
replace (
k8s.io/api => k8s.io/api v0.28.3
k8s.io/apiextensions-apiserver => k8s.io/apiextensions-apiserver v0.28.3
k8s.io/apimachinery => k8s.io/apimachinery v0.28.3
k8s.io/apiserver => k8s.io/apiserver v0.28.3
k8s.io/cli-runtime => k8s.io/cli-runtime v0.28.3
k8s.io/client-go => k8s.io/client-go v0.28.3
k8s.io/cloud-provider => k8s.io/cloud-provider v0.28.3
k8s.io/cluster-bootstrap => k8s.io/cluster-bootstrap v0.28.3
k8s.io/code-generator => k8s.io/code-generator v0.28.3
k8s.io/component-base => k8s.io/component-base v0.28.3
k8s.io/component-helpers => k8s.io/component-helpers v0.28.3
k8s.io/controller-manager => k8s.io/controller-manager v0.28.3
k8s.io/cri-api => k8s.io/cri-api v0.28.3
k8s.io/csi-translation-lib => k8s.io/csi-translation-lib v0.28.3
k8s.io/dynamic-resource-allocation => k8s.io/dynamic-resource-allocation v0.28.3
k8s.io/endpointslice => k8s.io/endpointslice v0.28.3
k8s.io/kms => k8s.io/kms v0.28.3
k8s.io/kube-aggregator => k8s.io/kube-aggregator v0.28.3
k8s.io/kube-controller-manager => k8s.io/kube-controller-manager v0.28.3
k8s.io/kube-proxy => k8s.io/kube-proxy v0.28.3
k8s.io/kube-scheduler => k8s.io/kube-scheduler v0.28.3
k8s.io/kubectl => k8s.io/kubectl v0.28.3
k8s.io/kubelet => k8s.io/kubelet v0.28.3
k8s.io/legacy-cloud-providers => k8s.io/legacy-cloud-providers v0.28.3
k8s.io/metrics => k8s.io/metrics v0.28.3
k8s.io/mount-utils => k8s.io/mount-utils v0.28.3
k8s.io/pod-security-admission => k8s.io/pod-security-admission v0.28.3
k8s.io/sample-apiserver => k8s.io/sample-apiserver v0.28.3
k8s.io/sample-cli-plugin => k8s.io/sample-cli-plugin v0.28.3
k8s.io/sample-controller => k8s.io/sample-controller v0.28.3
)
17 changes: 11 additions & 6 deletions vertical-pod-autoscaler/pkg/admission-controller/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,20 +14,25 @@

FROM --platform=$BUILDPLATFORM golang:1.23.4 AS builder

ENV GOPATH=/gopath/
ENV PATH=$GOPATH/bin:$PATH
WORKDIR /workspace

COPY . /gopath/src/k8s.io/autoscaler/vertical-pod-autoscaler
WORKDIR /gopath/src/k8s.io/autoscaler/vertical-pod-autoscaler
# Copy the Go Modules manifests
COPY go.mod go.mod
Copy link
Member

Choose a reason for hiding this comment

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

Could we replace all these copy statements with just one?

COPY . .

Similar to https://github.com/GoogleContainerTools/distroless?tab=readme-ov-file#examples-with-docker?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reason for these separate copy statements is to maximize layer caching. First copying the go.mod and go.sum and then downloading the dependencies before before copying any application code allows the layer containing all the dependencies to be cached during a container build. This can make a big difference for build times, especially when doing development. For this same reason the Go code is being explicitly copied into the container instead of a more global copy so that changes to other files (or the addition of new ones) that aren't part of the codebase don't trigger a new image build. Technically this could also be done with a .dockerignore file, but then any new files or directories would need to be added there, and in my opinion having it copied in the Dockerfile explicitly also helps for clarity. This is also the standard way Dockerfiles are created for projects using kubebuilder and I believe can be seen as best practice.

Copy link
Member

@adrianmoisey adrianmoisey Dec 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

What about:

COPY go.mod go.sum .

And later:

COPY common pkg .

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That should also work, although technically a little less good for layer caching.

Copy link
Member

Choose a reason for hiding this comment

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

Since both operations are a copy, I don't think there will be a noticeable different to the user if one those layers had to be re executed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@raywainman What's your view on this? I'd like to get this sorted so it can be merged and we can move forward with the dependency upgrade PR.

Copy link
Member

Choose a reason for hiding this comment

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

Layer caching seems like a reasonable reason to do this. LGTM.

COPY go.sum go.sum

RUN go mod download

COPY common common
COPY pkg pkg

ARG TARGETOS TARGETARCH

RUN CGO_ENABLED=0 LD_FLAGS=-s GOARCH=$TARGETARCH GOOS=$TARGETOS go build -C pkg/admission-controller -mod vendor -o admission-controller-$TARGETARCH
RUN CGO_ENABLED=0 LD_FLAGS=-s GOARCH=$TARGETARCH GOOS=$TARGETOS go build -C pkg/admission-controller -o admission-controller-$TARGETARCH

FROM gcr.io/distroless/static:latest

ARG TARGETARCH

COPY --from=builder /gopath/src/k8s.io/autoscaler/vertical-pod-autoscaler/pkg/admission-controller/admission-controller-$TARGETARCH /admission-controller
COPY --from=builder /workspace/pkg/admission-controller/admission-controller-$TARGETARCH /admission-controller

ENTRYPOINT ["/admission-controller"]
17 changes: 11 additions & 6 deletions vertical-pod-autoscaler/pkg/recommender/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,20 +14,25 @@

FROM --platform=$BUILDPLATFORM golang:1.23.4 AS builder

ENV GOPATH=/gopath/
ENV PATH=$GOPATH/bin:$PATH
WORKDIR /workspace

COPY . /gopath/src/k8s.io/autoscaler/vertical-pod-autoscaler
WORKDIR /gopath/src/k8s.io/autoscaler/vertical-pod-autoscaler
# Copy the Go Modules manifests
COPY go.mod go.mod
COPY go.sum go.sum

RUN go mod download

COPY common common
COPY pkg pkg

ARG TARGETOS TARGETARCH

RUN CGO_ENABLED=0 LD_FLAGS=-s GOARCH=$TARGETARCH GOOS=$TARGETOS go build -C pkg/recommender -mod vendor -o recommender-$TARGETARCH
RUN CGO_ENABLED=0 LD_FLAGS=-s GOARCH=$TARGETARCH GOOS=$TARGETOS go build -C pkg/recommender -o recommender-$TARGETARCH

FROM gcr.io/distroless/static:latest

ARG TARGETARCH

COPY --from=builder /gopath/src/k8s.io/autoscaler/vertical-pod-autoscaler/pkg/recommender/recommender-$TARGETARCH /recommender
COPY --from=builder /workspace/pkg/recommender/recommender-$TARGETARCH /recommender

ENTRYPOINT ["/recommender"]
17 changes: 11 additions & 6 deletions vertical-pod-autoscaler/pkg/updater/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,20 +14,25 @@

FROM --platform=$BUILDPLATFORM golang:1.23.4 AS builder

ENV GOPATH=/gopath/
ENV PATH=$GOPATH/bin:$PATH
WORKDIR /workspace

COPY . /gopath/src/k8s.io/autoscaler/vertical-pod-autoscaler
WORKDIR /gopath/src/k8s.io/autoscaler/vertical-pod-autoscaler
# Copy the Go Modules manifests
COPY go.mod go.mod
COPY go.sum go.sum

RUN go mod download

COPY common common
COPY pkg pkg

ARG TARGETOS TARGETARCH

RUN CGO_ENABLED=0 LD_FLAGS=-s GOARCH=$TARGETARCH GOOS=$TARGETOS go build -C pkg/updater -mod vendor -o updater-$TARGETARCH
RUN CGO_ENABLED=0 LD_FLAGS=-s GOARCH=$TARGETARCH GOOS=$TARGETOS go build -C pkg/updater -o updater-$TARGETARCH

FROM gcr.io/distroless/static:latest

ARG TARGETARCH

COPY --from=builder /gopath/src/k8s.io/autoscaler/vertical-pod-autoscaler/pkg/updater/updater-$TARGETARCH /updater
COPY --from=builder /workspace/pkg/updater/updater-$TARGETARCH /updater

ENTRYPOINT ["/updater"]
20 changes: 0 additions & 20 deletions vertical-pod-autoscaler/vendor/github.com/beorn7/perks/LICENSE
View file
Open in desktop

This file was deleted.

Loading
Loading