forked from canonical/pebble
-
Notifications
You must be signed in to change notification settings - Fork 0
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
flotter
wants to merge
42
commits into
master
Choose a base branch
from
plan-section-support
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
39bb63f
to
43fe543
Compare
flotter
commented
Jul 29, 2024
d6c0fd2
to
7ec6f4d
Compare
7ec6f4d
to
3438327
Compare
…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`.
flotter
commented
Jul 30, 2024
flotter
commented
Jul 30, 2024
anpep
reviewed
Jul 30, 2024
875a95e
to
6b8a0c2
Compare
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]>
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.
aa05ab3
to
58f54e6
Compare
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.
58f54e6
to
662fd6a
Compare
`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.
d3ae02c
to
8de6590
Compare
Added two more test cases to improve test coverage. If the path already exists but it's not a directory, a `syscall.ENOTDIR` error is expected.
) Once a month, send the latest code quality (including coverage) data to TIOBE for central storage. --------- Co-authored-by: Ben Hoyt <[email protected]>
743147c
to
9a51cfe
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add support for Pebble Plan extensions to allow managers to extend the existing Pebble Plan schema.