Skip to content

Commit 7ea02dc

Browse files
alexey-igrychevalexey-igrychev
committed
fix(build): fix stage selection does not take into account parent creation timestamp
Signed-off-by: Aleksei Igrychev <aleksei.igrychev@palark.com>
1 parent bc00022 commit 7ea02dc

File tree

5 files changed

+52
-24
lines changed

5 files changed

+52
-24
lines changed

‎pkg/build/build_phase.go‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -692,6 +692,15 @@ func (phase *BuildPhase) getPrevNonEmptyStageImageSize() int64 {
692692
return 0
693693
}
694694

695+
func (phase *BuildPhase) getPrevNonEmptyStageCreationTs() int64 {
696+
if phase.StagesIterator.PrevNonEmptyStage != nil {
697+
if phase.StagesIterator.PrevNonEmptyStage.GetStageImage().Image.GetStageDescription() != nil {
698+
return phase.StagesIterator.PrevNonEmptyStage.GetStageImage().Image.GetStageDescription().StageID.CreationTs
699+
}
700+
}
701+
return 0
702+
}
703+
695704
func (phase *BuildPhase) OnImageStage(ctx context.Context, img *image.Image, stg stage.Interface) error {
696705
return phase.StagesIterator.OnImageStage(ctx, img, stg, func(img *image.Image, stg stage.Interface, isEmpty bool) error {
697706
if isEmpty {
@@ -864,7 +873,7 @@ func (phase *BuildPhase) findAndFetchStageFromSecondaryStagesStorage(ctx context
864873

865874
ScanSecondaryStagesStorageList:
866875
for _, secondaryStagesStorage := range storageManager.GetSecondaryStagesStorageList() {
867-
secondaryStages, err := storageManager.GetStagesByDigestFromStagesStorageWithCache(ctx, stg.LogDetailedName(), stg.GetDigest(), secondaryStagesStorage)
876+
secondaryStages, err := storageManager.GetStagesByDigestFromStagesStorageWithCache(ctx, stg.LogDetailedName(), stg.GetDigest(), phase.getPrevNonEmptyStageCreationTs(), secondaryStagesStorage)
868877
if err != nil {
869878
return false, err
870879
} else {
@@ -929,7 +938,7 @@ func (phase *BuildPhase) calculateStage(ctx context.Context, img *image.Image, s
929938
Do(phase.Conveyor.GetStageDigestMutex(stg.GetDigest()).Lock)
930939

931940
storageManager := phase.Conveyor.StorageManager
932-
stages, err := storageManager.GetStagesByDigestWithCache(ctx, stg.LogDetailedName(), stageDigest)
941+
stages, err := storageManager.GetStagesByDigestWithCache(ctx, stg.LogDetailedName(), stageDigest, phase.getPrevNonEmptyStageCreationTs())
933942
if err != nil {
934943
return false, phase.Conveyor.GetStageDigestMutex(stg.GetDigest()).Unlock, err
935944
}
@@ -1130,7 +1139,7 @@ func (phase *BuildPhase) atomicBuildStageImage(ctx context.Context, img *image.I
11301139
defer unlockStage()
11311140
}
11321141

1133-
if stages, err := phase.Conveyor.StorageManager.GetStagesByDigest(ctx, stg.LogDetailedName(), stg.GetDigest()); err != nil {
1142+
if stages, err := phase.Conveyor.StorageManager.GetStagesByDigest(ctx, stg.LogDetailedName(), stg.GetDigest(), phase.getPrevNonEmptyStageCreationTs()); err != nil {
11341143
return err
11351144
} else {
11361145
stageDesc, err := phase.Conveyor.StorageManager.SelectSuitableStage(ctx, phase.Conveyor, stg, stages)

‎pkg/storage/local_stages_storage.go‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ func (storage *LocalStagesStorage) GetStagesIDs(ctx context.Context, projectName
117117
return images.ConvertToStages()
118118
}
119119

120-
func (storage *LocalStagesStorage) GetStagesIDsByDigest(ctx context.Context, projectName, digest string, opts ...Option) ([]image.StageID, error) {
120+
func (storage *LocalStagesStorage) GetStagesIDsByDigest(ctx context.Context, projectName, digest string, parentStageCreationTs int64, _ ...Option) ([]image.StageID, error) {
121121
imagesOpts := container_backend.ImagesOptions{}
122122
imagesOpts.Filters = append(imagesOpts.Filters, util.NewPair("reference", fmt.Sprintf(LocalStage_ImageRepoFormat, projectName)))
123123
// NOTE digest already depends on build-cache-version
@@ -127,7 +127,23 @@ func (storage *LocalStagesStorage) GetStagesIDsByDigest(ctx context.Context, pro
127127
if err != nil {
128128
return nil, fmt.Errorf("unable to get docker images: %w", err)
129129
}
130-
return images.ConvertToStages()
130+
131+
stagesIDs, err := images.ConvertToStages()
132+
if err != nil {
133+
return nil, fmt.Errorf("unable to convert images to stages: %w", err)
134+
}
135+
136+
var resultStageIDs []image.StageID
137+
for _, stageID := range stagesIDs {
138+
if parentStageCreationTs > stageID.CreationTs {
139+
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)
140+
continue
141+
}
142+
143+
resultStageIDs = append(resultStageIDs, stageID)
144+
}
145+
146+
return resultStageIDs, nil
131147
}
132148

133149
func (storage *LocalStagesStorage) GetStageDescription(ctx context.Context, projectName string, stageID image.StageID) (*image.StageDescription, error) {

‎pkg/storage/manager/storage_manager.go‎

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -70,10 +70,10 @@ type StorageManagerInterface interface {
7070
GetImageInfo(ctx context.Context, ref string, opts StorageOptions) (*image.Info, error)
7171

7272
LockStageImage(ctx context.Context, imageName string) error
73-
GetStagesByDigest(ctx context.Context, stageName, stageDigest string) ([]*image.StageDescription, error)
74-
GetStagesByDigestWithCache(ctx context.Context, stageName, stageDigest string) ([]*image.StageDescription, error)
75-
GetStagesByDigestFromStagesStorage(ctx context.Context, stageName, stageDigest string, stagesStorage storage.StagesStorage) ([]*image.StageDescription, error)
76-
GetStagesByDigestFromStagesStorageWithCache(ctx context.Context, stageName, stageDigest string, stagesStorage storage.StagesStorage) ([]*image.StageDescription, error)
73+
GetStagesByDigest(ctx context.Context, stageName, stageDigest string, parentStageCreationTs int64) ([]*image.StageDescription, error)
74+
GetStagesByDigestWithCache(ctx context.Context, stageName, stageDigest string, parentStageCreationTs int64) ([]*image.StageDescription, error)
75+
GetStagesByDigestFromStagesStorage(ctx context.Context, stageName, stageDigest string, parentStageCreationTs int64, stagesStorage storage.StagesStorage) ([]*image.StageDescription, error)
76+
GetStagesByDigestFromStagesStorageWithCache(ctx context.Context, stageName, stageDigest string, parentStageCreationTs int64, stagesStorage storage.StagesStorage) ([]*image.StageDescription, error)
7777
GetStageDescriptionList(ctx context.Context) ([]*image.StageDescription, error)
7878
GetStageDescriptionListWithCache(ctx context.Context) ([]*image.StageDescription, error)
7979
GetFinalStageDescriptionList(ctx context.Context) ([]*image.StageDescription, error)
@@ -744,16 +744,16 @@ func (m *StorageManager) SelectSuitableStage(ctx context.Context, c stage.Convey
744744
return stageDesc, nil
745745
}
746746

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

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

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

765-
return m.getStagesByDigestFromStagesStorage(ctx, stageName, stageDigest, stagesStorage)
765+
return m.getStagesByDigestFromStagesStorage(ctx, stageName, stageDigest, parentStageCreationTs, stagesStorage)
766766
}
767767

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

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

816-
func (m *StorageManager) getStagesIDsByDigestFromStagesStorage(ctx context.Context, stageName, stageDigest string, stagesStorage storage.StagesStorage, opts ...storage.Option) ([]image.StageID, error) {
816+
func (m *StorageManager) getStagesIDsByDigestFromStagesStorage(ctx context.Context, stageName, stageDigest string, parentStageCreationTs int64, stagesStorage storage.StagesStorage, opts ...storage.Option) ([]image.StageID, error) {
817817
var stageIDs []image.StageID
818818
if err := logboek.Context(ctx).Info().LogProcess("Get %s stages by digest %s from storage", stageName, stageDigest).
819819
DoError(func() error {
820820
var err error
821-
stageIDs, err = stagesStorage.GetStagesIDsByDigest(ctx, m.ProjectName, stageDigest, opts...)
821+
stageIDs, err = stagesStorage.GetStagesIDsByDigest(ctx, m.ProjectName, stageDigest, parentStageCreationTs, opts...)
822822
if err != nil {
823823
return fmt.Errorf("error getting project %s stage %s images from storage: %w", m.StagesStorage.String(), stageDigest, err)
824824
}

‎pkg/storage/repo_stages_storage.go‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,7 @@ func (storage *RepoStagesStorage) DeleteRepo(ctx context.Context) error {
192192
return storage.DockerRegistry.DeleteRepo(ctx, storage.RepoAddress)
193193
}
194194

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

198198
o := makeOptions(opts...)
@@ -237,6 +237,9 @@ func (storage *RepoStagesStorage) GetStagesIDsByDigest(ctx context.Context, _, d
237237
continue
238238
}
239239
return nil, fmt.Errorf("unable to get digest and creation timestamp from tag %q: %w", tag, err)
240+
} else if parentStageCreationTs > creationTs {
241+
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)
242+
continue
240243
} else {
241244
stageID := image.NewStageID(digest, creationTs)
242245

‎pkg/storage/stages_storage.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ type FilterStagesAndProcessRelatedDataOptions struct {
3434

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

0 commit comments

Comments
 (0)