chore: concurrent image build with context cancellation

[KollaAdithya]

Finally with this PR we can build container images in parallel.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the Apache 2.0 License.

[github-actions]

🍕 Here are the new binary sizes!

Name	New size (kiB)	size (kiB)	Delta (%)
macOS (amd)	50536	50328	+0.41
macOS (arm)	50724	50516	+0.41
linux (amd)	44484	44304	+0.41
linux (arm)	42820	42564	+0.60
windows (amd)	41368	41204	+0.40

[codecov-commenter]

Codecov Report

Merging #4760 (bfd918c) into mainline (ad57838) will decrease coverage by 0.01%.
The diff coverage is 77.50%.

@@             Coverage Diff              @@
##           mainline    #4760      +/-   ##
============================================
- Coverage     69.91%   69.91%   -0.01%     
============================================
  Files           284      284              
  Lines         40694    40795     +101     
  Branches        272      272              
============================================
+ Hits          28450    28520      +70     
- Misses        10869    10892      +23     
- Partials       1375     1383       +8     

Impacted Files	Coverage Δ
internal/pkg/cli/task_run.go	53.84% <0.00%> (ø)
internal/pkg/term/syncbuffer/termprinter.go	50.52% <60.00%> (+1.06%)	⬆️
internal/pkg/cli/deploy/workload.go	59.64% <71.15%> (+0.82%)	⬆️
internal/pkg/docker/dockerengine/dockerengine.go	79.89% <100.00%> (ø)
...ernal/pkg/docker/dockerengine/mock_dockerengine.go	100.00% <100.00%> (ø)
internal/pkg/repository/repository.go	69.23% <100.00%> (ø)
internal/pkg/term/syncbuffer/syncbuffer.go	92.85% <100.00%> (ø)

... and 5 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[paragbhingre]

This is really cool 😎

[Lou1415926]

What do you think of this

// In cli/deploy/workload.go
type termPrinter interface { // And use make gen-mocks to generate a mock for unit testing
  IsDone() bool
  Print() error
}
type workloadDeployer struct {
  newTermPrinter (fw FileWriter, bufs []*LabeledSyncBuffer, opts ...LabeledTermPrinterOption) termPrinter
}
func newWorkloadDeployer(in *WorkloadDeployerInput) (*workloadDeployer, error) {
    return &workloadDeployer{
        newTermPrinter: syncbuffer.NewLabeledTermPrinter,
        }
    }
}

func (d *workloadDeployer) uploadContainerImages(out *UploadArtifactsOutput) error {
   //...
   ltp := d.newTermPrinter(os.Stderr, labeledBuffers, syncbuffer.WithNumLines(numLinesForBuildAndPush), syncbuffer.WithPadding(paddingForBuildAndPush))
   //...
}

// In cli/deploy/workload_test.go
type deployMocks struct {
   //...
  mockTermPrinter *mocks.mockTermPrinter
}

func TestWorkloadDeployer_UploadArtifacts(t *testing.T) {
     m := &deployMocks{
		mockTermPrinter: mocks.NewMockTermPrinter(ctril)
      }
      m.mockTermPrinter.Expect().Print().Return(nil)
      wkldDeployer := &workloadDeployer{
           newTermPrinter: func(fw FileWriter, bufs []*LabeledSyncBuffer, opts ...LabeledTermPrinterOption) termPrinter {
            return m.mockTermPrinter
          }
       }
}

The reason why it is so hard to write unit test for UploadArtifacts is that the constructor for term printer is currently called within uploadContainerImages(). However, in order to properly test UploadArtifacts, we need to sub the labeled term printer with a mock. The solution is to mock the constructor function call - in our case newTermPrinter.

[Lou1415926]

nit:

Suggested change

	labeledTermPrinter: func(fw syncbuffer.FileWriter, bufs []*syncbuffer.LabeledSyncBuffer, opts ...syncbuffer.LabeledTermPrinterOption) labeledTermPrinter {
	labeledTermPrinter: syncbuffer.NewLabeledTermPrinter,

[KollaAdithya]

This will be aIncompatibleAssign 🤔 . if I directly use syncbuffer.NewLabeledTermPrinter 
because the the function signature of the labeledTermPrinter field in workloadDeployer struct does not match the signature of syncbuffer.NewLabeledTermPrinter function.

[efekarakus]

with a small tiny test adjustment request

[dannyrandall]

Suggested change

	digestsMu.Lock()
	digestsMu.Lock()
	defer digestsMu.Unlock()

[KollaAdithya]

is it good to unlock the map before return nil? I mean if I use defer then i will unlock after return is executed.

[dannyrandall]

looks great! good work ☺️🍾

[dannyrandall]

Suggested change

	time.Sleep(pollIntervalForBuildAndPush)
	select {
	case <-ctx.Done():
	return nil
	case <-time.After(pollIntervalForBuildAndPush):
	// loop again
	}

so this function stops if ctx is cancelled!