diff --git a/domain/objectstore/errors/errors.go b/domain/objectstore/errors/errors.go index 6785bb189af..174561c9f44 100644 --- a/domain/objectstore/errors/errors.go +++ b/domain/objectstore/errors/errors.go @@ -14,6 +14,7 @@ const ( // there is a collision in the hash function. ErrHashAndSizeAlreadyExists = errors.ConstError("hash exists for different file size") - // ErrHashAlreadyExists is returned when a hash already exists. - ErrHashAlreadyExists = errors.ConstError("hash already exists") + // ErrPathAlreadyExistsDifferentHash is returned when a path already exists + // with a different hash. + ErrPathAlreadyExistsDifferentHash = errors.ConstError("path already exists with different hash") ) diff --git a/domain/objectstore/state/state.go b/domain/objectstore/state/state.go index 5185db85711..e020ca91989 100644 --- a/domain/objectstore/state/state.go +++ b/domain/objectstore/state/state.go @@ -163,7 +163,7 @@ AND size = $dbMetadata.size`, dbMetadata, dbMetadataPath) err = tx.Query(ctx, pathStmt, dbMetadataPath).Get(&outcome) if database.IsErrConstraintPrimaryKey(err) { - return objectstoreerrors.ErrHashAlreadyExists + return objectstoreerrors.ErrPathAlreadyExistsDifferentHash } else if err != nil { return errors.Annotatef(err, "inserting metadata path") } diff --git a/domain/objectstore/state/state_test.go b/domain/objectstore/state/state_test.go index c850c355a79..08cf73c8961 100644 --- a/domain/objectstore/state/state_test.go +++ b/domain/objectstore/state/state_test.go @@ -102,7 +102,7 @@ func (s *stateSuite) TestPutMetadataConflict(c *gc.C) { _, err = st.PutMetadata(context.Background(), metadata) c.Assert(err, gc.Not(jc.ErrorIsNil)) - c.Check(err, jc.ErrorIs, objectstoreerrors.ErrHashAlreadyExists) + c.Check(err, jc.ErrorIs, objectstoreerrors.ErrPathAlreadyExistsDifferentHash) } func (s *stateSuite) TestPutMetadataWithSameHashAndSize(c *gc.C) { diff --git a/internal/bootstrap/agentbinary.go b/internal/bootstrap/agentbinary.go index 18e144afd3c..8db60ecf35e 100644 --- a/internal/bootstrap/agentbinary.go +++ b/internal/bootstrap/agentbinary.go @@ -19,7 +19,6 @@ import ( "github.com/juju/juju/core/logger" coreos "github.com/juju/juju/core/os" jujuversion "github.com/juju/juju/core/version" - objectstoreerrors "github.com/juju/juju/domain/objectstore/errors" "github.com/juju/juju/state/binarystorage" ) @@ -64,8 +63,7 @@ func PopulateAgentBinary(ctx context.Context, dataDir string, storage AgentBinar logger.Debugf("Adding agent binary: %v", agentTools.Version) - // If the hash already exists, we don't need to add it again. - if err := storage.Add(ctx, bytes.NewReader(data), metadata); err != nil && !errors.Is(err, objectstoreerrors.ErrHashAlreadyExists) { + if err := storage.Add(ctx, bytes.NewReader(data), metadata); err != nil { return nil, errors.Trace(err) } diff --git a/internal/bootstrap/agentbinary_test.go b/internal/bootstrap/agentbinary_test.go index 49f10332e21..03af84a9eab 100644 --- a/internal/bootstrap/agentbinary_test.go +++ b/internal/bootstrap/agentbinary_test.go @@ -19,7 +19,6 @@ import ( "github.com/juju/juju/core/arch" coreos "github.com/juju/juju/core/os" jujuversion "github.com/juju/juju/core/version" - objectstoreerrors "github.com/juju/juju/domain/objectstore/errors" "github.com/juju/juju/state/binarystorage" ) @@ -63,40 +62,6 @@ func (s *agentBinarySuite) TestPopulateAgentBinary(c *gc.C) { s.expectNoTools(c, toolsPath) } -func (s *agentBinarySuite) TestPopulateAgentBinaryTwiceShouldSucceed(c *gc.C) { - defer s.setupMocks(c).Finish() - - current := version.Binary{ - Number: jujuversion.Current, - Arch: arch.HostArch(), - Release: coreos.HostOSTypeName(), - } - - dir, toolsPath := s.ensureDirs(c, current) - size := int64(4) - - s.writeDownloadTools(c, toolsPath, downloadTools{ - Version: current.String(), - URL: filepath.Join(dir, "tools", fmt.Sprintf("%s.tgz", current.String())), - SHA256: "sha256", - Size: size, - }) - - s.writeAgentBinary(c, toolsPath, current) - - s.storage.EXPECT().Add(gomock.Any(), gomock.Any(), binarystorage.Metadata{ - Version: current.String(), - Size: size, - SHA256: "sha256", - }).Return(objectstoreerrors.ErrHashAlreadyExists) - - cleanup, err := PopulateAgentBinary(context.Background(), dir, s.storage, s.logger) - c.Assert(err, jc.ErrorIsNil) - cleanup() - - s.expectNoTools(c, toolsPath) -} - func (s *agentBinarySuite) TestPopulateAgentBinaryAddError(c *gc.C) { defer s.setupMocks(c).Finish() diff --git a/internal/charm/services/storage.go b/internal/charm/services/storage.go index 558715ebf4f..22222684515 100644 --- a/internal/charm/services/storage.go +++ b/internal/charm/services/storage.go @@ -72,7 +72,7 @@ func (s *CharmStorage) Store(ctx context.Context, charmURL string, downloadedCha } // If the blob is already stored, we can skip the upload. - if _, err := s.objectStore.Put(ctx, storagePath, downloadedCharm.CharmData, downloadedCharm.Size); err != nil && !errors.Is(err, objectstoreerrors.ErrHashAlreadyExists) { + if _, err := s.objectStore.Put(ctx, storagePath, downloadedCharm.CharmData, downloadedCharm.Size); err != nil && !errors.Is(err, objectstoreerrors.ErrPathAlreadyExistsDifferentHash) { return "", errors.Annotate(err, "cannot add charm to storage") } diff --git a/internal/charm/services/storage_test.go b/internal/charm/services/storage_test.go index 9375a82df6b..080d7350d6e 100644 --- a/internal/charm/services/storage_test.go +++ b/internal/charm/services/storage_test.go @@ -91,7 +91,7 @@ func (s *storageTestSuite) TestStoreBlobAlreadyStored(c *gc.C) { CharmVersion: "the-version", } - s.storageBackend.EXPECT().Put(gomock.Any(), expStoreCharmPath, gomock.AssignableToTypeOf(dlCharm.CharmData), int64(7337)).Return("", objectstoreerrors.ErrHashAlreadyExists) + s.storageBackend.EXPECT().Put(gomock.Any(), expStoreCharmPath, gomock.AssignableToTypeOf(dlCharm.CharmData), int64(7337)).Return("", objectstoreerrors.ErrPathAlreadyExistsDifferentHash) s.stateBackend.EXPECT().UpdateUploadedCharm(state.CharmInfo{ StoragePath: expStoreCharmPath, ID: curl, diff --git a/state/binarystorage/binarystorage.go b/state/binarystorage/binarystorage.go index dfc4162a29e..4a32037891c 100644 --- a/state/binarystorage/binarystorage.go +++ b/state/binarystorage/binarystorage.go @@ -65,7 +65,7 @@ func New( func (s *binaryStorage) Add(ctx context.Context, r io.Reader, metadata Metadata) (resultErr error) { // Add the binary file to storage. path := fmt.Sprintf("tools/%s-%s", metadata.Version, metadata.SHA256) - if _, err := s.managedStorage.Put(ctx, path, r, metadata.Size); err != nil && !errors.Is(err, objectstoreerrors.ErrHashAlreadyExists) { + if _, err := s.managedStorage.Put(ctx, path, r, metadata.Size); err != nil && !errors.Is(err, objectstoreerrors.ErrPathAlreadyExistsDifferentHash) { return errors.Annotate(err, "cannot store binary file") } defer func() {