Move CSI json file saving to SetUpAt()

When saving a json state file in NewMounter, we risk the json file will not
be cleaned when SetUpAt() fails. Move it to SetUpAt() instead.
This commit is contained in:
Jan Safranek 2022-03-22 11:01:52 +01:00
parent 83415e5c9e
commit f76efd0400
4 changed files with 121 additions and 103 deletions

View File

@ -255,6 +255,35 @@ func (c *csiMountMgr) SetUpAt(dir string, mounterArgs volume.MounterArgs) error
}
}
// Save volume info in pod dir
// persist volume info data for teardown
nodeName := string(c.plugin.host.GetNodeName())
volData := map[string]string{
volDataKey.specVolID: c.spec.Name(),
volDataKey.volHandle: volumeHandle,
volDataKey.driverName: string(c.driverName),
volDataKey.nodeName: nodeName,
volDataKey.volumeLifecycleMode: string(c.volumeLifecycleMode),
volDataKey.attachmentID: getAttachmentName(volumeHandle, string(c.driverName), nodeName),
}
err = saveVolumeData(parentDir, volDataFileName, volData)
defer func() {
// Only if there was an error and volume operation was considered
// finished, we should remove the directory.
if err != nil && volumetypes.IsOperationFinishedError(err) {
// attempt to cleanup volume mount dir
if removeerr := removeMountDir(c.plugin, dir); removeerr != nil {
klog.Error(log("mounter.SetUpAt failed to remove mount dir after error [%s]: %v", dir, removeerr))
}
}
}()
if err != nil {
errorMsg := log("mounter.SetUpAt failed to save volume info data: %v", err)
klog.Error(errorMsg)
return volumetypes.NewTransientOperationFailure(errorMsg)
}
err = csi.NodePublishVolume(
ctx,
volumeHandle,

View File

@ -61,6 +61,25 @@ var (
testAccount = "test-service-account"
)
func prepareVolumeInfoFile(mountPath string, plug *csiPlugin, specVolumeName, volumeID, driverName, lifecycleMode string) error {
nodeName := string(plug.host.GetNodeName())
volData := map[string]string{
volDataKey.specVolID: specVolumeName,
volDataKey.volHandle: volumeID,
volDataKey.driverName: driverName,
volDataKey.nodeName: nodeName,
volDataKey.attachmentID: getAttachmentName(volumeID, driverName, nodeName),
volDataKey.volumeLifecycleMode: lifecycleMode,
}
if err := os.MkdirAll(mountPath, 0755); err != nil {
return fmt.Errorf("failed to create dir for volume info file: %s", err)
}
if err := saveVolumeData(mountPath, volDataFileName, volData); err != nil {
return fmt.Errorf("failed to save volume info file: %s", err)
}
return nil
}
func TestMounterGetPath(t *testing.T) {
plug, tmpDir := newTestPlugin(t, nil)
defer os.RemoveAll(tmpDir)
@ -301,6 +320,37 @@ func TestMounterSetUp(t *testing.T) {
if !reflect.DeepEqual(vol.VolumeContext, test.expectedVolumeContext) {
t.Errorf("csi server expected volumeContext %+v, got %+v", test.expectedVolumeContext, vol.VolumeContext)
}
// ensure data file is created
dataDir := filepath.Dir(mounter.GetPath())
dataFile := filepath.Join(dataDir, volDataFileName)
if _, err := os.Stat(dataFile); err != nil {
if os.IsNotExist(err) {
t.Errorf("data file not created %s", dataFile)
} else {
t.Fatal(err)
}
}
data, err := loadVolumeData(dataDir, volDataFileName)
if err != nil {
t.Fatal(err)
}
if data[volDataKey.specVolID] != csiMounter.spec.Name() {
t.Error("volume data file unexpected specVolID:", data[volDataKey.specVolID])
}
if data[volDataKey.volHandle] != csiMounter.volumeID {
t.Error("volume data file unexpected volHandle:", data[volDataKey.volHandle])
}
if data[volDataKey.driverName] != string(csiMounter.driverName) {
t.Error("volume data file unexpected driverName:", data[volDataKey.driverName])
}
if data[volDataKey.nodeName] != string(csiMounter.plugin.host.GetNodeName()) {
t.Error("volume data file unexpected nodeName:", data[volDataKey.nodeName])
}
if data[volDataKey.volumeLifecycleMode] != string(csiMounter.volumeLifecycleMode) {
t.Error("volume data file unexpected volumeLifecycleMode:", data[volDataKey.volumeLifecycleMode])
}
})
}
}
@ -425,6 +475,36 @@ func TestMounterSetUpSimple(t *testing.T) {
if vol.Path != csiMounter.GetPath() {
t.Error("csi server may not have received NodePublishVolume call")
}
// ensure data file is created
dataDir := filepath.Dir(mounter.GetPath())
dataFile := filepath.Join(dataDir, volDataFileName)
if _, err := os.Stat(dataFile); err != nil {
if os.IsNotExist(err) {
t.Errorf("data file not created %s", dataFile)
} else {
t.Fatal(err)
}
}
data, err := loadVolumeData(dataDir, volDataFileName)
if err != nil {
t.Fatal(err)
}
if data[volDataKey.specVolID] != csiMounter.spec.Name() {
t.Error("volume data file unexpected specVolID:", data[volDataKey.specVolID])
}
if data[volDataKey.volHandle] != csiMounter.volumeID {
t.Error("volume data file unexpected volHandle:", data[volDataKey.volHandle])
}
if data[volDataKey.driverName] != string(csiMounter.driverName) {
t.Error("volume data file unexpected driverName:", data[volDataKey.driverName])
}
if data[volDataKey.nodeName] != string(csiMounter.plugin.host.GetNodeName()) {
t.Error("volume data file unexpected nodeName:", data[volDataKey.nodeName])
}
if data[volDataKey.volumeLifecycleMode] != string(tc.mode) {
t.Error("volume data file unexpected volumeLifecycleMode:", data[volDataKey.volumeLifecycleMode])
}
})
}
}

