Disk support, tools needed and implication on security model about rook HOT 4 CLOSED

There really are two categories of commands here. Consider the different command line parameters that apply to each scenario.

1. One-time (or rare) disk configuration. Requires privileged access.
2. Long-term orchestration. Should not require privileged access.

For the #2, you wouldn't expect to specify devices every time you launch the orchestrator. You would only expect to specify devices for #1, then the orchestrator would use them thereafter. If the parameters for #1 are passed during every run of the orchestrator (#2), you can run into issues. The same parameters may not have the same desired effect.

For example, there are potentially issues if you run this command at every boot: castled --devices=/dev/sdd. After reboot, sdd could represent a different disk. Under the covers we are using serial number for the disk so we are fine, but the user is passing a name that could change meaning.

• If the disk is unchanged, why should the parameter be passed again on subsequent launches? It is already configured.
• If the disk represents a different disk, should we start using the new disk? Should we destroy the osd on the old disk that moved? What is the user intention?

To eliminate the confusion here, it seems we need two distinct commands that don't overlap.

1. sudo casteld prepare --data-devices=sdb,sdc.
   • Preparing devices is done explicitly then the process exits.
2. castled

• long-running orchestration will use the devices that have already been provisioned. No confusion with the arguments on subsequent runs and no privileges necessary for the long-running process.

This means we would need to store state on the system partition such as in /var/lib/castle. The provisioning of disks will write the expected configuration there, and the orchestrator will read the state. Other config tied to this machine (such as a unique machine id) can be persisted to that folder as well.

from rook.

I think you're proposing that we get rid of auto-prepare mode and force users to call castled prepare to cleanly separate between code that requires root privs and code that does not. Also it would be nice if castled prepare performed no network operations at all.

the disk identification issue seems independent and we should probably enable letting users include and exclude devices use linux persistent disk names.

I'm unclear how new devices get added if we used the explicit prepare step.

from rook.

here's the patch to invoke libblkid from cgo:

commit b07e06e8f6063d8e8d63aacc2738c917cb42c662
Author: Bassam Tabbara <[email protected]>
Date:   Fri Oct 14 13:48:09 2016 -0700

    prototype lsblk using cgo

diff --git a/cmd/blkid/main.go b/cmd/blkid/main.go
new file mode 100644
index 0000000..655feaa
--- /dev/null
+++ b/cmd/blkid/main.go
@@ -0,0 +1,16 @@
+package main
+
+import (
+       "fmt"
+       "os"
+
+       "github.com/quantum/castle/pkg/system"
+)
+
+func main() {
+       value, err := system.LookupProbeValue(os.Args[1], os.Args[2])
+       if err != nil {
+               panic(err)
+       }
+       fmt.Printf(value)
+}
diff --git a/pkg/system/block_linux.go b/pkg/system/block_linux.go
new file mode 100644
index 0000000..455e662
--- /dev/null
+++ b/pkg/system/block_linux.go
@@ -0,0 +1,35 @@
+package system
+
+// #include <errno.h>
+// #include <blkid/blkid.h>
+// #cgo LDFLAGS: -lblkid -luuid
+import "C"
+
+import (
+       "fmt"
+       "unsafe"
+)
+
+func LookupProbeValue(device, label string) (string, error) {
+       var blkidProbe C.blkid_probe
+       var err C.int
+
+       blkidProbe = C.blkid_new_probe_from_filename(C.CString(device))
+       if blkidProbe == nil {
+               return "", fmt.Errorf("failed to create new blkid probe for device %s", device)
+       }
+       defer C.blkid_free_probe(blkidProbe)
+
+       err = C.blkid_do_probe(blkidProbe)
+       if err != 0 {
+               return "", fmt.Errorf("failed to probe device %s", device)
+       }
+
+       var bufPtr *C.char
+       err = C.blkid_probe_lookup_value(blkidProbe, C.CString(label), (**C.char)(unsafe.Pointer(&bufPtr)), nil)
+       if err != 0 {
+               return "", fmt.Errorf("failed to lookup probe value %s for device %s", label, device)
+       }
+
+       return C.GoString(bufPtr), nil
+}
diff --git a/pkg/system/block_linux_test.go b/pkg/system/block_linux_test.go
new file mode 100644
index 0000000..8f95c44
--- /dev/null
+++ b/pkg/system/block_linux_test.go
@@ -0,0 +1,10 @@
+package system
+
+import "testing"
+
+func TestLookupValue(t *testing.T) {
+       _, err := LookupProbeValue("/dev/sda1", "UUID")
+       if err != nil {
+               t.Fatal(err)
+       }
+}

from rook.

closing this in favor of #400.

from rook.