fix: azure file namespace issue in csi translation

fix build failure

fix comments
This commit is contained in:
andyzhangx 2021-04-19 09:36:09 +00:00
parent 425e33bd50
commit e10d3948f5
18 changed files with 72 additions and 32 deletions

View File

@ -21,7 +21,7 @@ import (
"fmt"
"strings"
"k8s.io/api/core/v1"
v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/sets"
utilfeature "k8s.io/apiserver/pkg/util/feature"
@ -84,7 +84,7 @@ func CreateVolumeSpec(podVolume v1.Volume, pod *v1.Pod, nodeName types.NodeName,
err)
}
volumeSpec, err = translateInTreeSpecToCSIIfNeeded(volumeSpec, nodeName, vpm, csiMigratedPluginManager, csiTranslator)
volumeSpec, err = translateInTreeSpecToCSIIfNeeded(volumeSpec, nodeName, vpm, csiMigratedPluginManager, csiTranslator, pod.Namespace)
if err != nil {
return nil, fmt.Errorf(
"error performing CSI migration checks and translation for PVC %q/%q: %v",
@ -109,7 +109,7 @@ func CreateVolumeSpec(podVolume v1.Volume, pod *v1.Pod, nodeName types.NodeName,
clonedPodVolume := podVolume.DeepCopy()
origspec := volume.NewSpecFromVolume(clonedPodVolume)
spec, err := translateInTreeSpecToCSIIfNeeded(origspec, nodeName, vpm, csiMigratedPluginManager, csiTranslator)
spec, err := translateInTreeSpecToCSIIfNeeded(origspec, nodeName, vpm, csiMigratedPluginManager, csiTranslator, pod.Namespace)
if err != nil {
return nil, fmt.Errorf(
"error performing CSI migration checks and translation for inline volume %q: %v",
@ -285,7 +285,7 @@ func ProcessPodVolumes(pod *v1.Pod, addVolumes bool, desiredStateOfWorld cache.D
return
}
func translateInTreeSpecToCSIIfNeeded(spec *volume.Spec, nodeName types.NodeName, vpm *volume.VolumePluginMgr, csiMigratedPluginManager csimigration.PluginManager, csiTranslator csimigration.InTreeToCSITranslator) (*volume.Spec, error) {
func translateInTreeSpecToCSIIfNeeded(spec *volume.Spec, nodeName types.NodeName, vpm *volume.VolumePluginMgr, csiMigratedPluginManager csimigration.PluginManager, csiTranslator csimigration.InTreeToCSITranslator, podNamespace string) (*volume.Spec, error) {
translatedSpec := spec
migratable, err := csiMigratedPluginManager.IsMigratable(spec)
if err != nil {
@ -300,7 +300,7 @@ func translateInTreeSpecToCSIIfNeeded(spec *volume.Spec, nodeName types.NodeName
return nil, err
}
if migratable && migrationSupportedOnNode {
translatedSpec, err = csimigration.TranslateInTreeSpecToCSI(spec, csiTranslator)
translatedSpec, err = csimigration.TranslateInTreeSpecToCSI(spec, podNamespace, csiTranslator)
if err != nil {
return nil, err
}

View File

@ -553,7 +553,7 @@ func (dswp *desiredStateOfWorldPopulator) createVolumeSpec(
return nil, nil, "", err
}
if migratable {
volumeSpec, err = csimigration.TranslateInTreeSpecToCSI(volumeSpec, dswp.intreeToCSITranslator)
volumeSpec, err = csimigration.TranslateInTreeSpecToCSI(volumeSpec, pod.Namespace, dswp.intreeToCSITranslator)
if err != nil {
return nil, nil, "", err
}
@ -595,7 +595,7 @@ func (dswp *desiredStateOfWorldPopulator) createVolumeSpec(
return nil, nil, "", err
}
if migratable {
spec, err = csimigration.TranslateInTreeSpecToCSI(spec, dswp.intreeToCSITranslator)
spec, err = csimigration.TranslateInTreeSpecToCSI(spec, pod.Namespace, dswp.intreeToCSITranslator)
if err != nil {
return nil, nil, "", err
}

View File

@ -122,19 +122,19 @@ func (pm PluginManager) IsMigratable(spec *volume.Spec) (bool, error) {
// from references to in-tree plugins to migrated CSI plugins
type InTreeToCSITranslator interface {
TranslateInTreePVToCSI(pv *v1.PersistentVolume) (*v1.PersistentVolume, error)
TranslateInTreeInlineVolumeToCSI(volume *v1.Volume) (*v1.PersistentVolume, error)
TranslateInTreeInlineVolumeToCSI(volume *v1.Volume, podNamespace string) (*v1.PersistentVolume, error)
}
// TranslateInTreeSpecToCSI translates a volume spec (either PV or inline volume)
// supported by an in-tree plugin to CSI
func TranslateInTreeSpecToCSI(spec *volume.Spec, translator InTreeToCSITranslator) (*volume.Spec, error) {
func TranslateInTreeSpecToCSI(spec *volume.Spec, podNamespace string, translator InTreeToCSITranslator) (*volume.Spec, error) {
var csiPV *v1.PersistentVolume
var err error
inlineVolume := false
if spec.PersistentVolume != nil {
csiPV, err = translator.TranslateInTreePVToCSI(spec.PersistentVolume)
} else if spec.Volume != nil {
csiPV, err = translator.TranslateInTreeInlineVolumeToCSI(spec.Volume)
csiPV, err = translator.TranslateInTreeInlineVolumeToCSI(spec.Volume, podNamespace)
inlineVolume = true
} else {
err = errors.New("not a valid volume spec")

View File

@ -58,7 +58,7 @@ type InTreeToCSITranslator interface {
GetInTreePluginNameFromSpec(pv *v1.PersistentVolume, vol *v1.Volume) (string, error)
GetCSINameFromInTreeName(pluginName string) (string, error)
TranslateInTreePVToCSI(pv *v1.PersistentVolume) (*v1.PersistentVolume, error)
TranslateInTreeInlineVolumeToCSI(volume *v1.Volume) (*v1.PersistentVolume, error)
TranslateInTreeInlineVolumeToCSI(volume *v1.Volume, podNamespace string) (*v1.PersistentVolume, error)
}
var _ OperationGenerator = &operationGenerator{}

View File

@ -86,7 +86,7 @@ func (t *awsElasticBlockStoreCSITranslator) TranslateInTreeStorageClassToCSI(sc
// TranslateInTreeInlineVolumeToCSI takes a Volume with AWSElasticBlockStore set from in-tree
// and converts the AWSElasticBlockStore source to a CSIPersistentVolumeSource
func (t *awsElasticBlockStoreCSITranslator) TranslateInTreeInlineVolumeToCSI(volume *v1.Volume) (*v1.PersistentVolume, error) {
func (t *awsElasticBlockStoreCSITranslator) TranslateInTreeInlineVolumeToCSI(volume *v1.Volume, podNamespace string) (*v1.PersistentVolume, error) {
if volume == nil || volume.AWSElasticBlockStore == nil {
return nil, fmt.Errorf("volume is nil or AWS EBS not defined on volume")
}

View File

@ -177,7 +177,7 @@ func TestTranslateInTreeInlineVolumeToCSI(t *testing.T) {
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
t.Logf("Testing %v", tc.name)
got, err := translator.TranslateInTreeInlineVolumeToCSI(&v1.Volume{Name: "volume", VolumeSource: tc.volumeSource})
got, err := translator.TranslateInTreeInlineVolumeToCSI(&v1.Volume{Name: "volume", VolumeSource: tc.volumeSource}, "")
if err != nil && !tc.expErr {
t.Fatalf("Did not expect error but got: %v", err)
}

View File

@ -93,7 +93,7 @@ func (t *azureDiskCSITranslator) TranslateInTreeStorageClassToCSI(sc *storage.St
// TranslateInTreeInlineVolumeToCSI takes a Volume with AzureDisk set from in-tree
// and converts the AzureDisk source to a CSIPersistentVolumeSource
func (t *azureDiskCSITranslator) TranslateInTreeInlineVolumeToCSI(volume *v1.Volume) (*v1.PersistentVolume, error) {
func (t *azureDiskCSITranslator) TranslateInTreeInlineVolumeToCSI(volume *v1.Volume, podNamespace string) (*v1.PersistentVolume, error) {
if volume == nil || volume.AzureDisk == nil {
return nil, fmt.Errorf("volume is nil or Azure Disk not defined on volume")
}

View File

@ -145,7 +145,7 @@ func TestTranslateAzureDiskInTreeStorageClassToCSI(t *testing.T) {
for _, tc := range cases {
t.Logf("Testing %v", tc.name)
got, err := translator.TranslateInTreeInlineVolumeToCSI(tc.volume)
got, err := translator.TranslateInTreeInlineVolumeToCSI(tc.volume, "")
if err != nil && !tc.expErr {
t.Errorf("Did not expect error but got: %v", err)
}

View File

@ -65,7 +65,7 @@ func (t *azureFileCSITranslator) TranslateInTreeStorageClassToCSI(sc *storage.St
// TranslateInTreeInlineVolumeToCSI takes a Volume with AzureFile set from in-tree
// and converts the AzureFile source to a CSIPersistentVolumeSource
func (t *azureFileCSITranslator) TranslateInTreeInlineVolumeToCSI(volume *v1.Volume) (*v1.PersistentVolume, error) {
func (t *azureFileCSITranslator) TranslateInTreeInlineVolumeToCSI(volume *v1.Volume, podNamespace string) (*v1.PersistentVolume, error) {
if volume == nil || volume.AzureFile == nil {
return nil, fmt.Errorf("volume is nil or Azure File not defined on volume")
}
@ -77,6 +77,11 @@ func (t *azureFileCSITranslator) TranslateInTreeInlineVolumeToCSI(volume *v1.Vol
accountName = azureSource.SecretName
}
secretNamespace := defaultSecretNamespace
if podNamespace != "" {
secretNamespace = podNamespace
}
var (
pv = &v1.PersistentVolume{
ObjectMeta: metav1.ObjectMeta{
@ -93,7 +98,7 @@ func (t *azureFileCSITranslator) TranslateInTreeInlineVolumeToCSI(volume *v1.Vol
VolumeAttributes: map[string]string{shareNameField: azureSource.ShareName},
NodeStageSecretRef: &v1.SecretReference{
Name: azureSource.SecretName,
Namespace: defaultSecretNamespace,
Namespace: secretNamespace,
},
},
},

View File

@ -102,10 +102,11 @@ func TestTranslateAzureFileInTreeStorageClassToCSI(t *testing.T) {
translator := NewAzureFileCSITranslator()
cases := []struct {
name string
volume *corev1.Volume
expVol *corev1.PersistentVolume
expErr bool
name string
volume *corev1.Volume
podNamespace string
expVol *corev1.PersistentVolume
expErr bool
}{
{
name: "empty volume",
@ -148,11 +149,44 @@ func TestTranslateAzureFileInTreeStorageClassToCSI(t *testing.T) {
},
},
},
{
name: "azure file volume with a pod namespace",
volume: &corev1.Volume{
VolumeSource: corev1.VolumeSource{
AzureFile: &corev1.AzureFileVolumeSource{
ReadOnly: true,
SecretName: "secretname",
ShareName: "sharename",
},
},
},
podNamespace: "test",
expVol: &corev1.PersistentVolume{
ObjectMeta: metav1.ObjectMeta{
Name: "file.csi.azure.com-sharename",
},
Spec: corev1.PersistentVolumeSpec{
PersistentVolumeSource: corev1.PersistentVolumeSource{
CSI: &corev1.CSIPersistentVolumeSource{
Driver: "file.csi.azure.com",
NodeStageSecretRef: &corev1.SecretReference{
Name: "secretname",
Namespace: "test",
},
ReadOnly: true,
VolumeAttributes: map[string]string{shareNameField: "sharename"},
VolumeHandle: "#secretname#sharename#",
},
},
AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteMany},
},
},
},
}
for _, tc := range cases {
t.Logf("Testing %v", tc.name)
got, err := translator.TranslateInTreeInlineVolumeToCSI(tc.volume)
got, err := translator.TranslateInTreeInlineVolumeToCSI(tc.volume, tc.podNamespace)
if err != nil && !tc.expErr {
t.Errorf("Did not expect error but got: %v", err)
}

View File

@ -162,7 +162,7 @@ func backwardCompatibleAccessModes(ams []v1.PersistentVolumeAccessMode) []v1.Per
// TranslateInTreeInlineVolumeToCSI takes a Volume with GCEPersistentDisk set from in-tree
// and converts the GCEPersistentDisk source to a CSIPersistentVolumeSource
func (g *gcePersistentDiskCSITranslator) TranslateInTreeInlineVolumeToCSI(volume *v1.Volume) (*v1.PersistentVolume, error) {
func (g *gcePersistentDiskCSITranslator) TranslateInTreeInlineVolumeToCSI(volume *v1.Volume, podNamespace string) (*v1.PersistentVolume, error) {
if volume == nil || volume.GCEPersistentDisk == nil {
return nil, fmt.Errorf("volume is nil or GCE PD not defined on volume")
}

View File

@ -273,7 +273,7 @@ func TestInlineReadOnly(t *testing.T) {
ReadOnly: true,
},
},
})
}, "")
if err != nil {
t.Fatalf("Failed to translate in tree inline volume to CSI: %v", err)
}

View File

@ -37,7 +37,8 @@ type InTreePlugin interface {
// TranslateInTreeInlineVolumeToCSI takes a inline volume and will translate
// the in-tree inline volume source to a CSIPersistentVolumeSource
// A PV object containing the CSIPersistentVolumeSource in it's spec is returned
TranslateInTreeInlineVolumeToCSI(volume *v1.Volume) (*v1.PersistentVolume, error)
// podNamespace is only needed for azurefile to fetch secret namespace, no need to be set for other plugins.
TranslateInTreeInlineVolumeToCSI(volume *v1.Volume, podNamespace string) (*v1.PersistentVolume, error)
// TranslateInTreePVToCSI takes a persistent volume and will translate
// the in-tree pv source to a CSI Source. The input persistent volume can be modified

View File

@ -75,7 +75,7 @@ func (t *osCinderCSITranslator) TranslateInTreeStorageClassToCSI(sc *storage.Sto
// TranslateInTreeInlineVolumeToCSI takes a Volume with Cinder set from in-tree
// and converts the Cinder source to a CSIPersistentVolumeSource
func (t *osCinderCSITranslator) TranslateInTreeInlineVolumeToCSI(volume *v1.Volume) (*v1.PersistentVolume, error) {
func (t *osCinderCSITranslator) TranslateInTreeInlineVolumeToCSI(volume *v1.Volume, podNamespace string) (*v1.PersistentVolume, error) {
if volume == nil || volume.Cinder == nil {
return nil, fmt.Errorf("volume is nil or Cinder not defined on volume")
}

View File

@ -111,7 +111,7 @@ func (t *vSphereCSITranslator) TranslateInTreeStorageClassToCSI(sc *storage.Stor
// TranslateInTreeInlineVolumeToCSI takes a Volume with VsphereVolume set from in-tree
// and converts the VsphereVolume source to a CSIPersistentVolumeSource
func (t *vSphereCSITranslator) TranslateInTreeInlineVolumeToCSI(volume *v1.Volume) (*v1.PersistentVolume, error) {
func (t *vSphereCSITranslator) TranslateInTreeInlineVolumeToCSI(volume *v1.Volume, podNamespace string) (*v1.PersistentVolume, error) {
if volume == nil || volume.VsphereVolume == nil {
return nil, fmt.Errorf("volume is nil or VsphereVolume not defined on volume")
}

View File

@ -314,7 +314,7 @@ func TestTranslatevSphereInTreeInlineVolumeToCSI(t *testing.T) {
for _, tc := range cases {
t.Logf("Testing %v", tc.name)
got, err := translator.TranslateInTreeInlineVolumeToCSI(tc.inlinevolume)
got, err := translator.TranslateInTreeInlineVolumeToCSI(tc.inlinevolume, "")
if err == nil && tc.expErr {
t.Errorf("Expected error, but did not get one.")
continue

View File

@ -62,13 +62,13 @@ func (CSITranslator) TranslateInTreeStorageClassToCSI(inTreePluginName string, s
// TranslateInTreeInlineVolumeToCSI takes a inline volume and will translate
// the in-tree volume source to a CSIPersistentVolumeSource (wrapped in a PV)
// if the translation logic has been implemented.
func (CSITranslator) TranslateInTreeInlineVolumeToCSI(volume *v1.Volume) (*v1.PersistentVolume, error) {
func (CSITranslator) TranslateInTreeInlineVolumeToCSI(volume *v1.Volume, podNamespace string) (*v1.PersistentVolume, error) {
if volume == nil {
return nil, fmt.Errorf("persistent volume was nil")
}
for _, curPlugin := range inTreePlugins {
if curPlugin.CanSupportInline(volume) {
pv, err := curPlugin.TranslateInTreeInlineVolumeToCSI(volume)
pv, err := curPlugin.TranslateInTreeInlineVolumeToCSI(volume, podNamespace)
if err != nil {
return nil, err
}

View File

@ -372,7 +372,7 @@ func TestTranslateInTreeInlineVolumeToCSINameUniqueness(t *testing.T) {
}
pv1, err := ctl.TranslateInTreeInlineVolumeToCSI(&v1.Volume{
VolumeSource: vs1,
})
}, "")
if err != nil {
t.Fatalf("Error when translating to CSI: %v", err)
}
@ -382,7 +382,7 @@ func TestTranslateInTreeInlineVolumeToCSINameUniqueness(t *testing.T) {
}
pv2, err := ctl.TranslateInTreeInlineVolumeToCSI(&v1.Volume{
VolumeSource: vs2,
})
}, "")
if err != nil {
t.Fatalf("Error when translating to CSI: %v", err)
}