Skip to content

Commit

Permalink
fix(build): fix stage selection does not take into account parent cre…
Browse files Browse the repository at this point in the history
…ation timestamp

Signed-off-by: Aleksei Igrychev <aleksei.igrychev@palark.com>
  • Loading branch information
alexey-igrychev committed Jul 2, 2024
1 parent 4e55ba7 commit 741ecea
Show file tree
Hide file tree
Showing 5 changed files with 52 additions and 24 deletions.
15 changes: 12 additions & 3 deletions pkg/build/build_phase.go
Original file line number Diff line number Diff line change
Expand Up @@ -702,6 +702,15 @@ func (phase *BuildPhase) getPrevNonEmptyStageImageSize() int64 {
return 0
}

func (phase *BuildPhase) getPrevNonEmptyStageCreationTs() int64 {
if phase.StagesIterator.PrevNonEmptyStage != nil {
if phase.StagesIterator.PrevNonEmptyStage.GetStageImage().Image.GetStageDescription() != nil {
return phase.StagesIterator.PrevNonEmptyStage.GetStageImage().Image.GetStageDescription().StageID.CreationTs
}
}
return 0
}

func (phase *BuildPhase) OnImageStage(ctx context.Context, img *image.Image, stg stage.Interface) error {
return phase.StagesIterator.OnImageStage(ctx, img, stg, func(img *image.Image, stg stage.Interface, isEmpty bool) error {
if isEmpty {
Expand Down Expand Up @@ -874,7 +883,7 @@ func (phase *BuildPhase) findAndFetchStageFromSecondaryStagesStorage(ctx context

ScanSecondaryStagesStorageList:
for _, secondaryStagesStorage := range storageManager.GetSecondaryStagesStorageList() {
secondaryStages, err := storageManager.GetStagesByDigestFromStagesStorageWithCache(ctx, stg.LogDetailedName(), stg.GetDigest(), secondaryStagesStorage)
secondaryStages, err := storageManager.GetStagesByDigestFromStagesStorageWithCache(ctx, stg.LogDetailedName(), stg.GetDigest(), phase.getPrevNonEmptyStageCreationTs(), secondaryStagesStorage)
if err != nil {
return false, err
} else {
Expand Down Expand Up @@ -942,7 +951,7 @@ func (phase *BuildPhase) calculateStage(ctx context.Context, img *image.Image, s
Do(phase.Conveyor.GetStageDigestMutex(stg.GetDigest()).Lock)

storageManager := phase.Conveyor.StorageManager
stages, err := storageManager.GetStagesByDigestWithCache(ctx, stg.LogDetailedName(), stageDigest)
stages, err := storageManager.GetStagesByDigestWithCache(ctx, stg.LogDetailedName(), stageDigest, phase.getPrevNonEmptyStageCreationTs())
if err != nil {
return false, phase.Conveyor.GetStageDigestMutex(stg.GetDigest()).Unlock, err
}
Expand Down Expand Up @@ -1143,7 +1152,7 @@ func (phase *BuildPhase) atomicBuildStageImage(ctx context.Context, img *image.I
defer unlockStage()
}

if stages, err := phase.Conveyor.StorageManager.GetStagesByDigest(ctx, stg.LogDetailedName(), stg.GetDigest()); err != nil {
if stages, err := phase.Conveyor.StorageManager.GetStagesByDigest(ctx, stg.LogDetailedName(), stg.GetDigest(), phase.getPrevNonEmptyStageCreationTs()); err != nil {
return err
} else {
stageDesc, err := phase.Conveyor.StorageManager.SelectSuitableStage(ctx, phase.Conveyor, stg, stages)
Expand Down
20 changes: 18 additions & 2 deletions pkg/storage/local_stages_storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ func (storage *LocalStagesStorage) GetStagesIDs(ctx context.Context, projectName
return images.ConvertToStages()
}

func (storage *LocalStagesStorage) GetStagesIDsByDigest(ctx context.Context, projectName, digest string, opts ...Option) ([]image.StageID, error) {
func (storage *LocalStagesStorage) GetStagesIDsByDigest(ctx context.Context, projectName, digest string, parentStageCreationTs int64, _ ...Option) ([]image.StageID, error) {
imagesOpts := container_backend.ImagesOptions{}
imagesOpts.Filters = append(imagesOpts.Filters, util.NewPair("reference", fmt.Sprintf(LocalStage_ImageRepoFormat, projectName)))
// NOTE digest already depends on build-cache-version
Expand All @@ -127,7 +127,23 @@ func (storage *LocalStagesStorage) GetStagesIDsByDigest(ctx context.Context, pro
if err != nil {
return nil, fmt.Errorf("unable to get docker images: %w", err)
}
return images.ConvertToStages()

stagesIDs, err := images.ConvertToStages()
if err != nil {
return nil, fmt.Errorf("unable to convert images to stages: %w", err)
}

var resultStageIDs []image.StageID
for _, stageID := range stagesIDs {
if parentStageCreationTs > stageID.CreationTs {
logboek.Context(ctx).Debug().LogF("Skip stage %s (parent stage creation timestamp %d is greater than the stage creation timestamp %d)\n", stageID.String(), parentStageCreationTs, stageID.CreationTs)
continue
}

resultStageIDs = append(resultStageIDs, stageID)
}

return resultStageIDs, nil
}

func (storage *LocalStagesStorage) GetStageDescription(ctx context.Context, projectName string, stageID image.StageID) (*image.StageDescription, error) {
Expand Down
34 changes: 17 additions & 17 deletions pkg/storage/manager/storage_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,10 @@ type StorageManagerInterface interface {
GetImageInfo(ctx context.Context, ref string, opts StorageOptions) (*image.Info, error)

LockStageImage(ctx context.Context, imageName string) error
GetStagesByDigest(ctx context.Context, stageName, stageDigest string) ([]*image.StageDescription, error)
GetStagesByDigestWithCache(ctx context.Context, stageName, stageDigest string) ([]*image.StageDescription, error)
GetStagesByDigestFromStagesStorage(ctx context.Context, stageName, stageDigest string, stagesStorage storage.StagesStorage) ([]*image.StageDescription, error)
GetStagesByDigestFromStagesStorageWithCache(ctx context.Context, stageName, stageDigest string, stagesStorage storage.StagesStorage) ([]*image.StageDescription, error)
GetStagesByDigest(ctx context.Context, stageName, stageDigest string, parentStageCreationTs int64) ([]*image.StageDescription, error)
GetStagesByDigestWithCache(ctx context.Context, stageName, stageDigest string, parentStageCreationTs int64) ([]*image.StageDescription, error)
GetStagesByDigestFromStagesStorage(ctx context.Context, stageName, stageDigest string, parentStageCreationTs int64, stagesStorage storage.StagesStorage) ([]*image.StageDescription, error)
GetStagesByDigestFromStagesStorageWithCache(ctx context.Context, stageName, stageDigest string, parentStageCreationTs int64, stagesStorage storage.StagesStorage) ([]*image.StageDescription, error)
GetStageDescriptionList(ctx context.Context) ([]*image.StageDescription, error)
GetStageDescriptionListWithCache(ctx context.Context) ([]*image.StageDescription, error)
GetFinalStageDescriptionList(ctx context.Context) ([]*image.StageDescription, error)
Expand Down Expand Up @@ -744,16 +744,16 @@ func (m *StorageManager) SelectSuitableStage(ctx context.Context, c stage.Convey
return stageDesc, nil
}

func (m *StorageManager) GetStagesByDigestWithCache(ctx context.Context, stageName, stageDigest string) ([]*image.StageDescription, error) {
return m.GetStagesByDigestFromStagesStorageWithCache(ctx, stageName, stageDigest, m.StagesStorage)
func (m *StorageManager) GetStagesByDigestWithCache(ctx context.Context, stageName, stageDigest string, parentStageCreationTs int64) ([]*image.StageDescription, error) {
return m.GetStagesByDigestFromStagesStorageWithCache(ctx, stageName, stageDigest, parentStageCreationTs, m.StagesStorage)
}

func (m *StorageManager) GetStagesByDigest(ctx context.Context, stageName, stageDigest string) ([]*image.StageDescription, error) {
return m.GetStagesByDigestFromStagesStorage(ctx, stageName, stageDigest, m.StagesStorage)
func (m *StorageManager) GetStagesByDigest(ctx context.Context, stageName, stageDigest string, parentStageCreationTs int64) ([]*image.StageDescription, error) {
return m.GetStagesByDigestFromStagesStorage(ctx, stageName, stageDigest, parentStageCreationTs, m.StagesStorage)
}

func (m *StorageManager) GetStagesByDigestFromStagesStorageWithCache(ctx context.Context, stageName, stageDigest string, stagesStorage storage.StagesStorage) ([]*image.StageDescription, error) {
cachedStageDescriptionList, err := m.getStagesByDigestFromStagesStorage(ctx, stageName, stageDigest, stagesStorage, storage.WithCache())
func (m *StorageManager) GetStagesByDigestFromStagesStorageWithCache(ctx context.Context, stageName, stageDigest string, parentStageCreationTs int64, stagesStorage storage.StagesStorage) ([]*image.StageDescription, error) {
cachedStageDescriptionList, err := m.getStagesByDigestFromStagesStorage(ctx, stageName, stageDigest, parentStageCreationTs, stagesStorage, storage.WithCache())
if err != nil {
return nil, err
}
Expand All @@ -762,15 +762,15 @@ func (m *StorageManager) GetStagesByDigestFromStagesStorageWithCache(ctx context
return cachedStageDescriptionList, nil
}

return m.getStagesByDigestFromStagesStorage(ctx, stageName, stageDigest, stagesStorage)
return m.getStagesByDigestFromStagesStorage(ctx, stageName, stageDigest, parentStageCreationTs, stagesStorage)
}

func (m *StorageManager) GetStagesByDigestFromStagesStorage(ctx context.Context, stageName, stageDigest string, stagesStorage storage.StagesStorage) ([]*image.StageDescription, error) {
return m.getStagesByDigestFromStagesStorage(ctx, stageName, stageDigest, stagesStorage)
func (m *StorageManager) GetStagesByDigestFromStagesStorage(ctx context.Context, stageName, stageDigest string, parentStageCreationTs int64, stagesStorage storage.StagesStorage) ([]*image.StageDescription, error) {
return m.getStagesByDigestFromStagesStorage(ctx, stageName, stageDigest, parentStageCreationTs, stagesStorage)
}

func (m *StorageManager) getStagesByDigestFromStagesStorage(ctx context.Context, stageName, stageDigest string, stagesStorage storage.StagesStorage, opts ...storage.Option) ([]*image.StageDescription, error) {
stageIDs, err := m.getStagesIDsByDigestFromStagesStorage(ctx, stageName, stageDigest, stagesStorage, opts...)
func (m *StorageManager) getStagesByDigestFromStagesStorage(ctx context.Context, stageName, stageDigest string, parentStageCreationTs int64, stagesStorage storage.StagesStorage, opts ...storage.Option) ([]*image.StageDescription, error) {
stageIDs, err := m.getStagesIDsByDigestFromStagesStorage(ctx, stageName, stageDigest, parentStageCreationTs, stagesStorage, opts...)
if err != nil {
return nil, fmt.Errorf("unable to get stages ids from %s by digest %s for stage %s: %w", stagesStorage.String(), stageDigest, stageName, err)
}
Expand Down Expand Up @@ -813,12 +813,12 @@ func (m *StorageManager) getWithLocalManifestCacheOption() bool {
return m.StagesStorage.Address() != storage.LocalStorageAddress
}

func (m *StorageManager) getStagesIDsByDigestFromStagesStorage(ctx context.Context, stageName, stageDigest string, stagesStorage storage.StagesStorage, opts ...storage.Option) ([]image.StageID, error) {
func (m *StorageManager) getStagesIDsByDigestFromStagesStorage(ctx context.Context, stageName, stageDigest string, parentStageCreationTs int64, stagesStorage storage.StagesStorage, opts ...storage.Option) ([]image.StageID, error) {
var stageIDs []image.StageID
if err := logboek.Context(ctx).Info().LogProcess("Get %s stages by digest %s from storage", stageName, stageDigest).
DoError(func() error {
var err error
stageIDs, err = stagesStorage.GetStagesIDsByDigest(ctx, m.ProjectName, stageDigest, opts...)
stageIDs, err = stagesStorage.GetStagesIDsByDigest(ctx, m.ProjectName, stageDigest, parentStageCreationTs, opts...)
if err != nil {
return fmt.Errorf("error getting project %s stage %s images from storage: %w", m.StagesStorage.String(), stageDigest, err)
}
Expand Down
5 changes: 4 additions & 1 deletion pkg/storage/repo_stages_storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ func (storage *RepoStagesStorage) DeleteRepo(ctx context.Context) error {
return storage.DockerRegistry.DeleteRepo(ctx, storage.RepoAddress)
}

func (storage *RepoStagesStorage) GetStagesIDsByDigest(ctx context.Context, _, digest string, opts ...Option) ([]image.StageID, error) {
func (storage *RepoStagesStorage) GetStagesIDsByDigest(ctx context.Context, _, digest string, parentStageCreationTs int64, opts ...Option) ([]image.StageID, error) {
var res []image.StageID

o := makeOptions(opts...)
Expand Down Expand Up @@ -237,6 +237,9 @@ func (storage *RepoStagesStorage) GetStagesIDsByDigest(ctx context.Context, _, d
continue
}
return nil, fmt.Errorf("unable to get digest and creation timestamp from tag %q: %w", tag, err)
} else if parentStageCreationTs > creationTs {
logboek.Context(ctx).Debug().LogF("Skip stage %s (parent stage creation timestamp %d is greater than the stage creation timestamp %d)\n", tag, parentStageCreationTs, creationTs)
continue
} else {
stageID := image.NewStageID(digest, creationTs)

Expand Down
2 changes: 1 addition & 1 deletion pkg/storage/stages_storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ type FilterStagesAndProcessRelatedDataOptions struct {

type StagesStorage interface {
GetStagesIDs(ctx context.Context, projectName string, opts ...Option) ([]image.StageID, error)
GetStagesIDsByDigest(ctx context.Context, projectName, digest string, opts ...Option) ([]image.StageID, error)
GetStagesIDsByDigest(ctx context.Context, projectName, digest string, parentStageCreationTs int64, opts ...Option) ([]image.StageID, error)
GetStageDescription(ctx context.Context, projectName string, stageID image.StageID) (*image.StageDescription, error)
ExportStage(ctx context.Context, stageDescription *image.StageDescription, destinationReference string, mutateConfigFunc func(config v1.Config) (v1.Config, error)) error
DeleteStage(ctx context.Context, stageDescription *image.StageDescription, options DeleteImageOptions) error
Expand Down

0 comments on commit 741ecea

Please sign in to comment.