The following pull request was submitted through Github.
It can be accessed and reviewed at: https://github.com/lxc/lxd/pull/6846
This e-mail was sent by the LXC bot, direct replies will not reach the author
unless they happen to be subscribed to this list.
=== Description (from pull-request) ===
Includes https://github.com/lxc/lxd/pull/6832
- The "nictype" property shouldn't be allowed when "network=" is used
From bd28effdb629aafc1a667b5980fd6f13b583eeea Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Tue, 4 Feb 2020 09:53:11 +0000
Subject: [PATCH 01/14] lxd/storage/drivers/driver/lvm: Uses d.thinpoolName()
rather than d.config["lvm.thinpool_name"]
During thin pool rename requests, use d.thinpoolName() rather than
d.config["lvm.thinpool_name"] in case d.config["lvm.thinpool_name"] is not
defined.
Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
lxd/storage/drivers/driver_lvm.go | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/lxd/storage/drivers/driver_lvm.go
b/lxd/storage/drivers/driver_lvm.go
index 04ccc4725c..09423c1ba1 100644
--- a/lxd/storage/drivers/driver_lvm.go
+++ b/lxd/storage/drivers/driver_lvm.go
@@ -466,11 +466,11 @@ func (d *lvm) Update(changedConfig map[string]string)
error {
}
if changedConfig["lvm.thinpool_name"] != "" {
- _, err := shared.TryRunCommand("lvrename",
d.config["lvm.vg_name"], d.config["lvm.thinpool_name"],
changedConfig["lvm.thinpool_name"])
+ _, err := shared.TryRunCommand("lvrename",
d.config["lvm.vg_name"], d.thinpoolName(), changedConfig["lvm.thinpool_name"])
if err != nil {
- return errors.Wrapf(err, "Error renaming LVM thin pool
from %q to %q", d.config["lvm.thinpool_name"],
changedConfig["lvm.thinpool_name"])
+ return errors.Wrapf(err, "Error renaming LVM thin pool
from %q to %q", d.thinpoolName(), changedConfig["lvm.thinpool_name"])
}
- d.logger.Debug("Thin pool volume renamed", log.Ctx{"vg_name":
d.config["lvm.vg_name"], "thinpool": d.config["lvm.thinpool_name"],
"new_thinpool": changedConfig["lvm.thinpool_name"]})
+ d.logger.Debug("Thin pool volume renamed", log.Ctx{"vg_name":
d.config["lvm.vg_name"], "thinpool": d.thinpoolName(), "new_thinpool":
changedConfig["lvm.thinpool_name"]})
}
return nil
From 4d97547265cb124b72e1a92aece3eb7da853020a Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Wed, 5 Feb 2020 09:55:47 +0000
Subject: [PATCH 02/14] lxd/patches: setupStorageDriver usage
Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
lxd/patches.go | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lxd/patches.go b/lxd/patches.go
index 4ff0e23157..8fe0db7509 100644
--- a/lxd/patches.go
+++ b/lxd/patches.go
@@ -451,7 +451,7 @@ func patchStorageApi(name string, d *Daemon) error {
return err
}
- return SetupStorageDriver(d.State(), true)
+ return setupStorageDriver(d.State(), true)
}
func upgradeFromStorageTypeBtrfs(name string, d *Daemon, defaultPoolName
string, defaultStorageTypeName string, cRegular []string, cSnapshots []string,
imgPublic []string, imgPrivate []string) error {
From 816d3f875714a495c91ab1cde6a36e2885ac1077 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Wed, 5 Feb 2020 09:58:10 +0000
Subject: [PATCH 03/14] lxd/storage: Renames SetupStorageDriver to
setupStorageDriver for consistency
Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
lxd/storage.go | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lxd/storage.go b/lxd/storage.go
index 38d30a404d..71d5b02a26 100644
--- a/lxd/storage.go
+++ b/lxd/storage.go
@@ -711,7 +711,7 @@ func resetContainerDiskIdmap(container *containerLXC,
srcIdmap *idmap.IdmapSet)
return nil
}
-func SetupStorageDriver(s *state.State, forceCheck bool) error {
+func setupStorageDriver(s *state.State, forceCheck bool) error {
pools, err := s.Cluster.StoragePoolsNotPending()
if err != nil {
if err == db.ErrNoSuchObject {
From 5f2468b5cb72e07ce2c689db1d81ebd5026e2217 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Wed, 5 Feb 2020 10:02:09 +0000
Subject: [PATCH 04/14] lxd/storage/drivers/driver/zfs: Adds zfs kernel module
load fail detection
Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
lxd/storage/drivers/driver_zfs.go | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/lxd/storage/drivers/driver_zfs.go
b/lxd/storage/drivers/driver_zfs.go
index 4071ec1a26..8b390a835e 100644
--- a/lxd/storage/drivers/driver_zfs.go
+++ b/lxd/storage/drivers/driver_zfs.go
@@ -50,7 +50,10 @@ func (d *zfs) load() error {
}
// Load the kernel module.
- util.LoadModule("zfs")
+ err := util.LoadModule("zfs")
+ if err != nil {
+ return errors.Wrapf(err, "Error loading %q module", "zfs")
+ }
// Validate the needed tools are present.
for _, tool := range []string{"zpool", "zfs"} {
From 7deb9534fd3d4c50a968cdb659ec8c52c19352be Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Thu, 6 Feb 2020 09:05:20 +0000
Subject: [PATCH 05/14] lxd/daemon: setupStorageDriver usage
Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
lxd/daemon.go | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lxd/daemon.go b/lxd/daemon.go
index fd0e4b57fc..77b074220a 100644
--- a/lxd/daemon.go
+++ b/lxd/daemon.go
@@ -815,7 +815,7 @@ func (d *Daemon) init() error {
/* Read the storage pools */
logger.Infof("Initializing storage pools")
- err = SetupStorageDriver(d.State(), false)
+ err = setupStorageDriver(d.State(), false)
if err != nil {
return err
}
From 5009b2fc77ef0a39b3f895f3d117a4b62a7c9ae0 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Thu, 6 Feb 2020 09:05:33 +0000
Subject: [PATCH 06/14] lxd/daemon: Comment consistency
Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
lxd/daemon.go | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/lxd/daemon.go b/lxd/daemon.go
index 77b074220a..6bdb4958b7 100644
--- a/lxd/daemon.go
+++ b/lxd/daemon.go
@@ -808,41 +808,41 @@ func (d *Daemon) init() error {
containersRestart(s)
}
- // Setup the user-agent
+ // Setup the user-agent.
if clustered {
version.UserAgentFeatures([]string{"cluster"})
}
- /* Read the storage pools */
+ // Read the storage pools.
logger.Infof("Initializing storage pools")
err = setupStorageDriver(d.State(), false)
if err != nil {
return err
}
- // Mount any daemon storage
+ // Mount any daemon storage.
err = daemonStorageMount(d.State())
if err != nil {
return err
}
- /* Apply all patches */
+ // Apply all patches.
err = patchesApplyAll(d)
if err != nil {
return err
}
- /* Setup the networks */
+ // Setup the networks.
logger.Infof("Initializing networks")
err = networkStartup(d.State())
if err != nil {
return err
}
- // Cleanup leftover images
+ // Cleanup leftover images.
pruneLeftoverImages(d)
- /* Setup the proxy handler, external authentication and MAAS */
+ // Setup the proxy handler, external authentication and MAAS.
candidAPIURL := ""
candidAPIKey := ""
candidDomains := ""
From d01d09942dfcb8e20e2dd3796f8b4789eadf54e6 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Thu, 6 Feb 2020 09:15:17 +0000
Subject: [PATCH 07/14] lxd/storage/drivers/driver/lvm: Makes lvm.vg_name
required for mounting
Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
lxd/storage/drivers/driver_lvm.go | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/lxd/storage/drivers/driver_lvm.go
b/lxd/storage/drivers/driver_lvm.go
index 09423c1ba1..34a3294aee 100644
--- a/lxd/storage/drivers/driver_lvm.go
+++ b/lxd/storage/drivers/driver_lvm.go
@@ -479,6 +479,10 @@ func (d *lvm) Update(changedConfig map[string]string)
error {
// Mount mounts the storage pool (this does nothing for external LVM pools,
but for loopback image
// LVM pools this creates a loop device).
func (d *lvm) Mount() (bool, error) {
+ if d.config["lvm.vg_name"] == "" {
+ return false, fmt.Errorf("Cannot mount pool as %q is not
specified", "lvm.vg_name")
+ }
+
// Open the loop file if the LVM device doesn't exist yet and the
source points to a file.
if !shared.IsDir(fmt.Sprintf("/dev/%s", d.config["lvm.vg_name"])) &&
filepath.IsAbs(d.config["source"]) &&
!shared.IsBlockdevPath(d.config["source"]) {
loopFile, err := d.openLoopFile(d.config["source"])
From 112be17b0df238d4919a07f8fbb4d21191b0eb89 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Thu, 6 Feb 2020 09:15:43 +0000
Subject: [PATCH 08/14] lxd/db/cluster/update: Adds updateFromV23 for ensuring
lvm.vg_name key is set
Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
lxd/db/cluster/update.go | 43 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 43 insertions(+)
diff --git a/lxd/db/cluster/update.go b/lxd/db/cluster/update.go
index 1466f9f13e..6b3d353a9a 100644
--- a/lxd/db/cluster/update.go
+++ b/lxd/db/cluster/update.go
@@ -59,6 +59,49 @@ var updates = map[int]schema.Update{
21: updateFromV20,
22: updateFromV21,
23: updateFromV22,
+ 24: updateFromV23,
+}
+
+// The lvm.vg_name config key is required for LVM to function.
+func updateFromV23(tx *sql.Tx) error {
+ // Fetch the IDs of all existing nodes.
+ nodeIDs, err := query.SelectIntegers(tx, "SELECT id FROM nodes")
+ if err != nil {
+ return errors.Wrap(err, "Failed to get IDs of current nodes")
+ }
+
+ // Fetch the IDs of all existing lvm pools.
+ poolIDs, err := query.SelectIntegers(tx, `SELECT id FROM storage_pools
WHERE driver='lvm'`)
+ if err != nil {
+ return errors.Wrap(err, "Failed to get IDs of current lvm
pools")
+ }
+
+ for _, poolID := range poolIDs {
+ for _, nodeID := range nodeIDs {
+ // Fetch the config for this lvm pool.
+ config, err := query.SelectConfig(tx,
"storage_pools_config", "storage_pool_id=? AND node_id=?", poolID, nodeID)
+ if err != nil {
+ return errors.Wrap(err, "Failed to fetch of lvm
pool config")
+ }
+
+ // Check if already set.
+ _, ok := config["lvm.vg_name"]
+ if ok {
+ continue
+ }
+
+ // Add lvm.vg_name config entry.
+ _, err = tx.Exec(`
+INSERT INTO storage_pools_config(storage_pool_id, node_id, key, value)
+SELECT ?, ?, 'lvm.vg_name', name FROM storage_pools WHERE id=?
+`, poolID, nodeID, poolID)
+ if err != nil {
+ return errors.Wrap(err, "Failed to create
lvm.vg_name node config")
+ }
+ }
+ }
+
+ return nil
}
// The zfs.pool_name config key is required for ZFS to function.
From 2463ccea11ff21c8b2bb9d02a3750eda6c364840 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Thu, 6 Feb 2020 09:16:03 +0000
Subject: [PATCH 09/14] lxd/db/cluster/update: Superfluous trailing whitespace
Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
lxd/db/cluster/update.go | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lxd/db/cluster/update.go b/lxd/db/cluster/update.go
index 6b3d353a9a..7b9763a238 100644
--- a/lxd/db/cluster/update.go
+++ b/lxd/db/cluster/update.go
@@ -174,12 +174,12 @@ CREATE TABLE images_profiles (
FOREIGN KEY (profile_id) REFERENCES profiles (id) ON DELETE CASCADE,
UNIQUE (image_id, profile_id)
);
-INSERT INTO images_profiles (image_id, profile_id)
+INSERT INTO images_profiles (image_id, profile_id)
SELECT images.id, profiles.id FROM images
JOIN profiles ON images.project_id = profiles.project_id
WHERE profiles.name = 'default';
INSERT INTO images_profiles (image_id, profile_id)
- SELECT images.id, profiles.id FROM projects_config AS R
+ SELECT images.id, profiles.id FROM projects_config AS R
JOIN projects_config AS S ON R.project_id = S.project_id
JOIN images ON images.project_id = R.project_id
JOIN profiles ON profiles.project_id = 1 AND profiles.name = "default"
From d10dd39459732167a4f542430e1dd629b2108877 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Thu, 6 Feb 2020 09:17:23 +0000
Subject: [PATCH 10/14] lxd/db/cluster/schema: v24 update
Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
lxd/db/cluster/schema.go | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lxd/db/cluster/schema.go b/lxd/db/cluster/schema.go
index a7bd5e2b05..02f2057d10 100644
--- a/lxd/db/cluster/schema.go
+++ b/lxd/db/cluster/schema.go
@@ -495,5 +495,5 @@ CREATE TABLE storage_volumes_config (
FOREIGN KEY (storage_volume_id) REFERENCES storage_volumes (id) ON DELETE
CASCADE
);
-INSERT INTO schema (version, updated_at) VALUES (23, strftime("%s"))
+INSERT INTO schema (version, updated_at) VALUES (24, strftime("%s"))
`
From ecde4cc3a3b1da1471e38c934d837d9779c20162 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Thu, 6 Feb 2020 10:19:50 +0000
Subject: [PATCH 11/14] lxd/device/config/devices: Adds NICType function on
Device type
This function centralises the logic used to derive the nictype property from
both the network and nictype properties.
Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
lxd/device/config/devices.go | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/lxd/device/config/devices.go b/lxd/device/config/devices.go
index f6ec31c1e4..d574e19adc 100644
--- a/lxd/device/config/devices.go
+++ b/lxd/device/config/devices.go
@@ -19,6 +19,21 @@ func (device Device) Clone() Device {
return copy
}
+// NICType returns the derived NIC Type for a NIC device.
+// If the "network" property is specified then this implicitly (at least for
now) means the nictype is "bridged".
+// Otherwise the "nictype" property is returned. If the device type is not a
NIC then an empty string is returned.
+func (device Device) NICType() string {
+ if device["type"] == "nic" {
+ if device["network"] != "" {
+ return "bridged"
+ }
+
+ return device["nictype"]
+ }
+
+ return ""
+}
+
// Validate accepts a map of field/validation functions to run against the
device's config.
func (device Device) Validate(rules map[string]func(value string) error) error
{
checkedFields := map[string]struct{}{}
From 0de66979ea782b5074db97956edb618531f925f8 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Thu, 6 Feb 2020 10:21:16 +0000
Subject: [PATCH 12/14] lxd/device: Device.NICType usage
Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
lxd/device/device_utils_network.go | 4 ++--
lxd/device/infiniband.go | 2 +-
lxd/device/nic.go | 2 +-
lxd/device/proxy.go | 2 +-
4 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/lxd/device/device_utils_network.go
b/lxd/device/device_utils_network.go
index 4c9c5e757d..a8850aa5c8 100644
--- a/lxd/device/device_utils_network.go
+++ b/lxd/device/device_utils_network.go
@@ -388,7 +388,7 @@ func networkSetupHostVethDevice(device deviceConfig.Device,
oldDevice deviceConf
func networkSetVethRoutes(m deviceConfig.Device) error {
// Decide whether the route should point to the veth parent or the
bridge parent.
routeDev := m["host_name"]
- if m["nictype"] == "bridged" {
+ if m.NICType() == "bridged" {
routeDev = m["parent"]
}
@@ -426,7 +426,7 @@ func networkSetVethRoutes(m deviceConfig.Device) error {
func networkRemoveVethRoutes(m deviceConfig.Device) {
// Decide whether the route should point to the veth parent or the
bridge parent
routeDev := m["host_name"]
- if m["nictype"] == "bridged" {
+ if m.NICType() == "bridged" {
routeDev = m["parent"]
}
diff --git a/lxd/device/infiniband.go b/lxd/device/infiniband.go
index 9fc586d941..adcb411c87 100644
--- a/lxd/device/infiniband.go
+++ b/lxd/device/infiniband.go
@@ -12,7 +12,7 @@ var infinibandTypes = map[string]func() device{
// infinibandLoadByType returns an Infiniband device instantiated with
supplied config.
func infinibandLoadByType(c deviceConfig.Device) device {
- f := infinibandTypes[c["nictype"]]
+ f := infinibandTypes[c.NICType()]
if f != nil {
return f()
}
diff --git a/lxd/device/nic.go b/lxd/device/nic.go
index f8b5bd3522..d33a8820a3 100644
--- a/lxd/device/nic.go
+++ b/lxd/device/nic.go
@@ -18,7 +18,7 @@ var nicTypes = map[string]func() device{
// nicLoadByType returns a NIC device instantiated with supplied config.
func nicLoadByType(c deviceConfig.Device) device {
- f := nicTypes[c["nictype"]]
+ f := nicTypes[c.NICType()]
if f != nil {
return f()
}
diff --git a/lxd/device/proxy.go b/lxd/device/proxy.go
index 77a1394575..3e6f38b558 100644
--- a/lxd/device/proxy.go
+++ b/lxd/device/proxy.go
@@ -267,7 +267,7 @@ func (d *proxy) setupNAT() error {
var IPv6Addr net.IP
for _, devConfig := range d.inst.ExpandedDevices() {
- if devConfig["type"] != "nic" || (devConfig["type"] == "nic" &&
devConfig["nictype"] != "bridged") {
+ if devConfig["type"] != "nic" || (devConfig["type"] == "nic" &&
devConfig.NICType() != "bridged") {
continue
}
From c786f6b3d88ef8bccc0f229dccd4ba4e439a467d Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Thu, 6 Feb 2020 10:22:36 +0000
Subject: [PATCH 13/14] lxd/device/nic/bridged: Bans use of nictype when using
network property
Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
lxd/device/nic_bridged.go | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lxd/device/nic_bridged.go b/lxd/device/nic_bridged.go
index af982f0a87..ad089ae747 100644
--- a/lxd/device/nic_bridged.go
+++ b/lxd/device/nic_bridged.go
@@ -76,7 +76,7 @@ func (d *nicBridged) validateConfig(instConf
instance.ConfigReader) error {
if d.config["network"] != "" {
requiredFields = append(requiredFields, "network")
- bannedKeys := []string{"parent", "mtu", "maas.subnet.ipv4",
"maas.subnet.ipv6"}
+ bannedKeys := []string{"nictype", "parent", "mtu",
"maas.subnet.ipv4", "maas.subnet.ipv6"}
for _, bannedKey := range bannedKeys {
if d.config[bannedKey] != "" {
return fmt.Errorf("Cannot use %q property in
conjunction with %q property", bannedKey, "network")
From b04915ea6febf7dd68778bb30e2507b7613baaf2 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Thu, 6 Feb 2020 10:22:59 +0000
Subject: [PATCH 14/14] test: Updates nic bridged tests for NICType logic
Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
test/suites/container_devices_nic_bridged.sh | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/test/suites/container_devices_nic_bridged.sh
b/test/suites/container_devices_nic_bridged.sh
index f78a7a7959..dbde3897dd 100644
--- a/test/suites/container_devices_nic_bridged.sh
+++ b/test/suites/container_devices_nic_bridged.sh
@@ -446,13 +446,14 @@ test_container_devices_nic_bridged() {
# Can't use network property when parent is set.
! lxc profile device set "${ctName}" eth0 network="${brName}"
- # Reset mtu and parent settings and assign network in one command.
- lxc profile device set "${ctName}" eth0 mtu="" parent="" network="${brName}"
+ # Remove mtu, nictype and parent settings and assign network in one command.
+ lxc profile device set "${ctName}" eth0 mtu="" parent="" nictype=""
network="${brName}"
# Can't remove network if parent not specified.
! lxc profile device unset "${ctName}" eth0 network
# Can't use some settings when network is set.
+ ! lxc profile device set "${ctName}" eth0 nictype="bridged"
! lxc profile device set "${ctName}" eth0 mtu="1400"
! lxc profile device set "${ctName}" eth0 maas.subnet.ipv4="test"
! lxc profile device set "${ctName}" eth0 maas.subnet.ipv6="test"
@@ -469,6 +470,17 @@ test_container_devices_nic_bridged() {
echo "container mtu has not been inherited from network"
false
fi
+
+ # Check profile routes are applied on boot (as these use derived nictype).
+ if ! ip -4 r list dev "${brName}" | grep "192.0.2.1${ipRand}" ; then
+ echo "ipv4.routes invalid"
+ false
+ fi
+ if ! ip -6 r list dev "${brName}" | grep "2001:db8::1${ipRand}" ; then
+ echo "ipv6.routes invalid"
+ false
+ fi
+
lxc stop -f "${ctName}"
lxc network unset "${brName}" bridge.mtu
_______________________________________________
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel
