Skip to content

Commit

Permalink
drivers: make internal DisableLogCollection capability public
Browse files Browse the repository at this point in the history
The `DisableLogCollection` capability was introduced as an experimental
interface for the Docker driver in 0.10.4. The interface has been stable and
allowing third-party task drivers the same capability would be useful for those
drivers that don't need the additional overhead of logmon.

This PR only makes the capability public. It doesn't yet add it to the
configuration options for the other internal drivers.

Fixes: #14636 #15686
  • Loading branch information
tgross committed May 15, 2023
1 parent a430abe commit e0143a3
Show file tree
Hide file tree
Showing 9 changed files with 315 additions and 273 deletions.
3 changes: 3 additions & 0 deletions .changelog/17196.txt
@@ -0,0 +1,3 @@
```release-note:improvement
drivers: Add `DisableLogCollection` to task driver capabilities interface
```
13 changes: 2 additions & 11 deletions client/allocrunner/taskrunner/logmon_hook.go
Expand Up @@ -19,7 +19,6 @@ import (
"github.com/hashicorp/nomad/helper/uuid"
"github.com/hashicorp/nomad/nomad/structs"
bstructs "github.com/hashicorp/nomad/plugins/base/structs"
"github.com/hashicorp/nomad/plugins/drivers"
pstructs "github.com/hashicorp/nomad/plugins/shared/structs"
"google.golang.org/grpc"
"google.golang.org/grpc/codes"
Expand Down Expand Up @@ -157,16 +156,8 @@ func (h *logmonHook) isLoggingDisabled() bool {
return true
}

// internal plugins have access to a capability to disable logging and
// metrics via a private interface; this is an "experimental" interface and
// currently only the docker driver exposes this to users.
ic, ok := h.runner.driver.(drivers.InternalCapabilitiesDriver)
if !ok {
return false
}

caps := ic.InternalCapabilities()
if caps.DisableLogCollection {
caps := h.runner.driverCapabilities
if caps != nil && caps.DisableLogCollection {
h.logger.Debug("log collection is disabled by driver")
return true
}
Expand Down
9 changes: 1 addition & 8 deletions drivers/docker/config.go
Expand Up @@ -793,13 +793,6 @@ func (d *Driver) TaskConfigSchema() (*hclspec.Spec, error) {
// Capabilities is returned by the Capabilities RPC and indicates what optional
// features this driver supports.
func (d *Driver) Capabilities() (*drivers.Capabilities, error) {
driverCapabilities.DisableLogCollection = d.config != nil && d.config.DisableLogCollection
return driverCapabilities, nil
}

var _ drivers.InternalCapabilitiesDriver = (*Driver)(nil)

func (d *Driver) InternalCapabilities() drivers.InternalCapabilities {
return drivers.InternalCapabilities{
DisableLogCollection: d.config != nil && d.config.DisableLogCollection,
}
}
54 changes: 42 additions & 12 deletions drivers/docker/config_test.go
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/hashicorp/nomad/ci"
"github.com/hashicorp/nomad/helper/pluginutils/hclutils"
"github.com/hashicorp/nomad/plugins/drivers"
"github.com/shoenig/test/must"
"github.com/stretchr/testify/require"
)

Expand Down Expand Up @@ -621,28 +622,55 @@ func TestConfig_DriverConfig_GC(t *testing.T) {
}
}

func TestConfig_InternalCapabilities(t *testing.T) {
func TestConfig_Capabilities(t *testing.T) {
ci.Parallel(t)

cases := []struct {
name string
config string
expected drivers.InternalCapabilities
expected *drivers.Capabilities
}{
{
name: "pure default",
config: `{}`,
expected: drivers.InternalCapabilities{},
name: "pure default",
config: `{}`,
expected: &drivers.Capabilities{
SendSignals: true,
Exec: true,
FSIsolation: "image",
NetIsolationModes: []drivers.NetIsolationMode{"host", "group", "task"},
MustInitiateNetwork: true,
MountConfigs: 0,
RemoteTasks: false,
DisableLogCollection: false,
},
},
{
name: "disabled",
config: `{ disable_log_collection = true }`,
expected: drivers.InternalCapabilities{DisableLogCollection: true},
name: "disabled",
config: `{ disable_log_collection = true }`,
expected: &drivers.Capabilities{
SendSignals: true,
Exec: true,
FSIsolation: "image",
NetIsolationModes: []drivers.NetIsolationMode{"host", "group", "task"},
MustInitiateNetwork: true,
MountConfigs: 0,
RemoteTasks: false,
DisableLogCollection: true,
},
},
{
name: "enabled explicitly",
config: `{ disable_log_collection = false }`,
expected: drivers.InternalCapabilities{},
name: "enabled explicitly",
config: `{ disable_log_collection = false }`,
expected: &drivers.Capabilities{
SendSignals: true,
Exec: true,
FSIsolation: "image",
NetIsolationModes: []drivers.NetIsolationMode{"host", "group", "task"},
MustInitiateNetwork: true,
MountConfigs: 0,
RemoteTasks: false,
DisableLogCollection: false,
},
},
}

Expand All @@ -652,7 +680,9 @@ func TestConfig_InternalCapabilities(t *testing.T) {
hclutils.NewConfigParser(configSpec).ParseHCL(t, "config "+c.config, &tc)

d := &Driver{config: &tc}
require.Equal(t, c.expected, d.InternalCapabilities())
caps, err := d.Capabilities()
must.NoError(t, err)
must.Eq(t, c.expected, caps)
})
}
}
Expand Down
1 change: 1 addition & 0 deletions plugins/drivers/client.go
Expand Up @@ -77,6 +77,7 @@ func (d *driverPluginClient) Capabilities() (*Capabilities, error) {

caps.MountConfigs = MountConfigSupport(resp.Capabilities.MountConfigs)
caps.RemoteTasks = resp.Capabilities.RemoteTasks
caps.DisableLogCollection = resp.Capabilities.DisableLogCollection
}

return caps, nil
Expand Down
4 changes: 4 additions & 0 deletions plugins/drivers/driver.go
Expand Up @@ -174,6 +174,10 @@ type Capabilities struct {
// adjust behavior such as propogating task handles between allocations
// to avoid downtime when a client is lost.
RemoteTasks bool

// DisableLogCollection indicates this driver has disabled log collection
// and the client should not start a logmon process.
DisableLogCollection bool
}

func (c *Capabilities) HasNetIsolationMode(m NetIsolationMode) bool {
Expand Down

0 comments on commit e0143a3

Please sign in to comment.