Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(plan): add plan sub-directory support #509

Merged
merged 15 commits into from
Oct 23, 2024
15 changes: 15 additions & 0 deletions client/plan.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@ type AddLayerOptions struct {
// has the given label. False (the default) means append a new layer.
Combine bool

// Inner set to true means a new layer append may go into an existing
// subdirectory, even though it may not result in appending it
// to the end of the layers slice (it becomes an insert).
Inner bool

// Label is the label for the new layer if appending, and the label of the
// layer to combine with if Combine is true.
Label string
Expand All @@ -38,16 +43,26 @@ func (client *Client) AddLayer(opts *AddLayerOptions) error {
var payload = struct {
Action string `json:"action"`
Combine bool `json:"combine"`
Inner bool `json:"inner"`
Label string `json:"label"`
Format string `json:"format"`
Layer string `json:"layer"`
}{
Action: "add",
Combine: opts.Combine,
Inner: opts.Inner,
Label: opts.Label,
Format: "yaml",
Layer: string(opts.LayerData),
}

// Add label validation here once layer persistence is supported over
// the API. We cannot do this in the plan library because JUJU already
// has labels in production systems that violates the layers file
// naming convention (which includes the label). Since JUJU uses its
// own client, we can enforce the label naming convention on all other
// systems using the Pebble supplied client by validating it here.

var body bytes.Buffer
if err := json.NewEncoder(&body).Encode(&payload); err != nil {
return err
Expand Down
23 changes: 20 additions & 3 deletions client/plan_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,22 @@ import (
)

func (cs *clientSuite) TestAddLayer(c *check.C) {
for _, combine := range []bool{false, true} {
for _, option := range []struct {
combine bool
inner bool
}{{
combine: false,
inner: false,
}, {
combine: true,
inner: false,
}, {
combine: false,
inner: true,
}, {
combine: true,
inner: true,
}} {
cs.rsp = `{
"type": "sync",
"status-code": 200,
Expand All @@ -37,7 +52,8 @@ services:
command: cmd
`[1:]
err := cs.cli.AddLayer(&client.AddLayerOptions{
Combine: combine,
Combine: option.combine,
Inner: option.inner,
Label: "foo",
LayerData: []byte(layerYAML),
})
Expand All @@ -49,10 +65,11 @@ services:
c.Assert(json.NewDecoder(cs.req.Body).Decode(&body), check.IsNil)
c.Assert(body, check.DeepEquals, map[string]interface{}{
"action": "add",
"combine": combine,
"combine": option.combine,
"label": "foo",
"format": "yaml",
"layer": layerYAML,
"inner": option.inner,
})
}
}
Expand Down
2 changes: 2 additions & 0 deletions docs/reference/cli-commands/add.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,7 @@ label (or append if the label is not found).
[add command options]
--combine Combine the new layer with an existing layer that has
the given label (default is to append)
--inner Allow appending a new layer inside an existing
subdirectory
```
<!-- END AUTOMATED OUTPUT -->
3 changes: 3 additions & 0 deletions internals/cli/cmd_add.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ type cmdAdd struct {
client *client.Client

Combine bool `long:"combine"`
Inner bool `long:"inner"`
Positional struct {
Label string `positional-arg-name:"<label>" required:"1"`
LayerPath string `positional-arg-name:"<layer-path>" required:"1"`
Expand All @@ -48,6 +49,7 @@ func init() {
Description: cmdAddDescription,
ArgsHelp: map[string]string{
"--combine": "Combine the new layer with an existing layer that has the given label (default is to append)",
"--inner": "Allow appending a new layer inside an existing subdirectory",
},
New: func(opts *CmdOptions) flags.Commander {
return &cmdAdd{client: opts.Client}
Expand All @@ -65,6 +67,7 @@ func (cmd *cmdAdd) Execute(args []string) error {
}
opts := client.AddLayerOptions{
Combine: cmd.Combine,
Inner: cmd.Inner,
Label: cmd.Positional.Label,
LayerData: data,
}
Expand Down
1 change: 1 addition & 0 deletions internals/cli/cmd_add_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ services:
"label": "foo",
"format": "yaml",
"layer": layerYAML,
"inner": false,
})
fmt.Fprint(w, `{
"type": "sync",
Expand Down
4 changes: 2 additions & 2 deletions internals/daemon/api_exec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ func (s *execSuite) TestContextNoOverrides(c *C) {
Environment: map[string]string{"FOO": "foo", "BAR": "bar"},
WorkingDir: dir,
}},
})
}, false)
c.Assert(err, IsNil)

