[lxc-devel] [lxd/master] Network: Make OVN updates more nuanced and less destructive

2020-12-18 Thread tomponline on Github
The following pull request was submitted through Github.
It can be accessed and reviewed at: https://github.com/lxc/lxd/pull/8276

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) ===
Don't tear down all OVN config and rebuild, instead try and apply only changes, so as to reduce impact on instance port config.
From cba9502715754f6ae05cfa8bb4a4dae4e62c8465 Mon Sep 17 00:00:00 2001
From: Thomas Parrott 
Date: Fri, 18 Dec 2020 11:45:16 +
Subject: [PATCH 01/16] lxd/network/openvswitch/ovn: Adds mayExist argument to
 LogicalRouterAdd

Signed-off-by: Thomas Parrott 
---
 lxd/network/openvswitch/ovn.go | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/lxd/network/openvswitch/ovn.go b/lxd/network/openvswitch/ovn.go
index d7ad3079ee..86625846c2 100644
--- a/lxd/network/openvswitch/ovn.go
+++ b/lxd/network/openvswitch/ovn.go
@@ -125,8 +125,14 @@ func (o *OVN) nbctl(args ...string) (string, error) {
 }
 
 // LogicalRouterAdd adds a named logical router.
-func (o *OVN) LogicalRouterAdd(routerName OVNRouter) error {
-   _, err := o.nbctl("lr-add", string(routerName))
+func (o *OVN) LogicalRouterAdd(routerName OVNRouter, mayExist bool) error {
+   args := []string{}
+
+   if mayExist {
+   args = append(args, "--may-exist")
+   }
+
+   _, err := o.nbctl(append(args, "lr-add", string(routerName))...)
if err != nil {
return err
}

From a67d1ed533580a1e7829ca9afd7a99363eba4ce4 Mon Sep 17 00:00:00 2001
From: Thomas Parrott 
Date: Fri, 18 Dec 2020 11:46:13 +
Subject: [PATCH 02/16] lxd/network/openvswitch/ovn: Adds mayExist argument to
 LogicalRouterSNATAdd

Signed-off-by: Thomas Parrott 
---
 lxd/network/openvswitch/ovn.go | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/lxd/network/openvswitch/ovn.go b/lxd/network/openvswitch/ovn.go
index 86625846c2..f529d39394 100644
--- a/lxd/network/openvswitch/ovn.go
+++ b/lxd/network/openvswitch/ovn.go
@@ -151,8 +151,14 @@ func (o OVN) LogicalRouterDelete(routerName OVNRouter) 
error {
 }
 
 // LogicalRouterSNATAdd adds an SNAT rule to a logical router to translate 
packets from intNet to extIP.
-func (o *OVN) LogicalRouterSNATAdd(routerName OVNRouter, intNet *net.IPNet, 
extIP net.IP) error {
-   _, err := o.nbctl("lr-nat-add", string(routerName), "snat", 
extIP.String(), intNet.String())
+func (o *OVN) LogicalRouterSNATAdd(routerName OVNRouter, intNet *net.IPNet, 
extIP net.IP, mayExist bool) error {
+   args := []string{}
+
+   if mayExist {
+   args = append(args, "--may-exist")
+   }
+
+   _, err := o.nbctl(append(args, "lr-nat-add", string(routerName), 
"snat", extIP.String(), intNet.String())...)
if err != nil {
return err
}

From 94fdc43a5e424d0a9ec4d35e70f12af2a1a5dd65 Mon Sep 17 00:00:00 2001
From: Thomas Parrott 
Date: Fri, 18 Dec 2020 11:46:41 +
Subject: [PATCH 03/16] lxd/network/openvswitch/ovn: Simplifies
 LogicalRouterRouteAdd

Signed-off-by: Thomas Parrott 
---
 lxd/network/openvswitch/ovn.go | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/lxd/network/openvswitch/ovn.go b/lxd/network/openvswitch/ovn.go
index f529d39394..fa6a409b6c 100644
--- a/lxd/network/openvswitch/ovn.go
+++ b/lxd/network/openvswitch/ovn.go
@@ -204,8 +204,7 @@ func (o *OVN) LogicalRouterRouteAdd(routerName OVNRouter, 
destination *net.IPNet
args = append(args, "--may-exist")
}
 
-   args = append(args, "lr-route-add", string(routerName), 
destination.String(), nextHop.String())
-   _, err := o.nbctl(args...)
+   _, err := o.nbctl(append(args, "lr-route-add", string(routerName), 
destination.String(), nextHop.String())...)
if err != nil {
return err
}

From e47204f3c35bc466639591ff1b13d6302e4d6a21 Mon Sep 17 00:00:00 2001
From: Thomas Parrott 
Date: Fri, 18 Dec 2020 11:46:57 +
Subject: [PATCH 04/16] lxd/network/openvswitch/ovn: Adds mayExist argument to
 LogicalRouterPortAdd

Signed-off-by: Thomas Parrott 
---
 lxd/network/openvswitch/ovn.go | 24 +++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/lxd/network/openvswitch/ovn.go b/lxd/network/openvswitch/ovn.go
index fa6a409b6c..f4bd7f186f 100644
--- a/lxd/network/openvswitch/ovn.go
+++ b/lxd/network/openvswitch/ovn.go
@@ -230,7 +230,29 @@ func (o *OVN) LogicalRouterRouteDelete(routerName 
OVNRouter, destination *net.IP
 }
 
 // LogicalRouterPortAdd adds a named logical router port to a logical router.
-func (o *OVN) LogicalRouterPortAdd(routerName OVNRouter, portName 
OVNRouterPort, mac net.HardwareAddr, ipAddr ...*net.IPNet) error {
+func (o *OVN) LogicalRouterPortAdd(routerName OVNRouter, portName 
OVNRouterPort, mac net.HardwareAddr, ipAddr []*net.IPNet, mayExist bool) error {
+   if mayExist {
+

[lxc-devel] [lxd/master] Instance: Fix copying snapshot to new instance in different project

2020-12-18 Thread tomponline on Github
The following pull request was submitted through Github.
It can be accessed and reviewed at: https://github.com/lxc/lxd/pull/8275

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) ===
Fixes https://github.com/lxc/lxd/issues/8273

Adds test for this scenario.
From 954eadfa158f37860127f06422658a6d73ced0a0 Mon Sep 17 00:00:00 2001
From: Thomas Parrott 
Date: Fri, 18 Dec 2020 14:02:06 +
Subject: [PATCH 1/5] lxd/instances/post: Use source.Project when loading
 instance to get instance type in containersPost

Fixes #8273

Signed-off-by: Thomas Parrott 
---
 lxd/instances_post.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lxd/instances_post.go b/lxd/instances_post.go
index 92047db448..e10f4d79e1 100644
--- a/lxd/instances_post.go
+++ b/lxd/instances_post.go
@@ -838,7 +838,7 @@ func containersPost(d *Daemon, r *http.Request) 
response.Response {
return fmt.Errorf("Must specify a 
source instance")
}
 
-   source, err := 
instance.LoadInstanceDatabaseObject(tx, project, req.Source.Source)
+   source, err := 
instance.LoadInstanceDatabaseObject(tx, req.Source.Project, req.Source.Source)
if err != nil {
return errors.Wrap(err, "Load source 
instance from database")
}

From 48df8c87eef5e4e106d1f8f740464773541d9233 Mon Sep 17 00:00:00 2001
From: Thomas Parrott 
Date: Fri, 18 Dec 2020 14:06:24 +
Subject: [PATCH 2/5] lxd/instances/post: Rename project to targetProject to
 differentiate between source.Project in containersPost

Signed-off-by: Thomas Parrott 
---
 lxd/instances_post.go | 22 +++---
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/lxd/instances_post.go b/lxd/instances_post.go
index e10f4d79e1..edc32e78ba 100644
--- a/lxd/instances_post.go
+++ b/lxd/instances_post.go
@@ -722,12 +722,12 @@ func createFromBackup(d *Daemon, projectName string, data 
io.Reader, pool string
 }
 
 func containersPost(d *Daemon, r *http.Request) response.Response {
-   project := projectParam(r)
+   targetProject := projectParam(r)
logger.Debugf("Responding to instance create")
 
// If we're getting binary content, process separately
if r.Header.Get("Content-Type") == "application/octet-stream" {
-   return createFromBackup(d, project, r.Body, 
r.Header.Get("X-LXD-pool"), r.Header.Get("X-LXD-name"))
+   return createFromBackup(d, targetProject, r.Body, 
r.Header.Get("X-LXD-pool"), r.Header.Get("X-LXD-name"))
}
 
// Parse the request
@@ -754,7 +754,7 @@ func containersPost(d *Daemon, r *http.Request) 
response.Response {
// the selected node is the local one, this is effectively a
// no-op, since GetNodeWithLeastInstances() will return an empty
// string.
-   architectures, err := instance.SuitableArchitectures(d.State(), 
project, req)
+   architectures, err := instance.SuitableArchitectures(d.State(), 
targetProject, req)
if err != nil {
return response.BadRequest(err)
}
@@ -780,7 +780,7 @@ func containersPost(d *Daemon, r *http.Request) 
response.Response {
return response.SmartError(err)
}
 
-   client = client.UseProject(project)
+   client = client.UseProject(targetProject)
client = client.UseTarget(targetNode)
 
logger.Debugf("Forward instance post request to %s", 
address)
@@ -790,7 +790,7 @@ func containersPost(d *Daemon, r *http.Request) 
response.Response {
}
 
opAPI := op.Get()
-   return operations.ForwardedOperationResponse(project, 
&opAPI)
+   return 
operations.ForwardedOperationResponse(targetProject, &opAPI)
}
}
 
@@ -849,13 +849,13 @@ func containersPost(d *Daemon, r *http.Request) 
response.Response {
}
}
 
-   err := projecthelpers.AllowInstanceCreation(tx, project, req)
+   err := projecthelpers.AllowInstanceCreation(tx, targetProject, 
req)
if err != nil {
return err
}
 
if req.Name == "" {
-   names, err := tx.GetInstanceNames(project)
+   names, err := tx.GetInstanceNames(targetProject)
if err != nil {
return err
}
@@ -883,13 +883,13 @@ func containersPost(d *Daemon, r *http.Request) 
response.Response {

[lxc-devel] [lxd/master] Storage: Fix snapshot remove subsequent

2020-12-18 Thread tomponline on Github
The following pull request was submitted through Github.
It can be accessed and reviewed at: https://github.com/lxc/lxd/pull/8274

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) ===
When setting `volume.zfs.remove_snapshots=true` on a ZFS storage pool, this fixes several issues:

- The wrong snapshot was being checked for deletion suitability (resulting in not deleting any snapshots).
- Once that was fixed, there was also an issue with only the storage volume and storage volume DB record of the snapshot being deleted, not the instance snapshot record as well. Leaving orphaned snapshots in `lxc info ` output and preventing deletion of instance (because snapshot volume DB record had been removed).
- Because of the scope of the `err` being returned, it was likely that as a new `err` was created inside the subsequent snapshot deletion block, that the original error would be returned even on successful restore. Added `return nil` after successful restore.
- Modified `DeleteInstanceSnapshot` to not fail if the storage volume DB record has already been removed (as that is the desired result anyway).

Fixes https://discuss.linuxcontainers.org/t/snapshot-c1-20201218-03-cannot-be-restored-due-to-subsequent-snapshot-s-set-zfs-remove-snapshots-to-override/9742

From 35398d973bb5e87d12a40fe46449a7da849c7f7d Mon Sep 17 00:00:00 2001
From: Thomas Parrott 
Date: Fri, 18 Dec 2020 12:13:10 +
Subject: [PATCH 1/3] lxd/storage/drivers/driver/zfs/volumes: Error quoting in
 RestoreVolume

Signed-off-by: Thomas Parrott 
---
 lxd/storage/drivers/driver_zfs_volumes.go | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lxd/storage/drivers/driver_zfs_volumes.go 
b/lxd/storage/drivers/driver_zfs_volumes.go
index 29e38998d8..cec417f814 100644
--- a/lxd/storage/drivers/driver_zfs_volumes.go
+++ b/lxd/storage/drivers/driver_zfs_volumes.go
@@ -1788,14 +1788,14 @@ func (d *zfs) RestoreVolume(vol Volume, snapshotName 
string, op *operations.Oper
 
if strings.HasPrefix(entry, "@") {
// Located an internal snapshot.
-   return fmt.Errorf("Snapshot '%s' cannot be restored due 
to subsequent internal snapshot(s) (from a copy)", snapshotName)
+   return fmt.Errorf("Snapshot %q cannot be restored due 
to subsequent internal snapshot(s) (from a copy)", snapshotName)
}
}
 
// Check if snapshot removal is allowed.
if len(snapshots) > 0 {
if !shared.IsTrue(vol.ExpandedConfig("zfs.remove_snapshots")) {
-   return fmt.Errorf("Snapshot '%s' cannot be restored due 
to subsequent snapshot(s). Set zfs.remove_snapshots to override", snapshotName)
+   return fmt.Errorf("Snapshot %q cannot be restored due 
to subsequent snapshot(s). Set zfs.remove_snapshots to override", snapshotName)
}
 
// Setup custom error to tell the backend what to delete.

From 4efdfbc4fc20fdca860f99e9ac55d0948c3bd8ca Mon Sep 17 00:00:00 2001
From: Thomas Parrott 
Date: Fri, 18 Dec 2020 12:13:35 +
Subject: [PATCH 2/3] lxd/storage/backend/lxd: Don't fail in
 DeleteInstanceSnapshot if volume DB record already deleted

Signed-off-by: Thomas Parrott 
---
 lxd/storage/backend_lxd.go | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lxd/storage/backend_lxd.go b/lxd/storage/backend_lxd.go
index 2ca935dc0c..8d3358b2ca 100644
--- a/lxd/storage/backend_lxd.go
+++ b/lxd/storage/backend_lxd.go
@@ -1996,9 +1996,9 @@ func (b *lxdBackend) DeleteInstanceSnapshot(inst 
instance.Instance, op *operatio
return err
}
 
-   // Remove the snapshot volume record from the database.
+   // Remove the snapshot volume record from the database if exists.
err = b.state.Cluster.RemoveStoragePoolVolume(inst.Project(), 
drivers.GetSnapshotVolumeName(parentName, snapName), volDBType, b.ID())
-   if err != nil {
+   if err != nil && err != db.ErrNoSuchObject {
return err
}
 

From 841fcd1491216e470944a6c087a2e6fb61988e30 Mon Sep 17 00:00:00 2001
From: Thomas Parrott 
Date: Fri, 18 Dec 2020 12:14:16 +
Subject: [PATCH 3/3] lxd/storage/backend/lxd: Fix deleting subsequent
 snapshots for ZFS in RestoreInstanceSnapshot

Signed-off-by: Thomas Parrott 
---
 lxd/storage/backend_lxd.go | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/lxd/storage/backend_lxd.go b/lxd/storage/backend_lxd.go
index 8d3358b2ca..09ff3c95c5 100644
--- a/lxd/storage/backend_lxd.go
+++ b/lxd/storage/backend_lxd.go
@@ -2064,23 +2064,25 @@ func (b *lxdBackend) RestoreInstanceSnapshot(inst 
instance.Instance, src instanc
 
// Go through all the snapshots.