From bac85e14a9b2f350881a4c7daf80818afa6f33a4 Mon Sep 17 00:00:00 2001 From: Gaelle Fournier Date: Fri, 26 May 2023 17:33:35 +0200 Subject: [PATCH] feat(core): Refactoring S2I BC and IS build code Ref #4297 --- pkg/builder/s2i.go | 28 ++------------ pkg/controller/catalog/initialize.go | 58 +++++++++------------------- pkg/util/s2i/build.go | 38 ++++++++++++++++++ 3 files changed, 60 insertions(+), 64 deletions(-) diff --git a/pkg/builder/s2i.go b/pkg/builder/s2i.go index 043ba70cf2..da6a66b36d 100644 --- a/pkg/builder/s2i.go +++ b/pkg/builder/s2i.go @@ -31,7 +31,6 @@ import ( "strings" corev1 "k8s.io/api/core/v1" - apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime/schema" @@ -39,7 +38,6 @@ import ( ctrl "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/apiutil" - ctrlutil "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" buildv1 "github.com/openshift/api/build/v1" imagev1 "github.com/openshift/api/image/v1" @@ -90,11 +88,6 @@ func (t *s2iTask) Do(ctx context.Context) v1.BuildStatus { }, } - err := t.c.Delete(ctx, bc) - if err != nil && !apierrors.IsNotFound(err) { - return status.Failed(fmt.Errorf("cannot delete build config: %w", err)) - } - // Set the build controller as owner reference owner := t.getControllerReference() if owner == nil { @@ -102,13 +95,9 @@ func (t *s2iTask) Do(ctx context.Context) v1.BuildStatus { owner = t.build } - if err := ctrlutil.SetOwnerReference(owner, bc, t.c.GetScheme()); err != nil { - return status.Failed(fmt.Errorf("cannot set owner reference on BuildConfig: %s: %w", bc.Name, err)) - } - - err = t.c.Create(ctx, bc) + err := s2i.BuildConfig(ctx, t.c, bc, owner) if err != nil { - return status.Failed(fmt.Errorf("cannot create build config: %w", err)) + return status.Failed(err) } is := &imagev1.ImageStream{ @@ -128,18 +117,9 @@ func (t *s2iTask) Do(ctx context.Context) v1.BuildStatus { }, } - err = t.c.Delete(ctx, is) - if err != nil && !apierrors.IsNotFound(err) { - return status.Failed(fmt.Errorf("cannot delete image stream: %w", err)) - } - - if err := ctrlutil.SetOwnerReference(owner, is, t.c.GetScheme()); err != nil { - return status.Failed(fmt.Errorf("cannot set owner reference on ImageStream: %s: %w", is.Name, err)) - } - - err = t.c.Create(ctx, is) + err = s2i.ImageStream(ctx, t.c, is, owner) if err != nil { - return status.Failed(fmt.Errorf("cannot create image stream: %w", err)) + return status.Failed(err) } err = util.WithTempDir(t.build.Name+"-s2i-", func(tmpDir string) error { diff --git a/pkg/controller/catalog/initialize.go b/pkg/controller/catalog/initialize.go index 40a0618abb..64947d9de3 100644 --- a/pkg/controller/catalog/initialize.go +++ b/pkg/controller/catalog/initialize.go @@ -39,15 +39,20 @@ import ( "github.com/apache/camel-k/v2/pkg/util" "github.com/apache/camel-k/v2/pkg/util/kubernetes" "github.com/apache/camel-k/v2/pkg/util/s2i" + spectrum "github.com/container-tools/spectrum/pkg/builder" gcrv1 "github.com/google/go-containerregistry/pkg/v1" + buildv1 "github.com/openshift/api/build/v1" imagev1 "github.com/openshift/api/image/v1" + corev1 "k8s.io/api/core/v1" k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/runtime/serializer" + ctrl "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/apiutil" ) @@ -175,6 +180,8 @@ func initializeS2i(ctx context.Context, c client.Client, ip *v1.IntegrationPlatf ADD /usr/share/maven/mvnw/ /usr/share/maven/mvnw/ `)) + owner := catalogReference(catalog) + // BuildConfig bc := &buildv1.BuildConfig{ TypeMeta: metav1.TypeMeta{ @@ -192,14 +199,6 @@ func initializeS2i(ctx context.Context, c client.Client, ip *v1.IntegrationPlatf "camel.apache.org/runtime.version": catalog.Spec.Runtime.Version, "camel.apache.org/runtime.provider": string(catalog.Spec.Runtime.Provider), }, - OwnerReferences: []metav1.OwnerReference{ - { - APIVersion: catalog.APIVersion, - Kind: catalog.Kind, - Name: catalog.Name, - UID: catalog.UID, - }, - }, }, Spec: buildv1.BuildConfigSpec{ CommonSpec: buildv1.CommonSpec{ @@ -236,14 +235,6 @@ func initializeS2i(ctx context.Context, c client.Client, ip *v1.IntegrationPlatf kubernetes.CamelCreatorLabelVersion: catalog.ResourceVersion, "camel.apache.org/runtime.provider": string(catalog.Spec.Runtime.Provider), }, - OwnerReferences: []metav1.OwnerReference{ - { - APIVersion: catalog.APIVersion, - Kind: catalog.Kind, - Name: catalog.Name, - UID: catalog.UID, - }, - }, }, Spec: imagev1.ImageStreamSpec{ LookupPolicy: imagev1.ImageLookupPolicy{ @@ -264,18 +255,7 @@ func initializeS2i(ctx context.Context, c client.Client, ip *v1.IntegrationPlatf return target, nil } - err := c.Delete(ctx, bc) - if err != nil && !k8serrors.IsNotFound(err) { - target.Status.Phase = v1.CamelCatalogPhaseError - target.Status.SetErrorCondition( - v1.CamelCatalogConditionReady, - "Builder Image", - err, - ) - return target, err - } - - err = c.Create(ctx, bc) + err := s2i.BuildConfig(ctx, c, bc, owner) if err != nil { target.Status.Phase = v1.CamelCatalogPhaseError target.Status.SetErrorCondition( @@ -286,18 +266,7 @@ func initializeS2i(ctx context.Context, c client.Client, ip *v1.IntegrationPlatf return target, err } - err = c.Delete(ctx, is) - if err != nil && !k8serrors.IsNotFound(err) { - target.Status.Phase = v1.CamelCatalogPhaseError - target.Status.SetErrorCondition( - v1.CamelCatalogConditionReady, - "Builder Image", - err, - ) - return target, err - } - - err = c.Create(ctx, is) + err = s2i.ImageStream(ctx, c, is, owner) if err != nil { target.Status.Phase = v1.CamelCatalogPhaseError target.Status.SetErrorCondition( @@ -569,3 +538,12 @@ func tarEntries(writer io.Writer, files ...string) error { } return nil } + +func catalogReference(catalog *v1.CamelCatalog) *unstructured.Unstructured { + owner := &unstructured.Unstructured{} + owner.SetName(catalog.Name) + owner.SetUID(catalog.UID) + owner.SetAPIVersion(catalog.APIVersion) + owner.SetKind(catalog.Kind) + return owner +} diff --git a/pkg/util/s2i/build.go b/pkg/util/s2i/build.go index d30207d59d..82e68d40eb 100644 --- a/pkg/util/s2i/build.go +++ b/pkg/util/s2i/build.go @@ -21,12 +21,18 @@ package s2i import ( "context" "errors" + "fmt" "time" "github.com/apache/camel-k/v2/pkg/client" buildv1 "github.com/openshift/api/build/v1" + imagev1 "github.com/openshift/api/image/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + ctrl "sigs.k8s.io/controller-runtime/pkg/client" + ctrlutil "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" ) // Cancel the s2i Build by updating its status. @@ -68,3 +74,35 @@ func WaitForS2iBuildCompletion(ctx context.Context, c client.Client, build *buil } } } + +// Create the BuildConfig of the build with the right owner after having deleted it it already existed. +func BuildConfig(ctx context.Context, c client.Client, bc *buildv1.BuildConfig, owner metav1.Object) error { + if err := c.Delete(ctx, bc); err != nil && !apierrors.IsNotFound(err) { + return fmt.Errorf("cannot delete build config: %w", err) + } + + if err := ctrlutil.SetOwnerReference(owner, bc, c.GetScheme()); err != nil { + return fmt.Errorf("cannot set owner reference on BuildConfig: %s: %w", bc.Name, err) + } + + if err := c.Create(ctx, bc); err != nil { + return fmt.Errorf("cannot create build config: %w", err) + } + return nil +} + +// Create the ImageStream for the builded image with the right owner after having deleted it it already existed. +func ImageStream(ctx context.Context, c client.Client, is *imagev1.ImageStream, owner metav1.Object) error { + if err := c.Delete(ctx, is); err != nil && !apierrors.IsNotFound(err) { + return fmt.Errorf("cannot delete image stream: %w", err) + } + + if err := ctrlutil.SetOwnerReference(owner, is, c.GetScheme()); err != nil { + return fmt.Errorf("cannot set owner reference on ImageStream: %s: %w", is.Name, err) + } + + if err := c.Create(ctx, is); err != nil { + return fmt.Errorf("cannot create image stream: %w", err) + } + return nil +}