Repository: threefoldtech/zos
Pull request #2002 (2023-07-11 10:11:02)

Bootstrap retry
Potential fix for #1991


Patch:
diff --git a/.github/workflows/publish-bootstrap.yaml b/.github/workflows/publish-bootstrap.yaml
index 53ed2a39..d841919c 100644
--- a/.github/workflows/publish-bootstrap.yaml
+++ b/.github/workflows/publish-bootstrap.yaml
@@ -17,12 +17,9 @@ jobs:
           sudo apt-get update
           sudo apt-get install -y musl musl-tools
       - name: Setup toolchain
-        uses: actions-rs/toolchain@v1
+        uses: dtolnay/rust-toolchain@stable
         with:
-          toolchain: stable
-          target: x86_64-unknown-linux-musl
-          override: true
-      - uses: actions-rs/cargo@v1
+          targets: x86_64-unknown-linux-musl
       - name: Build bootstrap
         run: make release
         working-directory: bootstrap/bootstrap
diff --git a/bootstrap/bootstrap/src/bootstrap.rs b/bootstrap/bootstrap/src/bootstrap.rs
index fcae16b8..afe595c8 100644
--- a/bootstrap/bootstrap/src/bootstrap.rs
+++ b/bootstrap/bootstrap/src/bootstrap.rs
@@ -191,24 +191,28 @@ fn install_package(flist: &hub::Flist) -> Result<()> {
 
         // the entire point of this match is the
         // logging of the error.
-        match flist.download(&flist.name) {
-            Ok(ok) => Ok(ok),
-            Err(err) => {
-                error!("failed to download flist '{}': {}", flist.url, err);
-                bail!("failed to download flist '{}': {}", flist.url, err);
-            }
-        }
+        flist
+            .download(&flist.name)
+            .with_context(|| format!("failed to download flist: {}", flist.url))
     });
 
-    // I can't use the ? because error from retry
-    // is not compatible with failure::Error for
-    // some reason.
     match result {
-        Err(err) => bail!("{:?}", err),
+        Err(retry::Error::Operation { error, .. }) => return Err(error),
+        Err(retry::Error::Internal(msg)) => bail!("failed retrying to download flist: {}", msg),
         _ => (),
     };
 
-    let fs = Zfs::mount("backend", &flist.name, "root")?;
+    let fs = retry::retry(retry::delay::Exponential::from_millis(500).take(10), || {
+        Zfs::mount("backend", &flist.name, "root")
+            .with_context(|| format!("failed to mount flist: {}", flist.url))
+    });
+
+    let fs = match fs {
+        Ok(fs) => fs,
+        Err(retry::Error::Operation { error, .. }) => return Err(error),
+        Err(retry::Error::Internal(msg)) => bail!("failed retrying to mount flist: {}", msg),
+    };
+
     debug!("zfs started, now copying all files");
 
     fs.copy("/").context("failed to copy files")?;
diff --git a/cmds/modules/provisiond/api.go b/cmds/modules/provisiond/api.go
index 094ca054..bf6dd5e5 100644
--- a/cmds/modules/provisiond/api.go
+++ b/cmds/modules/provisiond/api.go
@@ -86,8 +86,8 @@ func setupGPURmb(router rmb.Router, store provision.Storage) {
 		}
 
 		var list []Info
