feat: model life worker #19434
Merged
feat: model life worker #19434
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
SimonRichardson
force-pushed
the
create-model-life-worker
branch
from
3bdb2b0 to
05111da
Compare
SimonRichardson
requested review from
nvinuesa,
jack-w-shaw,
manadart and
tlm
as code owners
Adds GetModelLife and WatchModel methods to the model domain. We want to know when a model is created, changed etc. This will be used by the new model life watcher. This will replace the lifeflag worker that depends on the api and in turn the apiserver.
The model life worker will replace the life flag on the controller. The controller doesn't want to create many connections to the api just to get the information that is readily available via the domain services.
Starts add worker tests to ensure we've got good coverage of behaviour.
This wires up modellife worker with the controller manifolds.
SimonRichardson
force-pushed
the
create-model-life-worker
branch
from
db4b120 to
08d4745
Compare
manadart
approved these changes
Apr 9, 2025
| @@ -744,8 +744,7 @@ func (s *Service) GetApplicationLife(ctx context.Context, appName string) (corel | |||
| if err != nil { | |||
| return "", errors.Errorf("getting life for %q: %w", appName, err) | |||
| } | |||
| result := appLife.Value() | |||
| return result, errors.Capture(err) | |||
| return appLife.Value() | |||
There was a problem hiding this comment.
Nice to keep running capture over these.
| @@ -3,7 +3,10 @@ | |||
|
|
|||
| package life | |||
|
|
|||
| import corelife "github.com/juju/juju/core/life" | |||
| import ( | |||
| corelife "github.com/juju/juju/core/life" | |||
There was a problem hiding this comment.
I'd like to converge these one day.
| stmt, err := st.Prepare(` | ||
| SELECT &dbModelLife.* | ||
| FROM model | ||
| WHERE uuid = $dbModelLife.uuid; |
There was a problem hiding this comment.
Little formatting inconsistency.
nvinuesa
approved these changes
Apr 9, 2025
|
/merge |
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.
This removes another slice of the pie in getting the model worker manager off the API server in terms of a dependency. By depending directly on the domain, we alleviate the burden of every request going through the API server, causing additional load on both the API and in turn the database for login requests. We're reducing the surface area of the API server creating a more focused curated API.
We're unable to remove the controller lifeflag controller facade, as the container agent requires access to it. I suspect though, that rather than having such a generic facade, we could make it more specific.
The
api-callerdependencies have not quite gone down yet, and that is because of two more workers:Once those have made the transition, we should be free of the
api-callerfor most of the workers. A bounce of the API server should not cause Juju to go into a death spiral.QA steps
$ juju bootstrap lxd test $ juju add-model default $ juju deploy ubuntu