refactor(caasapplicationprovisioner): cut over to DQLite #18571
Conversation
The caasapplicationprovisioner facade relied on legacy Mongo-backed state for it's charm calls. However, we no longer store charms in Mongo, only DQLite. This was causing failures. Particularly, the state application methods .Charm & .CharmPendingToBeDownloaded were affected. Use domain methods for these calls instead. Drop these methods from the state interface. --- In this facade, we retrieved charms from state for the purpose of retrieving parts of their metadata. Specifically, their resources and storage specifications. This means it would be wasteful to retrieve the entire metadata. To remedy this, I have written some new domain service methods to retrieve only the data required. --- Remove unused state methods
jack-w-shaw
force-pushed
the
JUJU-7155_cutover_caasapplicationprovisioner
branch
from
5ea42bf to
d2a4e0e
Compare
| @@ -200,9 +200,9 @@ func (s *State) IsCharmAvailable(ctx context.Context, id corecharm.ID) (bool, er | |||
| ident := charmID{UUID: id.String()} | |||
|
|
|||
| query := ` | |||
| SELECT charm.available AS &charmAvailable.available | |||
| SELECT &charmAvailable.* | |||
There was a problem hiding this comment.
Not that important, but for these queries I prefer SELECT &charmAvailale.available because it makes it clear that thats all you're selecting there.
There was a problem hiding this comment.
Simon and I prefer this
| return expected | ||
| }) | ||
|
|
||
| storage, err := st.GetCharmMetadataStorage(context.Background(), id) |
There was a problem hiding this comment.
Can these be put into seperate functions for testing? The preamble for this could just be put into a function that all the individual tests call. That'd give a clearer signal if any test starts failing.
There was a problem hiding this comment.
Could do, but that would lead to a lot of code duplication in the tests. I'd rather keep them like this tbh
| @@ -3490,24 +3490,6 @@ func (a *Application) UnitNames() ([]string, error) { | |||
| return u, errors.Trace(err) | |||
| } | |||
|
|
|||
| // CharmPendingToBeDownloaded returns true if the charm referenced by this | |||
| // application is pending to be downloaded. | |||
| func (a *Application) CharmPendingToBeDownloaded() bool { | |||
There was a problem hiding this comment.
Funny that these just didn't have any tests.
There was a problem hiding this comment.
This is because we recently removed all tests for state
|
/merge |
The caasapplicationprovisioner facade relied on legacy Mongo-backed state for it's charm calls.
However, we no longer store charms in Mongo, only DQLite. This was causing failures.
Particularly, the state application methods .Charm & .CharmPendingToBeDownloaded were affected.
Use domain methods for these calls instead. Drop these methods from the state interface.
In this facade, we retrieved charms from state for the purpose of retrieving parts of their metadata. Specifically, their resources and storage specifications.
This means it would be wasteful to retrieve the entire metadata. To remedy this, I have written some new domain service methods to retrieve only the data required.
Remove unused state methods
QA steps
Unfortunately, deploy on k8s is broken for reasons independent of this PR. So we can only rely on unit tests