add unit tests to validate mount options#1043
add unit tests to validate mount options#1043Sneha-at wants to merge 2 commits intoGoogleCloudPlatform:mainfrom
Conversation
pkg/csi_driver/node_test.go
Outdated
| }, | ||
| }, | ||
| { | ||
| name: "validate sidecar bucket access check is disabled when host network is enabled", |
There was a problem hiding this comment.
Please include scenario and expected outcome in the test name: https://engdoc.corp.google.com/eng/doc/tott/episodes/660.md?cl=head
pkg/csi_driver/node.go
Outdated
|
|
||
| if enableCloudProfilerForSidecar && gcsFuseSidecarImage != "" && isSidecarVersionSupportedForGivenFeature(gcsFuseSidecarImage, SidecarCloudProfilerMinVersion) { | ||
| fuseMountOptions = joinMountOptions(fuseMountOptions, []string{util.EnableCloudProfilerForSidecarConst + "=" + strconv.FormatBool(enableCloudProfilerForSidecar)}) | ||
| enableCloudProfilerForVersion := enableCloudProfilerForSidecar && gcsFuseSidecarImage != "" && isSidecarVersionSupportedForGivenFeature(gcsFuseSidecarImage, SidecarCloudProfilerMinVersion) |
There was a problem hiding this comment.
Can we return error at the beginning if gcsFuseSidecarImage == "" instead of continuing to check for this throughout? gcsFuseSidecarImage == "" should should never happen, and if it does, we should return error
| return "" | ||
| } | ||
|
|
||
| func addFuseMountOptions(identityProvider string, identityPool string, fuseMountOptions []string, vc map[string]string, hostNetworkEnabled bool, enableSidecarBucketAccessCheckForSidecarVersion bool, enableCloudProfilerForVersion bool) ([]string, error) { |
There was a problem hiding this comment.
Please add function comment .
Is there a reason why we didn't put everything into addFuseMountOptions. For example, testing the MachineTypeAutoConfig?
There was a problem hiding this comment.
I have added all the references for joinMountptions call in this addFuseMountOptionsfunction. MachineTypeAutoConfig seems to be using a slightly different approach to pass in the mount options so did not combine that.
| } | ||
| } | ||
|
|
||
| func TestAddFuseMountOptions(t *testing.T) { |
There was a problem hiding this comment.
Please add additional missing test cases.
- Case where inputFuseMountOptions != []string{} (What happens if a field already exists that we need to add?)
Sneha-at
force-pushed
the
add_token_retry
branch
from
bdfdbf1 to
4e52f8c
Compare
2 participants
What type of PR is this?
/kind failing-test
What this PR does / why we need it:
Adds unit tests to validate fuse mount options are correctly set for respective features.
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Testing
Feature overlay
Stable overlay
Does this PR introduce a user-facing change?: