Skip to content

Commit

Permalink
Fix Artifact type to a pointer
Browse files Browse the repository at this point in the history
Prior to this, the Artifacts in TaskRunStatusFields was not set to
a pointer of the underlying struct. As a result, it was incompatible
with `tkn`. Here we fix that issue by updating it to a `pointer`.
  • Loading branch information
chitrangpatel authored and tekton-robot committed Aug 30, 2024
1 parent a3bd23e commit c910594
Show file tree
Hide file tree
Showing 19 changed files with 184 additions and 118 deletions.
77 changes: 40 additions & 37 deletions pkg/apis/pipeline/v1/artifact_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,29 +52,30 @@ type Artifacts struct {
Outputs []Artifact `json:"outputs,omitempty"`
}

func (a *Artifacts) Merge(another Artifacts) {
func (a *Artifacts) Merge(another *Artifacts) {
inputMap := make(map[string][]ArtifactValue)
var newInputs []Artifact

for _, v := range a.Inputs {
inputMap[v.Name] = v.Values
}

for _, v := range another.Inputs {
_, ok := inputMap[v.Name]
if !ok {
inputMap[v.Name] = []ArtifactValue{}
}
for _, vv := range v.Values {
exists := false
for _, av := range inputMap[v.Name] {
if cmp.Equal(vv, av) {
exists = true
break
}
if another != nil {
for _, v := range another.Inputs {
_, ok := inputMap[v.Name]
if !ok {
inputMap[v.Name] = []ArtifactValue{}
}
if !exists {
inputMap[v.Name] = append(inputMap[v.Name], vv)
for _, vv := range v.Values {
exists := false
for _, av := range inputMap[v.Name] {
if cmp.Equal(vv, av) {
exists = true
break
}
}
if !exists {
inputMap[v.Name] = append(inputMap[v.Name], vv)
}
}
}
}
Expand All @@ -92,31 +93,33 @@ func (a *Artifacts) Merge(another Artifacts) {
outputMap[v.Name] = v
}

for _, v := range another.Outputs {
_, ok := outputMap[v.Name]
if !ok {
outputMap[v.Name] = Artifact{Name: v.Name, Values: []ArtifactValue{}, BuildOutput: v.BuildOutput}
}
// only update buildOutput to true.
// Do not convert to false if it was true before.
if v.BuildOutput {
art := outputMap[v.Name]
art.BuildOutput = v.BuildOutput
outputMap[v.Name] = art
}
for _, vv := range v.Values {
exists := false
for _, av := range outputMap[v.Name].Values {
if cmp.Equal(vv, av) {
exists = true
break
}
if another != nil {
for _, v := range another.Outputs {
_, ok := outputMap[v.Name]
if !ok {
outputMap[v.Name] = Artifact{Name: v.Name, Values: []ArtifactValue{}, BuildOutput: v.BuildOutput}
}
if !exists {
// only update buildOutput to true.
// Do not convert to false if it was true before.
if v.BuildOutput {
art := outputMap[v.Name]
art.Values = append(art.Values, vv)
art.BuildOutput = v.BuildOutput
outputMap[v.Name] = art
}
for _, vv := range v.Values {
exists := false
for _, av := range outputMap[v.Name].Values {
if cmp.Equal(vv, av) {
exists = true
break
}
}
if !exists {
art := outputMap[v.Name]
art.Values = append(art.Values, vv)
outputMap[v.Name] = art
}
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/apis/pipeline/v1/artifact_types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ func TestArtifactsMerge(t *testing.T) {

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
tc.a1.Merge(tc.a2)
tc.a1.Merge(&tc.a2)
got := tc.a1
if d := cmp.Diff(tc.expected, got, cmpopts.SortSlices(func(a, b Artifact) bool { return a.Name > b.Name })); d != "" {
t.Errorf("TestArtifactsMerge() did not produce expected artifacts for test %s: %s", tc.name, diff.PrintWantGot(d))
Expand Down
2 changes: 0 additions & 2 deletions pkg/apis/pipeline/v1/openapi_generated.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 0 additions & 2 deletions pkg/apis/pipeline/v1/swagger.json
Original file line number Diff line number Diff line change
Expand Up @@ -2137,7 +2137,6 @@
},
"artifacts": {
"description": "Artifacts are the list of artifacts written out by the task's containers",
"default": {},
"$ref": "#/definitions/v1.Artifacts",
"x-kubernetes-list-type": "atomic"
},
Expand Down Expand Up @@ -2232,7 +2231,6 @@
"properties": {
"artifacts": {
"description": "Artifacts are the list of artifacts written out by the task's containers",
"default": {},
"$ref": "#/definitions/v1.Artifacts",
"x-kubernetes-list-type": "atomic"
},
Expand Down
2 changes: 1 addition & 1 deletion pkg/apis/pipeline/v1/taskrun_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ type TaskRunStatusFields struct {
// Artifacts are the list of artifacts written out by the task's containers
// +optional
// +listType=atomic
Artifacts Artifacts `json:"artifacts,omitempty"`
Artifacts *Artifacts `json:"artifacts,omitempty"`

// The list has one entry per sidecar in the manifest. Each entry is
// represents the imageid of the corresponding sidecar.
Expand Down
6 changes: 5 additions & 1 deletion pkg/apis/pipeline/v1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions pkg/pod/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ func setTaskRunStatusBasedOnStepStatus(ctx context.Context, logger *zap.SugaredL
logger.Errorf("Failed to set artifacts value from sidecar logs: %v", err)
merr = multierror.Append(merr, err)
} else {
trs.Artifacts = tras
trs.Artifacts = &tras
}
}

Expand Down Expand Up @@ -343,8 +343,8 @@ func setTaskRunStatusBasedOnStepStatus(ctx context.Context, logger *zap.SugaredL
logger.Errorf("error setting step artifacts in taskrun %q: %v", tr.Name, err)
merr = multierror.Append(merr, err)
}
trs.Artifacts.Merge(tras)
trs.Artifacts.Merge(sas)
trs.Artifacts.Merge(&tras)
trs.Artifacts.Merge(&sas)
}
msg, err = createMessageFromResults(filteredResults)
if err != nil {
Expand Down
Loading

0 comments on commit c910594

Please sign in to comment.