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

Jump to bottom

Add Cloud Storage FUSE Metadata/Stat cache prefetch from kubernetes. #360

Merged
merged 1 commit into from
Nov 12, 2024
Merged

Add Cloud Storage FUSE Metadata/Stat cache prefetch from kubernetes. #360

merged 1 commit into from
Nov 12, 2024

Conversation

hime
Copy link
Collaborator

@hime hime commented Oct 14, 2024 •
edited
Loading

What type of PR is this?

Uncomment only one /kind <> line, hit enter to put that in a new line, and remove leading whitespaces from that line:

/kind api-change
/kind bug
/kind cleanup
/kind design
/kind documentation
/kind failing-test

/kind feature

/kind flake

What this PR does / why we need it:
Prefill stat/metadata cache as pod starts to reduce number of list calls during workload execution.

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:
Yes, this PR adds a new boolean API field within volumeAttributes called gcsfuseMetadataPrefetchOnMount.

Add Cloud Storage FUSE Metadata/Stat cache prefetch from Kubernetes.

@hime hime requested review from saikat-royc and msau42 October 14, 2024 21:23
@hime hime force-pushed the metadata-prefetch branch from 2655502 to c418cf0 Compare October 14, 2024 21:58
@hime hime changed the title Add Cloud Storage FUSE Metadata/Stat cache prefetch. Add Cloud Storage FUSE Metadata/Stat cache prefetch from kubernetes. Oct 14, 2024
@hime hime force-pushed the metadata-prefetch branch 3 times, most recently from b514089 to b6c810b Compare October 15, 2024 20:44
hime
hime commented Oct 16, 2024
View reviewed changes
pkg/webhook/client.go Outdated Show resolved Hide resolved
hime
hime commented Oct 16, 2024
View reviewed changes
pkg/webhook/injection.go Outdated Show resolved Hide resolved
hime
hime commented Oct 16, 2024
View reviewed changes
pkg/webhook/injection.go Outdated Show resolved Hide resolved
hime
hime commented Oct 16, 2024
View reviewed changes
pkg/webhook/injection.go Outdated Show resolved Hide resolved
hime
hime commented Oct 16, 2024
View reviewed changes
pkg/webhook/sidecar_spec.go Outdated Show resolved Hide resolved
hime
hime commented Oct 16, 2024
View reviewed changes
pkg/webhook/sidecar_spec.go Outdated Show resolved Hide resolved
hime
hime commented Oct 16, 2024
View reviewed changes
pkg/webhook/sidecar_spec.go Outdated Show resolved Hide resolved
@hime hime force-pushed the metadata-prefetch branch 2 times, most recently from 8296c5e to 47c84dd Compare October 17, 2024 19:19
@hime hime marked this pull request as ready for review October 17, 2024 20:13
@hime hime added go Pull requests that update Go code enhancement New feature or request labels Oct 18, 2024
@hime hime force-pushed the metadata-prefetch branch 3 times, most recently from 3fa65bb to 098d32a Compare October 24, 2024 00:08
msau42
msau42 reviewed Nov 1, 2024
View reviewed changes
cmd/metadata_prefetch/Dockerfile Show resolved Hide resolved
cmd/metadata_prefetch/main.go Outdated Show resolved Hide resolved
cmd/webhook/main.go Outdated Show resolved Hide resolved
saikat-royc
saikat-royc reviewed Nov 4, 2024
View reviewed changes
cmd/metadata_prefetch/Dockerfile Show resolved Hide resolved
cmd/metadata_prefetch/main.go Show resolved Hide resolved
cmd/metadata_prefetch/main.go Outdated Show resolved Hide resolved
cmd/metadata_prefetch/main.go Outdated Show resolved Hide resolved
cmd/metadata_prefetch/main.go Show resolved Hide resolved
pkg/webhook/mutatingwebhook.go Outdated Show resolved Hide resolved
pkg/webhook/injection.go Outdated Show resolved Hide resolved
pkg/webhook/injection.go Outdated Show resolved Hide resolved
pkg/webhook/injection.go Outdated Show resolved Hide resolved
pkg/webhook/injection.go Outdated
// Privately Hosted Sidecar Image feature for clusters running with limited internet access.
privateMetadataPrefetchSidecarImage, err := ParseSidecarContainerImage(&pod.Spec, SidecarMetadataPrefetchName)
if err != nil {
klog.Errorf("failed to get privately hosted metadata prefetch image... skipping injection.")
Copy link
Collaborator

Choose a reason for hiding this comment

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

this can be a info or warning since we donot always manually inject containers

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This should be an error. We only emit this error when we're sure the user provided a custom sidecar image that was invalid.

@hime hime force-pushed the metadata-prefetch branch from 80906b1 to 713ff01 Compare November 4, 2024 21:45
@hime hime force-pushed the metadata-prefetch branch 6 times, most recently from b281615 to c441b09 Compare November 7, 2024 20:30
hime
hime commented Nov 8, 2024
View reviewed changes
pkg/webhook/client.go Outdated Show resolved Hide resolved
@hime hime force-pushed the metadata-prefetch branch from c441b09 to bb70b50 Compare November 8, 2024 23:11
saikat-royc
saikat-royc reviewed Nov 8, 2024
View reviewed changes
pkg/webhook/client.go Outdated
}

// GetPV returns an error if pvc.Spec.VolumeName is not empty and the associated PV object is not found in the API server.
fetchedPv, err := si.GetPV(pvc.Spec.VolumeName)
Copy link
Collaborator

Choose a reason for hiding this comment

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

we can reuse the same pv variable that we defined in L35

@saikat-royc
Copy link
Collaborator

For the new tests added for this feature in generateTestSkip() 1. we need to skip managed driver tests 2. eventually add a min version (based on the expected gke component version)

@hime hime force-pushed the metadata-prefetch branch 3 times, most recently from 220da5e to a9d13b6 Compare November 12, 2024 03:15
@hime
Copy link
Collaborator Author

hime commented Nov 12, 2024 •
edited
Loading

For the new tests added for this feature in generateTestSkip() 1. we need to skip managed driver tests 2. eventually add a min version (based on the expected gke component version)

@saikat-royc Disabled tests for managed driver and when running on clusters that do not support native sidecar.

@hime hime force-pushed the metadata-prefetch branch 2 times, most recently from 4ff9462 to 588aa4b Compare November 12, 2024 04:46
@saikat-royc
Copy link
Collaborator

/lgtm

@hime hime added the Approved label Nov 12, 2024
@hime hime force-pushed the metadata-prefetch branch from 588aa4b to 0d0dd42 Compare November 12, 2024 21:12
@hime hime force-pushed the metadata-prefetch branch from 0d0dd42 to ce6fa17 Compare November 12, 2024 21:23
@hime hime merged commit 531aa7b into main Nov 12, 2024
8 checks passed
This was referenced Nov 13, 2024
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@saikat-royc saikat-royc saikat-royc left review comments

@msau42 msau42 msau42 left review comments

Assignees
No one assigned
Labels
Approved enhancement New feature or request go Pull requests that update Go code
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@hime @saikat-royc @msau42