-
Notifications
You must be signed in to change notification settings - Fork 501
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: object store services #18108
feat: object store services #18108
Conversation
"provider-tracker", | ||
"is-controller-flag", | ||
"lease-manager", | ||
"object-store-s3-caller", | ||
"provider-service-factory", | ||
"object-store-services", | ||
"query-logger", | ||
"s3-http-client", | ||
"service-factory", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We no longer require the full-blown service factory for the object store. This means we can drop the provider tracker as well!
dfc725d
to
b1316e7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code is fine, as is QA.
The "singular" naming though. Is it a model object store getter, a singleton store getter? I think we can land on a better name.
The objectstore can not be embedded into the service factory because the objectstore metadata services are inside the service factory. The DAG of the dependencies prevent this, so to move forward, we can copy what we've done with the provider tracker worker. This introduces the concept for the workers. Adding it to the agent manifolds and pushing it through the API is still required.
The objectstore dependencies have been changed to the objectstore services (old terminology: object store service factory). This will allow us to inject the objectstore into the service factory. There is still a lot to fix, but according to the manifold tests, we can clearly see that it's possible. So given that signal, I'm going to press on and complete this.
The controller also has a object store, that also needs to be migrated across to the new object store services. The service factory (soon to be renamed) is becoming lighter and is only offering the services that are serviable by the API server. The domain services are in fact real services, not infrastructure.
This is end game. If this works, it means that we can access the blob store as a dependency as a first class citizen. This has already shown real value when it was done for the provider tracker.
The following wires up all the service factory dependencies correctly. It's now possible to have the object store in the service factory. There is a question around if the service factory should have a deprendency on the upgrade database. Originally it didn't because it's lazy evaluated. Now it's been introduced by the objectstore. I wonder if that's going to cause us issues.
Now that objectstore has been dropped from the service factory, we need to regenerate the mocks.
24fe58d
to
aa90be7
Compare
Model is much more discriptive than singular and it gives context to the resulting expectation.
aa90be7
to
1c24175
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM and QA passes, thanks!
/merge |
This introduces 2 concepts:
Using these two concepts, we can start interning all the object store usages at the API server level into the domain services. The work to do this was pre-seeded by the provider tracker, so applying it to the object store wasn't a huge lift. The only surprising aspect to this, one that I had forgotten was that we required the controller config. This was easy to implement in the end.
We need to understand if the service factory depending on the upgrade database flag/gate is a wise idea. We can easily change it in the future, but originally was left off because the service factory is lazy evaluated.
Checklist
QA steps
Ensure that we can still deploy charms and agents. There shouldn't be any real concerns, as the services still exist and those haven't changed. Just that they come from a different worker.
$ juju bootstrap lxd test $ juju add-model default $ juju deploy ubuntu
Links
Jira card: JUJU-6549