diff --git a/README.md b/README.md index f0ab8e42..315e9d08 100644 --- a/README.md +++ b/README.md @@ -403,6 +403,60 @@ $ pebble run --verbose ... ``` + + ## Container usage Pebble works well as a local service manager, but if running Pebble in a separate container, you can use the exec and file management APIs to coordinate with the remote system over the shared unix socket. diff --git a/internals/plan/plan.go b/internals/plan/plan.go index 76fc1feb..22d403c9 100644 --- a/internals/plan/plan.go +++ b/internals/plan/plan.go @@ -29,6 +29,7 @@ import ( "github.com/canonical/x-go/strutil/shlex" "gopkg.in/yaml.v3" + "github.com/canonical/pebble/internals/logger" "github.com/canonical/pebble/internals/osutil" ) @@ -89,9 +90,6 @@ type Service struct { BackoffFactor OptionalFloat `yaml:"backoff-factor,omitempty"` BackoffLimit OptionalDuration `yaml:"backoff-limit,omitempty"` KillDelay OptionalDuration `yaml:"kill-delay,omitempty"` - - // Log forwarding - LogTargets []string `yaml:"log-targets,omitempty"` } // Copy returns a deep copy of the service. @@ -120,7 +118,6 @@ func (s *Service) Copy() *Service { copied.OnCheckFailure[k] = v } } - copied.LogTargets = append([]string(nil), s.LogTargets...) return &copied } @@ -188,23 +185,6 @@ func (s *Service) Merge(other *Service) { if other.BackoffLimit.IsSet { s.BackoffLimit = other.BackoffLimit } - s.LogTargets = appendUnique(s.LogTargets, other.LogTargets...) -} - -// appendUnique appends into a the elements from b which are not yet present -// and returns the modified slice. -// TODO: move this function into canonical/x-go/strutil -func appendUnique(a []string, b ...string) []string { -Outer: - for _, bn := range b { - for _, an := range a { - if an == bn { - continue Outer - } - } - a = append(a, bn) - } - return a } // Equal returns true when the two services are equal in value. @@ -275,23 +255,22 @@ func CommandString(base, extra []string) string { } // LogsTo returns true if the logs from s should be forwarded to target t. -// This happens if: -// - t.Selection is "opt-out" or empty, and s.LogTargets is empty; or -// - t.Selection is not "disabled", and s.LogTargets contains t. func (s *Service) LogsTo(t *LogTarget) bool { - if t.Selection == DisabledSelection { - return false - } - if len(s.LogTargets) == 0 { - if t.Selection == UnsetSelection || t.Selection == OptOutSelection { + // Iterate backwards through t.Services until we find something matching + // s.Name. + for i := len(t.Services) - 1; i >= 0; i-- { + switch t.Services[i] { + case s.Name: return true - } - } - for _, targetName := range s.LogTargets { - if targetName == t.Name { + case ("-" + s.Name): + return false + case "all": return true + case "-all": + return false } } + // Nothing matching the service name, so it was not specified. return false } @@ -513,11 +492,11 @@ func (c *ExecCheck) Merge(other *ExecCheck) { // LogTarget specifies a remote server to forward logs to. type LogTarget struct { - Name string `yaml:"-"` - Type LogTargetType `yaml:"type"` - Location string `yaml:"location"` - Selection Selection `yaml:"selection,omitempty"` - Override Override `yaml:"override,omitempty"` + Name string `yaml:"-"` + Type LogTargetType `yaml:"type"` + Location string `yaml:"location"` + Services []string `yaml:"services"` + Override Override `yaml:"override,omitempty"` } // LogTargetType defines the protocol to use to forward logs. @@ -529,19 +508,10 @@ const ( UnsetLogTarget LogTargetType = "" ) -// Selection describes which services' logs will be forwarded to this target. -type Selection string - -const ( - OptOutSelection Selection = "opt-out" - OptInSelection Selection = "opt-in" - DisabledSelection Selection = "disabled" - UnsetSelection Selection = "" -) - // Copy returns a deep copy of the log target configuration. func (t *LogTarget) Copy() *LogTarget { copied := *t + copied.Services = append([]string(nil), t.Services...) return &copied } @@ -553,9 +523,7 @@ func (t *LogTarget) Merge(other *LogTarget) { if other.Location != "" { t.Location = other.Location } - if other.Selection != "" { - t.Selection = other.Selection - } + t.Services = append(t.Services, other.Services...) } // FormatError is the error returned when a layer has a format error, such as @@ -796,35 +764,28 @@ func CombineLayers(layers ...*Layer) (*Layer, error) { } } - switch target.Selection { - case OptOutSelection, OptInSelection, DisabledSelection, UnsetSelection: - // valid, continue - default: + // Validate service names specified in log target + for _, serviceName := range target.Services { + serviceName = strings.TrimPrefix(serviceName, "-") + if serviceName == "all" { + continue + } + if _, ok := combined.Services[serviceName]; ok { + continue + } return nil, &FormatError{ - Message: fmt.Sprintf(`log target %q has invalid selection %q, must be %q, %q or %q`, - name, target.Selection, OptOutSelection, OptInSelection, DisabledSelection), + Message: fmt.Sprintf(`log target %q specifies unknown service %q`, + target.Name, serviceName), } } - if target.Location == "" && target.Selection != DisabledSelection { + if target.Location == "" { return nil, &FormatError{ Message: fmt.Sprintf(`plan must define "location" for log target %q`, name), } } } - // Validate service log targets - for serviceName, service := range combined.Services { - for _, targetName := range service.LogTargets { - _, ok := combined.LogTargets[targetName] - if !ok { - return nil, &FormatError{ - Message: fmt.Sprintf(`unknown log target %q for service %q`, targetName, serviceName), - } - } - } - } - // Ensure combined layers don't have cycles. err := combined.checkCycles() if err != nil { @@ -963,6 +924,15 @@ func ParseLayer(order int, label string, data []byte) (*Layer, error) { Message: fmt.Sprintf("cannot use reserved service name %q", name), } } + // Deprecated service names + if name == "all" || name == "default" || name == "none" { + logger.Noticef("Using keyword %q as a service name is deprecated", name) + } + if strings.HasPrefix(name, "-") { + return nil, &FormatError{ + Message: fmt.Sprintf(`cannot use service name %q: starting with "-" not allowed`, name), + } + } if service == nil { return nil, &FormatError{ Message: fmt.Sprintf("service object cannot be null for service %q", name), diff --git a/internals/plan/plan_test.go b/internals/plan/plan_test.go index 2fe9f6e7..1883ece4 100644 --- a/internals/plan/plan_test.go +++ b/internals/plan/plan_test.go @@ -831,24 +831,21 @@ var planTests = []planTest{{ command: foo override: merge startup: enabled - log-targets: - - tgt1 svc2: command: bar override: merge startup: enabled - log-targets: - - tgt1 - - tgt2 log-targets: tgt1: type: loki location: http://10.1.77.196:3100/loki/api/v1/push + services: [all] override: merge tgt2: type: syslog location: udp://0.0.0.0:514 + services: [svc2] override: merge `}, result: &plan.Layer{ @@ -861,7 +858,6 @@ var planTests = []planTest{{ BackoffDelay: plan.OptionalDuration{Value: defaultBackoffDelay}, BackoffFactor: plan.OptionalFloat{Value: defaultBackoffFactor}, BackoffLimit: plan.OptionalDuration{Value: defaultBackoffLimit}, - LogTargets: []string{"tgt1"}, }, "svc2": { Name: "svc2", @@ -871,7 +867,6 @@ var planTests = []planTest{{ BackoffDelay: plan.OptionalDuration{Value: defaultBackoffDelay}, BackoffFactor: plan.OptionalFloat{Value: defaultBackoffFactor}, BackoffLimit: plan.OptionalDuration{Value: defaultBackoffLimit}, - LogTargets: []string{"tgt1", "tgt2"}, }, }, Checks: map[string]*plan.Check{}, @@ -880,12 +875,14 @@ var planTests = []planTest{{ Name: "tgt1", Type: plan.LokiTarget, Location: "http://10.1.77.196:3100/loki/api/v1/push", + Services: []string{"all"}, Override: plan.MergeOverride, }, "tgt2": { Name: "tgt2", Type: plan.SyslogTarget, Location: "udp://0.0.0.0:514", + Services: []string{"svc2"}, Override: plan.MergeOverride, }, }, @@ -898,50 +895,50 @@ var planTests = []planTest{{ command: foo override: merge startup: enabled - log-targets: - - tgt1 svc2: command: bar override: merge startup: enabled - log-targets: - - tgt1 - - tgt2 log-targets: tgt1: type: loki location: http://10.1.77.196:3100/loki/api/v1/push + services: [all] override: merge tgt2: type: syslog location: udp://0.0.0.0:514 + services: [svc2] + override: merge + tgt3: + type: loki + location: http://10.1.77.206:3100/loki/api/v1/push + services: [all] override: merge `, ` services: svc1: command: foo override: merge - log-targets: - - tgt3 svc2: command: bar override: replace startup: enabled - log-targets: - - tgt3 log-targets: tgt1: + services: [-all, svc1] override: merge - selection: opt-in tgt2: type: syslog + location: udp://1.2.3.4:514 + services: [] override: replace - selection: disabled tgt3: - type: loki - location: http://10.1.77.206:3100/loki/api/v1/push + type: syslog + location: udp://0.0.0.0:514 + services: [-svc1] override: merge `}, layers: []*plan.Layer{{ @@ -949,18 +946,16 @@ var planTests = []planTest{{ Order: 0, Services: map[string]*plan.Service{ "svc1": { - Name: "svc1", - Command: "foo", - Override: plan.MergeOverride, - Startup: plan.StartupEnabled, - LogTargets: []string{"tgt1"}, + Name: "svc1", + Command: "foo", + Override: plan.MergeOverride, + Startup: plan.StartupEnabled, }, "svc2": { - Name: "svc2", - Command: "bar", - Override: plan.MergeOverride, - Startup: plan.StartupEnabled, - LogTargets: []string{"tgt1", "tgt2"}, + Name: "svc2", + Command: "bar", + Override: plan.MergeOverride, + Startup: plan.StartupEnabled, }, }, Checks: map[string]*plan.Check{}, @@ -969,12 +964,21 @@ var planTests = []planTest{{ Name: "tgt1", Type: plan.LokiTarget, Location: "http://10.1.77.196:3100/loki/api/v1/push", + Services: []string{"all"}, Override: plan.MergeOverride, }, "tgt2": { Name: "tgt2", Type: plan.SyslogTarget, Location: "udp://0.0.0.0:514", + Services: []string{"svc2"}, + Override: plan.MergeOverride, + }, + "tgt3": { + Name: "tgt3", + Type: plan.LokiTarget, + Location: "http://10.1.77.206:3100/loki/api/v1/push", + Services: []string{"all"}, Override: plan.MergeOverride, }, }, @@ -983,36 +987,36 @@ var planTests = []planTest{{ Order: 1, Services: map[string]*plan.Service{ "svc1": { - Name: "svc1", - Command: "foo", - Override: plan.MergeOverride, - LogTargets: []string{"tgt3"}, + Name: "svc1", + Command: "foo", + Override: plan.MergeOverride, }, "svc2": { - Name: "svc2", - Command: "bar", - Override: plan.ReplaceOverride, - Startup: plan.StartupEnabled, - LogTargets: []string{"tgt3"}, + Name: "svc2", + Command: "bar", + Override: plan.ReplaceOverride, + Startup: plan.StartupEnabled, }, }, Checks: map[string]*plan.Check{}, LogTargets: map[string]*plan.LogTarget{ "tgt1": { - Name: "tgt1", - Override: plan.MergeOverride, - Selection: plan.OptInSelection, + Name: "tgt1", + Services: []string{"-all", "svc1"}, + Override: plan.MergeOverride, }, "tgt2": { - Name: "tgt2", - Type: plan.SyslogTarget, - Override: plan.ReplaceOverride, - Selection: plan.DisabledSelection, + Name: "tgt2", + Type: plan.SyslogTarget, + Location: "udp://1.2.3.4:514", + Services: []string{}, + Override: plan.ReplaceOverride, }, "tgt3": { Name: "tgt3", - Type: plan.LokiTarget, - Location: "http://10.1.77.206:3100/loki/api/v1/push", + Type: plan.SyslogTarget, + Location: "udp://0.0.0.0:514", + Services: []string{"-svc1"}, Override: plan.MergeOverride, }, }, @@ -1027,7 +1031,6 @@ var planTests = []planTest{{ BackoffDelay: plan.OptionalDuration{Value: defaultBackoffDelay}, BackoffFactor: plan.OptionalFloat{Value: defaultBackoffFactor}, BackoffLimit: plan.OptionalDuration{Value: defaultBackoffLimit}, - LogTargets: []string{"tgt1", "tgt3"}, }, "svc2": { Name: "svc2", @@ -1037,28 +1040,28 @@ var planTests = []planTest{{ BackoffDelay: plan.OptionalDuration{Value: defaultBackoffDelay}, BackoffFactor: plan.OptionalFloat{Value: defaultBackoffFactor}, BackoffLimit: plan.OptionalDuration{Value: defaultBackoffLimit}, - LogTargets: []string{"tgt3"}, }, }, Checks: map[string]*plan.Check{}, LogTargets: map[string]*plan.LogTarget{ "tgt1": { - Name: "tgt1", - Type: plan.LokiTarget, - Location: "http://10.1.77.196:3100/loki/api/v1/push", - Override: plan.MergeOverride, - Selection: plan.OptInSelection, + Name: "tgt1", + Type: plan.LokiTarget, + Location: "http://10.1.77.196:3100/loki/api/v1/push", + Services: []string{"all", "-all", "svc1"}, + Override: plan.MergeOverride, }, "tgt2": { - Name: "tgt2", - Type: plan.SyslogTarget, - Override: plan.ReplaceOverride, - Selection: plan.DisabledSelection, + Name: "tgt2", + Type: plan.SyslogTarget, + Location: "udp://1.2.3.4:514", + Override: plan.ReplaceOverride, }, "tgt3": { Name: "tgt3", - Type: plan.LokiTarget, - Location: "http://10.1.77.206:3100/loki/api/v1/push", + Type: plan.SyslogTarget, + Location: "udp://0.0.0.0:514", + Services: []string{"all", "-svc1"}, Override: plan.MergeOverride, }, }, @@ -1079,6 +1082,7 @@ var planTests = []planTest{{ log-targets: tgt1: type: loki + services: [all] override: merge `}}, { summary: "Unsupported log target type", @@ -1091,31 +1095,24 @@ var planTests = []planTest{{ override: merge `}, }, { - summary: "Invalid selection for log target", - error: `log target "tgt1" has invalid selection "foobar", must be "opt-out", "opt-in" or "disabled"`, + summary: "Log target specifies invalid service", + error: `log target "tgt1" specifies unknown service "nonexistent"`, input: []string{` log-targets: tgt1: type: loki location: http://10.1.77.196:3100/loki/api/v1/push + services: [nonexistent] override: merge - selection: foobar `}, }, { - summary: "Service specifies unknown log target", - error: `unknown log target "tgt2" for service "svc1"`, + summary: `Service name can't start with "-"`, + error: `cannot use service name "-svc1": starting with "-" not allowed`, input: []string{` services: - svc1: + -svc1: command: foo override: merge - log-targets: - - tgt2 - log-targets: - tgt1: - type: loki - location: http://10.1.77.196:3100/loki/api/v1/push - override: merge `}, }} @@ -1462,41 +1459,89 @@ func (s *S) TestParseCommand(c *C) { } } -func (s *S) TestSelectTargets(c *C) { - logTargets := []*plan.LogTarget{ - {Name: "unset", Selection: plan.UnsetSelection}, - {Name: "optout", Selection: plan.OptOutSelection}, - {Name: "optin", Selection: plan.OptInSelection}, - {Name: "disabled", Selection: plan.DisabledSelection}, - } - services := []*plan.Service{ - {Name: "svc1", LogTargets: nil}, - {Name: "svc2", LogTargets: []string{}}, - {Name: "svc3", LogTargets: []string{"unset"}}, - {Name: "svc4", LogTargets: []string{"optout"}}, - {Name: "svc5", LogTargets: []string{"optin"}}, - {Name: "svc6", LogTargets: []string{"disabled"}}, - {Name: "svc7", LogTargets: []string{"unset", "optin", "disabled"}}, - } +func (s *S) TestLogsTo(c *C) { + tests := []struct { + services []string + logsTo map[string]bool + }{{ + services: nil, + logsTo: map[string]bool{ + "svc1": false, + "svc2": false, + }, + }, { + services: []string{}, + logsTo: map[string]bool{ + "svc1": false, + "svc2": false, + }, + }, { + services: []string{"all"}, + logsTo: map[string]bool{ + "svc1": true, + "svc2": true, + }, + }, { + services: []string{"svc1"}, + logsTo: map[string]bool{ + "svc1": true, + "svc2": false, + }, + }, { + services: []string{"svc1", "svc2"}, + logsTo: map[string]bool{ + "svc1": true, + "svc2": true, + "svc3": false, + }, + }, { + services: []string{"all", "-svc2"}, + logsTo: map[string]bool{ + "svc1": true, + "svc2": false, + "svc3": true, + }, + }, { + services: []string{"svc1", "svc2", "-svc1", "all"}, + logsTo: map[string]bool{ + "svc1": true, + "svc2": true, + "svc3": true, + }, + }, { + services: []string{"svc1", "svc2", "-all"}, + logsTo: map[string]bool{ + "svc1": false, + "svc2": false, + "svc3": false, + }, + }, { + services: []string{"all", "-all"}, + logsTo: map[string]bool{ + "svc1": false, + "svc2": false, + "svc3": false, + }, + }, { + services: []string{"svc1", "svc2", "-all", "svc3", "svc1", "-svc3"}, + logsTo: map[string]bool{ + "svc1": true, + "svc2": false, + "svc3": false, + }, + }} - // Use pointers to bools so the test will fail if we forget to set a value - t, f := true, false - expected := map[string]map[string]*bool{ - "svc1": {"unset": &t, "optout": &t, "optin": &f, "disabled": &f}, - "svc2": {"unset": &t, "optout": &t, "optin": &f, "disabled": &f}, - "svc3": {"unset": &t, "optout": &f, "optin": &f, "disabled": &f}, - "svc4": {"unset": &f, "optout": &t, "optin": &f, "disabled": &f}, - "svc5": {"unset": &f, "optout": &f, "optin": &t, "disabled": &f}, - "svc6": {"unset": &f, "optout": &f, "optin": &f, "disabled": &f}, - "svc7": {"unset": &t, "optout": &f, "optin": &t, "disabled": &f}, - } + for _, test := range tests { + target := &plan.LogTarget{ + Services: test.services, + } - for _, service := range services { - for _, target := range logTargets { - exp := expected[service.Name][target.Name] - c.Assert(exp, NotNil, Commentf("no expected value defined for %s.LogsTo(%s)", service.Name, target.Name)) - c.Check(service.LogsTo(target), Equals, *exp, - Commentf("unexpected value for %s.LogsTo(%s)", service.Name, target.Name)) + for serviceName, shouldLogTo := range test.logsTo { + service := &plan.Service{ + Name: serviceName, + } + c.Check(service.LogsTo(target), Equals, shouldLogTo, + Commentf("matching service %q against 'services: %v'", serviceName, test.services)) } } }