View File

@ -425,51 +425,9 @@ func (p *csiPlugin) NewMounter(
}
mounter.csiClientGetter.driverName = csiDriverName(driverName)
// Save volume info in pod dir
dir := mounter.GetPath()
dataDir := filepath.Dir(dir) // dropoff /mount at end
if err := os.MkdirAll(dataDir, 0750); err != nil {
return nil, errors.New(log("failed to create dir %#v: %v", dataDir, err))
}
klog.V(4).Info(log("created path successfully [%s]", dataDir))
mounter.MetricsProvider = NewMetricsCsi(volumeHandle, dir, csiDriverName(driverName))
// persist volume info data for teardown
node := string(p.host.GetNodeName())
volData := map[string]string{
volDataKey.specVolID: spec.Name(),
volDataKey.volHandle: volumeHandle,
volDataKey.driverName: driverName,
volDataKey.nodeName: node,
volDataKey.volumeLifecycleMode: string(volumeLifecycleMode),
}
attachID := getAttachmentName(volumeHandle, driverName, node)
volData[volDataKey.attachmentID] = attachID
err = saveVolumeData(dataDir, volDataFileName, volData)
defer func() {
// Only if there was an error and volume operation was considered
// finished, we should remove the directory.
if err != nil && volumetypes.IsOperationFinishedError(err) {
// attempt to cleanup volume mount dir.
if err = removeMountDir(p, dir); err != nil {
klog.Error(log("attacher.MountDevice failed to remove mount dir after error [%s]: %v", dir, err))
}
}
}()
if err != nil {
errorMsg := log("csi.NewMounter failed to save volume info data: %v", err)
klog.Error(errorMsg)
return nil, errors.New(errorMsg)
}
klog.V(4).Info(log("mounter created successfully"))
return mounter, nil
}

View File

