From 04a8757de9ffff465a6d8d7123756cb57798009e Mon Sep 17 00:00:00 2001 From: Pasquale Congiusti Date: Tue, 28 May 2024 15:27:12 +0200 Subject: [PATCH] fix(builder): add root and base image to S2I report --- .../ROOT/partials/apis/camel-k-crds.adoc | 9 +- helm/camel-k/crds/crd-build.yaml | 27 +++++ pkg/apis/camel/v1/build_types.go | 5 +- pkg/apis/camel/v1/zz_generated.deepcopy.go | 1 + pkg/builder/builder_test.go | 98 ++++++++++++++++++- pkg/builder/jib.go | 25 ++--- pkg/builder/s2i.go | 4 +- pkg/builder/spectrum.go | 27 ++--- pkg/builder/util.go | 17 ++++ .../applyconfiguration/camel/v1/s2itask.go | 30 +++++- .../crd/bases/camel.apache.org_builds.yaml | 27 +++++ pkg/trait/builder.go | 4 + pkg/trait/builder_test.go | 20 ++++ 13 files changed, 244 insertions(+), 50 deletions(-) diff --git a/docs/modules/ROOT/partials/apis/camel-k-crds.adoc b/docs/modules/ROOT/partials/apis/camel-k-crds.adoc index b11fe645ae..195ada4f87 100644 --- a/docs/modules/ROOT/partials/apis/camel-k-crds.adoc +++ b/docs/modules/ROOT/partials/apis/camel-k-crds.adoc @@ -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. @@ -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 diff --git a/helm/camel-k/crds/crd-build.yaml b/helm/camel-k/crds/crd-build.yaml index 31635f79dd..44a2cb99d2 100644 --- a/helm/camel-k/crds/crd-build.yaml +++ b/helm/camel-k/crds/crd-build.yaml @@ -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. @@ -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 diff --git a/pkg/apis/camel/v1/build_types.go b/pkg/apis/camel/v1/build_types.go index 7c1dc4e43b..273482e22e 100644 --- a/pkg/apis/camel/v1/build_types.go +++ b/pkg/apis/camel/v1/build_types.go @@ -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"` } diff --git a/pkg/apis/camel/v1/zz_generated.deepcopy.go b/pkg/apis/camel/v1/zz_generated.deepcopy.go index 8d1e9dc134..c1e2625c07 100644 --- a/pkg/apis/camel/v1/zz_generated.deepcopy.go +++ b/pkg/apis/camel/v1/zz_generated.deepcopy.go @@ -2866,6 +2866,7 @@ func (in *RuntimeSpec) DeepCopy() *RuntimeSpec { func (in *S2iTask) DeepCopyInto(out *S2iTask) { *out = *in in.BaseTask.DeepCopyInto(&out.BaseTask) + out.PublishTask = in.PublishTask } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new S2iTask. diff --git a/pkg/builder/builder_test.go b/pkg/builder/builder_test.go index 1f8c643b8b..dd12778634 100644 --- a/pkg/builder/builder_test.go +++ b/pkg/builder/builder_test.go @@ -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) @@ -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) +} diff --git a/pkg/builder/jib.go b/pkg/builder/jib.go index 8285c907ec..1c3255ade8 100644 --- a/pkg/builder/jib.go +++ b/pkg/builder/jib.go @@ -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 == "" { @@ -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 != "" { @@ -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, ",") @@ -154,7 +143,7 @@ func (t *jibTask) Do(ctx context.Context) v1.BuildStatus { } } - return status + return *status } func cleanRegistryConfig(registryConfigDir string) error { diff --git a/pkg/builder/s2i.go b/pkg/builder/s2i.go index 718e9eb05b..e88387bfaf 100644 --- a/pkg/builder/s2i.go +++ b/pkg/builder/s2i.go @@ -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{ @@ -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 { diff --git a/pkg/builder/spectrum.go b/pkg/builder/spectrum.go index dbe70d7b5f..ffc9ca8818 100644 --- a/pkg/builder/spectrum.go +++ b/pkg/builder/spectrum.go @@ -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 == "" { @@ -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 @@ -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, @@ -149,7 +138,7 @@ func (t *spectrumTask) Do(ctx context.Context) v1.BuildStatus { } } - return status + return *status } func readSpectrumLogs(newStdOut io.Reader) { diff --git a/pkg/builder/util.go b/pkg/builder/util.go index aad11bc3a0..721306f6f8 100644 --- a/pkg/builder/util.go +++ b/pkg/builder/util.go @@ -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 +} diff --git a/pkg/client/camel/applyconfiguration/camel/v1/s2itask.go b/pkg/client/camel/applyconfiguration/camel/v1/s2itask.go index da1a36d56f..820fb4b352 100644 --- a/pkg/client/camel/applyconfiguration/camel/v1/s2itask.go +++ b/pkg/client/camel/applyconfiguration/camel/v1/s2itask.go @@ -22,9 +22,9 @@ package v1 // S2iTaskApplyConfiguration represents an declarative configuration of the S2iTask type for use // with apply. type S2iTaskApplyConfiguration struct { - BaseTaskApplyConfiguration `json:",inline"` - ContextDir *string `json:"contextDir,omitempty"` - Tag *string `json:"tag,omitempty"` + BaseTaskApplyConfiguration `json:",inline"` + PublishTaskApplyConfiguration `json:",inline"` + Tag *string `json:"tag,omitempty"` } // S2iTaskApplyConfiguration constructs an declarative configuration of the S2iTask type for use with @@ -57,6 +57,30 @@ func (b *S2iTaskApplyConfiguration) WithContextDir(value string) *S2iTaskApplyCo return b } +// WithBaseImage sets the BaseImage field in the declarative configuration to the given value +// and returns the receiver, so that objects can be built by chaining "With" function invocations. +// If called multiple times, the BaseImage field is set to the value of the last call. +func (b *S2iTaskApplyConfiguration) WithBaseImage(value string) *S2iTaskApplyConfiguration { + b.BaseImage = &value + return b +} + +// WithImage sets the Image field in the declarative configuration to the given value +// and returns the receiver, so that objects can be built by chaining "With" function invocations. +// If called multiple times, the Image field is set to the value of the last call. +func (b *S2iTaskApplyConfiguration) WithImage(value string) *S2iTaskApplyConfiguration { + b.Image = &value + return b +} + +// WithRegistry sets the Registry field in the declarative configuration to the given value +// and returns the receiver, so that objects can be built by chaining "With" function invocations. +// If called multiple times, the Registry field is set to the value of the last call. +func (b *S2iTaskApplyConfiguration) WithRegistry(value *RegistrySpecApplyConfiguration) *S2iTaskApplyConfiguration { + b.Registry = value + return b +} + // WithTag sets the Tag field in the declarative configuration to the given value // and returns the receiver, so that objects can be built by chaining "With" function invocations. // If called multiple times, the Tag field is set to the value of the last call. diff --git a/pkg/resources/config/crd/bases/camel.apache.org_builds.yaml b/pkg/resources/config/crd/bases/camel.apache.org_builds.yaml index 31635f79dd..44a2cb99d2 100644 --- a/pkg/resources/config/crd/bases/camel.apache.org_builds.yaml +++ b/pkg/resources/config/crd/bases/camel.apache.org_builds.yaml @@ -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. @@ -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 diff --git a/pkg/trait/builder.go b/pkg/trait/builder.go index d3c20dc083..fb92721128 100644 --- a/pkg/trait/builder.go +++ b/pkg/trait/builder.go @@ -292,6 +292,10 @@ func (t *builderTrait) Apply(e *Environment) error { Name: "s2i", Configuration: *taskConfOrDefault(tasksConf, "s2i"), }, + PublishTask: v1.PublishTask{ + BaseImage: t.getBaseImage(e), + Image: imageName, + }, Tag: e.IntegrationKit.ResourceVersion, }}) } diff --git a/pkg/trait/builder_test.go b/pkg/trait/builder_test.go index dd831b6409..fc6b18b56b 100644 --- a/pkg/trait/builder_test.go +++ b/pkg/trait/builder_test.go @@ -90,6 +90,25 @@ func TestS2IBuilderTrait(t *testing.T) { assert.NotNil(t, env.Pipeline[0].Builder) assert.NotNil(t, env.Pipeline[1].Package) assert.NotNil(t, env.Pipeline[2].S2i) + assert.Equal(t, "root-jdk-image", env.Pipeline[2].S2i.BaseImage) + assert.Empty(t, env.Pipeline[2].S2i.Registry) +} + +func TestJibBuilderTrait(t *testing.T) { + env := createBuilderTestEnv(v1.IntegrationPlatformClusterOpenShift, v1.IntegrationPlatformBuildPublishStrategyJib, v1.BuildStrategyRoutine) + conditions, err := NewBuilderTestCatalog().apply(env) + + require.NoError(t, err) + assert.NotEmpty(t, conditions) + assert.NotEmpty(t, env.ExecutedTraits) + assert.NotNil(t, env.GetTrait("builder")) + assert.NotEmpty(t, env.Pipeline) + assert.Len(t, env.Pipeline, 3) + assert.NotNil(t, env.Pipeline[0].Builder) + assert.NotNil(t, env.Pipeline[1].Package) + assert.NotNil(t, env.Pipeline[2].Jib) + assert.Equal(t, "root-jdk-image", env.Pipeline[2].Jib.BaseImage) + assert.NotEmpty(t, env.Pipeline[2].Jib.Registry) } func createBuilderTestEnv(cluster v1.IntegrationPlatformCluster, strategy v1.IntegrationPlatformBuildPublishStrategy, buildStrategy v1.BuildStrategy) *Environment { @@ -134,6 +153,7 @@ func createBuilderTestEnv(cluster v1.IntegrationPlatformCluster, strategy v1.Int BuildConfiguration: v1.BuildConfiguration{ Strategy: buildStrategy, }, + BaseImage: "root-jdk-image", }, }, Status: v1.IntegrationPlatformStatus{