diff --git a/pkg/controllers/csi/driver/volumes/host/publisher.go b/pkg/controllers/csi/driver/volumes/host/publisher.go index 45a7e5bd4e..77629bec0d 100644 --- a/pkg/controllers/csi/driver/volumes/host/publisher.go +++ b/pkg/controllers/csi/driver/volumes/host/publisher.go @@ -37,18 +37,23 @@ const failedToGetOsAgentVolumePrefix = "failed to get OSMount from database: " func NewHostVolumePublisher(fs afero.Afero, mounter mount.Interface, db metadata.Access, path metadata.PathResolver) csivolumes.Publisher { return &HostVolumePublisher{ - fs: fs, - mounter: mounter, - db: db, - path: path, + fs: fs, + mounter: mounter, + db: db, + path: path, + isNotMounted: mount.IsNotMountPoint, } } +// necessary for mocking, as the MounterMock will use the os package +type mountChecker func(mounter mount.Interface, file string) (bool, error) + type HostVolumePublisher struct { - fs afero.Afero - mounter mount.Interface - db metadata.Access - path metadata.PathResolver + fs afero.Afero + mounter mount.Interface + db metadata.Access + isNotMounted mountChecker + path metadata.PathResolver } func (publisher *HostVolumePublisher) PublishVolume(ctx context.Context, volumeCfg csivolumes.VolumeConfig) (*csi.NodePublishVolumeResponse, error) { @@ -62,7 +67,19 @@ func (publisher *HostVolumePublisher) PublishVolume(ctx context.Context, volumeC return nil, status.Error(codes.Internal, failedToGetOsAgentVolumePrefix+err.Error()) } - if osMount != nil && osMount.DeletedAt.Valid { + if osMount != nil { + if !osMount.DeletedAt.Valid { + // If the OSAgents were removed forcefully, we might not get the unmount request, so we can't fully relay on the database, and have to directly check if its mounted or not + isNotMounted, err := publisher.isNotMounted(publisher.mounter, osMount.Location) + if err != nil { + return nil, err + } + + if !isNotMounted { + return &csi.NodePublishVolumeResponse{}, goerrors.New("previous OSMount is yet to be unmounted, there can be only 1 OSMount per tenant per node, blocking until unmount") // don't want to have the stacktrace here, it just pollutes the logs + } + } + osMount.VolumeMeta = metadata.VolumeMeta{ ID: volumeCfg.VolumeID, PodName: volumeCfg.PodName, @@ -79,9 +96,7 @@ func (publisher *HostVolumePublisher) PublishVolume(ctx context.Context, volumeC } return &csi.NodePublishVolumeResponse{}, nil - } - - if osMount == nil && errors.Is(err, gorm.ErrRecordNotFound) { + } else { osMount := metadata.OSMount{ VolumeMeta: metadata.VolumeMeta{ID: volumeCfg.VolumeID, PodName: volumeCfg.PodName}, VolumeMetaID: volumeCfg.VolumeID, @@ -98,8 +113,6 @@ func (publisher *HostVolumePublisher) PublishVolume(ctx context.Context, volumeC if err := publisher.db.CreateOSMount(&osMount); err != nil { return nil, status.Error(codes.Internal, fmt.Sprintf("failed to insert OSMount to database. info: %v err: %s", osMount, err.Error())) } - } else { - return &csi.NodePublishVolumeResponse{}, goerrors.New("previous OSMount is yet to be unmounted, there can be only 1 OSMount per tenant per node, blocking until unmount") // don't want to have the stacktrace here, it just pollutes the logs } return &csi.NodePublishVolumeResponse{}, nil diff --git a/pkg/controllers/csi/driver/volumes/host/publisher_test.go b/pkg/controllers/csi/driver/volumes/host/publisher_test.go index 08d46d2829..c5b8b84e87 100644 --- a/pkg/controllers/csi/driver/volumes/host/publisher_test.go +++ b/pkg/controllers/csi/driver/volumes/host/publisher_test.go @@ -68,6 +68,28 @@ func TestPublishVolume(t *testing.T) { require.Error(t, err) assert.NotNil(t, response) }) + + t.Run("publish volume when previous OSMount force deleted => do mount", func(t *testing.T) { + mounter := mount.NewFakeMounter([]mount.MountPoint{}) + publisher := newPublisherForTesting(mounter) + mockDynakube(t, &publisher) + + response, err := publisher.PublishVolume(ctx, createTestVolumeConfig()) + + require.NoError(t, err) + assert.NotNil(t, response) + assert.NotEmpty(t, mounter.MountPoints) + assertReferencesForPublishedVolume(t, &publisher, mounter) + + publisher.isNotMounted = func(mounter mount.Interface, file string) (bool, error) { + return true, nil + } + response, err = publisher.PublishVolume(ctx, createTestVolumeConfig()) + require.NoError(t, err) + assert.NotNil(t, response) + assert.NotEmpty(t, mounter.MountPoints) + assertReferencesForPublishedVolume(t, &publisher, mounter) + }) } func TestUnpublishVolume(t *testing.T) { @@ -134,6 +156,9 @@ func newPublisherForTesting(mounter *mount.FakeMounter) HostVolumePublisher { mounter: mounter, db: metadata.FakeMemoryDB(), path: metadata.PathResolver{RootDir: csiOptions.RootDir}, + isNotMounted: func(mounter mount.Interface, file string) (bool, error) { + return false, nil + }, } }