@ -418,6 +418,12 @@ func TestPluginConstructVolumeSpec(t *testing.T) {
}
csiMounter := mounter.(*csiMountMgr)
mountPath := filepath.Dir(csiMounter.GetPath())
err = prepareVolumeInfoFile(mountPath, plug, tc.originSpec.Name(), csiMounter.volumeID, testDriver, string(csiMounter.volumeLifecycleMode))
if err != nil {
t.Fatalf("failed to save fake volume info file: %s", err)
}
// rebuild spec
spec, err := plug.ConstructVolumeSpec("test-pv", filepath.Dir(csiMounter.GetPath()))
if err != nil {
@ -452,7 +458,6 @@ func TestPluginConstructVolumeSpec(t *testing.T) {
if spec.Name() != tc.specVolID {
t.Errorf("Unexpected spec name constructed %s", spec.Name())
}
})
}
}
@ -547,6 +552,12 @@ func TestPluginConstructVolumeSpecWithInline(t *testing.T) {
}
csiMounter := mounter.(*csiMountMgr)
mountPath := filepath.Dir(csiMounter.GetPath())
err = prepareVolumeInfoFile(mountPath, plug, tc.originSpec.Name(), csiMounter.volumeID, testDriver, string(csiMounter.volumeLifecycleMode))
if err != nil {
t.Fatalf("failed to save fake volume info file: %s", err)
}
// rebuild spec
spec, err := plug.ConstructVolumeSpec("test-pv", filepath.Dir(csiMounter.GetPath()))
if err != nil {
@ -672,36 +683,6 @@ func TestPluginNewMounter(t *testing.T) {
if csiMounter.volumeLifecycleMode != test.volumeLifecycleMode {
t.Error("unexpected driver mode:", csiMounter.volumeLifecycleMode)
}
// ensure data file is created
dataDir := filepath.Dir(mounter.GetPath())
dataFile := filepath.Join(dataDir, volDataFileName)
if _, err := os.Stat(dataFile); err != nil {
if os.IsNotExist(err) {
t.Errorf("data file not created %s", dataFile)
} else {
t.Fatal(err)
}
}
data, err := loadVolumeData(dataDir, volDataFileName)
if err != nil {
t.Fatal(err)
}
if data[volDataKey.specVolID] != csiMounter.spec.Name() {
t.Error("volume data file unexpected specVolID:", data[volDataKey.specVolID])
}
if data[volDataKey.volHandle] != csiMounter.volumeID {
t.Error("volume data file unexpected volHandle:", data[volDataKey.volHandle])
}
if data[volDataKey.driverName] != string(csiMounter.driverName) {
t.Error("volume data file unexpected driverName:", data[volDataKey.driverName])
}
if data[volDataKey.nodeName] != string(csiMounter.plugin.host.GetNodeName()) {
t.Error("volume data file unexpected nodeName:", data[volDataKey.nodeName])
}
if data[volDataKey.volumeLifecycleMode] != string(test.volumeLifecycleMode) {
t.Error("volume data file unexpected volumeLifecycleMode:", data[volDataKey.volumeLifecycleMode])
}
})
}
}
@ -809,36 +790,6 @@ func TestPluginNewMounterWithInline(t *testing.T) {
if csiMounter.volumeLifecycleMode != test.volumeLifecycleMode {
t.Error("unexpected driver mode:", csiMounter.volumeLifecycleMode)
}
// ensure data file is created
dataDir := filepath.Dir(mounter.GetPath())
dataFile := filepath.Join(dataDir, volDataFileName)
if _, err := os.Stat(dataFile); err != nil {
if os.IsNotExist(err) {
t.Errorf("data file not created %s", dataFile)
} else {
t.Fatal(err)
}
}
data, err := loadVolumeData(dataDir, volDataFileName)
if err != nil {
t.Fatal(err)
}
if data[volDataKey.specVolID] != csiMounter.spec.Name() {
t.Error("volume data file unexpected specVolID:", data[volDataKey.specVolID])
}
if data[volDataKey.volHandle] != csiMounter.volumeID {
t.Error("volume data file unexpected volHandle:", data[volDataKey.volHandle])
}
if data[volDataKey.driverName] != string(csiMounter.driverName) {
t.Error("volume data file unexpected driverName:", data[volDataKey.driverName])
}
if data[volDataKey.nodeName] != string(csiMounter.plugin.host.GetNodeName()) {
t.Error("volume data file unexpected nodeName:", data[volDataKey.nodeName])
}
if data[volDataKey.volumeLifecycleMode] != string(csiMounter.volumeLifecycleMode) {
t.Error("volume data file unexpected volumeLifecycleMode:", data[volDataKey.volumeLifecycleMode])
}
})
}
}