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.

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

Notice if a filesystem overwrites a partitioned disk #1397

Closed
bgilbert opened this issue Apr 19, 2022 · 6 comments
Closed

Notice if a filesystem overwrites a partitioned disk #1397

bgilbert opened this issue Apr 19, 2022 · 6 comments
Assignees
Labels
good first issue jira for syncing to jira

Comments

@bgilbert
Copy link
Contributor

bgilbert commented Apr 19, 2022

A common configuration error is to create a filesystem over a whole-disk device rather than the partition that was intended:

variant: fcos
version: 1.4.0
storage:
  disks:
  - device: /dev/sdb
    wipe_table: false
    partitions:
    - label: data
  filesystems:
    - device: /dev/sdb
      path: /mnt
      format: ext4
      with_mount_unit: true

Add a warning if a storage.filesystems.device matches a storage.disks.device.

@dustymabe
Copy link
Member

I'm wondering if we should even do this warning at the Ignition level (which I think butane also surfaces Ignition warnings). That means the user would actually get the warning on the console because of our recent addition in coreos/fedora-coreos-config#1621

@bgilbert
Copy link
Contributor Author

Actually, yeah, that's a good point. I'll leave this issue here for now so other people can weigh in, but if there's consensus we can move it over to Ignition.

@jlebon
Copy link
Member

jlebon commented Apr 26, 2022

This seems relevant to any distro using Ignition, so having this in Ignition directly SGTM!

@bgilbert bgilbert transferred this issue from coreos/butane Jun 13, 2022
@bgilbert
Copy link
Contributor Author

Moved to Ignition.

@jlebon
Copy link
Member

jlebon commented Jul 6, 2022

Add a warning if a storage.filesystems.path matches a storage.disks.device.

Shouldn't this be:

Add a warning if a storage.filesystems.device matches a storage.disks.device.

? (And the sample Butane config updated accordingly.)

I guess we could check also that path isn't under /dev, but mounting a filesystem there is an odd thing to do so it doesn't seem worth a check.

@bgilbert
Copy link
Contributor Author

bgilbert commented Jul 6, 2022

@jlebon Whoops, yes, good catch. 😖

Also, we should probably only warn if there are any storage.disks.partitions. Someone might decide to mention a disk without intending to partition it, and the spec notably does not say that storage.disks is only meant for partitioning. I've just tried it and Ignition does not automatically create a partition table solely because a disk was mentioned, with or without wipeTable.

As a further refinement, we could warn if wipeTable is true but wipeFilesystem is false. That's technically a valid way to recreate the filesystem on every provision, but it's pretty indirect and probably wasn't what the user intended.

prestist added a commit to prestist/ignition that referenced this issue Aug 17, 2022
Fixes coreos#1397, a common configuration error is to create a filesystem over
a whole-disk device rather than a partition. Add a warning for a
filesystem device matching the disk device.

Additionally, another configuration error was identified for a way to
recreate the filesystem on every provision, while the configuration to
do so is valid, it might be done unitionally. Add a warning when
wipeTable is true but wipeFilesystem is false.
prestist added a commit to prestist/ignition that referenced this issue Sep 29, 2022
Fixes coreos#1397, a common configuration error is to create a filesystem over
a whole-disk device rather than a partition. Add a warning for a
filesystem device matching the disk device.

Additionally, another configuration error was identified for a way to
recreate the filesystem on every provision, while the configuration to
do so is valid, it might be done unitionally. Add a warning when
wipeTable is true but wipeFilesystem is false.
prestist added a commit to prestist/ignition that referenced this issue Sep 30, 2022
Fixes coreos#1397, a common configuration error is to create a filesystem over
a whole-disk device rather than a partition. Add a warning for a
filesystem device matching the disk device.

Additionally, another configuration error was identified for a way to
recreate the filesystem on every provision, while the configuration to
do so is valid, it might be done unitionally. Add a warning when
wipeTable is true but wipeFilesystem is false.
prestist added a commit to prestist/ignition that referenced this issue Oct 4, 2022
Fixes coreos#1397, a common configuration error is to create a filesystem over
a whole-disk device rather than a partition. Add a warning for a
filesystem device matching the disk device.

Additionally, another configuration error was identified for a way to
recreate the filesystem on every provision, while the configuration to
do so is valid, it might be done unitionally. Add a warning when
wipeTable is true but wipeFilesystem is false.
prestist added a commit to prestist/ignition that referenced this issue Oct 24, 2022
Fixes coreos#1397, a common configuration error is to create a filesystem over
a whole-disk device rather than a partition. Add a warning for a
filesystem device matching the disk device.

Additionally, another configuration error was identified for a way to
recreate the filesystem on every provision, while the configuration to
do so is valid, it might be done unitionally. Add a warning when
wipeTable is true but wipeFilesystem is false.
prestist added a commit to prestist/ignition that referenced this issue Oct 25, 2022
Fixes coreos#1397, a common configuration error is to create a filesystem over
a whole-disk device rather than a partition. Add a warning for a
filesystem device matching the disk device.

Additionally, another configuration error was identified for a way to
recreate the filesystem on every provision, while the configuration to
do so is valid, it might be done unitionally. Add a warning when
wipeTable is true but wipeFilesystem is false.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue jira for syncing to jira
Projects
None yet
Development

No branches or pull requests

4 participants