Skip to content

Commit

Permalink
osbuild1: make the conversion from v2 result stable
Browse files Browse the repository at this point in the history
A result from manifest v2 contains logs from pipelines. The individual
pipelines are stored as an object, thus they have no order. Well, at least
in Go, because it doesn't guarantee one when parsing maps, see:

golang/go#27179

Unfortunately, this makes the Result.fromV2 method return unpredictable
results because the pipeline results are processed in basically a random
order.

This caused the TestUnmarshalV2Failure test (result_test.go:124) to randomly
fail because it expects the ordering to be stable. I decided to fix this by
ordering the pipeline results by their name. When this fix is applied, the
output from Result.fromV2 is well-defined.

Signed-off-by: Ondřej Budai <ondrej@budai.cz>
  • Loading branch information
ondrejbudai authored and achilleas-k committed Apr 26, 2021
1 parent 9a15c20 commit 9b5a661
Showing 1 changed file with 33 additions and 3 deletions.
36 changes: 33 additions & 3 deletions internal/osbuild1/result.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"encoding/json"
"fmt"
"io"
"sort"

"github.com/osbuild/osbuild-composer/internal/osbuild2"
)
Expand Down Expand Up @@ -197,13 +198,42 @@ func (cr *Result) fromV2(crv2 osbuild2.Result) {
cr.Build = new(buildResult)
cr.Assembler = new(rawAssemblerResult)

// crv2.Log contains a map of pipelines. Unfortunately, Go doesn't
// preserve the order of keys in a map. See:
// https://github.com/golang/go/issues/27179
//
// I think it makes sense for this function to always return
// a well-defined output, therefore we need to invent an ordering
// for pipeline results. Otherwise, the ordering is basically random.
//
// The following lines convert the map of pipeline results to an array
// of pipeline results. In the last step, the array is sorted by
// the pipeline name. This isn't ideal but at least it's predictable.
//
// See: https://github.com/osbuild/osbuild/issues/619
type pipelineResult struct {
pipelineName string
stageResults []osbuild2.StageResult
}

var pipelineResults []pipelineResult

for pname, stageResults := range crv2.Log {
pipelineResults = append(pipelineResults, pipelineResult{pipelineName: pname, stageResults: stageResults})
}

// Sort the pipelineResult array by the pipeline name to ensure a stable order.
sort.Slice(pipelineResults, func(i, j int) bool {
return pipelineResults[i].pipelineName < pipelineResults[j].pipelineName
})

// convert all stages logs from all pipelines into v1 StageResult objects
for pname, stages := range crv2.Log {
for idx, stage := range stages {
for _, pr := range pipelineResults {
for idx, stage := range pr.stageResults {
stageResult := StageResult{
// Create uniquely identifiable name for the stage:
// <pipeline name>:<stage index>-<stage type>
Name: fmt.Sprintf("%s:%d-%s", pname, idx, stage.Type),
Name: fmt.Sprintf("%s:%d-%s", pr.pipelineName, idx, stage.Type),
Success: stage.Success,
Output: stage.Output,
}
Expand Down

0 comments on commit 9b5a661

Please sign in to comment.