canonical / lxd

Powerful system container and virtual machine manager
https://canonical.com/lxd
GNU Affero General Public License v3.0
4.38k stars 931 forks source link

lxd: Unsetting `core.metric_address` does not stop from serving metrics #12105

Closed gabrielmougard closed 1 year ago

gabrielmougard commented 1 year ago

Required information

   * Kernel version: `5.15.0-78-generic`
   * LXC version: `5.16`
   * LXD version: `5.16`
   * Storage backend in use: `zfs`

# Issue description

After a metric server is started (`lxc config set core.metric_address <ADDR>`), unsetting the property (`lxc config unset core.metric_address`) does not stop the server (the metric page is still available). Looking at the logs, it seems that the underlying listener is closed but I suspect that the server shutdown is not triggered by the listener shutting down. Looking in `lxd/endpoint/endpoints.go` in `func (e *Endpoints) serve(kind kind)`, I see a final call to: 

```go
...

e.tomb.Go(func() error {
   return server.Serve(listener)
})

With no signal handling whatsoever (i.e, when the listener closes, is this goroutine terminated ? Because it seems I still can access the endpoint eventhough this one is not listening for new incoming data anymore)

I didn't check if this happens for other existing endpoints but I noticed the same behaviour here with this new endpoint I recently introduced.

Is it a bug or a feature (a.k.a is it intended to keep the endpoint reachable eventhough the property defining the endpoint address is not in the server core config) ?

Steps to reproduce

  1. lxc config set core.metric_address :8444
  2. Go to http://0.0.0.0:8444/1.0 in a browser and see the metrics.
  3. lxc config unset core.metric_address
  4. lxc config show , now core.metric_address should have disappeared which is as expected.
  5. See a trace of closing socket in the log, proof that func (e *Endpoints) closeListener(kind kind) error has been called successfully.
  6. Go to http://0.0.0.0:8444/1.0 in a browser and witness that the metric server is still reachable (we don't want that).
gabrielmougard commented 1 year ago

@monstermunchkin @tomponline have you witnessed this already ?

tomponline commented 1 year ago

Please show "lxc config show"

gabrielmougard commented 1 year ago
$ lxc config show
config: {}
monstermunchkin commented 1 year ago

This is not a bug.

I can reproduce what you're seeing when using the browser. That must have to do with caching. If you try the same thing with curl, you cannot connect to the server. Also, the output of ss -tln doesn't show that it's listening on the port anymore.

tomponline commented 1 year ago

Thanks!