fix: remove sources of target_agent_version from controller database

Removes reads and writes of target_agent_version from controller databases. In a couple of places, these are sourced as they should be from the model database. In the case of model summaries they are flat removed - this will need to be rethought at a later time.
manadart · Nov 26, 2024 · 89d2088 · 89d2088
1 parent 119b4c4
commit 89d2088
Show file tree

Hide file tree

Showing 19 changed files with 64 additions and 185 deletions.
diff --git a/cmd/jujud-controller/agent/machine_test.go b/cmd/jujud-controller/agent/machine_test.go
@@ -37,7 +37,6 @@ import (
 	"github.com/juju/juju/core/network"
 	coreos "github.com/juju/juju/core/os"
 	jujuversion "github.com/juju/juju/core/version"
-	"github.com/juju/juju/domain"
 	blockdevicestate "github.com/juju/juju/domain/blockdevice/state"
 	"github.com/juju/juju/environs/filestorage"
 	envstorage "github.com/juju/juju/environs/storage"
@@ -283,21 +282,13 @@ func (s *MachineSuite) testUpgradeRequest(c *gc.C, agent runner, tag string, cur
 
 // setAgentVersion sets the agent version for the controller model in dqlite.
 func (s *MachineSuite) setAgentVersion(c *gc.C, vers string) {
-	st := domain.NewStateBase(s.TxnRunnerFactory())
-	db, err := st.DB()
-	c.Assert(err, jc.ErrorIsNil)
+	db := s.ModelTxnRunner(c, s.ControllerModelUUID())
 
-	q := `
-UPDATE model_agent
-SET target_version = $M.target_agent_version
-WHERE model_uuid = $M.model_id
-`
-	args := sqlair.M{
-		"model_id":             s.ControllerModelUUID(),
-		"target_agent_version": vers,
-	}
+	q := "INSERT INTO agent_version (target_version) values ($M.target_version)"
+
+	args := sqlair.M{"target_version": vers}
 
-	stmt, err := st.Prepare(q, args)
+	stmt, err := sqlair.Prepare(q, args)
 	c.Assert(err, jc.ErrorIsNil)
 
 	err = db.Txn(context.Background(), func(ctx context.Context, tx *sqlair.TX) error {

diff --git a/core/model/readonlymodel.go b/core/model/readonlymodel.go
@@ -4,22 +4,17 @@
 package model
 
 import (
-	"github.com/juju/version/v2"
-
 	"github.com/juju/juju/core/user"
 	"github.com/juju/juju/internal/uuid"
 )
 
 // ReadOnlyModel represents the state of a read-only model found in the
 // model database, not the controller database.
-// All the fields are are denormalized from the model database.
+// All the fields are denormalized from the controller database.
 type ReadOnlyModel struct {
 	// UUID represents the model UUID.
 	UUID UUID
 
-	// AgentVersion reports the current target agent version for the model.
-	AgentVersion version.Number
-
 	// ControllerUUID represents the controller UUID.
 	ControllerUUID uuid.UUID
 

diff --git a/domain/application/service_test.go b/domain/application/service_test.go
@@ -20,7 +20,6 @@ import (
 	coresecrets "github.com/juju/juju/core/secrets"
 	corestorage "github.com/juju/juju/core/storage"
 	coreunit "github.com/juju/juju/core/unit"
-	jujuversion "github.com/juju/juju/core/version"
 	"github.com/juju/juju/domain"
 	"github.com/juju/juju/domain/application"
 	applicationerrors "github.com/juju/juju/domain/application/errors"
@@ -71,9 +70,9 @@ func (s *serviceSuite) SetUpTest(c *gc.C) {
 	modelUUID := uuid.MustNewUUID()
 	err := s.TxnRunner().StdTxn(context.Background(), func(ctx context.Context, tx *sql.Tx) error {
 		_, err := tx.ExecContext(ctx, `
-			INSERT INTO model (uuid, controller_uuid, target_agent_version, name, type, cloud, cloud_type)
-			VALUES (?, ?, ?, "test", "iaas", "test-model", "ec2")
-		`, modelUUID.String(), coretesting.ControllerTag.Id(), jujuversion.Current.String())
+			INSERT INTO model (uuid, controller_uuid,  name, type, cloud, cloud_type)
+			VALUES (?, ?, "test", "iaas", "test-model", "ec2")
+		`, modelUUID.String(), coretesting.ControllerTag.Id())
 		return err
 	})
 	c.Assert(err, jc.ErrorIsNil)

diff --git a/domain/application/state/application_test.go b/domain/application/state/application_test.go
@@ -21,7 +21,6 @@ import (
 	"github.com/juju/juju/core/secrets"
 	coreunit "github.com/juju/juju/core/unit"
 	unittesting "github.com/juju/juju/core/unit/testing"
-	jujuversion "github.com/juju/juju/core/version"
 	"github.com/juju/juju/domain"
 	"github.com/juju/juju/domain/application"
 	"github.com/juju/juju/domain/application/charm"
@@ -58,9 +57,9 @@ func (s *applicationStateSuite) TestGetModelType(c *gc.C) {
 	modelUUID := uuid.MustNewUUID()
 	err := s.TxnRunner().StdTxn(context.Background(), func(ctx context.Context, tx *sql.Tx) error {
 		_, err := tx.ExecContext(ctx, `
-			INSERT INTO model (uuid, controller_uuid, target_agent_version, name, type, cloud, cloud_type)
-			VALUES (?, ?, ?, "test", "iaas", "test-model", "ec2")
-		`, modelUUID.String(), coretesting.ControllerTag.Id(), jujuversion.Current.String())
+			INSERT INTO model (uuid, controller_uuid, name, type, cloud, cloud_type)
+			VALUES (?, ?, "test", "iaas", "test-model", "ec2")
+		`, modelUUID.String(), coretesting.ControllerTag.Id())
 		return err
 	})
 	c.Assert(err, jc.ErrorIsNil)

diff --git a/domain/application/watcher_test.go b/domain/application/watcher_test.go
@@ -17,7 +17,6 @@ import (
 	corecharm "github.com/juju/juju/core/charm"
 	"github.com/juju/juju/core/database"
 	corestorage "github.com/juju/juju/core/storage"
-	jujuversion "github.com/juju/juju/core/version"
 	"github.com/juju/juju/core/watcher/watchertest"
 	"github.com/juju/juju/domain"
 	"github.com/juju/juju/domain/application/charm"
@@ -45,9 +44,9 @@ func (s *watcherSuite) SetUpTest(c *gc.C) {
 	modelUUID := uuid.MustNewUUID()
 	err := s.TxnRunner().StdTxn(context.Background(), func(ctx context.Context, tx *sql.Tx) error {
 		_, err := tx.ExecContext(ctx, `
-			INSERT INTO model (uuid, controller_uuid, target_agent_version, name, type, cloud, cloud_type)
-			VALUES (?, ?, ?, "test", "iaas", "test-model", "ec2")
-		`, modelUUID.String(), coretesting.ControllerTag.Id(), jujuversion.Current.String())
+			INSERT INTO model (uuid, controller_uuid, name, type, cloud, cloud_type)
+			VALUES (?, ?, "test", "iaas", "test-model", "ec2")
+		`, modelUUID.String(), coretesting.ControllerTag.Id())
 		return err
 	})
 	c.Assert(err, jc.ErrorIsNil)

diff --git a/domain/model/bootstrap/bootstrap_test.go b/domain/model/bootstrap/bootstrap_test.go
@@ -185,15 +185,14 @@ func (s *modelBootstrapSuite) TestCreateModelWithDifferingBuildNumber(c *gc.C) {
 }
 
 type dbReadOnlyModel struct {
-	UUID               string         `db:"uuid"`
-	ControllerUUID     string         `db:"controller_uuid"`
-	Name               string         `db:"name"`
-	Type               string         `db:"type"`
-	TargetAgentVersion sql.NullString `db:"target_agent_version"`
-	Cloud              string         `db:"cloud"`
-	CloudType          string         `db:"cloud_type"`
-	CloudRegion        string         `db:"cloud_region"`
-	CredentialOwner    string         `db:"credential_owner"`
-	CredentialName     string         `db:"credential_name"`
-	IsControllerModel  bool           `db:"is_controller_model"`
+	UUID              string `db:"uuid"`
+	ControllerUUID    string `db:"controller_uuid"`
+	Name              string `db:"name"`
+	Type              string `db:"type"`
+	Cloud             string `db:"cloud"`
+	CloudType         string `db:"cloud_type"`
+	CloudRegion       string `db:"cloud_region"`
+	CredentialOwner   string `db:"credential_owner"`
+	CredentialName    string `db:"credential_name"`
+	IsControllerModel bool   `db:"is_controller_model"`
 }
diff --git a/domain/model/state/controllerstate.go b/domain/model/state/controllerstate.go
@@ -160,15 +160,6 @@ func Create(
 		)
 	}
 
-	// Creates a record for the newly created model and register the target
-	// agent version.
-	if err := createModelAgent(ctx, preparer, tx, modelID, input.AgentVersion); err != nil {
-		return fmt.Errorf(
-			"creating model %q with id %q agent: %w",
-			input.Name, modelID, err,
-		)
-	}
-
 	// Sets the secret backend to be used for the newly created model.
 	if err := setModelSecretBackend(ctx, preparer, tx, modelID, input.SecretBackend); err != nil {
 		return fmt.Errorf(
@@ -378,56 +369,6 @@ WHERE uuid = $dbModel.uuid
 	return coreModel, nil
 }
 
-// createModelAgent is responsible for creating a new model's agent record for
-// the given model id. If a model agent record already exists for the given
-// model uuid then an error satisfying [modelerrors.AlreadyExists] is returned.
-// If no model exists for the provided UUID then a [modelerrors.NotFound] is
-// returned.
-func createModelAgent(
-	ctx context.Context,
-	preparer domain.Preparer,
-	tx *sqlair.TX,
-	modelUUID coremodel.UUID,
-	agentVersion version.Number,
-) error {
-	modelAgent := dbModelAgent{
-		UUID:            modelUUID.String(),
-		PreviousVersion: agentVersion.String(),
-		TargetVersion:   agentVersion.String(),
-	}
-	stmt, err := preparer.Prepare(`
-INSERT INTO model_agent (*) VALUES ($dbModelAgent.*)
-`, modelAgent)
-	if err != nil {
-		return errors.Annotatef(err, "preparing insert model agent statement")
-	}
-
-	var outcome sqlair.Outcome
-	err = tx.Query(ctx, stmt, modelAgent).Get(&outcome)
-	if jujudb.IsErrConstraintPrimaryKey(err) {
-		return fmt.Errorf(
-			"%w for uuid %q while setting model agent version",
-			modelerrors.AlreadyExists, modelUUID,
-		)
-	} else if jujudb.IsErrConstraintForeignKey(err) {
-		return fmt.Errorf(
-			"%w for uuid %q while setting model agent version",
-			modelerrors.NotFound,
-			modelUUID,
-		)
-	} else if err != nil {
-		return fmt.Errorf("creating model %q agent information: %w", modelUUID, err)
-	}
-
-	if num, err := outcome.Result().RowsAffected(); err != nil {
-		return errors.Trace(err)
-	} else if num != 1 {
-		return fmt.Errorf("creating model agent record, expected 1 row to be inserted got %d", num)
-	}
-
-	return nil
-}
-
 // setModelSecretBackend sets the secret backend for a given model id. If the
 // secret backend does not exist a [secretbackenderrors.NotFound] error will be
 // returned. Should the model already have a secret backend set an error
@@ -908,7 +849,7 @@ OR uuid IN (SELECT grant_on
 		return nil, errors.Annotatef(err, "preparing select model statement")
 	}
 
-	rval := []coremodel.Model{}
+	var rval []coremodel.Model
 	err = db.Txn(ctx, func(ctx context.Context, tx *sqlair.TX) error {
 		var result []dbModel
 		if err := tx.Query(ctx, modelStmt, uUUID).GetAll(&result); errors.Is(err, sqlair.ErrNoRows) {
@@ -918,12 +859,12 @@ OR uuid IN (SELECT grant_on
 		}
 
 		for _, r := range result {
-			model, err := r.toCoreModel()
+			mod, err := r.toCoreModel()
 			if err != nil {
 				return errors.Trace(err)
 			}
 
-			rval = append(rval, model)
+			rval = append(rval, mod)
 		}
 
 		return nil
@@ -1004,7 +945,7 @@ func (s *State) ListModelSummariesForUser(ctx context.Context, userName user.Nam
 SELECT    (p.access_type, m.uuid, m.name, m.cloud_name, m.cloud_region_name, 
           m.model_type, m.cloud_type, m.owner_name, m.cloud_credential_name, 
           m.cloud_credential_cloud_name, m.cloud_credential_owner_name,
-          m.life, mll.time, m.target_agent_version) AS (&dbModelSummary.*)
+          m.life, mll.time) AS (&dbModelSummary.*)
 FROM      v_user_auth u
 JOIN      v_permission p ON p.grant_to = u.uuid
 JOIN      v_model m ON m.uuid = p.grant_on
@@ -1077,7 +1018,7 @@ func (s *State) ListAllModelSummaries(ctx context.Context) ([]coremodel.ModelSum
 SELECT    (m.uuid, m.name, m.cloud_name, m.cloud_region_name, 
           m.model_type, m.cloud_type, m.owner_name, m.cloud_credential_name, 
           m.cloud_credential_cloud_name, m.cloud_credential_owner_name,
-          m.life, m.target_agent_version) AS (&dbModelSummary.*)
+          m.life) AS (&dbModelSummary.*)
 FROM      v_model m 
 `, dbModelSummary{})
 	if err != nil {

diff --git a/domain/model/state/modelstate.go b/domain/model/state/modelstate.go
@@ -153,16 +153,6 @@ func (s *ModelState) Model(ctx context.Context) (coremodel.ReadOnlyModel, error)
 		s.logger.Infof("model %s: cloud credential owner name is empty", model.Name)
 	}
 
-	var agentVersion string
-	if m.TargetAgentVersion.Valid {
-		agentVersion = m.TargetAgentVersion.String
-	}
-
-	model.AgentVersion, err = version.Parse(agentVersion)
-	if err != nil {
-		return coremodel.ReadOnlyModel{}, fmt.Errorf("parsing model agent version %q: %w", agentVersion, err)
-	}
-
 	model.ControllerUUID, err = uuid.UUIDFromString(m.ControllerUUID)
 	if err != nil {
 		return coremodel.ReadOnlyModel{}, fmt.Errorf("parsing controller uuid %q: %w", m.ControllerUUID, err)

diff --git a/domain/model/state/types.go b/domain/model/state/types.go
@@ -5,12 +5,9 @@ package state
 
 import (
 	"database/sql"
-	"fmt"
 	"time"
 
 	"github.com/juju/errors"
-	"github.com/juju/version/v2"
-
 	"github.com/juju/juju/core/credential"
 	corelife "github.com/juju/juju/core/life"
 	coremodel "github.com/juju/juju/core/model"
@@ -35,9 +32,6 @@ type dbModel struct {
 	// ModelType is the type of model.
 	ModelType string `db:"model_type"`
 
-	// AgentVersion is the target version for agents running under this model.
-	AgentVersion string `db:"target_agent_version"`
-
 	// CloudName is the name of the cloud to associate with the model.
 	CloudName string `db:"cloud_name"`
 
@@ -64,10 +58,6 @@ type dbModel struct {
 }
 
 func (m *dbModel) toCoreModel() (coremodel.Model, error) {
-	agentVersion, err := version.Parse(m.AgentVersion)
-	if err != nil {
-		return coremodel.Model{}, fmt.Errorf("parsing model %q agent version %q: %w", m.UUID, agentVersion, err)
-	}
 	ownerName, err := user.NewName(m.OwnerName)
 	if err != nil {
 		return coremodel.Model{}, errors.Trace(err)
@@ -96,14 +86,13 @@ func (m *dbModel) toCoreModel() (coremodel.Model, error) {
 	}
 
 	return coremodel.Model{
-		Name:         m.Name,
-		Life:         corelife.Value(m.Life),
-		UUID:         coremodel.UUID(m.UUID),
-		ModelType:    coremodel.ModelType(m.ModelType),
-		AgentVersion: agentVersion,
-		Cloud:        m.CloudName,
-		CloudType:    m.CloudType,
-		CloudRegion:  cloudRegion,
+		Name:        m.Name,
+		Life:        corelife.Value(m.Life),
+		UUID:        coremodel.UUID(m.UUID),
+		ModelType:   coremodel.ModelType(m.ModelType),
+		Cloud:       m.CloudName,
+		CloudType:   m.CloudType,
+		CloudRegion: cloudRegion,
 		Credential: credential.Key{
 			Name:  credentialName,
 			Cloud: credentialCloudName,
@@ -195,9 +184,6 @@ type dbModelSummary struct {
 	Access permission.Access `db:"access_type"`
 	// UserLastConnection is the last time this user has accessed this model
 	UserLastConnection *time.Time `db:"time"`
-
-	// AgentVersion is the agent version for this model.
-	AgentVersion string `db:"target_agent_version"`
 }
 
 // decodeModelSummary transforms a dbModelSummary into a coremodel.ModelSummary.
@@ -215,16 +201,6 @@ func (m dbModelSummary) decodeUserModelSummary(controllerInfo dbController) (cor
 
 // decodeModelSummary transforms a dbModelSummary into a coremodel.ModelSummary.
 func (m dbModelSummary) decodeModelSummary(controllerInfo dbController) (coremodel.ModelSummary, error) {
-	var agentVersion version.Number
-	if m.AgentVersion != "" {
-		var err error
-		agentVersion, err = version.Parse(m.AgentVersion)
-		if err != nil {
-			return coremodel.ModelSummary{}, errors.Annotatef(
-				err, "parsing model %q agent version %q", m.Name, agentVersion,
-			)
-		}
-	}
 	ownerName, err := user.NewName(m.OwnerName)
 	if err != nil {
 		return coremodel.ModelSummary{}, errors.Trace(err)
@@ -252,7 +228,6 @@ func (m dbModelSummary) decodeModelSummary(controllerInfo dbController) (coremod
 		IsController:   m.UUID == controllerInfo.ModelUUID,
 		OwnerName:      ownerName,
 		Life:           corelife.Value(m.Life),
-		AgentVersion:   agentVersion,
 	}, nil
 }
 
@@ -304,17 +279,16 @@ func (info *dbModelUserInfo) toModelUserInfo() (coremodel.ModelUserInfo, error)
 }
 
 type dbReadOnlyModel struct {
-	UUID               string         `db:"uuid"`
-	ControllerUUID     string         `db:"controller_uuid"`
-	Name               string         `db:"name"`
-	Type               string         `db:"type"`
-	TargetAgentVersion sql.NullString `db:"target_agent_version"`
-	Cloud              string         `db:"cloud"`
-	CloudType          string         `db:"cloud_type"`
-	CloudRegion        string         `db:"cloud_region"`
-	CredentialOwner    string         `db:"credential_owner"`
-	CredentialName     string         `db:"credential_name"`
-	IsControllerModel  bool           `db:"is_controller_model"`
+	UUID              string `db:"uuid"`
+	ControllerUUID    string `db:"controller_uuid"`
+	Name              string `db:"name"`
+	Type              string `db:"type"`
+	Cloud             string `db:"cloud"`
+	CloudType         string `db:"cloud_type"`
+	CloudRegion       string `db:"cloud_region"`
+	CredentialOwner   string `db:"credential_owner"`
+	CredentialName    string `db:"credential_name"`
+	IsControllerModel bool   `db:"is_controller_model"`
 }
 
 type dbCloudType struct {