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.

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

[WIP] Get rid of kubectl depepdency in container image #1137

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[WIP] Get rid of kubectl depepdency in container image #1137

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,11 @@ gpu-operator:
CGO_ENABLED=0 GOOS=$(GOOS) \
go build -ldflags "-s -w -X $(VERSION_PKG).gitCommit=$(GIT_COMMIT) -X $(VERSION_PKG).version=$(VERSION)" -o gpu-operator ./cmd/gpu-operator/...

# Build apply-crds binary
apply-crds:
CGO_ENABLED=0 GOOS=$(GOOS) \
go build -ldflags "-s -w -X $(VERSION_PKG).gitCommit=$(GIT_COMMIT) -X $(VERSION_PKG).version=$(VERSION)" -o apply-crds ./cmd/apply-crds/...

Comment on lines +89 to +93
Copy link
Member

Choose a reason for hiding this comment

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

In other projects we using the following:

CMDS := $(patsubst ./cmd/%/,%,$(sort $(dir $(wildcard ./cmd/*/))))
CMD_TARGETS := $(patsubst %,cmd-%,$(CMDS))

ifneq ($(PREFIX),)
cmd-%: COMMAND_BUILD_OPTIONS = -o $(PREFIX)/$(*)
endif
ifneq ($(shell uname),Darwin)
EXTLDFLAGS = -Wl,--export-dynamic -Wl,--unresolved-symbols=ignore-in-object-files
else
EXTLDFLAGS = -Wl,-undefined,dynamic_lookup
endif
BUILDFLAGS = -ldflags "-s -w '-extldflags=$(EXTLDFLAGS)' -X $(CLI_VERSION_PACKAGE).gitCommit=$(GIT_COMMIT) -X $(CLI_VERSION_PACKAGE).version=$(CLI_VERSION)"
build:
	go build $(BUILDFLAGS) ./...

cmds: $(CMD_TARGETS)
$(CMD_TARGETS): cmd-%:
	go build $(BUILDFLAGS) $(COMMAND_BUILD_OPTIONS) $(MODULE)/cmd/$(*)

to auto-generate make cmd-* targets for any binary that we place in cmd. Would that be a useful addition here?

# Run against the configured Kubernetes cluster in ~/.kube/config
run: generate check manifests
go run ./cmd/gpu-operator/...
Expand Down
27 changes: 27 additions & 0 deletions cmd/apply-crds/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/*
* Copyright (c), NVIDIA CORPORATION. All rights reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

// Package main uses the crdutil package to ensure CRDs are created
// or updated in the cluster during Helm chart installation.
package main

import (
"github.com/NVIDIA/k8s-operator-libs/pkg/crdutil"
)

func main() {
crdutil.EnsureCRDsCmd()
}
3 changes: 2 additions & 1 deletion deployments/gpu-operator/templates/cleanup_crd.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,12 @@ spec:
{{- end }}
containers:
- name: cleanup-crd
image: {{ include "gpu-operator.fullimage" . }}
image: {{ .Values.operator.imageCleanupCRD }}
imagePullPolicy: {{ .Values.operator.imagePullPolicy }}
command:
- /bin/sh
- -c
args:
- >
kubectl delete clusterpolicy cluster-policy;
kubectl delete crd clusterpolicies.nvidia.com;
Expand Down
11 changes: 5 additions & 6 deletions deployments/gpu-operator/templates/upgrade_crd.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,13 +83,12 @@ spec:
image: {{ include "gpu-operator.fullimage" . }}
imagePullPolicy: {{ .Values.operator.imagePullPolicy }}
command:
- /bin/sh
- -c
- >
kubectl apply -f /opt/gpu-operator/nvidia.com_clusterpolicies.yaml;
kubectl apply -f /opt/gpu-operator/nvidia.com_nvidiadrivers.yaml;
- /usr/bin/apply-crds
args:
- --crds-file=/opt/gpu-operator/nvidia.com_clusterpolicies.yaml
- --crds-file=/opt/gpu-operator/nvidia.com_nvidiadrivers.yaml
{{- if .Values.nfd.enabled }}
kubectl apply -f /opt/gpu-operator/nfd-api-crds.yaml;
- --crds-file=/opt/gpu-operator/nfd-api-crds.yaml
{{- end }}
restartPolicy: OnFailure
{{- end }}
1 change: 1 addition & 0 deletions deployments/gpu-operator/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ operator:
use_ocp_driver_toolkit: false
# cleanup CRD on chart un-install
cleanupCRD: false
imageCleanupCRD: bitnami/kubectl:1.31
Copy link
Member Author

@tobiasgiese tobiasgiese Nov 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

We can use the upstream bitnami/kubectl image here as we don't need the CRDs in the container for the cleanup task.

Copy link
Contributor

Choose a reason for hiding this comment

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

The cleanest approach would be to add a DeleteCRD method to the k8s-operator-libs. I think we can revisit this PR after that. Thoughts?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good call, was also thinking about this. Let me add a command to delete CRDs to the cmdutil

Copy link
Member

Choose a reason for hiding this comment

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

There may be other requirements in terms of using a thirdparty image. Implementing the required logic in code may be simpler in the long run.

# upgrade CRD on chart upgrade, requires --disable-openapi-validation flag
# to be passed during helm upgrade.
upgradeCRD: true
Expand Down
9 changes: 2 additions & 7 deletions docker/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ COPY *.mk .
# Build
ARG VERSION="unknown"
ARG GIT_COMMIT="unknown"
RUN make gpu-operator
RUN make gpu-operator apply-crds

FROM nvcr.io/nvidia/cuda:12.6.2-base-ubi9

Expand All @@ -76,19 +76,14 @@ LABEL vsc-ref=${GIT_COMMIT}

WORKDIR /
COPY --from=builder /workspace/gpu-operator /usr/bin/
COPY --from=builder /workspace/apply-crds /usr/bin/

RUN mkdir -p /opt/gpu-operator/manifests
COPY assets /opt/gpu-operator/
COPY manifests /opt/gpu-operator/manifests
RUN mkdir /licenses && mv /NGC-DL-CONTAINER-LICENSE /licenses/NGC-DL-CONTAINER-LICENSE
COPY hack/must-gather.sh /usr/bin/gather

# Install must-gather dependency: `kubectl`
ARG TARGETARCH
RUN OS_ARCH=${TARGETARCH/x86_64/amd64} && OS_ARCH=${OS_ARCH/aarch64/arm64} && curl -LO https://storage.googleapis.com/kubernetes-release/release/$(curl -s https://storage.googleapis.com/kubernetes-release/release/stable.txt)/bin/linux/${OS_ARCH}/kubectl
RUN chmod +x ./kubectl
RUN mv ./kubectl /usr/local/bin

# Add CRD resource into the image for helm upgrades
COPY deployments/gpu-operator/crds/nvidia.com_clusterpolicies.yaml /opt/gpu-operator/nvidia.com_clusterpolicies.yaml
COPY deployments/gpu-operator/crds/nvidia.com_nvidiadrivers.yaml /opt/gpu-operator/nvidia.com_nvidiadrivers.yaml
Expand Down
11 changes: 8 additions & 3 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ require (
github.com/Masterminds/sprig/v3 v3.3.0
github.com/NVIDIA/go-nvlib v0.7.0
github.com/NVIDIA/k8s-kata-manager v0.2.2
github.com/NVIDIA/k8s-operator-libs v0.0.0-20240826221728-249ba446fa35
github.com/NVIDIA/k8s-operator-libs v0.0.0-20241120073822-1ad8938d7274
github.com/NVIDIA/nvidia-container-toolkit v1.17.2
github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc
github.com/go-logr/logr v1.4.2
Expand Down Expand Up @@ -169,14 +169,19 @@ require (
gopkg.in/yaml.v3 v3.0.1 // indirect
helm.sh/helm/v3 v3.16.1 // indirect
k8s.io/apiserver v0.31.2 // indirect
k8s.io/cli-runtime v0.31.1 // indirect
k8s.io/cli-runtime v0.31.2 // indirect
k8s.io/component-base v0.31.2 // indirect
k8s.io/kube-openapi v0.0.0-20240228011516-70dd3763d340 // indirect
k8s.io/kubectl v0.31.0 // indirect
k8s.io/kubectl v0.31.2 // indirect
k8s.io/utils v0.0.0-20240921022957-49e7df575cb6 // indirect
oras.land/oras-go v1.2.5 // indirect
sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect
sigs.k8s.io/kustomize/api v0.17.2 // indirect
sigs.k8s.io/kustomize/kyaml v0.17.1 // indirect
sigs.k8s.io/structured-merge-diff/v4 v4.4.1 // indirect
)

// DROP BEFORE MERGE!
// Implements https://github.com/NVIDIA/k8s-operator-libs/pull/58
// This is only for testing.
replace github.com/NVIDIA/k8s-operator-libs => github.com/tobiasgiese/k8s-operator-libs v0.0.0-20241125092837-e8a080621717
Comment on lines +184 to +187
Copy link
Member Author

Choose a reason for hiding this comment

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

must be dropped before merging, just as a reminder

12 changes: 6 additions & 6 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,6 @@ github.com/NVIDIA/go-nvlib v0.7.0 h1:Z/J7skMdLbTiHvomKVsGYsttfQMZj5FwNYIFXhZ4i/c
github.com/NVIDIA/go-nvlib v0.7.0/go.mod h1:9UrsLGx/q1OrENygXjOuM5Ey5KCtiZhbvBlbUIxtGWY=
github.com/NVIDIA/k8s-kata-manager v0.2.2 h1:+xVIp4yLfCjZ31Dfrm9LOKo4T47b4g+DV6XkwAqalns=
github.com/NVIDIA/k8s-kata-manager v0.2.2/go.mod h1:UGjGQUcpXTegwyOc5IwcyLTzPKwO9lOIkqw/qUzk8Q0=
github.com/NVIDIA/k8s-operator-libs v0.0.0-20240826221728-249ba446fa35 h1:w9DXPTJCq9k2PVpdBQJrWE4vAmZcFaSHKLpM/xos9WI=
github.com/NVIDIA/k8s-operator-libs v0.0.0-20240826221728-249ba446fa35/go.mod h1:sw6XRI5wq0Q+nSgaWa1Pyo/ZKxQebc70x6VIznDAxtM=
github.com/NVIDIA/nvidia-container-toolkit v1.17.2 h1:iE6PK9SQH3HyDrOolu27xn3CJgURR3bDtnbfFrxdML8=
github.com/NVIDIA/nvidia-container-toolkit v1.17.2/go.mod h1:R6bNf6ca0IjjACa0ncKGvsrx6zSjsgz8QkFyBDk5szU=
github.com/Shopify/logrus-bugsnag v0.0.0-20171204204709-577dee27f20d h1:UrqY+r/OJnIp5u0s1SbQ8dVfLCZJsnvazdBP5hS4iRs=
Expand Down Expand Up @@ -351,6 +349,8 @@ github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/
github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsTg=
github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY=
github.com/tobiasgiese/k8s-operator-libs v0.0.0-20241125092837-e8a080621717 h1:456lFgNispD2ff9fpni9sYB3838p14O30zN0cyoeFmI=
github.com/tobiasgiese/k8s-operator-libs v0.0.0-20241125092837-e8a080621717/go.mod h1:g8DW2t4Vit91uLdqCxsjKbKYrwCdb/oB9q/YOXdUjmQ=
github.com/ulikunitz/xz v0.5.12 h1:37Nm15o69RwBkXM0J6A5OlE67RZTfzUxTj8fB3dfcsc=
github.com/ulikunitz/xz v0.5.12/go.mod h1:nbz6k7qbPmH4IRqmfOplQw/tblSgqTqBwxkY0oWt/14=
github.com/urfave/cli/v2 v2.27.5 h1:WoHEJLdsXr6dDWoJgMq/CboDmyY/8HMMH1fTECbih+w=
Expand Down Expand Up @@ -488,8 +488,8 @@ k8s.io/apimachinery v0.31.2 h1:i4vUt2hPK56W6mlT7Ry+AO8eEsyxMD1U44NR22CLTYw=
k8s.io/apimachinery v0.31.2/go.mod h1:rsPdaZJfTfLsNJSQzNHQvYoTmxhoOEofxtOsF3rtsMo=
k8s.io/apiserver v0.31.2 h1:VUzOEUGRCDi6kX1OyQ801m4A7AUPglpsmGvdsekmcI4=
k8s.io/apiserver v0.31.2/go.mod h1:o3nKZR7lPlJqkU5I3Ove+Zx3JuoFjQobGX1Gctw6XuE=
k8s.io/cli-runtime v0.31.1 h1:/ZmKhmZ6hNqDM+yf9s3Y4KEYakNXUn5sod2LWGGwCuk=
k8s.io/cli-runtime v0.31.1/go.mod h1:pKv1cDIaq7ehWGuXQ+A//1OIF+7DI+xudXtExMCbe9U=
k8s.io/cli-runtime v0.31.2 h1:7FQt4C4Xnqx8V1GJqymInK0FFsoC+fAZtbLqgXYVOLQ=
k8s.io/cli-runtime v0.31.2/go.mod h1:XROyicf+G7rQ6FQJMbeDV9jqxzkWXTYD6Uxd15noe0Q=
k8s.io/client-go v0.31.2 h1:Y2F4dxU5d3AQj+ybwSMqQnpZH9F30//1ObxOKlTI9yc=
k8s.io/client-go v0.31.2/go.mod h1:NPa74jSVR/+eez2dFsEIHNa+3o09vtNaWwWwb1qSxSs=
k8s.io/component-base v0.31.2 h1:Z1J1LIaC0AV+nzcPRFqfK09af6bZ4D1nAOpWsy9owlA=
Expand All @@ -498,8 +498,8 @@ k8s.io/klog/v2 v2.130.1 h1:n9Xl7H1Xvksem4KFG4PYbdQCQxqc/tTUyrgXaOhHSzk=
k8s.io/klog/v2 v2.130.1/go.mod h1:3Jpz1GvMt720eyJH1ckRHK1EDfpxISzJ7I9OYgaDtPE=
k8s.io/kube-openapi v0.0.0-20240228011516-70dd3763d340 h1:BZqlfIlq5YbRMFko6/PM7FjZpUb45WallggurYhKGag=
k8s.io/kube-openapi v0.0.0-20240228011516-70dd3763d340/go.mod h1:yD4MZYeKMBwQKVht279WycxKyM84kkAx2DPrTXaeb98=
k8s.io/kubectl v0.31.0 h1:kANwAAPVY02r4U4jARP/C+Q1sssCcN/1p9Nk+7BQKVg=
k8s.io/kubectl v0.31.0/go.mod h1:pB47hhFypGsaHAPjlwrNbvhXgmuAr01ZBvAIIUaI8d4=
k8s.io/kubectl v0.31.2 h1:gTxbvRkMBwvTSAlobiTVqsH6S8Aa1aGyBcu5xYLsn8M=
k8s.io/kubectl v0.31.2/go.mod h1:EyASYVU6PY+032RrTh5ahtSOMgoDRIux9V1JLKtG5xM=
k8s.io/utils v0.0.0-20240921022957-49e7df575cb6 h1:MDF6h2H/h4tbzmtIKTuctcwZmY0tY9mD9fNT47QO6HI=
k8s.io/utils v0.0.0-20240921022957-49e7df575cb6/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0=
oras.land/oras-go v1.2.5 h1:XpYuAwAb0DfQsunIyMfeET92emK8km3W4yEzZvUbsTo=
Expand Down
61 changes: 61 additions & 0 deletions vendor/github.com/NVIDIA/k8s-operator-libs/pkg/crdutil/README.md
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading