Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Release 2.3.x] fix(builder): add root and base image to S2I report #5556

Merged
merged 1 commit into from
May 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions docs/modules/ROOT/partials/apis/camel-k-crds.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -5029,6 +5029,7 @@ Properties -- .
* <<#_camel_apache_org_v1_BuildahTask, BuildahTask>>
* <<#_camel_apache_org_v1_JibTask, JibTask>>
* <<#_camel_apache_org_v1_KanikoTask, KanikoTask>>
* <<#_camel_apache_org_v1_S2iTask, S2iTask>>
* <<#_camel_apache_org_v1_SpectrumTask, SpectrumTask>>

PublishTask image publish configuration.
Expand Down Expand Up @@ -5338,12 +5339,12 @@ S2iTask is used to configure S2I.



|`contextDir` +
string
|
|`PublishTask` +
*xref:#_camel_apache_org_v1_PublishTask[PublishTask]*
|(Members of `PublishTask` are embedded into this type.)



can be useful to share info with other tasks

|`tag` +
string
Expand Down
27 changes: 27 additions & 0 deletions helm/camel-k/crds/crd-build.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1717,6 +1717,9 @@ spec:
s2i:
description: a S2iTask, for S2I strategy
properties:
baseImage:
description: base image layer
type: string
configuration:
description: The configuration that should be used to perform
the Build.
Expand Down Expand Up @@ -1781,9 +1784,33 @@ spec:
contextDir:
description: can be useful to share info with other tasks
type: string
image:
description: final image name
type: string
name:
description: name of the task
type: string
registry:
description: where to publish the final image
properties:
address:
description: the URI to access
type: string
ca:
description: the configmap which stores the Certificate
Authority
type: string
insecure:
description: if the container registry is insecure (ie,
http only)
type: boolean
organization:
description: the registry organization
type: string
secret:
description: the secret where credentials are stored
type: string
type: object
tag:
description: used by the ImageStream
type: string
Expand Down
5 changes: 2 additions & 3 deletions pkg/apis/camel/v1/build_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -175,9 +175,8 @@ type SpectrumTask struct {

// S2iTask is used to configure S2I.
type S2iTask struct {
BaseTask `json:",inline"`
// can be useful to share info with other tasks
ContextDir string `json:"contextDir,omitempty"`
BaseTask `json:",inline"`
PublishTask `json:",inline"`
// used by the ImageStream
Tag string `json:"tag,omitempty"`
}
Expand Down
1 change: 1 addition & 0 deletions pkg/apis/camel/v1/zz_generated.deepcopy.go

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

98 changes: 97 additions & 1 deletion pkg/builder/builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ type errorTestSteps struct {
Step2 Step
}

func TestFailure(t *testing.T) {
func TestBuilderFailure(t *testing.T) {
c, err := test.NewFakeClient()
require.NoError(t, err)

Expand Down Expand Up @@ -74,3 +74,99 @@ func TestFailure(t *testing.T) {
assert.Equal(t, v1.BuildPhaseFailed, status.Phase)
assert.Equal(t, "an error", status.Error)
}

func TestS2IPublishingFailure(t *testing.T) {
c, err := test.NewFakeClient()
require.NoError(t, err)
b := New(c)
build := &v1.Build{
Spec: v1.BuildSpec{
Tasks: []v1.Task{
{
S2i: &v1.S2iTask{
BaseTask: v1.BaseTask{
Name: "s2i",
},
PublishTask: v1.PublishTask{
BaseImage: "base-image",
},
},
},
},
},
Status: v1.BuildStatus{
RootImage: "root-image",
},
}

ctx := cancellable.NewContext()
status := b.Build(build).TaskByName("s2i").Do(ctx)
assert.Equal(t, v1.BuildPhaseFailed, status.Phase)
assert.NotEmpty(t, status.Error)
assert.Equal(t, "base-image", status.BaseImage)
assert.Equal(t, "root-image", status.RootImage)
}

func TestJibPublishingFailure(t *testing.T) {
c, err := test.NewFakeClient()
require.NoError(t, err)
b := New(c)
build := &v1.Build{
Spec: v1.BuildSpec{
Tasks: []v1.Task{
{
Jib: &v1.JibTask{
BaseTask: v1.BaseTask{
Name: "jib",
},
PublishTask: v1.PublishTask{
BaseImage: "base-image",
},
},
},
},
},
Status: v1.BuildStatus{
RootImage: "root-image",
},
}

ctx := cancellable.NewContext()
status := b.Build(build).TaskByName("jib").Do(ctx)
assert.Equal(t, v1.BuildPhaseFailed, status.Phase)
assert.NotEmpty(t, status.Error)
assert.Equal(t, "base-image", status.BaseImage)
assert.Equal(t, "root-image", status.RootImage)
}

func TestSpectrumPublishingFailure(t *testing.T) {
c, err := test.NewFakeClient()
require.NoError(t, err)
b := New(c)
build := &v1.Build{
Spec: v1.BuildSpec{
Tasks: []v1.Task{
{
Spectrum: &v1.SpectrumTask{
BaseTask: v1.BaseTask{
Name: "spectrum",
},
PublishTask: v1.PublishTask{
BaseImage: "base-image",
},
},
},
},
},
Status: v1.BuildStatus{
RootImage: "root-image",
},
}

ctx := cancellable.NewContext()
status := b.Build(build).TaskByName("spectrum").Do(ctx)
assert.Equal(t, v1.BuildPhaseFailed, status.Phase)
assert.NotEmpty(t, status.Error)
assert.Equal(t, "base-image", status.BaseImage)
assert.Equal(t, "root-image", status.RootImage)
}
25 changes: 7 additions & 18 deletions pkg/builder/jib.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,18 +43,7 @@ type jibTask struct {
var _ Task = &jibTask{}

func (t *jibTask) Do(ctx context.Context) v1.BuildStatus {
status := v1.BuildStatus{}

baseImage := t.build.Status.BaseImage
if baseImage == "" {
baseImage = t.task.BaseImage
}
status.BaseImage = baseImage
rootImage := t.build.Status.RootImage
if rootImage == "" {
rootImage = t.task.BaseImage
}
status.RootImage = rootImage
status := initializeStatusFrom(t.build.Status, t.task.BaseImage)

contextDir := t.task.ContextDir
if contextDir == "" {
Expand All @@ -80,14 +69,14 @@ func (t *jibTask) Do(ctx context.Context) v1.BuildStatus {
if !exists || empty {
// this can only indicate that there are no more resources to add to the base image,
// because transitive resolution is the same even if spec differs.
log.Infof("No new image to build, reusing existing image %s", baseImage)
status.Image = baseImage
return status
status.Image = status.BaseImage
log.Infof("No new image to build, reusing existing image %s", status.Image)
return *status
}
mavenDir := strings.ReplaceAll(contextDir, ContextDir, "maven")

log.Debugf("Registry address: %s", t.task.Registry.Address)
log.Debugf("Base image: %s", baseImage)
log.Debugf("Base image: %s", status.BaseImage)

registryConfigDir := ""
if t.task.Registry.Secret != "" {
Expand All @@ -109,7 +98,7 @@ func (t *jibTask) Do(ctx context.Context) v1.BuildStatus {
mavenArgs = append(mavenArgs, strings.Split(string(mavenCommand), " ")...)
mavenArgs = append(mavenArgs, "-P", "jib")
mavenArgs = append(mavenArgs, jib.JibMavenToImageParam+t.task.Image)
mavenArgs = append(mavenArgs, jib.JibMavenFromImageParam+baseImage)
mavenArgs = append(mavenArgs, jib.JibMavenFromImageParam+status.BaseImage)
mavenArgs = append(mavenArgs, jib.JibMavenBaseImageCache+mavenDir+"/jib")
if t.task.Configuration.ImagePlatforms != nil {
platforms := strings.Join(t.task.Configuration.ImagePlatforms, ",")
Expand Down Expand Up @@ -154,7 +143,7 @@ func (t *jibTask) Do(ctx context.Context) v1.BuildStatus {
}
}

return status
return *status
}

func cleanRegistryConfig(registryConfigDir string) error {
Expand Down
4 changes: 2 additions & 2 deletions pkg/builder/s2i.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ type s2iTask struct {
var _ Task = &s2iTask{}

func (t *s2iTask) Do(ctx context.Context) v1.BuildStatus {
status := v1.BuildStatus{}
status := initializeStatusFrom(t.build.Status, t.task.BaseImage)

bc := &buildv1.BuildConfig{
TypeMeta: metav1.TypeMeta{
Expand Down Expand Up @@ -220,7 +220,7 @@ func (t *s2iTask) Do(ctx context.Context) v1.BuildStatus {
return status.Failed(err)
}

return status
return *status
}

func (t *s2iTask) getControllerReference() metav1.Object {
Expand Down
27 changes: 8 additions & 19 deletions pkg/builder/spectrum.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,18 +44,7 @@ type spectrumTask struct {
var _ Task = &spectrumTask{}

func (t *spectrumTask) Do(ctx context.Context) v1.BuildStatus {
status := v1.BuildStatus{}

baseImage := t.build.Status.BaseImage
if baseImage == "" {
baseImage = t.task.BaseImage
}
status.BaseImage = baseImage
rootImage := t.build.Status.RootImage
if rootImage == "" {
rootImage = t.task.BaseImage
}
status.RootImage = rootImage
status := initializeStatusFrom(t.build.Status, t.task.BaseImage)

contextDir := t.task.ContextDir
if contextDir == "" {
Expand Down Expand Up @@ -83,17 +72,17 @@ func (t *spectrumTask) Do(ctx context.Context) v1.BuildStatus {
if !exists || empty {
// this can only indicate that there are no more resources to add to the base image,
// because transitive resolution is the same even if spec differs.
log.Infof("No new image to build, reusing existing image %s", baseImage)
status.Image = baseImage
return status
status.Image = status.BaseImage
log.Infof("No new image to build, reusing existing image %s", status.Image)
return *status
}

pullInsecure := t.task.Registry.Insecure // incremental build case

log.Debugf("Registry address: %s", t.task.Registry.Address)
log.Debugf("Base image: %s", baseImage)
log.Debugf("Base image: %s", status.BaseImage)

if !strings.HasPrefix(baseImage, t.task.Registry.Address) {
if !strings.HasPrefix(status.BaseImage, t.task.Registry.Address) {
if pullInsecure {
log.Info("Assuming secure pull because the registry for the base image and the main registry are different")
pullInsecure = false
Expand Down Expand Up @@ -122,7 +111,7 @@ func (t *spectrumTask) Do(ctx context.Context) v1.BuildStatus {
PushInsecure: t.task.Registry.Insecure,
PullConfigDir: registryConfigDir,
PushConfigDir: registryConfigDir,
Base: baseImage,
Base: status.BaseImage,
Target: t.task.Image,
Stdout: newStdW,
Stderr: newStdW,
Expand All @@ -149,7 +138,7 @@ func (t *spectrumTask) Do(ctx context.Context) v1.BuildStatus {
}
}

return status
return *status
}

func readSpectrumLogs(newStdOut io.Reader) {
Expand Down
17 changes: 17 additions & 0 deletions pkg/builder/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,20 @@ func artifactIDs(artifacts []v1.Artifact) []string {

return result
}

// initializeStatusFrom helps creating a BuildStatus from scratch filling with base and root images.
func initializeStatusFrom(buildStatus v1.BuildStatus, taskBaseImage string) *v1.BuildStatus {
status := v1.BuildStatus{}
baseImage := buildStatus.BaseImage
if baseImage == "" {
baseImage = taskBaseImage
}
status.BaseImage = baseImage
rootImage := buildStatus.RootImage
if rootImage == "" {
rootImage = taskBaseImage
}
status.RootImage = rootImage

return &status
}
30 changes: 27 additions & 3 deletions pkg/client/camel/applyconfiguration/camel/v1/s2itask.go

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

Loading
Loading