Skip to content

Commit

Permalink
Address improvements.
Browse files Browse the repository at this point in the history
  • Loading branch information
hime committed Nov 4, 2024
1 parent d05e65a commit 713ff01
Show file tree
Hide file tree
Showing 9 changed files with 86 additions and 37 deletions.
14 changes: 12 additions & 2 deletions cmd/metadata_prefetch/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

Expand Down
37 changes: 33 additions & 4 deletions cmd/metadata_prefetch/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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
}
1 change: 0 additions & 1 deletion cmd/webhook/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down
2 changes: 1 addition & 1 deletion pkg/webhook/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand Down
13 changes: 6 additions & 7 deletions pkg/webhook/injection.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
}
Expand Down
12 changes: 6 additions & 6 deletions pkg/webhook/injection_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -445,7 +445,7 @@ func TestInjectMetadataPrefetchSidecar(t *testing.T) {
Spec: corev1.PodSpec{
InitContainers: []corev1.Container{
{
Name: "gke-gcsfuse-sidecar",
Name: GcsFuseSidecarName,
},
{
Name: "two",
Expand Down Expand Up @@ -513,7 +513,7 @@ func TestInjectMetadataPrefetchSidecar(t *testing.T) {
Spec: corev1.PodSpec{
InitContainers: []corev1.Container{
{
Name: "gke-gcsfuse-sidecar",
Name: GcsFuseSidecarName,
},
{
Name: "two",
Expand Down Expand Up @@ -594,7 +594,7 @@ func TestInjectMetadataPrefetchSidecar(t *testing.T) {
Spec: corev1.PodSpec{
InitContainers: []corev1.Container{
{
Name: "gke-gcsfuse-sidecar",
Name: GcsFuseSidecarName,
},
{
Name: "two",
Expand Down Expand Up @@ -755,7 +755,7 @@ func TestInjectMetadataPrefetchSidecar(t *testing.T) {
Spec: corev1.PodSpec{
InitContainers: []corev1.Container{
{
Name: "gke-gcsfuse-sidecar",
Name: GcsFuseSidecarName,
},
{
Name: MetadataPrefetchSidecarName,
Expand Down Expand Up @@ -872,7 +872,7 @@ func TestInjectMetadataPrefetchSidecar(t *testing.T) {
Spec: corev1.PodSpec{
InitContainers: []corev1.Container{
{
Name: "gke-gcsfuse-sidecar",
Name: GcsFuseSidecarName,
},
{
Name: MetadataPrefetchSidecarName,
Expand Down Expand Up @@ -993,7 +993,7 @@ func TestInjectMetadataPrefetchSidecar(t *testing.T) {
Spec: corev1.PodSpec{
InitContainers: []corev1.Container{
{
Name: "gke-gcsfuse-sidecar",
Name: GcsFuseSidecarName,
},
{
Name: MetadataPrefetchSidecarName,
Expand Down
6 changes: 3 additions & 3 deletions pkg/webhook/mutatingwebhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
36 changes: 24 additions & 12 deletions pkg/webhook/parsers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion pkg/webhook/parsers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down

0 comments on commit 713ff01

Please sign in to comment.