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

CSI volume source fstype incompatible with CSI spec #65122

Closed
msau42 opened this issue Jun 15, 2018 · 5 comments
Closed

CSI volume source fstype incompatible with CSI spec #65122

msau42 opened this issue Jun 15, 2018 · 5 comments
Labels
kind/bug Categorizes issue or PR as related to a bug. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. sig/storage Categorizes an issue or PR as relevant to SIG Storage.
Milestone
v1.12

Comments

@msau42
Copy link
Member

msau42 commented Jun 15, 2018

Is this a BUG REPORT or FEATURE REQUEST?:
@kubernetes/sig-storage-bugs

What happened:
CSI spec allows for an empty fstype string. This is intended for non-block plugins like nfs and gluster where filesystems are not separately created on the volume.

  // Indicate that the volume will be accessed via the filesystem API.
  message MountVolume {
    // The filesystem type. This field is OPTIONAL.
    // An empty string is equal to an unspecified field value.
    string fs_type = 1;
  }

The Kubernetes CSI volume source API defaults fstype to "ext4" and doesn't allow an empty string:

        // Filesystem type to mount.
        // Must be a filesystem type supported by the host operating system.
        // Ex. "ext4", "xfs", "ntfs". Implicitly inferred to be "ext4" if unspecified.
        // +optional
        FSType string

This makes it impossible for Kubernetes to pass in an empty fs_type to the plugin.

What you expected to happen:
Since we currently leave fs creation and formatting up to the plugin, the plugin should be the one to decide the default.
@k8s-ci-robot k8s-ci-robot added sig/storage Categorizes an issue or PR as relevant to SIG Storage. kind/bug Categorizes issue or PR as related to a bug. labels Jun 15, 2018
@saad-ali
Copy link
Member

We should change the default FSType value for CSIPersistentVolumeSource empty string. And plugins will be responsible for setting a sane default.

@saad-ali saad-ali added this to the v1.12 milestone Jun 15, 2018
@saad-ali saad-ali added the priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. label Jun 15, 2018
@msau42
Copy link
Member Author

msau42 commented Jun 15, 2018

From the CSI side though, it is unclear from the spec what the plugin should do if fsType is empty and it requires one

@saad-ali
Copy link
Member

From the CSI side though, it is unclear from the spec what the plugin should do if fsType is empty and it requires one

The plugin can decide what makes sense for it, since the spec does not dictate.

@k8s-github-robot
Copy link

[MILESTONENOTIFIER] Milestone Issue: Up-to-date for process

@msau42

Issue Labels
  • sig/storage: Issue will be escalated to these SIGs if needed.
  • priority/important-soon: Escalate to the issue owners and SIG owner; move out of milestone after several unsuccessful escalation attempts.
  • kind/bug: Fixes a bug discovered during the current release.
Help

@krunaljain krunaljain mentioned this issue Jun 26, 2018
Merged
@codenrhoden codenrhoden mentioned this issue Jun 27, 2018
Closed
k8s-github-robot pushed a commit that referenced this issue Jul 10, 2018
Merge pull request #65499 from krunaljain/bugfix/csi_default_fs_type
3b269e1 
Automatic merge from submit-queue (batch tested with PRs 55023, 65499). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Bugfix/csi default fs type

This PR address the issue mentioned in the following ticket #65122 
The FSType string will now not be defaulted to ext4. Removes defaulting of CSI file system type to ext4. CSI plugins that depended on this default need to be updated as the fsType would remain an empty string if not provided and would not default to ext4. CSI spec allows for an empty fstype string. This is intended for non-block plugins like nfs and gluster where filesystems are not separately created on the volume. But currently the default file system is overridden to ext4  which makes the above case redundant. This commit prevents such an overridding. 


```release-note  
ACTION REQUIRED: Removes defaulting of CSI file system type to ext4. All the production drivers listed under https://kubernetes-csi.github.io/docs/Drivers.html were tested and work as expected after this change. If you are using a driver not in that list, please test the drivers on an updated test cluster first. ```
k8s-publishing-bot added a commit to kubernetes/api that referenced this issue Jul 10, 2018
@k8s-publishing-bot
Merge pull request #65499 from krunaljain/bugfix/csi_default_fs_type
86095a2 
Automatic merge from submit-queue (batch tested with PRs 55023, 65499). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Bugfix/csi default fs type

This PR address the issue mentioned in the following ticket kubernetes/kubernetes#65122
The FSType string will now not be defaulted to ext4. Removes defaulting of CSI file system type to ext4. CSI plugins that depended on this default need to be updated as the fsType would remain an empty string if not provided and would not default to ext4. CSI spec allows for an empty fstype string. This is intended for non-block plugins like nfs and gluster where filesystems are not separately created on the volume. But currently the default file system is overridden to ext4  which makes the above case redundant. This commit prevents such an overridding.

```release-note
ACTION REQUIRED: Removes defaulting of CSI file system type to ext4. All the production drivers listed under https://kubernetes-csi.github.io/docs/Drivers.html were tested and work as expected after this change. If you are using a driver not in that list, please test the drivers on an updated test cluster first. ```

Kubernetes-commit: 3b269e182d24cdda5b75384c4e7cf2a3814532fb
@msau42
Copy link
Member Author

msau42 commented Jul 18, 2018

Fixed by #65499
/close

@vladimirvivien vladimirvivien mentioned this issue Aug 8, 2018
Closed
2 tasks
@msau42 msau42 mentioned this issue Aug 13, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
kind/bug Categorizes issue or PR as related to a bug. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. sig/storage Categorizes an issue or PR as relevant to SIG Storage.
Projects
None yet
Milestone
v1.12
Development

No branches or pull requests
4 participants
@saad-ali @k8s-github-robot @k8s-ci-robot @msau42