stdout, stderr, err := s.exec(c, "", &client.ExecOptions{
Expand All @@ -228,7 +228,7 @@ func (s *execSuite) TestContextOverrides(c *C) {
Environment: map[string]string{"FOO": "foo", "BAR": "bar"},
WorkingDir: c.MkDir(),
}},
})
}, false)
c.Assert(err, IsNil)

overrideDir := c.MkDir()
Expand Down
5 changes: 3 additions & 2 deletions internals/daemon/api_plan.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ func v1PostLayers(c *Command, r *http.Request, _ *UserState) Response {
var payload struct {
Action string `json:"action"`
Combine bool `json:"combine"`
Inner bool `json:"inner"`
flotter marked this conversation as resolved.
Show resolved Hide resolved
Label string `json:"label"`
Format string `json:"format"`
Layer string `json:"layer"`
Expand All @@ -68,9 +69,9 @@ func v1PostLayers(c *Command, r *http.Request, _ *UserState) Response {

planMgr := overlordPlanManager(c.d.overlord)
if payload.Combine {
err = planMgr.CombineLayer(layer)
err = planMgr.CombineLayer(layer, payload.Inner)
} else {
err = planMgr.AppendLayer(layer)
err = planMgr.AppendLayer(layer, payload.Inner)
}
if err != nil {
if _, ok := err.(*planstate.LabelExists); ok {
Expand Down
94 changes: 80 additions & 14 deletions internals/overlord/planstate/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ package planstate

import (
"fmt"
"slices"
"strings"
"sync"

"github.com/canonical/pebble/internals/plan"
Expand Down Expand Up @@ -100,8 +102,10 @@ func (m *PlanManager) Plan() *plan.Plan {

// AppendLayer takes a Layer, appends it to the plan's layers and updates the
// layer.Order field to the new order. If a layer with layer.Label already
// exists, return an error of type *LabelExists.
func (m *PlanManager) AppendLayer(layer *plan.Layer) error {
// exists, return an error of type *LabelExists. Inner must be set to true
// if the append operation may be demoted to an insert due to the layer
// configuration being located in a sub-directory.
func (m *PlanManager) AppendLayer(layer *plan.Layer, inner bool) error {
var newPlan *plan.Plan
defer func() { m.callChangeListeners(newPlan) }()

Expand All @@ -113,14 +117,17 @@ func (m *PlanManager) AppendLayer(layer *plan.Layer) error {
return &LabelExists{Label: layer.Label}
}

newPlan, err := m.appendLayer(layer)
newPlan, err := m.appendLayer(layer, inner)
return err
}

// CombineLayer takes a Layer, combines it to an existing layer that has the
// same label. If no existing layer has the label, append a new one. In either
// case, update the layer.Order field to the new order.
func (m *PlanManager) CombineLayer(layer *plan.Layer) error {
// case, update the layer.Order field to the new order. Inner must be set to
// true if a combine operation gets demoted to an append operation (due to the
// layer not yet existing), and if the configuration layer is located in a
// sub-directory (see AppendLayer).
func (m *PlanManager) CombineLayer(layer *plan.Layer, inner bool) error {
var newPlan *plan.Plan
defer func() { m.callChangeListeners(newPlan) }()

Expand All @@ -131,7 +138,7 @@ func (m *PlanManager) CombineLayer(layer *plan.Layer) error {
if index < 0 {
// No layer found with this label, append new one.
var err error
newPlan, err = m.appendLayer(layer)
newPlan, err = m.appendLayer(layer, inner)
return err
}

Expand All @@ -155,19 +162,78 @@ func (m *PlanManager) CombineLayer(layer *plan.Layer) error {
return nil
}

func (m *PlanManager) appendLayer(layer *plan.Layer) (*plan.Plan, error) {
newOrder := 1
if len(m.plan.Layers) > 0 {
last := m.plan.Layers[len(m.plan.Layers)-1]
newOrder = last.Order + 1
// appendLayer appends (or inserts) a new layer configuration
// into the layers slice of the current plan. One important
// task of this method is to determine the new order of the layer.
//
// | File (inside layersDir) | Order | Label |
// | -------------------------- | --------------- | ------- |
// | 001-foo.yaml | 001-000 => 1000 | foo |
// | 002-bar.d/001-aaa.yaml | 002-001 => 2001 | bar/aaa |
// | 002-bar.d/002-bbb.yaml | 002-002 => 2002 | bar/bbb |
// | 003-baz.yaml | 003-000 => 3000 | baz |
//
// The new incoming layer only supplies the label, which may include
// a sub-directory prefix. Normally without a sub-directory prefix,
// the new layer will always be appended, which means incrementing
// the root level order (+ 1000). If a sub-directory already exists
// in the layers slice, its order was already allocated, which means
// we can at most insert the layer as the last entry in the
// sub-directory. However, this insert is only allowed if explicitly
// requested by the user (inner=true).
func (m *PlanManager) appendLayer(newLayer *plan.Layer, inner bool) (*plan.Plan, error) {
// The starting index and order assumes no existing layers.
newIndex := 0
newOrder := 1000

// If we have existing layers, things get a little bit more complex.
layersCount := len(m.plan.Layers)
if layersCount > 0 {
// We know at this point the complete label does not yet exist.
// However, let's see if the first part of the label is a
// sub-directory that exists and for which we already allocated
// an order?
newSubLabel, _, hasSub := strings.Cut(newLayer.Label, "/")
for i := layersCount - 1; i >= 0; i-- {
layer := m.plan.Layers[i]
layerSubLabel, _, _ := strings.Cut(layer.Label, "/")
// If we have a sub-directory match we know it already exists.
// Since we searched backwards, we know the order should be the
// next integer value.
if layerSubLabel == newSubLabel {
newOrder = layer.Order + 1
newIndex = i + 1
break
}
}

// If we did not match a sub-directory, this is simply an append.
// However, we need to know if this a inside a sub-directory or not
// as it has an impact on how we allocate the order.
if newIndex == 0 {
newIndex = layersCount
newOrder = ((m.plan.Layers[layersCount-1].Order / 1000) + 1) * 1000
if hasSub {
// The first file in the sub-directory gets an order of "001".
newOrder += 1
}
}
}

// If the append operation requires an insert because the layer is added
// inside an already existing sub-directory, with higher order items already
// allocated beyond it, we allow it only if the request specifically
// authorised it (inner=true).
if newIndex != layersCount && !inner {
return nil, fmt.Errorf("cannot insert sub-directory layer without 'inner' attribute set")
}

newLayers := append(m.plan.Layers, layer)
newLayers := slices.Insert(m.plan.Layers, newIndex, newLayer)
newPlan, err := m.updatePlanLayers(newLayers)
if err != nil {
return nil, err
}
layer.Order = newOrder
newLayer.Order = newOrder
return newPlan, nil
}

Expand Down Expand Up @@ -243,6 +309,6 @@ func (m *PlanManager) SetServiceArgs(serviceArgs map[string][]string) error {
}
}

newPlan, err := m.appendLayer(newLayer)
newPlan, err := m.appendLayer(newLayer, false)
return err
}
Loading
Loading