Refactor state resources persistence code to remove abstractions

addyess · May 26, 2022 · b72909e · b72909e
1 parent a623f79
commit b72909e
Show file tree

Hide file tree

Showing 54 changed files with 1,891 additions and 3,641 deletions.
diff --git a/api/client/resources/client_listresources_test.go b/api/client/resources/client_listresources_test.go
@@ -4,8 +4,6 @@
 package resources_test
 
 import (
-	"fmt"
-
 	"github.com/golang/mock/gomock"
 	"github.com/juju/errors"
 	jc "github.com/juju/testing/checkers"
@@ -115,6 +113,5 @@ func (s *ListResourcesSuite) TestConversionFailed(c *gc.C) {
 	s.facade.EXPECT().FacadeCall("ListResources", args, gomock.Any()).SetArg(2, resultParams).Return(nil)
 
 	_, err := s.client.ListResources([]string{"a-application"})
-	fmt.Println(err.Error())
 	c.Assert(err, gc.ErrorMatches, "boom")
 }
diff --git a/apiserver/apiserver.go b/apiserver/apiserver.go
@@ -724,10 +724,7 @@ func (srv *Server) endpoints() []apihttp.Endpoint {
 			if err != nil {
 				return nil, nil, nil, errors.Trace(err)
 			}
-			rst, err := st.Resources()
-			if err != nil {
-				return nil, nil, nil, errors.Trace(err)
-			}
+			rst := st.Resources()
 			return rst, st, entity.Tag(), nil
 		},
 		ChangeAllowedFunc: func(req *http.Request) error {

diff --git a/apiserver/facades/agent/resourceshookcontext/register.go b/apiserver/facades/agent/resourceshookcontext/register.go
@@ -58,9 +58,6 @@ func newStateFacade(ctx facade.Context) (*UnitFacade, error) {
 		return nil, errors.Errorf("expected names.UnitTag or names.ApplicationTag, got %T", tag)
 	}
 
-	res, err := st.Resources()
-	if err != nil {
-		return nil, errors.Trace(err)
-	}
+	res := st.Resources()
 	return NewUnitFacade(&resourcesUnitDataStore{res, unit}), nil
 }
diff --git a/apiserver/facades/agent/resourceshookcontext/unitfacade.go b/apiserver/facades/agent/resourceshookcontext/unitfacade.go
@@ -13,15 +13,6 @@ import (
 	"github.com/juju/juju/state"
 )
 
-// NewHookContextFacade adapts NewUnitFacade for facade registration.
-func NewHookContextFacade(st *state.State, unit *state.Unit) (interface{}, error) {
-	res, err := st.Resources()
-	if err != nil {
-		return nil, errors.Trace(err)
-	}
-	return NewUnitFacade(&resourcesUnitDataStore{res, unit}), nil
-}
-
 // resourcesUnitDatastore is a shim to elide serviceName from
 // ListResources.
 type resourcesUnitDataStore struct {

diff --git a/apiserver/facades/client/application/application.go b/apiserver/facades/client/application/application.go
@@ -426,11 +426,7 @@ func (api *APIBase) Deploy(args params.ApplicationsDeploy) (params.ErrorResults,
 			// TODO(babbageclunk): rework the deploy API so the
 			// resources are created transactionally to avoid needing
 			// to do this.
-			resources, err := api.backend.Resources()
-			if err != nil {
-				logger.Errorf("couldn't get backend.Resources")
-				continue
-			}
+			resources := api.backend.Resources()
 			err = resources.RemovePendingAppResources(arg.ApplicationName, arg.Resources)
 			if err != nil {
 				logger.Errorf("couldn't remove pending resources for %q", arg.ApplicationName)

diff --git a/apiserver/facades/client/application/application_test.go b/apiserver/facades/client/application/application_test.go
@@ -632,8 +632,7 @@ func (s *applicationSuite) testApplicationDeployWithPlacementLockedError(
 
 func (s *applicationSuite) TestApplicationDeploymentRemovesPendingResourcesOnFailure(c *gc.C) {
 	charm := s.AddTestingCharm(c, "dummy-resource")
-	resources, err := s.State.Resources()
-	c.Assert(err, jc.ErrorIsNil)
+	resources := s.State.Resources()
 	pendingID, err := resources.AddPendingResource("haha/borken", "user", charmresource.Resource{
 		Meta:   charm.Meta().Resources["dummy"],
 		Origin: charmresource.OriginUpload,
@@ -655,13 +654,13 @@ func (s *applicationSuite) TestApplicationDeploymentRemovesPendingResourcesOnFai
 
 	res, err := resources.ListPendingResources("haha/borken")
 	c.Assert(err, jc.ErrorIsNil)
-	c.Assert(res, gc.HasLen, 0)
+	c.Assert(res.Resources, gc.HasLen, 0)
+	c.Assert(res.UnitResources, gc.HasLen, 0)
 }
 
 func (s *applicationSuite) TestApplicationDeploymentLeavesResourcesOnSuccess(c *gc.C) {
 	charm := s.AddTestingCharm(c, "dummy-resource")
-	resources, err := s.State.Resources()
-	c.Assert(err, jc.ErrorIsNil)
+	resources := s.State.Resources()
 	pendingID, err := resources.AddPendingResource("unborken", "user", charmresource.Resource{
 		Meta:   charm.Meta().Resources["dummy"],
 		Origin: charmresource.OriginUpload,

diff --git a/apiserver/facades/client/application/backend.go b/apiserver/facades/client/application/backend.go
@@ -50,7 +50,7 @@ type Backend interface {
 	SaveController(info crossmodel.ControllerInfo, modelUUID string) (ExternalController, error)
 	ControllerTag() names.ControllerTag
 	ControllerConfig() (controller.Config, error)
-	Resources() (Resources, error)
+	Resources() Resources
 	OfferConnectionForRelation(string) (OfferConnection, error)
 	SaveEgressNetworks(relationKey string, cidrs []string) (state.RelationNetworks, error)
 	Branch(string) (Generation, error)
@@ -411,7 +411,7 @@ func (s stateShim) UnitsInError() ([]Unit, error) {
 	return result, nil
 }
 
-func (s stateShim) Resources() (Resources, error) {
+func (s stateShim) Resources() Resources {
 	return s.State.Resources()
 }
 

diff --git a/apiserver/facades/client/resources/facade.go b/apiserver/facades/client/resources/facade.go
@@ -57,10 +57,7 @@ func NewFacade(ctx facade.Context) (*API, error) {
 	}
 
 	st := ctx.State()
-	rst, err := st.Resources()
-	if err != nil {
-		return nil, errors.Trace(err)
-	}
+	rst := st.Resources()
 
 	controllerCfg, err := st.ControllerConfig()
 	if err != nil {

diff --git a/apiserver/facades/controller/caasapplicationprovisioner/mock_test.go b/apiserver/facades/controller/caasapplicationprovisioner/mock_test.go
@@ -100,9 +100,9 @@ func (st *mockState) ResolveConstraints(cons constraints.Value) (constraints.Val
 	return cons, nil
 }
 
-func (st *mockState) Resources() (caasapplicationprovisioner.Resources, error) {
+func (st *mockState) Resources() caasapplicationprovisioner.Resources {
 	st.MethodCall(st, "Resources")
-	return st.resource, nil
+	return st.resource
 }
 
 type mockResources struct {

diff --git a/apiserver/facades/controller/caasapplicationprovisioner/state.go b/apiserver/facades/controller/caasapplicationprovisioner/state.go
@@ -26,7 +26,7 @@ type CAASApplicationProvisionerState interface {
 	Model() (Model, error)
 	Application(string) (Application, error)
 	ResolveConstraints(cons constraints.Value) (constraints.Value, error)
-	Resources() (Resources, error)
+	Resources() Resources
 	WatchApplications() state.StringsWatcher
 }
 
@@ -104,7 +104,7 @@ func (s stateShim) Application(name string) (Application, error) {
 	return &applicationShim{app}, nil
 }
 
-func (s stateShim) Resources() (Resources, error) {
+func (s stateShim) Resources() Resources {
 	return s.State.Resources()
 }
 

diff --git a/apiserver/facades/controller/charmrevisionupdater/interface.go b/apiserver/facades/controller/charmrevisionupdater/interface.go
@@ -25,7 +25,7 @@ type State interface {
 	ControllerConfig() (controller.Config, error)
 	ControllerUUID() string
 	Model() (Model, error)
-	Resources() (state.Resources, error)
+	Resources() state.Resources
 	AliveRelationKeys() []string
 }
 

diff --git a/apiserver/facades/controller/charmrevisionupdater/mocks/mocks.go b/apiserver/facades/controller/charmrevisionupdater/mocks/mocks.go
diff --git a/apiserver/facades/controller/charmrevisionupdater/updater.go b/apiserver/facades/controller/charmrevisionupdater/updater.go
@@ -77,10 +77,7 @@ func (api *CharmRevisionUpdaterAPI) updateLatestRevisions() error {
 	}
 
 	// Process the resulting info for each charm.
-	resources, err := api.state.Resources()
-	if err != nil {
-		return errors.Trace(err)
-	}
+	resources := api.state.Resources()
 	for _, info := range latest {
 		// First, add a charm placeholder to the model for each.
 		if err := api.state.AddCharmPlaceholder(info.url); err != nil {

diff --git a/apiserver/facades/controller/charmrevisionupdater/updater_test.go b/apiserver/facades/controller/charmrevisionupdater/updater_test.go
@@ -280,7 +280,7 @@ func (s *updaterSuite) TestCharmhubUpdateWithResources(c *gc.C) {
 		},
 	}, nil)
 
-	s.state.EXPECT().Resources().Return(s.resources, nil).AnyTimes()
+	s.state.EXPECT().Resources().Return(s.resources).AnyTimes()
 	s.state.EXPECT().AllApplications().Return([]charmrevisionupdater.Application{
 		makeApplication(ctrl, "ch", "resourcey", "charm-3", "app-1", 1),
 	}, nil).AnyTimes()
@@ -415,7 +415,7 @@ func (s *updaterSuite) setupMocksNoResources(c *gc.C) *gomock.Controller {
 	s.state.EXPECT().Cloud(gomock.Any()).Return(cloud.Cloud{Type: "cloud"}, nil).AnyTimes()
 	s.state.EXPECT().ControllerUUID().Return("controller-1").AnyTimes()
 	s.state.EXPECT().Model().Return(s.model, nil).AnyTimes()
-	s.state.EXPECT().Resources().Return(s.resources, nil).AnyTimes()
+	s.state.EXPECT().Resources().Return(s.resources).AnyTimes()
 	return ctrl
 }
 

diff --git a/apiserver/facades/controller/migrationmaster/mocks/precheckbackend.go b/apiserver/facades/controller/migrationmaster/mocks/precheckbackend.go
diff --git a/apiserver/resources_mig.go b/apiserver/resources_mig.go
@@ -73,10 +73,7 @@ func (h *resourcesMigrationUploadHandler) processPost(r *http.Request, st *state
 	if err != nil {
 		return empty, errors.Trace(err)
 	}
-	rSt, err := st.Resources()
-	if err != nil {
-		return empty, errors.Trace(err)
-	}
+	rSt := st.Resources()
 
 	reader := r.Body
 

diff --git a/apiserver/resources_mig_test.go b/apiserver/resources_mig_test.go
@@ -156,8 +156,7 @@ func (s *resourcesUploadSuite) TestUpload(c *gc.C) {
 	c.Check(outResp.ID, gc.Not(gc.Equals), "")
 	c.Check(outResp.Timestamp.IsZero(), jc.IsFalse)
 
-	rSt, err := s.importingState.Resources()
-	c.Assert(err, jc.ErrorIsNil)
+	rSt := s.importingState.Resources()
 	res, reader, err := rSt.OpenResource(s.appName, "bin")
 	c.Assert(err, jc.ErrorIsNil)
 	defer reader.Close()
@@ -193,8 +192,7 @@ func (s *resourcesUploadSuite) TestPlaceholder(c *gc.C) {
 	c.Check(outResp.ID, gc.Not(gc.Equals), "")
 	c.Check(outResp.Timestamp.IsZero(), jc.IsTrue)
 
-	rSt, err := s.importingState.Resources()
-	c.Assert(err, jc.ErrorIsNil)
+	rSt := s.importingState.Resources()
 	res, err := rSt.GetResource(s.appName, "bin")
 	c.Assert(err, jc.ErrorIsNil)
 	c.Check(res.IsPlaceholder(), jc.IsTrue)

diff --git a/cmd/juju/application/bundle_test.go b/cmd/juju/application/bundle_test.go
@@ -475,8 +475,7 @@ func (s *BundleDeployCharmStoreSuite) TestDeployBundleResources(c *gc.C) {
 func (s *BundleDeployCharmStoreSuite) checkResources(c *gc.C, app string, expected []resource.Resource) {
 	_, err := s.State.Application(app)
 	c.Check(err, jc.ErrorIsNil)
-	st, err := s.State.Resources()
-	c.Assert(err, jc.ErrorIsNil)
+	st := s.State.Resources()
 	svcResources, err := st.ListResources(app)
 	c.Assert(err, jc.ErrorIsNil)
 	resources := svcResources.Resources

diff --git a/cmd/juju/application/deploy_test.go b/cmd/juju/application/deploy_test.go
@@ -204,10 +204,7 @@ func deployResources(
 	if len(resources) == 0 {
 		return nil, nil
 	}
-	stRes, err := st.Resources()
-	if err != nil {
-		return nil, err
-	}
+	stRes := st.Resources()
 	ids = make(map[string]string)
 	for _, res := range resources {
 		content := res.Name + " content"

diff --git a/cmd/juju/application/refresh_resources_test.go b/cmd/juju/application/refresh_resources_test.go
@@ -88,8 +88,7 @@ resources:
 		"riak", "--path="+myriakPath.Path, "--resource", "data="+resourceFile)
 	c.Assert(err, jc.ErrorIsNil)
 
-	resources, err := s.State.Resources()
-	c.Assert(err, jc.ErrorIsNil)
+	resources := s.State.Resources()
 
 	sr, err := resources.ListResources("riak")
 	c.Assert(err, jc.ErrorIsNil)
@@ -168,8 +167,7 @@ Deploying charm "cs:bionic/starsay-1".`
 	c.Assert(err, jc.ErrorIsNil)
 	c.Assert(errs, gc.DeepEquals, []error{nil})
 
-	res, err := s.State.Resources()
-	c.Assert(err, jc.ErrorIsNil)
+	res := s.State.Resources()
 	appResources, err := res.ListResources("starsay")
 	c.Assert(err, jc.ErrorIsNil)
 
@@ -284,8 +282,7 @@ Deploying charm "cs:bionic/starsay-1".`
 	charmAdder.CheckCall(c, 0,
 		"AddCharm", charm.MustParseURL("cs:bionic/starsay-2"), csclientparams.NoChannel, false)
 
-	res, err = s.State.Resources()
-	c.Assert(err, jc.ErrorIsNil)
+	res = s.State.Resources()
 	appResources, err = res.ListResources("starsay")
 	c.Assert(err, jc.ErrorIsNil)
 

diff --git a/migration/precheck.go b/migration/precheck.go
@@ -14,7 +14,6 @@ import (
 	"github.com/juju/juju/apiserver/common"
 	coremigration "github.com/juju/juju/core/migration"
 	"github.com/juju/juju/core/presence"
-	"github.com/juju/juju/core/resource"
 	"github.com/juju/juju/core/status"
 	"github.com/juju/juju/state"
 	"github.com/juju/juju/tools"
@@ -35,7 +34,6 @@ type PrecheckBackend interface {
 	AllRelations() ([]PrecheckRelation, error)
 	ControllerBackend() (PrecheckBackend, error)
 	CloudCredential(tag names.CloudCredentialTag) (state.Credential, error)
-	ListPendingResources(string) ([]resource.Resource, error)
 }
 
 // Pool defines the interface to a StatePool used by the migration

diff --git a/migration/precheck_shim.go b/migration/precheck_shim.go
@@ -7,20 +7,14 @@ import (
 	"github.com/juju/errors"
 	"github.com/juju/version/v2"
 
-	"github.com/juju/juju/core/resource"
 	"github.com/juju/juju/state"
 )
 
 // PrecheckShim wraps a pair of *state.States to implement PrecheckBackend.
 func PrecheckShim(modelState, controllerState *state.State) (PrecheckBackend, error) {
-	rSt, err := modelState.Resources()
-	if err != nil {
-		return nil, errors.Trace(err)
-	}
 	return &precheckShim{
 		State:           modelState,
 		controllerState: controllerState,
-		resourcesSt:     rSt,
 	}, nil
 }
 
@@ -29,7 +23,6 @@ func PrecheckShim(modelState, controllerState *state.State) (PrecheckBackend, er
 type precheckShim struct {
 	*state.State
 	controllerState *state.State
-	resourcesSt     state.Resources
 }
 
 // Model implements PrecheckBackend.
@@ -99,12 +92,6 @@ func (s *precheckShim) AllRelations() ([]PrecheckRelation, error) {
 	return out, nil
 }
 
-// ListPendingResources implements PrecheckBackend.
-func (s *precheckShim) ListPendingResources(app string) ([]resource.Resource, error) {
-	resources, err := s.resourcesSt.ListPendingResources(app)
-	return resources, errors.Trace(err)
-}
-
 // ControllerBackend implements PrecheckBackend.
 func (s *precheckShim) ControllerBackend() (PrecheckBackend, error) {
 	return PrecheckShim(s.controllerState, s.controllerState)