From 713ff01fbfc1fd32ba91b9f6a6991ba5132559d6 Mon Sep 17 00:00:00 2001 From: hime Date: Mon, 4 Nov 2024 19:44:40 +0000 Subject: [PATCH] Address improvements. --- cmd/metadata_prefetch/Dockerfile | 14 ++++++++++-- cmd/metadata_prefetch/main.go | 37 ++++++++++++++++++++++++++++---- cmd/webhook/main.go | 1 - pkg/webhook/client.go | 2 +- pkg/webhook/injection.go | 13 ++++++----- pkg/webhook/injection_test.go | 12 +++++------ pkg/webhook/mutatingwebhook.go | 6 +++--- pkg/webhook/parsers.go | 36 ++++++++++++++++++++----------- pkg/webhook/parsers_test.go | 2 +- 9 files changed, 86 insertions(+), 37 deletions(-) diff --git a/cmd/metadata_prefetch/Dockerfile b/cmd/metadata_prefetch/Dockerfile index f3ccb8a4..0a09a13f 100644 --- a/cmd/metadata_prefetch/Dockerfile +++ b/cmd/metadata_prefetch/Dockerfile @@ -26,8 +26,7 @@ RUN make metadata-prefetch BINDIR=/bin FROM gke.gcr.io/debian-base:bookworm-v1.0.4-gke.2 AS debian # go/gke-releasing-policies#base-images -FROM gcr.io/distroless/base-debian12 -ARG TARGETPLATFORM +FROM gcr.io/distroless/base-debian12 AS output-image # Copy existing binaries. COPY --from=debian /bin/ls /bin/ls @@ -38,6 +37,17 @@ COPY --from=debian /lib/x86_64-linux-gnu/libc.so.6 /lib/x86_64-linux-gnu/libc.so COPY --from=debian /lib/x86_64-linux-gnu/libpcre2-8.so.0 /lib/x86_64-linux-gnu/libpcre2-8.so.0 COPY --from=debian /lib64/ld-linux-x86-64.so.2 /lib64/ld-linux-x86-64.so.2 +# Validate dependencies +FROM output-image AS validator-image +COPY --from=debian /bin/bash /bin/bash +COPY --from=debian /usr/bin/ldd /usr/bin/ldd +COPY --from=debian /bin/grep /bin/grep +SHELL ["/bin/bash", "-c"] +RUN if ldd /bin/ls | grep "not found"; then echo "!!! Missing deps for ls command !!!" && exit 1; fi + +# Final image +FROM output-image + # Copy the built binaries COPY --from=metadata-prefetch-builder /bin/gcs-fuse-csi-driver-metadata-prefetch /gcs-fuse-csi-driver-metadata-prefetch diff --git a/cmd/metadata_prefetch/main.go b/cmd/metadata_prefetch/main.go index ba6fde1c..e556f95a 100644 --- a/cmd/metadata_prefetch/main.go +++ b/cmd/metadata_prefetch/main.go @@ -23,11 +23,16 @@ import ( "os" "os/exec" "os/signal" + "strings" "syscall" "k8s.io/klog/v2" ) +const ( + mountPathsLocation = "/volumes/" +) + func main() { klog.InitFlags(nil) flag.Parse() @@ -49,25 +54,49 @@ func main() { // Start the "ls" command in the background. // All our volumes are mounted under the /volumes/ directory. - cmd := exec.CommandContext(ctx, "ls", "-R", "/volumes/") + cmd := exec.CommandContext(ctx, "ls", "-R", mountPathsLocation) cmd.Stdout = nil // Connects file descriptor to the null device (os.DevNull). // TODO(hime): We should research stratergies to parallelize ls execution and speed up cache population. err := cmd.Start() if err == nil { - klog.Info("Running ls on bucket(s)") + mountPaths, err := getDirectoryNames(mountPathsLocation) + if err != nil { + klog.Infof("Running ls on mountPath(s): %s", strings.Join(mountPaths, ", ")) + } else { + klog.Warningf("failed to get mountPaths: %v", err) + } + err = cmd.Wait() if err != nil { klog.Errorf("Error while executing ls command: %v", err) } else { - klog.Info("Metadata prefetch complete.") + klog.Info("Metadata prefetch complete") } } else { klog.Errorf("Error starting ls command: %v.", err) } - klog.Info("Going to sleep...") + klog.Info("going to sleep...") // Keep the process running. select {} } + +// getDirectoryNames returns a list of strings with the names of all the directories +// present within the provided directory path. +func getDirectoryNames(dirPath string) ([]string, error) { + directories := []string{} + items, err := os.ReadDir(dirPath) + if err != nil { + return directories, err + } + + for _, item := range items { + if item.IsDir() { + directories = append(directories, item.Name()) + } + } + + return directories, nil +} diff --git a/cmd/webhook/main.go b/cmd/webhook/main.go index e88fb9c3..01134799 100644 --- a/cmd/webhook/main.go +++ b/cmd/webhook/main.go @@ -104,7 +104,6 @@ func main() { // Setup Informer informerFactory := informers.NewSharedInformerFactory(client, resyncDuration) nodeLister := informerFactory.Core().V1().Nodes().Lister() - // need tests that use these listers. pvcLister := informerFactory.Core().V1().PersistentVolumeClaims().Lister() pvLister := informerFactory.Core().V1().PersistentVolumes().Lister() diff --git a/pkg/webhook/client.go b/pkg/webhook/client.go index a5befbea..63745d61 100644 --- a/pkg/webhook/client.go +++ b/pkg/webhook/client.go @@ -23,8 +23,8 @@ import ( corev1 "k8s.io/api/core/v1" ) +// IsPreprovisionCSIVolume checks whether the volume is a pre-provisioned volume for the desired csiDriver. func (si *SidecarInjector) IsPreprovisionCSIVolume(csiDriver string, pvc *corev1.PersistentVolumeClaim) (bool, *corev1.PersistentVolume, error) { - // IsPreprovisionCSIVolume checks whether the volume is a pre-provisioned volume for the desired csiDriver. if csiDriver == "" { return false, nil, errors.New("failed to check IsPreprovisionCSIVolume, csiDriver is empty") } diff --git a/pkg/webhook/injection.go b/pkg/webhook/injection.go index 4b173f66..42797720 100644 --- a/pkg/webhook/injection.go +++ b/pkg/webhook/injection.go @@ -94,15 +94,14 @@ func (si *SidecarInjector) injectMetadataPrefetchSidecarContainer(pod *corev1.Po var containerSpec corev1.Container var index int - // We allow sidecar to be present on regular container list to support - // Privately Hosted Sidecar Image feature for clusters running with limited internet access. - privateMetadataPrefetchSidecarImage, err := ParseSidecarContainerImage(&pod.Spec, MetadataPrefetchSidecarName) + // Extract user provided metadata prefetch sidecar image. + userProvidedMetadataPrefetchSidecarImage, err := ExtractContainerImageAndDeleteContainer(&pod.Spec, MetadataPrefetchSidecarName) if err != nil { - klog.Errorf("failed to get privately hosted metadata prefetch image... skipping injection.") + klog.Errorf("failed to get user provided metadata prefetch image... skipping injection.") } - if privateMetadataPrefetchSidecarImage != "" { - config.MetadataContainerImage = privateMetadataPrefetchSidecarImage + if userProvidedMetadataPrefetchSidecarImage != "" { + config.MetadataContainerImage = userProvidedMetadataPrefetchSidecarImage } if injectAsNativeSidecar { @@ -121,7 +120,7 @@ func (si *SidecarInjector) injectMetadataPrefetchSidecarContainer(pod *corev1.Po // This should not happen as we always inject the sidecar after injecting our primary gcsfuse sidecar. if index == 0 { - klog.Warning("gke-gcsfuse-sidecar not found when attempting to inject metadata prefetch sidecar... skipping injection") + klog.Warningf("%s not found when attempting to inject metadata prefetch sidecar... skipping injection", GcsFuseSidecarName) return } diff --git a/pkg/webhook/injection_test.go b/pkg/webhook/injection_test.go index 3ccb559c..97d283db 100644 --- a/pkg/webhook/injection_test.go +++ b/pkg/webhook/injection_test.go @@ -445,7 +445,7 @@ func TestInjectMetadataPrefetchSidecar(t *testing.T) { Spec: corev1.PodSpec{ InitContainers: []corev1.Container{ { - Name: "gke-gcsfuse-sidecar", + Name: GcsFuseSidecarName, }, { Name: "two", @@ -513,7 +513,7 @@ func TestInjectMetadataPrefetchSidecar(t *testing.T) { Spec: corev1.PodSpec{ InitContainers: []corev1.Container{ { - Name: "gke-gcsfuse-sidecar", + Name: GcsFuseSidecarName, }, { Name: "two", @@ -594,7 +594,7 @@ func TestInjectMetadataPrefetchSidecar(t *testing.T) { Spec: corev1.PodSpec{ InitContainers: []corev1.Container{ { - Name: "gke-gcsfuse-sidecar", + Name: GcsFuseSidecarName, }, { Name: "two", @@ -755,7 +755,7 @@ func TestInjectMetadataPrefetchSidecar(t *testing.T) { Spec: corev1.PodSpec{ InitContainers: []corev1.Container{ { - Name: "gke-gcsfuse-sidecar", + Name: GcsFuseSidecarName, }, { Name: MetadataPrefetchSidecarName, @@ -872,7 +872,7 @@ func TestInjectMetadataPrefetchSidecar(t *testing.T) { Spec: corev1.PodSpec{ InitContainers: []corev1.Container{ { - Name: "gke-gcsfuse-sidecar", + Name: GcsFuseSidecarName, }, { Name: MetadataPrefetchSidecarName, @@ -993,7 +993,7 @@ func TestInjectMetadataPrefetchSidecar(t *testing.T) { Spec: corev1.PodSpec{ InitContainers: []corev1.Container{ { - Name: "gke-gcsfuse-sidecar", + Name: GcsFuseSidecarName, }, { Name: MetadataPrefetchSidecarName, diff --git a/pkg/webhook/mutatingwebhook.go b/pkg/webhook/mutatingwebhook.go index adeb9d52..5530aecd 100644 --- a/pkg/webhook/mutatingwebhook.go +++ b/pkg/webhook/mutatingwebhook.go @@ -94,9 +94,9 @@ func (si *SidecarInjector) Handle(_ context.Context, req admission.Request) admi return admission.Errored(http.StatusBadRequest, err) } - if image, err := ParseSidecarContainerImage(&pod.Spec, GcsFuseSidecarName); err == nil { - if image != "" { - config.ContainerImage = image + if userProvidedGcsFuseSidecarImage, err := ExtractContainerImageAndDeleteContainer(&pod.Spec, GcsFuseSidecarName); err == nil { + if userProvidedGcsFuseSidecarImage != "" { + config.ContainerImage = userProvidedGcsFuseSidecarImage } } else { return admission.Errored(http.StatusBadRequest, err) diff --git a/pkg/webhook/parsers.go b/pkg/webhook/parsers.go index 65acd461..e59023b0 100644 --- a/pkg/webhook/parsers.go +++ b/pkg/webhook/parsers.go @@ -38,22 +38,25 @@ func ParseBool(str string) (bool, error) { } } -// parseSidecarContainerImage supports our Privately Hosted Sidecar Image option -// by iterating the container list and finding a container named "gke-gcsfuse-sidecar" -// If we find "gke-gcsfuse-sidecar": -// - extract the container image and check if the image is valid +// ExtractContainerImageAndDeleteContainer supports the injection of custom sidecar images. +// We iterate the container list and find a container named "containerName" +// If we find "containerName": +// - extract the container image // - removes the container definition from the container list. -// - remove any mentions of "gke-gcsfuse-sidecar" from initContainer list. +// - verifies if the image is valid // - return image // -// Note: This methos MUST delete all containers using containerName in the Pod Spec. -func ParseSidecarContainerImage(podSpec *corev1.PodSpec, containerName string) (string, error) { +// We support custom sidecar images because: +// - Requirement for Privately Hosted Sidecar Image feature, for clusters running with limited internet access. +// - Allow fast testing of new sidecar image on a production environemnt, usually related to a new gcsfuse binary. +func ExtractContainerImageAndDeleteContainer(podSpec *corev1.PodSpec, containerName string) (string, error) { var image string - // Find container named "gke-gcsfuse-sidecar" (sidecarContainerName), extract its image, and remove from list. + // Find Container named containerName, extract its image, and remove from list. if index, present := containerPresent(podSpec.Containers, containerName); present { image = podSpec.Containers[index].Image + // The next webhook step is to reinject the sidecar, removing user declaration to prevent dual injection creation failures. if image != "" { copy(podSpec.Containers[index:], podSpec.Containers[index+1:]) podSpec.Containers = podSpec.Containers[:len(podSpec.Containers)-1] @@ -64,10 +67,19 @@ func ParseSidecarContainerImage(podSpec *corev1.PodSpec, containerName string) ( } } - // Remove any mention of gke-gcsfuse-sidecar from init container list. - if index, present := containerPresent(podSpec.InitContainers, containerName); present { - copy(podSpec.InitContainers[index:], podSpec.InitContainers[index+1:]) - podSpec.InitContainers = podSpec.InitContainers[:len(podSpec.InitContainers)-1] + // Find initContainer named containerName, extract its image, and remove from list. + if index, present := containerPresent(podSpec.Containers, containerName); present { + image = podSpec.Containers[index].Image + + // The next webhook step is to reinject the sidecar, removing user declaration to prevent dual injection creation failures. + if image != "" { + copy(podSpec.Containers[index:], podSpec.Containers[index+1:]) + podSpec.Containers = podSpec.Containers[:len(podSpec.Containers)-1] + } + + if _, _, _, err := parsers.ParseImageName(image); err != nil { + return "", fmt.Errorf("could not parse input image: %q, error: %w", image, err) + } } return image, nil diff --git a/pkg/webhook/parsers_test.go b/pkg/webhook/parsers_test.go index 9e3e1581..b6c48c0f 100644 --- a/pkg/webhook/parsers_test.go +++ b/pkg/webhook/parsers_test.go @@ -334,7 +334,7 @@ func TestParseSidecarContainerImage(t *testing.T) { for _, tc := range testCases { pod := tc.pod - image, err := ParseSidecarContainerImage(&pod.Spec, GcsFuseSidecarName) + image, err := ExtractContainerImageAndDeleteContainer(&pod.Spec, GcsFuseSidecarName) if image != tc.expectedImage { t.Errorf(`unexpected image: want: "%s" but got: "%s"`, tc.expectedImage, image) }