From e62cd78ad86c0736c61f5a0b773bc3c77ad2cd27 Mon Sep 17 00:00:00 2001 From: Timofey Kirillov Date: Tue, 6 Dec 2022 18:51:04 +0300 Subject: [PATCH] fix(report): fix panic occured when using final-repo and report Panic occurs when published image in the primary repo becomes broken (MANIFEST_UNKNOWN), despite the fact it is listed in the repository tags. * Reworked report information gathering to use cached stage description for report when possible. * Check final repo for image existance and repush this image when manifest becomes broken. Signed-off-by: Timofey Kirillov --- pkg/build/build_phase.go | 20 ++-------- pkg/container_backend/legacy_base_image.go | 15 ++++++-- pkg/container_backend/legacy_interface.go | 5 ++- pkg/storage/manager/storage_manager.go | 45 ++++++++++++++-------- 4 files changed, 48 insertions(+), 37 deletions(-) diff --git a/pkg/build/build_phase.go b/pkg/build/build_phase.go index 2f9086a007..bfd8fba15e 100644 --- a/pkg/build/build_phase.go +++ b/pkg/build/build_phase.go @@ -179,22 +179,10 @@ func (phase *BuildPhase) createReport(ctx context.Context) error { continue } - var desc *imagePkg.StageDescription - stageImage := img.GetLastNonEmptyStage().GetStageImage() - stageID := stageImage.Image.GetStageDescription().StageID - - if phase.Conveyor.StorageManager.GetFinalStagesStorage() != nil { - var err error - desc, err = phase.Conveyor.StorageManager.GetFinalStagesStorage().GetStageDescription(ctx, phase.Conveyor.ProjectName(), stageID.Digest, stageID.UniqueID) - if err != nil { - return fmt.Errorf("unable to get stage %s descriptor from final repo %s: %w", stageID.String(), phase.Conveyor.StorageManager.GetFinalStagesStorage().String(), err) - } - } else { - var err error - desc, err = phase.Conveyor.StorageManager.GetStagesStorage().GetStageDescription(ctx, phase.Conveyor.ProjectName(), stageID.Digest, stageID.UniqueID) - if err != nil { - return fmt.Errorf("unable to get stage %s descriptor from primary repo %s: %w", stageID.String(), phase.Conveyor.StorageManager.GetStagesStorage().String(), err) - } + stageImage := img.GetLastNonEmptyStage().GetStageImage().Image + desc := stageImage.GetFinalStageDescription() + if desc == nil { + desc = stageImage.GetStageDescription() } phase.ImagesReport.SetImageRecord(img.GetName(), ReportImageRecord{ diff --git a/pkg/container_backend/legacy_base_image.go b/pkg/container_backend/legacy_base_image.go index 7b6a925bc4..f1f221c684 100644 --- a/pkg/container_backend/legacy_base_image.go +++ b/pkg/container_backend/legacy_base_image.go @@ -8,9 +8,10 @@ import ( ) type legacyBaseImage struct { - name string - info *image.Info - stageDesc *image.StageDescription + name string + info *image.Info + stageDesc *image.StageDescription + finalStageDesc *image.StageDescription ContainerBackend ContainerBackend } @@ -64,6 +65,14 @@ func (i *legacyBaseImage) GetStageDescription() *image.StageDescription { return i.stageDesc } +func (i *legacyBaseImage) SetFinalStageDescription(stageDesc *image.StageDescription) { + i.finalStageDesc = stageDesc +} + +func (i *legacyBaseImage) GetFinalStageDescription() *image.StageDescription { + return i.finalStageDesc +} + func (i *legacyBaseImage) IsExistsLocally() bool { return i.info != nil } diff --git a/pkg/container_backend/legacy_interface.go b/pkg/container_backend/legacy_interface.go index 44695d4605..c7a9ab3cd7 100644 --- a/pkg/container_backend/legacy_interface.go +++ b/pkg/container_backend/legacy_interface.go @@ -22,6 +22,7 @@ type LegacyImageInterface interface { // TODO: should be under a single separate interface Container() LegacyContainer BuilderContainer() LegacyBuilderContainer + SetCommitChangeOptions(opts LegacyCommitChangeOptions) Build(context.Context, BuildOptions) error SetBuiltID(builtID string) @@ -37,7 +38,9 @@ type LegacyImageInterface interface { SetStageDescription(stage *image.StageDescription) GetStageDescription() *image.StageDescription - SetCommitChangeOptions(opts LegacyCommitChangeOptions) + + GetFinalStageDescription() *image.StageDescription + SetFinalStageDescription(stage *image.StageDescription) GetCopy() LegacyImageInterface } diff --git a/pkg/storage/manager/storage_manager.go b/pkg/storage/manager/storage_manager.go index 90778ad936..6a909df093 100644 --- a/pkg/storage/manager/storage_manager.go +++ b/pkg/storage/manager/storage_manager.go @@ -415,28 +415,28 @@ func doFetchStage(ctx context.Context, projectName string, stagesStorage storage }) } -func copyStageIntoStagesStorage(ctx context.Context, projectName string, stageID image.StageID, img container_backend.LegacyImageInterface, stagesStorage storage.StagesStorage, containerBackend container_backend.ContainerBackend) error { +func copyStageIntoStagesStorage(ctx context.Context, projectName string, stageID image.StageID, img container_backend.LegacyImageInterface, stagesStorage storage.StagesStorage, containerBackend container_backend.ContainerBackend) (*image.StageDescription, error) { newImg := img.GetCopy() targetStagesStorageImageName := stagesStorage.ConstructStageImageName(projectName, stageID.Digest, stageID.UniqueID) if err := containerBackend.RenameImage(ctx, newImg, targetStagesStorageImageName, false); err != nil { - return fmt.Errorf("unable to rename image %s to %s: %w", img.Name(), targetStagesStorageImageName, err) + return nil, fmt.Errorf("unable to rename image %s to %s: %w", img.Name(), targetStagesStorageImageName, err) } if err := stagesStorage.StoreImage(ctx, newImg); err != nil { - return fmt.Errorf("unable to store stage %s into the cache stages storage %s: %w", stageID.String(), stagesStorage.String(), err) + return nil, fmt.Errorf("unable to store stage %s into the cache stages storage %s: %w", stageID.String(), stagesStorage.String(), err) } if err := storeStageDescriptionIntoLocalManifestCache(ctx, projectName, stageID, stagesStorage, ConvertStageDescriptionForStagesStorage(newImg.GetStageDescription(), stagesStorage)); err != nil { - return fmt.Errorf("error storing stage %s description into local manifest cache: %w", targetStagesStorageImageName, err) + return nil, fmt.Errorf("error storing stage %s description into local manifest cache: %w", targetStagesStorageImageName, err) } if err := lrumeta.CommonLRUImagesCache.AccessImage(ctx, targetStagesStorageImageName); err != nil { - return fmt.Errorf("error accessing last recently used images cache for %s: %w", targetStagesStorageImageName, err) + return nil, fmt.Errorf("error accessing last recently used images cache for %s: %w", targetStagesStorageImageName, err) } - return nil + return newImg.GetStageDescription(), nil } func (m *StorageManager) FetchStage(ctx context.Context, containerBackend container_backend.ContainerBackend, stg stage.Interface) error { @@ -579,14 +579,14 @@ func (m *StorageManager) FetchStage(ctx context.Context, containerBackend contai }) if IsErrStageNotFound(err) { - logboek.Context(ctx).Error().LogF("Stage is no longer available in the %q!\n", stg.LogDetailedName(), stg.GetStageImage().Image.Name(), m.StagesStorage.String(), m.ProjectName) + logboek.Context(ctx).Error().LogF("Stage %s image %s is no longer available!\n", stg.LogDetailedName(), stg.GetStageImage().Image.Name()) return ErrUnexpectedStagesStorageState } if storage.IsErrBrokenImage(err) { - logboek.Context(ctx).Error().LogF("Invalid stage %q!\n", stg.LogDetailedName(), stg.GetStageImage().Image.Name(), m.StagesStorage.String(), m.ProjectName) + logboek.Context(ctx).Error().LogF("Broken stage %s image %s!\n", stg.LogDetailedName(), stg.GetStageImage().Image.Name()) - logboek.Context(ctx).Error().LogF("Will mark image %q as rejected in the stages storage %s\n", stg.GetStageImage().Image.Name(), m.StagesStorage.String()) + logboek.Context(ctx).Error().LogF("Will mark image %s as rejected in the stages storage %s\n", stg.GetStageImage().Image.Name(), m.StagesStorage.String()) if err := m.StagesStorage.RejectStage(ctx, m.ProjectName, stageID.Digest, stageID.UniqueID); err != nil { return fmt.Errorf("unable to reject stage %s image %s in the stages storage %s: %w", stg.LogDetailedName(), stg.GetStageImage().Image.Name(), m.StagesStorage.String(), err) } @@ -606,7 +606,7 @@ func (m *StorageManager) FetchStage(ctx context.Context, containerBackend contai err := logboek.Context(ctx).Default().LogProcess("Copy stage %s into cache %s", stg.LogDetailedName(), cacheStagesStorage.String()). DoError(func() error { - if err := copyStageIntoStagesStorage(ctx, m.ProjectName, *stageID, fetchedImg, cacheStagesStorage, containerBackend); err != nil { + if _, err := copyStageIntoStagesStorage(ctx, m.ProjectName, *stageID, fetchedImg, cacheStagesStorage, containerBackend); err != nil { return fmt.Errorf("unable to copy stage %s into cache stages storage %s: %w", stageID.String(), cacheStagesStorage.String(), err) } return nil @@ -626,7 +626,7 @@ func (m *StorageManager) CopyStageIntoCacheStorages(ctx context.Context, stg sta err := logboek.Context(ctx).Default().LogProcess("Copy stage %s into cache %s", stg.LogDetailedName(), cacheStagesStorage.String()). DoError(func() error { - if err := copyStageIntoStagesStorage(ctx, m.ProjectName, *stageID, img.Image, cacheStagesStorage, containerBackend); err != nil { + if _, err := copyStageIntoStagesStorage(ctx, m.ProjectName, *stageID, img.Image, cacheStagesStorage, containerBackend); err != nil { return fmt.Errorf("unable to copy stage %s into cache stages storage %s: %w", stageID.String(), cacheStagesStorage.String(), err) } return nil @@ -669,16 +669,25 @@ func (m *StorageManager) CopyStageIntoFinalStorage(ctx context.Context, stg stag logboek.Context(ctx).Debug().LogF("[%p] Got existing final stages list cache: %#v\n", m, existingStagesListCache.StageIDs) stageID := stg.GetStageImage().Image.GetStageDescription().StageID - finalImageName := m.FinalStagesStorage.ConstructStageImageName(m.ProjectName, stageID.Digest, stageID.UniqueID) + + finalImageName := m.GetFinalStagesStorage().ConstructStageImageName(m.ProjectName, stageID.Digest, stageID.UniqueID) for _, existingStg := range existingStagesListCache.GetStageIDs() { if existingStg.IsEqual(*stageID) { - logboek.Context(ctx).Info().LogF("Stage %s already exists in the final repo, skipping\n", stageID.String()) + desc, err := m.GetFinalStagesStorage().GetStageDescription(ctx, m.ProjectName, stageID.Digest, stageID.UniqueID) + if err != nil { + return fmt.Errorf("unable to get stage %s descriptor from final repo %s: %w", stageID.String(), m.GetFinalStagesStorage().String(), err) + } + if desc != nil { + stg.GetStageImage().Image.SetFinalStageDescription(desc) - logboek.Context(ctx).Default().LogFHighlight("Use cache final image for %s\n", stg.LogDetailedName()) - container_backend.LogImageName(ctx, finalImageName) + logboek.Context(ctx).Info().LogF("Stage %s already exists in the final repo, skipping\n", stageID.String()) - return nil + logboek.Context(ctx).Default().LogFHighlight("Use cache final image for %s\n", stg.LogDetailedName()) + container_backend.LogImageName(ctx, finalImageName) + + return nil + } } } @@ -697,8 +706,10 @@ func (m *StorageManager) CopyStageIntoFinalStorage(ctx context.Context, stg stag options.Style(style.Highlight()) }). DoError(func() error { - if err := copyStageIntoStagesStorage(ctx, m.ProjectName, *stageID, img.Image, m.FinalStagesStorage, containerBackend); err != nil { + if desc, err := copyStageIntoStagesStorage(ctx, m.ProjectName, *stageID, img.Image, m.FinalStagesStorage, containerBackend); err != nil { return fmt.Errorf("unable to copy stage %s into the final repo %s: %w", stageID.String(), m.FinalStagesStorage.String(), err) + } else { + stg.GetStageImage().Image.SetFinalStageDescription(desc) } logboek.Context(ctx).Default().LogFDetails(" name: %s\n", finalImageName)