-		for _, device := range devices {
-			id := device.ShortID()
+		for _, pciDevice := range devices {
+			id := pciDevice.ShortID()
 			info := Info{
 				ID:       id,
 				Vendor:   "unknown",
@@ -95,12 +95,17 @@ func setupGPURmb(router rmb.Router, store provision.Storage) {
 				Contract: used[id],
 			}
 
-			vendor, device, ok := device.GetDevice()
+			vendor, device, ok := pciDevice.GetDevice()
 			if ok {
 				info.Vendor = vendor.Name
 				info.Device = device.Name
 			}
 
+			subdevice, ok := pciDevice.GetSubdevice()
+			if ok {
+				info.Device = subdevice.Name
+			}
+
 			list = append(list, info)
 		}
 
diff --git a/etc/zinit/zui.yaml b/etc/zinit/zui.yaml
index 7ff9940c..28536cbd 100644
--- a/etc/zinit/zui.yaml
+++ b/etc/zinit/zui.yaml
@@ -11,4 +11,5 @@ exec: |
     fi
   '
 after:
+  - node-ready
   - quiet
diff --git a/pkg/capacity/pci.go b/pkg/capacity/pci.go
index 9662ad04..fb60b1cd 100644
--- a/pkg/capacity/pci.go
+++ b/pkg/capacity/pci.go
@@ -16,12 +16,24 @@ const (
 	pciDir = "/sys/bus/pci/devices"
 )
 
+// Subdevice is subdevice information to a PCI device
+type Subdevice struct {
+	// SubsystemVendorID is device subsystem vendor ID according to PCI database
+	SubsystemVendorID uint16
+	// SubsystemVendorID is device subsystem ID according to PCI database
+	SubsystemDeviceID uint16
+	// Name is subdevice name according to PCI database
+	Name string
+}
+
 // Device is a PCI device
 type Device struct {
 	// ID is device id according to PCI database
 	ID uint16
 	// Name is device name according to PCI database
 	Name string
+	// Subdevices is a slice of all subdevices according to PCI database
+	Subdevices []Subdevice
 }
 
 // Vendor is Vendor information
@@ -41,7 +53,7 @@ var (
 	//go:embed pci/pci.ids
 	src []byte
 
-	pattern = regexp.MustCompile(`^(\t*)([\d|a-f]{4})(.+)$`)
+	pattern = regexp.MustCompile(`^(\t*)([\d|a-f]{4}) ([\d|a-f]{4}){0,1}(.+)$`)
 	data    map[uint16]Vendor
 
 	gpuVendorsWhitelist = []uint16{
@@ -63,6 +75,7 @@ func loadVendors(src []byte) (map[uint16]Vendor, error) {
 
 	scanner := bufio.NewScanner(bytes.NewBuffer(src))
 	var current *Vendor
+	var deviceID uint16
 	for scanner.Scan() {
 		line := scanner.Text()
 		if strings.HasPrefix(line, "#") {
@@ -73,13 +86,13 @@ func loadVendors(src []byte) (map[uint16]Vendor, error) {
 		if matches == nil {
 			continue
 		}
-		// this should have `4 parts`
+		// this should have `5 parts`
 		tabs := matches[1]
 		id64, err := strconv.ParseUint(matches[2], 16, 16)
 		if err != nil {
 			return nil, fmt.Errorf("failed to parse vendor or device id: %w", err)
 		}
-		rest := strings.TrimSpace(matches[3])
+		rest := strings.TrimSpace(matches[4])
 
 		id := uint16(id64)
 		// the line has no leading tabs. this is a new vendor
@@ -99,9 +112,32 @@ func loadVendors(src []byte) (map[uint16]Vendor, error) {
 			}
 
 			current.Devices[id] = Device{
-				ID:   id,
-				Name: rest,
+				ID:         id,
+				Name:       rest,
+				Subdevices: make([]Subdevice, 0),
+			}
+			deviceID = id
+		} else if len(tabs) == 2 {
+			if current == nil {
+				return nil, fmt.Errorf("subsystem appeared before a vendor: %s", line)
+			}
+			if len(current.Devices) == 0 {
+				return nil, fmt.Errorf("subsystem appeared before a device: %s", line)
 			}
+			ssid64, err := strconv.ParseUint(matches[3], 16, 16)
+			if err != nil {
+				return nil, fmt.Errorf("failed to parse subsystem id: %w", err)
+			}
+			ssid := uint16(ssid64)
+
+			device := current.Devices[deviceID]
+			device.Subdevices = append(device.Subdevices, Subdevice{
+				SubsystemVendorID: id,
+				SubsystemDeviceID: ssid,
+				Name:              rest,
+			})
+			current.Devices[deviceID] = device
+
 		}
 	}
 
@@ -125,6 +161,20 @@ func GetDevice(vendor uint16, device uint16) (v Vendor, d Device, ok bool) {
 	return
 }
 
+// GetSubdevice looks up the subdevice using devices db
+func GetSubdevice(vendor uint16, device uint16, subsystemVendorID uint16, subsystemDeviceID uint16) (Subdevice, bool) {
+	_, d, ok := GetDevice(vendor, device)
+	if !ok {
+		return Subdevice{}, false
+	}
+	for _, subdevice := range d.Subdevices {
+		if subdevice.SubsystemDeviceID == subsystemDeviceID && subdevice.SubsystemVendorID == subsystemVendorID {
+			return subdevice, true
+		}
+	}
+	return Subdevice{}, false
+}
+
 // Filter over the PCI for listing
 type Filter func(pci *PCI) bool
 
@@ -138,6 +188,10 @@ type PCI struct {
 	Device uint16
 	// Class of the device
 	Class uint32
+	// Subsystem Vendor of the device
+	SubsystemVendor uint16
+	// Subsystem ID of the device
+	SubsystemDevice uint16
 }
 
 // GetDevice gets the attached PCI device information (vendor and device)
@@ -145,6 +199,11 @@ func (p *PCI) GetDevice() (Vendor, Device, bool) {
 	return GetDevice(p.Vendor, p.Device)
 }
 
+// GetSubdevice gets the attached PCI subdevice information
+func (p *PCI) GetSubdevice() (Subdevice, bool) {
+	return GetSubdevice(p.Vendor, p.Device, p.SubsystemVendor, p.SubsystemDevice)
+}
+
 // ShortID returns a short identification string
 // for the device in the format `slot/vendor/device`
 func (p *PCI) ShortID() string {
@@ -162,9 +221,11 @@ func (p *PCI) Flag(name string) (uint64, error) {
 
 func pciDeviceFromSlot(slot string) (PCI, error) {
 	const (
-		classFile  = "class"
-		vendorFile = "vendor"
-		deviceFile = "device"
+		classFile           = "class"
+		vendorFile          = "vendor"
+		deviceFile          = "device"
+		subsystemVendorFile = "subsystem_vendor"
+		subsystemDeviceFile = "subsystem_device"
 	)
 	class, err := readUint64(filepath.Join(pciDir, slot, classFile), 32)
 	if err != nil {
@@ -172,19 +233,29 @@ func pciDeviceFromSlot(slot string) (PCI, error) {
 	}
 	vendor, err := readUint64(filepath.Join(pciDir, slot, vendorFile), 16)
 	if err != nil {
-		return PCI{}, fmt.Errorf("failed to get device '%s' class: %w", slot, err)
+		return PCI{}, fmt.Errorf("failed to get device '%s' vendor: %w", slot, err)
 	}
 
 	device, err := readUint64(filepath.Join(pciDir, slot, deviceFile), 16)
 	if err != nil {
-		return PCI{}, fmt.Errorf("failed to get device '%s' class: %w", slot, err)
+		return PCI{}, fmt.Errorf("failed to get device '%s' device: %w", slot, err)
+	}
+	subsystemVendor, err := readUint64(filepath.Join(pciDir, slot, subsystemVendorFile), 16)
+	if err != nil {
+		return PCI{}, fmt.Errorf("failed to get device '%s' subsystem vendor: %w", slot, err)
+	}
+	subsystemDevice, err := readUint64(filepath.Join(pciDir, slot, subsystemDeviceFile), 16)
+	if err != nil {
+		return PCI{}, fmt.Errorf("failed to get device '%s' subsystem device: %w", slot, err)
 	}
 
 	pci := PCI{
-		Slot:   slot,
-		Class:  uint32(class),
-		Vendor: uint16(vendor),
-		Device: uint16(device),
+		Slot:            slot,
+		Class:           uint32(class),
+		Vendor:          uint16(vendor),
+		Device:          uint16(device),
+		SubsystemVendor: uint16(subsystemVendor),
+		SubsystemDevice: uint16(subsystemDevice),
 	}
 
 	return pci, err


Test patch:
diff --git a/.github/workflows/test-bootstrap.yaml b/.github/workflows/test-bootstrap.yaml
index b5a35f30..c5100f8e 100644
--- a/.github/workflows/test-bootstrap.yaml
+++ b/.github/workflows/test-bootstrap.yaml
@@ -2,8 +2,8 @@ name: Bootstrap Tests
 on:
   push:
     paths:
-      - 'bootstrap/bootstrap/**'
-      - '.github/workflows/test-bootstrap.yaml'
+      - "bootstrap/bootstrap/**"
+      - ".github/workflows/test-bootstrap.yaml"
 jobs:
   bootstrap:
     name: Running Bootstrap Tests
@@ -16,12 +16,9 @@ jobs:
           sudo apt-get update
           sudo apt-get install -y musl musl-tools
       - name: Setup toolchain
-        uses: actions-rs/toolchain@v1
+        uses: dtolnay/rust-toolchain@stable
         with:
-          toolchain: stable
-          target: x86_64-unknown-linux-musl
-          override: true
-      - uses: actions-rs/cargo@v1
+          targets: x86_64-unknown-linux-musl
       - name: Test bootstrap
         run: make test
         working-directory: bootstrap/bootstrap
diff --git a/pkg/capacity/pci_test.go b/pkg/capacity/pci_test.go
index 3e28e639..4bf9c0c0 100644
--- a/pkg/capacity/pci_test.go
+++ b/pkg/capacity/pci_test.go
@@ -15,6 +15,18 @@ func TestGetDevice(t *testing.T) {
 	require.Equal(t, "Navi 10 [Radeon RX 5600 OEM/5600 XT / 5700/5700 XT]", device.Name)
 }
 
+func TestGetSubdevice(t *testing.T) {
+	subdevice, ok := GetSubdevice(0x10de, 0x1e30, 0x10de, 0x129e)
+
+	require.True(t, ok)
+	require.Equal(t, "Quadro RTX 8000", subdevice.Name)
+
+	subdevice, ok = GetSubdevice(0x10de, 0x1e30, 0x10de, 0x12ba)
+
+	require.True(t, ok)
+	require.Equal(t, "Quadro RTX 6000", subdevice.Name)
+}
+
 func TestListPCI(t *testing.T) {
 	devices, err := ListPCI()
 	require.NoError(t, err)