The following pull request was submitted through Github.
It can be accessed and reviewed at: https://github.com/lxc/lxd/pull/8155
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) ===
This change ensures that if the instance ID changes while it is running (which can happen if one performs an `lxd import` over the top of a running container) that the container's stop hooks will still be able to find the instance by name and be able to cleanly shutdown.
The start hook is also updated for consistency with the others.
All 3 hooks still handle being passed instance ID in order to allow clean shutdown of instances started before this change.
From d9d2a053653613b8ad593fbcbacd93dbecb3ec19 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Fri, 13 Nov 2020 12:00:31 +0000
Subject: [PATCH 1/4] lxd/instance/drivers/driver/lxc: Updates initLXC to use
project and instance name in callhook hook commands
Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
lxd/instance/drivers/driver_lxc.go | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/lxd/instance/drivers/driver_lxc.go
b/lxd/instance/drivers/driver_lxc.go
index 101a0078d7..4645fc6dc1 100644
--- a/lxd/instance/drivers/driver_lxc.go
+++ b/lxd/instance/drivers/driver_lxc.go
@@ -943,17 +943,20 @@ func (c *lxc) initLXC(config bool) error {
return err
}
- err = lxcSetConfigItem(cc, "lxc.hook.pre-start",
fmt.Sprintf("/proc/%d/exe callhook %s %d start", os.Getpid(),
shared.VarPath(""), c.id))
+ // Call the onstart hook on start.
+ err = lxcSetConfigItem(cc, "lxc.hook.pre-start",
fmt.Sprintf("/proc/%d/exe callhook %s %s %s start", os.Getpid(),
shared.VarPath(""), strconv.Quote(c.Project()), strconv.Quote(c.Name())))
if err != nil {
return err
}
- err = lxcSetConfigItem(cc, "lxc.hook.stop", fmt.Sprintf("%s callhook %s
%d stopns", c.state.OS.ExecPath, shared.VarPath(""), c.id))
+ // Call the onstopns hook on stop but before namespaces are unmounted.
+ err = lxcSetConfigItem(cc, "lxc.hook.stop", fmt.Sprintf("%s callhook %s
%s %s stopns", c.state.OS.ExecPath, shared.VarPath(""),
strconv.Quote(c.Project()), strconv.Quote(c.Name())))
if err != nil {
return err
}
- err = lxcSetConfigItem(cc, "lxc.hook.post-stop", fmt.Sprintf("%s
callhook %s %d stop", c.state.OS.ExecPath, shared.VarPath(""), c.id))
+ // Call the onstop hook on stop.
+ err = lxcSetConfigItem(cc, "lxc.hook.post-stop", fmt.Sprintf("%s
callhook %s %s %s stop", c.state.OS.ExecPath, shared.VarPath(""),
strconv.Quote(c.Project()), strconv.Quote(c.Name())))
if err != nil {
return err
}
From 25bf93b31a892f662de892f6c4b4c0ef86e8f468 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Fri, 13 Nov 2020 12:01:12 +0000
Subject: [PATCH 2/4] lxd/instance/drivers/driver/lxc: Updates startCommon to
quote hook command arguments
Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
lxd/instance/drivers/driver_lxc.go | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/lxd/instance/drivers/driver_lxc.go
b/lxd/instance/drivers/driver_lxc.go
index 4645fc6dc1..4705a36d90 100644
--- a/lxd/instance/drivers/driver_lxc.go
+++ b/lxd/instance/drivers/driver_lxc.go
@@ -2187,19 +2187,19 @@ func (c *lxc) startCommon() (string, []func() error,
error) {
if c.state.OS.Shiftfs && !c.IsPrivileged() && diskIdmap
== nil {
// Host side mark mount.
- err = lxcSetConfigItem(c.c,
"lxc.hook.pre-start", fmt.Sprintf("/bin/mount -t shiftfs -o mark,passthrough=3
%s %s", c.RootfsPath(), c.RootfsPath()))
+ err = lxcSetConfigItem(c.c,
"lxc.hook.pre-start", fmt.Sprintf("/bin/mount -t shiftfs -o mark,passthrough=3
%s %s", strconv.Quote(c.RootfsPath()), strconv.Quote(c.RootfsPath())))
if err != nil {
return "", nil, errors.Wrapf(err,
"Failed to setup device mount shiftfs '%s'", dev.Name)
}
// Container side shift mount.
- err = lxcSetConfigItem(c.c,
"lxc.hook.pre-mount", fmt.Sprintf("/bin/mount -t shiftfs -o passthrough=3 %s
%s", c.RootfsPath(), c.RootfsPath()))
+ err = lxcSetConfigItem(c.c,
"lxc.hook.pre-mount", fmt.Sprintf("/bin/mount -t shiftfs -o passthrough=3 %s
%s", strconv.Quote(c.RootfsPath()), strconv.Quote(c.RootfsPath())))
if err != nil {
return "", nil, errors.Wrapf(err,
"Failed to setup device mount shiftfs '%s'", dev.Name)
}
// Host side umount of mark mount.
- err = lxcSetConfigItem(c.c,
"lxc.hook.start-host", fmt.Sprintf("/bin/umount -l %s", c.RootfsPath()))
+ err = lxcSetConfigItem(c.c,
"lxc.hook.start-host", fmt.Sprintf("/bin/umount -l %s",
strconv.Quote(c.RootfsPath())))
if err != nil {
return "", nil, errors.Wrapf(err,
"Failed to setup device mount shiftfs '%s'", dev.Name)
}
@@ -2228,17 +2228,17 @@ func (c *lxc) startCommon() (string, []func() error,
error) {
return "", nil,
errors.Wrapf(fmt.Errorf("shiftfs is required but isn't supported on system"),
"Failed to setup device mount '%s'", dev.Name)
}
- err = lxcSetConfigItem(c.c,
"lxc.hook.pre-start", fmt.Sprintf("/bin/mount -t shiftfs -o mark,passthrough=3
%s %s", mount.DevPath, mount.DevPath))
+ err = lxcSetConfigItem(c.c,
"lxc.hook.pre-start", fmt.Sprintf("/bin/mount -t shiftfs -o mark,passthrough=3
%s %s", strconv.Quote(mount.DevPath), strconv.Quote(mount.DevPath)))
if err != nil {
return "", nil,
errors.Wrapf(err, "Failed to setup device mount shiftfs '%s'", dev.Name)
}
- err = lxcSetConfigItem(c.c,
"lxc.hook.pre-mount", fmt.Sprintf("/bin/mount -t shiftfs -o passthrough=3 %s
%s", mount.DevPath, mount.DevPath))
+ err = lxcSetConfigItem(c.c,
"lxc.hook.pre-mount", fmt.Sprintf("/bin/mount -t shiftfs -o passthrough=3 %s
%s", strconv.Quote(mount.DevPath), strconv.Quote(mount.DevPath)))
if err != nil {
return "", nil,
errors.Wrapf(err, "Failed to setup device mount shiftfs '%s'", dev.Name)
}
- err = lxcSetConfigItem(c.c,
"lxc.hook.start-host", fmt.Sprintf("/bin/umount -l %s", mount.DevPath))
+ err = lxcSetConfigItem(c.c,
"lxc.hook.start-host", fmt.Sprintf("/bin/umount -l %s",
strconv.Quote(mount.DevPath)))
if err != nil {
return "", nil,
errors.Wrapf(err, "Failed to setup device mount shiftfs '%s'", dev.Name)
}
From 63351b856b3ddefc4b7bb12631731b19cfb0c2f5 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Fri, 13 Nov 2020 11:08:49 +0000
Subject: [PATCH 3/4] lxd/main/callhook: Updates cmdCallhook to support using
project name and instance name in arguments
Removes support for unused hook "network-up".
Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
lxd/main_callhook.go | 68 +++++++++++++++++++++++++-------------------
1 file changed, 38 insertions(+), 30 deletions(-)
diff --git a/lxd/main_callhook.go b/lxd/main_callhook.go
index e2c7adfb11..84390f87ec 100644
--- a/lxd/main_callhook.go
+++ b/lxd/main_callhook.go
@@ -2,6 +2,7 @@ package main
import (
"fmt"
+ "net/url"
"os"
"path/filepath"
"time"
@@ -17,7 +18,7 @@ type cmdCallhook struct {
func (c *cmdCallhook) Command() *cobra.Command {
cmd := &cobra.Command{}
- cmd.Use = "callhook <path> <id> <hook>"
+ cmd.Use = "callhook <path> [<instance id>|<instance project> <instance
name>] <hook>"
cmd.Short = "Call container lifecycle hook in LXD"
cmd.Long = `Description:
Call container lifecycle hook in LXD
@@ -32,7 +33,7 @@ func (c *cmdCallhook) Command() *cobra.Command {
}
func (c *cmdCallhook) Run(cmd *cobra.Command, args []string) error {
- // Sanity checks
+ // Sanity checks.
if len(args) < 2 {
cmd.Help()
@@ -44,16 +45,28 @@ func (c *cmdCallhook) Run(cmd *cobra.Command, args
[]string) error {
}
path := args[0]
- id := args[1]
- state := args[2]
+
+ var projectName string
+ var instanceRef string
+ var hook string
+
+ if len(args) == 3 {
+ instanceRef = args[1]
+ hook = args[2]
+ } else if len(args) == 4 {
+ projectName = args[1]
+ instanceRef = args[2]
+ hook = args[3]
+ }
+
target := ""
- // Only root should run this
+ // Only root should run this.
if os.Geteuid() != 0 {
return fmt.Errorf("This must be run as root")
}
- // Connect to LXD
+ // Connect to LXD.
socket := os.Getenv("LXD_SOCKET")
if socket == "" {
socket = filepath.Join(path, "unix.socket")
@@ -67,40 +80,35 @@ func (c *cmdCallhook) Run(cmd *cobra.Command, args
[]string) error {
return err
}
- // Prepare the request URL
- url := fmt.Sprintf("/internal/containers/%s/on%s", id, state)
- if state == "stopns" {
- target = os.Getenv("LXC_TARGET")
- netns := os.Getenv("LXC_NET_NS")
- if target == "" {
- target = "unknown"
- }
- url = fmt.Sprintf("%s?target=%s&netns=%s", url, target, netns)
- } else if state == "stop" {
+ // Prepare the request URL query parameters.
+ v := url.Values{}
+ if projectName != "" {
+ v.Set("project", projectName)
+ }
+
+ if hook == "stop" || hook == "stopns" {
target = os.Getenv("LXC_TARGET")
if target == "" {
target = "unknown"
}
- url = fmt.Sprintf("%s?target=%s", url, target)
- } else if state == "network-up" {
- url = fmt.Sprintf("%s?device=%s&host_name=%s", url, args[3],
os.Getenv("LXC_NET_PEER"))
+ v.Set("target", target)
+ }
+
+ if hook == "stopns" {
+ v.Set("netns", os.Getenv("LXC_NET_NS"))
}
- // Setup the request
- hook := make(chan error, 1)
+ // Setup the request.
+ response := make(chan error, 1)
go func() {
+ url := fmt.Sprintf("/internal/containers/%s/%s?%s",
url.PathEscape(instanceRef), url.PathEscape(fmt.Sprintf("on%s", hook)),
v.Encode())
_, _, err := d.RawQuery("GET", url, nil, "")
- if err != nil {
- hook <- err
- return
- }
-
- hook <- nil
+ response <- err
}()
- // Handle the timeout
+ // Handle the timeout.
select {
- case err := <-hook:
+ case err := <-response:
if err != nil {
return err
}
@@ -112,7 +120,7 @@ func (c *cmdCallhook) Run(cmd *cobra.Command, args
[]string) error {
// If the container is rebooting, we purposefully tell LXC that this
hook failed so that
// it won't reboot the container, which lets LXD start it again in the
OnStop function.
// Other hook types can return without error safely.
- if state == "stop" && target == "reboot" {
+ if hook == "stop" && target == "reboot" {
return fmt.Errorf("Reboot must be handled by LXD")
}
From 67b7c35ac82086b304945f3b3c663d5b953e4067 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Fri, 13 Nov 2020 11:06:25 +0000
Subject: [PATCH 4/4] lxd/api/internal: Adds support for using instance name
and project name in container hook routes
This allows the stop hooks to be run even if the instance has been recovered to
a different DB ID while the container is running.
However in order to ensure previously started containers can still stop cleanly
we keep support for using instance ID too.
Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
lxd/api_internal.go | 74 ++++++++++++++++++++++++---------------------
1 file changed, 39 insertions(+), 35 deletions(-)
diff --git a/lxd/api_internal.go b/lxd/api_internal.go
index e10e3404c0..5bc6d48b80 100644
--- a/lxd/api_internal.go
+++ b/lxd/api_internal.go
@@ -25,6 +25,7 @@ import (
"github.com/lxc/lxd/lxd/instance/instancetype"
"github.com/lxc/lxd/lxd/project"
"github.com/lxc/lxd/lxd/response"
+ "github.com/lxc/lxd/lxd/state"
storagePools "github.com/lxc/lxd/lxd/storage"
storageDrivers "github.com/lxc/lxd/lxd/storage/drivers"
"github.com/lxc/lxd/shared"
@@ -65,19 +66,19 @@ var internalReadyCmd = APIEndpoint{
}
var internalContainerOnStartCmd = APIEndpoint{
- Path: "containers/{id}/onstart",
+ Path: "containers/{instanceRef}/onstart",
Get: APIEndpointAction{Handler: internalContainerOnStart},
}
var internalContainerOnStopNSCmd = APIEndpoint{
- Path: "containers/{id}/onstopns",
+ Path: "containers/{instanceRef}/onstopns",
Get: APIEndpointAction{Handler: internalContainerOnStopNS},
}
var internalContainerOnStopCmd = APIEndpoint{
- Path: "containers/{id}/onstop",
+ Path: "containers/{instanceRef}/onstop",
Get: APIEndpointAction{Handler: internalContainerOnStop},
}
@@ -138,24 +139,43 @@ func internalShutdown(d *Daemon, r *http.Request)
response.Response {
return response.EmptySyncResponse
}
-func internalContainerOnStart(d *Daemon, r *http.Request) response.Response {
- id, err := strconv.Atoi(mux.Vars(r)["id"])
- if err != nil {
- return response.SmartError(err)
- }
+// internalContainerHookLoadFromRequestReference loads the container from the
instance reference in the request.
+// It detects whether the instance reference is an instance ID or instance
name and loads instance accordingly.
+func internalContainerHookLoadFromReference(s *state.State, r *http.Request)
(instance.Instance, error) {
+ var inst instance.Instance
+ instanceRef := mux.Vars(r)["instanceRef"]
+ projectName := projectParam(r)
- inst, err := instance.LoadByID(d.State(), id)
- if err != nil {
- return response.SmartError(err)
+ instanceID, err := strconv.Atoi(instanceRef)
+ if err == nil {
+ inst, err = instance.LoadByID(s, instanceID)
+ if err != nil {
+ return nil, err
+ }
+ } else {
+ inst, err = instance.LoadByProjectAndName(s, projectName,
instanceRef)
+ if err != nil {
+ return nil, err
+ }
}
if inst.Type() != instancetype.Container {
- return response.SmartError(fmt.Errorf("Instance is not
container type"))
+ return nil, fmt.Errorf("Instance is not container type")
+ }
+
+ return inst, nil
+}
+
+func internalContainerOnStart(d *Daemon, r *http.Request) response.Response {
+ inst, err := internalContainerHookLoadFromReference(d.State(), r)
+ if err != nil {
+ logger.Error("The start hook failed to load",
log.Ctx{"instance": inst.Name(), "err": err})
+ return response.SmartError(err)
}
err = inst.OnHook(instance.HookStart, nil)
if err != nil {
- logger.Error("The start hook failed", log.Ctx{"container":
inst.Name(), "err": err})
+ logger.Error("The start hook failed", log.Ctx{"instance":
inst.Name(), "err": err})
return response.SmartError(err)
}
@@ -163,8 +183,9 @@ func internalContainerOnStart(d *Daemon, r *http.Request)
response.Response {
}
func internalContainerOnStopNS(d *Daemon, r *http.Request) response.Response {
- id, err := strconv.Atoi(mux.Vars(r)["id"])
+ inst, err := internalContainerHookLoadFromReference(d.State(), r)
if err != nil {
+ logger.Error("The stopns hook failed to load",
log.Ctx{"instance": inst.Name(), "err": err})
return response.SmartError(err)
}
@@ -174,15 +195,6 @@ func internalContainerOnStopNS(d *Daemon, r *http.Request)
response.Response {
}
netns := queryParam(r, "netns")
- inst, err := instance.LoadByID(d.State(), id)
- if err != nil {
- return response.SmartError(err)
- }
-
- if inst.Type() != instancetype.Container {
- return response.SmartError(fmt.Errorf("Instance is not
container type"))
- }
-
args := map[string]string{
"target": target,
"netns": netns,
@@ -190,7 +202,7 @@ func internalContainerOnStopNS(d *Daemon, r *http.Request)
response.Response {
err = inst.OnHook(instance.HookStopNS, args)
if err != nil {
- logger.Error("The stopns hook failed", log.Ctx{"container":
inst.Name(), "err": err})
+ logger.Error("The stopns hook failed", log.Ctx{"instance":
inst.Name(), "err": err})
return response.SmartError(err)
}
@@ -198,8 +210,9 @@ func internalContainerOnStopNS(d *Daemon, r *http.Request)
response.Response {
}
func internalContainerOnStop(d *Daemon, r *http.Request) response.Response {
- id, err := strconv.Atoi(mux.Vars(r)["id"])
+ inst, err := internalContainerHookLoadFromReference(d.State(), r)
if err != nil {
+ logger.Error("The stop hook failed to load",
log.Ctx{"instance": inst.Name(), "err": err})
return response.SmartError(err)
}
@@ -208,22 +221,13 @@ func internalContainerOnStop(d *Daemon, r *http.Request)
response.Response {
target = "unknown"
}
- inst, err := instance.LoadByID(d.State(), id)
- if err != nil {
- return response.SmartError(err)
- }
-
- if inst.Type() != instancetype.Container {
- return response.SmartError(fmt.Errorf("Instance is not
container type"))
- }
-
args := map[string]string{
"target": target,
}
err = inst.OnHook(instance.HookStop, args)
if err != nil {
- logger.Error("The stop hook failed", log.Ctx{"container":
inst.Name(), "err": err})
+ logger.Error("The stop hook failed", log.Ctx{"instance":
inst.Name(), "err": err})
return response.SmartError(err)
}
_______________________________________________
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel
