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 section support #16

Draft
wants to merge 42 commits into
base: master
Choose a base branch
from
Draft

Conversation

flotter
Copy link
Owner

@flotter flotter commented Jul 25, 2024

Add support for Pebble Plan extensions to allow managers to extend the existing Pebble Plan schema.

// LayerSectionExtension allows the plan layer schema to be extended without
// adding centralised schema knowledge to the plan library.
type LayerSectionExtension interface {
	// ParseSection returns a newly allocated concrete type containing the
	// unmarshalled section content.
	ParseSection(data yaml.Node) (LayerSection, error)

	// CombineSections returns a newly allocated concrete type containing the
	// result of combining the supplied sections in order.
	CombineSections(sections ...LayerSection) (LayerSection, error)

	// ValidatePlan takes the complete plan as input, and allows the
	// extension to validate the plan. This can be used for cross section
	// dependency validation.
	ValidatePlan(plan *Plan) error
}

type LayerSection interface {
	// Ask the section to validate itself.
	Validate() error

	// Returns true if the section is empty.
	IsZero() bool
}

AnneCYH and others added 2 commits July 26, 2024 14:08
)

Put services that depend on each other (requires) in the same lane to start/stop in parallel.
@flotter flotter force-pushed the plan-section-support branch 12 times, most recently from 39bb63f to 43fe543 Compare July 29, 2024 12:02
internals/plan/suite_test.go Outdated Show resolved Hide resolved
@flotter flotter force-pushed the plan-section-support branch 2 times, most recently from d6c0fd2 to 7ec6f4d Compare July 29, 2024 13:25
@flotter flotter requested review from paul-rodriguez and anpep July 29, 2024 13:32
@flotter flotter force-pushed the plan-section-support branch from 7ec6f4d to 3438327 Compare July 29, 2024 15:59
…nonical#456)

This allows having the layers directory in a different directory than
the "layers" sub-directory inside the pebble directory.

The defaults are unchanged, but one can pass an optional non-empty
string in `daemon.Options{LayersDir: ...}` when calling `daemon.New()`
to use a custom location for the layers.

This also removes the need to pass `pebbleDir` into `plan` and
`planstate`, as it really only needs the `layersDir`.
internals/plan/plan.go Outdated Show resolved Hide resolved
internals/plan/plan.go Outdated Show resolved Hide resolved
internals/plan/plan.go Outdated Show resolved Hide resolved
internals/plan/plan.go Outdated Show resolved Hide resolved
internals/plan/plan.go Show resolved Hide resolved
@flotter flotter force-pushed the plan-section-support branch from 875a95e to 6b8a0c2 Compare July 30, 2024 12:16
Currently managers are started and stopped in a FIFO (first in, first
out) order, but I would expect managers to behave more like a stack, in
that they are started and stopped in a LIFO (last in first out) order.
So the stopping of the managers should be done in reverse.

Co-authored-by: Ben Hoyt <[email protected]>
thp-canonical and others added 2 commits August 16, 2024 16:15
When using a `logger.MockLogger()`, a background goroutine calling
`logger.Noticef()` might race the underlying `bytes.Buffer` for
read/write access. To avoid that, limit the returned buffer type for
`MockLogger()` to just `fmt.Stringer` (and mutex-protect its reading
from the buffer) as well as mutex-protect `Write()` calls to the
underlying buffer.

To reproduce the race condition, run `go test -race ./internals/logger`
with `TestMockLoggerReadWriteThreadsafe` added, but with the unmodified
`MockLogger` implementation.

If more functions from `bytes.Buffer` are needed from the `MockLogger`
return value, the interface can be expanded with additional pass-through
lock-protected calls, but in all of Pebble, it seems only the
`.String()` function is used, so only that is exposed for now.

While `logger.Noticef()` calls are serialized by `logger.loggerLock`,
the same cannot be said for parallel access to the underlying
`bytes.Buffer` that is returned from `MockLogger` when calling
`logger.Noticef()` in one goroutine, and accessing the `bytes.Buffer`
from another goroutine.
Update the "In this documentation" section on the main page.
@flotter flotter force-pushed the plan-section-support branch 3 times, most recently from aa05ab3 to 58f54e6 Compare August 19, 2024 21:50
flotter and others added 2 commits August 19, 2024 23:56
Managers can explicitly call ```state.EnsureBefore()``` if they wish to
apply changes sooner than the scheduled 5min ensure overlord timer.

Although managers get a state instance at creation time, during
```overlord.New()```, it is currently forbidden to call ensure early on
before the ```Daemon``` started the ```overlord.Loop()```. The current
restriction stems from the fact that ```state.EnsureBefore()``` adjusts
the ensure timer expiry time, and the timer is only created when
```overlord.Loop()``` is called.

**The problem:**

Since the overlord ensure loop is only started later on during
```daemon.Start()```, a manager has no direct way to know when it's safe
to call Ensure. Managers currently have to use the first call to their
```Ensure()``` method as an indication the overlord loop was entered,
which is unnecessary manager boilerplate code. See
```overlord/checkstate/manager.go```.

Managers (and managers with dependencies on other managers), may want to
implement state changes during callbacks. These events can come from
outside the overlord framework (e.g. from the Linux kernel or early on
during manager ```StartUp```), meaning that handlers using state may be
triggered asynchronously from the Daemon startup sequence, and arrive
before the overlord Loop is started.

**Solution:**

Allow ```state.EnsureBefore()``` to be called before the timer exists.
Since an implicit ensure is performed on overlord loop entry, simply
returning while the timer does not exist is the same as saying an ensure
is already scheduled.
This deletes the legacy warnings code and re-implements `pebble
warnings` and `pebble okay` using notices as warnings (as we originally
intended). The `/v1/warnings` endpoints have been removed, and `pebble
warnings` now hits `/v1/notices?types=warning`.

On every request, instead of returning the count of warnings and the
latest timestamp, we only return the timestamp of the latest (most
recent last-repeated time). This way the server doesn't have to track
which notices have been okayed -- the client can compare against its
stored timestamp and see if the server-reported timestamp is later.

So there are a few breaking changes in this PR:

* `Client.WarningsSummary() (count int, timestamp time.Time)` is now
`Client.LatestWarningTime() time.Time`, as there's no count anymore
* `GET /v1/warnings` (list warnings) is gone: `pebble warnings` now uses
`GET /v1/notices?types=warning`
* `POST /v1/warnings` (okay warnings) are gone: `pebble okay` now just
updates the local state file, and doesn't use the API/client at all
* Similarly, `Client.Warnings()` and `Client.Okay()` are gone; use
`Client.Notices()` and maintain local client-side state, respectively
* `State.Warnf` is still around, but it's now just a helper that calls
`State.AddNotice` with the right type (warning) and key (message)

Fixes canonical#475.
@flotter flotter force-pushed the plan-section-support branch from 58f54e6 to 662fd6a Compare August 20, 2024 06:54
`daemonSuite.socketPath` appears to be assigned outside of the test, but
only when it has a default value, which results in a stale `socketPath`
value in this test.

Running `go test -count=2 -v ./internals/daemon -check.v` with this fix
now passes.

Fixes canonical#282.
@flotter flotter force-pushed the plan-section-support branch from d3ae02c to 8de6590 Compare August 26, 2024 14:35
@flotter flotter force-pushed the plan-section-support branch from 743147c to 9a51cfe Compare August 28, 2024 14:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.