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 testing infra for checking whether an in-tree plugin is using backend that is shimmed to CSI #77101

Merged
merged 2 commits into from
May 1, 2019
Merged

Add testing infra for checking whether an in-tree plugin is using backend that is shimmed to CSI #77101

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
213cc99
Operation generator migration metric fixes and test metrics retrieval…
davidz627 Apr 25, 2019
1271237
Add migration metrics checking to all test suites
davidz627 Apr 25, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
46 changes: 42 additions & 4 deletions pkg/volume/util/operationexecutor/operation_generator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -295,15 +295,16 @@ func (og *operationGenerator) GenerateBulkVolumeVerifyFunc(
func (og *operationGenerator) GenerateAttachVolumeFunc(
volumeToAttach VolumeToAttach,
actualStateOfWorld ActualStateOfWorldAttacherUpdater) volumetypes.GeneratedOperations {
originalSpec := volumeToAttach.VolumeSpec
attachVolumeFunc := func() (error, error) {
var attachableVolumePlugin volume.AttachableVolumePlugin
originalSpec := volumeToAttach.VolumeSpec

nu, err := nodeUsingCSIPlugin(og, volumeToAttach.VolumeSpec, volumeToAttach.NodeName)
if err != nil {
return volumeToAttach.GenerateError("AttachVolume.NodeUsingCSIPlugin failed", err)
}

// useCSIPlugin will check both CSIMigration and the plugin specific feature gate
// useCSIPlugin will check both CSIMigration and the plugin specific feature gates
if useCSIPlugin(og.volumePluginMgr, volumeToAttach.VolumeSpec) && nu {
// The volume represented by this spec is CSI and thus should be migrated
attachableVolumePlugin, err = og.volumePluginMgr.FindAttachablePluginByName(csi.CSIPluginName)
Expand Down Expand Up @@ -382,8 +383,32 @@ func (og *operationGenerator) GenerateAttachVolumeFunc(
}
}

// Get attacher plugin
attachableVolumePluginName := unknownAttachableVolumePlugin
// TODO(dyzz) Ignoring this error means that if the plugin is migrated and
// any transient error is encountered (API unavailable, driver not installed)
// the operation will have it's metric registered with the in-tree plugin instead
// of the CSI Driver we migrated to. Fixing this requires a larger refactor that
// involves determining the plugin_name for the metric generating "CompleteFunc"
// during the actual "OperationFunc" and not during this generation function

nu, err := nodeUsingCSIPlugin(og, volumeToAttach.VolumeSpec, volumeToAttach.NodeName)
if err != nil {
klog.Errorf("GenerateAttachVolumeFunc failed to check if node is using CSI Plugin, metric for this operation may be inaccurate: %v", err)
}

// Need to translate the spec here if the plugin is migrated so that the metrics
// emitted show the correct (migrated) plugin
if useCSIPlugin(og.volumePluginMgr, volumeToAttach.VolumeSpec) && nu {
csiSpec, err := translateSpec(volumeToAttach.VolumeSpec)
if err == nil {
volumeToAttach.VolumeSpec = csiSpec
}
// If we have an error here we ignore it, the metric emitted will then be for the
// in-tree plugin. This error case(skipped one) will also trigger an error
// while the generated function is executed. And those errors will be handled during the execution of the generated
// function with a back off policy.
}
// Get attacher plugin
attachableVolumePlugin, err :=
og.volumePluginMgr.FindAttachablePluginBySpec(volumeToAttach.VolumeSpec)
// It's ok to ignore the error, returning error is not expected from this function.
Expand Down Expand Up @@ -528,15 +553,28 @@ func (og *operationGenerator) GenerateMountVolumeFunc(
actualStateOfWorld ActualStateOfWorldMounterUpdater,
isRemount bool) volumetypes.GeneratedOperations {
// Get mounter plugin
originalSpec := volumeToMount.VolumeSpec
volumePluginName := unknownVolumePlugin
// Need to translate the spec here if the plugin is migrated so that the metrics
// emitted show the correct (migrated) plugin
if useCSIPlugin(og.volumePluginMgr, volumeToMount.VolumeSpec) {
csiSpec, err := translateSpec(volumeToMount.VolumeSpec)
if err == nil {
volumeToMount.VolumeSpec = csiSpec
}
// If we have an error here we ignore it, the metric emitted will then be for the
// in-tree plugin. This error case(skipped one) will also trigger an error
// while the generated function is executed. And those errors will be handled during the execution of the generated
// function with a back off policy.
}
volumePlugin, err :=
og.volumePluginMgr.FindPluginBySpec(volumeToMount.VolumeSpec)
if err == nil && volumePlugin != nil {
volumePluginName = volumePlugin.GetPluginName()
}

mountVolumeFunc := func() (error, error) {
originalSpec := volumeToMount.VolumeSpec

// Get mounter plugin
if useCSIPlugin(og.volumePluginMgr, volumeToMount.VolumeSpec) {
Copy link
Member

Choose a reason for hiding this comment

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

Do we still need to translate the spec here too since it's done earlier now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it is safer to as the one done earlier just swallows any errors it encounters. This issue can be resolved along with this: https://github.com/kubernetes/kubernetes/pull/77101/files#diff-450e811a4953f760ff1594ede8b2037eR388

I think the idea would be to determine which plugin to emit the metric for when the operation is being run (since this now actually depends on some runtime state), instead of prior in the operation generator func.

csiSpec, err := translateSpec(volumeToMount.VolumeSpec)
Expand Down
78 changes: 46 additions & 32 deletions test/e2e/storage/drivers/in_tree.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,9 @@ var _ testsuites.DynamicPVTestDriver = &nfsDriver{}
func InitNFSDriver() testsuites.TestDriver {
return &nfsDriver{
driverInfo: testsuites.DriverInfo{
Name: "nfs",
MaxFileSize: testpatterns.FileSizeLarge,
Name: "nfs",
InTreePluginName: "kubernetes.io/nfs",
MaxFileSize: testpatterns.FileSizeLarge,
SupportedFsType: sets.NewString(
"", // Default fsType
),
Expand Down Expand Up @@ -228,8 +229,9 @@ var _ testsuites.PreprovisionedPVTestDriver = &glusterFSDriver{}
func InitGlusterFSDriver() testsuites.TestDriver {
return &glusterFSDriver{
driverInfo: testsuites.DriverInfo{
Name: "gluster",
MaxFileSize: testpatterns.FileSizeMedium,
Name: "gluster",
InTreePluginName: "kubernetes.io/glusterfs",
MaxFileSize: testpatterns.FileSizeMedium,
SupportedFsType: sets.NewString(
"", // Default fsType
),
Expand Down Expand Up @@ -345,9 +347,10 @@ var _ testsuites.PreprovisionedPVTestDriver = &iSCSIDriver{}
func InitISCSIDriver() testsuites.TestDriver {
return &iSCSIDriver{
driverInfo: testsuites.DriverInfo{
Name: "iscsi",
FeatureTag: "[Feature:Volumes]",
MaxFileSize: testpatterns.FileSizeMedium,
Name: "iscsi",
InTreePluginName: "kubernetes.io/iscsi",
FeatureTag: "[Feature:Volumes]",
MaxFileSize: testpatterns.FileSizeMedium,
SupportedFsType: sets.NewString(
"", // Default fsType
"ext2",
Expand Down Expand Up @@ -457,9 +460,10 @@ var _ testsuites.PreprovisionedPVTestDriver = &rbdDriver{}
func InitRbdDriver() testsuites.TestDriver {
return &rbdDriver{
driverInfo: testsuites.DriverInfo{
Name: "rbd",
FeatureTag: "[Feature:Volumes]",
MaxFileSize: testpatterns.FileSizeMedium,
Name: "rbd",
InTreePluginName: "kubernetes.io/rbd",
FeatureTag: "[Feature:Volumes]",
MaxFileSize: testpatterns.FileSizeMedium,
SupportedFsType: sets.NewString(
"", // Default fsType
"ext2",
Expand Down Expand Up @@ -584,9 +588,10 @@ var _ testsuites.PreprovisionedPVTestDriver = &cephFSDriver{}
func InitCephFSDriver() testsuites.TestDriver {
return &cephFSDriver{
driverInfo: testsuites.DriverInfo{
Name: "ceph",
FeatureTag: "[Feature:Volumes]",
MaxFileSize: testpatterns.FileSizeMedium,
Name: "ceph",
InTreePluginName: "kubernetes.io/cephfs",
FeatureTag: "[Feature:Volumes]",
MaxFileSize: testpatterns.FileSizeMedium,
SupportedFsType: sets.NewString(
"", // Default fsType
),
Expand Down Expand Up @@ -683,8 +688,9 @@ var _ testsuites.InlineVolumeTestDriver = &hostPathDriver{}
func InitHostPathDriver() testsuites.TestDriver {
return &hostPathDriver{
driverInfo: testsuites.DriverInfo{
Name: "hostPath",
MaxFileSize: testpatterns.FileSizeMedium,
Name: "hostPath",
InTreePluginName: "kubernetes.io/host-path",
MaxFileSize: testpatterns.FileSizeMedium,
SupportedFsType: sets.NewString(
"", // Default fsType
),
Expand Down Expand Up @@ -755,8 +761,9 @@ var _ testsuites.InlineVolumeTestDriver = &hostPathSymlinkDriver{}
func InitHostPathSymlinkDriver() testsuites.TestDriver {
return &hostPathSymlinkDriver{
driverInfo: testsuites.DriverInfo{
Name: "hostPathSymlink",
MaxFileSize: testpatterns.FileSizeMedium,
Name: "hostPathSymlink",
InTreePluginName: "kubernetes.io/host-path",
MaxFileSize: testpatterns.FileSizeMedium,
SupportedFsType: sets.NewString(
"", // Default fsType
),
Expand Down Expand Up @@ -895,8 +902,9 @@ var _ testsuites.InlineVolumeTestDriver = &emptydirDriver{}
func InitEmptydirDriver() testsuites.TestDriver {
return &emptydirDriver{
driverInfo: testsuites.DriverInfo{
Name: "emptydir",
MaxFileSize: testpatterns.FileSizeMedium,
Name: "emptydir",
InTreePluginName: "kubernetes.io/empty-dir",
MaxFileSize: testpatterns.FileSizeMedium,
SupportedFsType: sets.NewString(
"", // Default fsType
),
Expand Down Expand Up @@ -960,8 +968,9 @@ var _ testsuites.DynamicPVTestDriver = &cinderDriver{}
func InitCinderDriver() testsuites.TestDriver {
return &cinderDriver{
driverInfo: testsuites.DriverInfo{
Name: "cinder",
MaxFileSize: testpatterns.FileSizeMedium,
Name: "cinder",
InTreePluginName: "kubernetes.io/cinder",
MaxFileSize: testpatterns.FileSizeMedium,
SupportedFsType: sets.NewString(
"", // Default fsType
"ext3",
Expand Down Expand Up @@ -1129,6 +1138,7 @@ func InitGcePdDriver() testsuites.TestDriver {
return &gcePdDriver{
driverInfo: testsuites.DriverInfo{
Name: "gcepd",
InTreePluginName: "kubernetes.io/gce-pd",
MaxFileSize: testpatterns.FileSizeMedium,
SupportedFsType: supportedTypes,
SupportedMountOption: sets.NewString("debug", "nouid32"),
Expand Down Expand Up @@ -1255,8 +1265,9 @@ var _ testsuites.DynamicPVTestDriver = &vSphereDriver{}
func InitVSphereDriver() testsuites.TestDriver {
return &vSphereDriver{
driverInfo: testsuites.DriverInfo{
Name: "vSphere",
MaxFileSize: testpatterns.FileSizeMedium,
Name: "vSphere",
InTreePluginName: "kubernetes.io/vsphere-volume",
MaxFileSize: testpatterns.FileSizeMedium,
SupportedFsType: sets.NewString(
"", // Default fsType
"ext4",
Expand Down Expand Up @@ -1376,8 +1387,9 @@ var _ testsuites.DynamicPVTestDriver = &azureDriver{}
func InitAzureDriver() testsuites.TestDriver {
return &azureDriver{
driverInfo: testsuites.DriverInfo{
Name: "azure",
MaxFileSize: testpatterns.FileSizeMedium,
Name: "azure",
InTreePluginName: "kubernetes.io/azure-file",
MaxFileSize: testpatterns.FileSizeMedium,
SupportedFsType: sets.NewString(
"", // Default fsType
"ext4",
Expand Down Expand Up @@ -1494,8 +1506,9 @@ var _ testsuites.DynamicPVTestDriver = &awsDriver{}
func InitAwsDriver() testsuites.TestDriver {
return &awsDriver{
driverInfo: testsuites.DriverInfo{
Name: "aws",
MaxFileSize: testpatterns.FileSizeMedium,
Name: "aws",
InTreePluginName: "kubernetes.io/aws-ebs",
MaxFileSize: testpatterns.FileSizeMedium,
SupportedFsType: sets.NewString(
"", // Default fsType
"ext3",
Expand Down Expand Up @@ -1660,11 +1673,12 @@ func InitLocalDriverWithVolumeType(volumeType utils.LocalVolumeType) func() test
}
return &localDriver{
driverInfo: testsuites.DriverInfo{
Name: "local",
FeatureTag: featureTag,
MaxFileSize: maxFileSize,
SupportedFsType: supportedFsTypes,
Capabilities: capabilities,
Name: "local",
InTreePluginName: "kubernetes.io/local-volume",
FeatureTag: featureTag,
MaxFileSize: maxFileSize,
SupportedFsType: supportedFsTypes,
Capabilities: capabilities,
},
volumeType: volumeType,
}
Expand Down
2 changes: 2 additions & 0 deletions test/e2e/storage/testsuites/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,9 @@ go_library(
"//staging/src/k8s.io/apiserver/pkg/storage/names:go_default_library",
"//staging/src/k8s.io/client-go/dynamic:go_default_library",
"//staging/src/k8s.io/client-go/kubernetes:go_default_library",
"//staging/src/k8s.io/csi-translation-lib:go_default_library",
"//test/e2e/framework:go_default_library",
"//test/e2e/framework/metrics:go_default_library",
"//test/e2e/framework/podlogs:go_default_library",
"//test/e2e/framework/volume:go_default_library",
"//test/e2e/storage/testpatterns:go_default_library",
Expand Down