From a6fff152278424e5a8fa037d5ef929ce60bacb03 Mon Sep 17 00:00:00 2001 From: Danail Branekov Date: Thu, 29 Jun 2023 15:55:41 +0000 Subject: [PATCH] Build workloads do not own custom kpack builders via ControllerReference Custom kpack builders (the ones that are created on the fly for build workloads with buildpacks specified) are shared by build workloads that have the same set of buildpacks. Therefore, setting ControllerReference from the build workload to the builder is incorrect as the ControllerReference can only be set once. Issue: https://github.com/cloudfoundry/korifi/issues/2645 --- .../controllers/buildworkload_controller.go | 2 +- .../buildworkload_controller_test.go | 36 +++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/kpack-image-builder/controllers/buildworkload_controller.go b/kpack-image-builder/controllers/buildworkload_controller.go index eac4b7357..791713eb8 100644 --- a/kpack-image-builder/controllers/buildworkload_controller.go +++ b/kpack-image-builder/controllers/buildworkload_controller.go @@ -466,7 +466,7 @@ func (r *BuildWorkloadReconciler) ensureKpackBuilder(ctx context.Context, log lo }, } _, err = ctrl.CreateOrUpdate(ctx, r.k8sClient, builder, func() error { - if err = controllerutil.SetControllerReference(buildWorkload, builder, r.scheme); err != nil { + if err = controllerutil.SetOwnerReference(buildWorkload, builder, r.scheme); err != nil { log.Info("unable to set owner reference on Builder", "reason", err) return err } diff --git a/kpack-image-builder/controllers/buildworkload_controller_test.go b/kpack-image-builder/controllers/buildworkload_controller_test.go index c2e38b971..d067abb93 100644 --- a/kpack-image-builder/controllers/buildworkload_controller_test.go +++ b/kpack-image-builder/controllers/buildworkload_controller_test.go @@ -258,6 +258,7 @@ var _ = Describe("BuildWorkloadReconciler", func() { g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(builder), builder)).To(Succeed()) g.Expect(builder.OwnerReferences).To(HaveLen(1)) g.Expect(builder.OwnerReferences[0].Name).To(Equal(buildWorkload.Name)) + g.Expect(builder.OwnerReferences[0].Controller).To(BeNil()) g.Expect(builder.Spec.Tag).To(Equal("my.repository/my-prefix/builders-" + builderName)) g.Expect(builder.Spec.Stack).To(Equal(clusterBuilder.Spec.Stack)) @@ -286,6 +287,41 @@ var _ = Describe("BuildWorkloadReconciler", func() { }).Should(Succeed()) }) + When("there is another buildworkload referencing the same buildpacks", func() { + var ( + anotherBuildworkloadGUID string + sharedBuilder *buildv1alpha2.Builder + ) + + BeforeEach(func() { + anotherBuildworkloadGUID = PrefixedGUID("another-buildworkload-") + anotherBuildWorkload := buildWorkloadObject(anotherBuildworkloadGUID, namespaceGUID, source, env, services, reconcilerName, buildpacks) + Expect(k8sClient.Create(ctx, anotherBuildWorkload)).To(Succeed()) + + sharedBuilder = &buildv1alpha2.Builder{ + ObjectMeta: metav1.ObjectMeta{ + Name: controllers.ComputeBuilderName(anotherBuildWorkload.Spec.Buildpacks), + Namespace: namespaceGUID, + }, + } + Eventually(func(g Gomega) { + g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(sharedBuilder), sharedBuilder)).To(Succeed()) + }).Should(Succeed()) + }) + + It("shares the kpack builder", func() { + Eventually(func(g Gomega) { + g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(sharedBuilder), sharedBuilder)).To(Succeed()) + g.Expect(sharedBuilder.OwnerReferences).To(HaveLen(2)) + g.Expect( + []string{sharedBuilder.OwnerReferences[0].Name, sharedBuilder.OwnerReferences[1].Name}, + ).To(ConsistOf( + []string{anotherBuildworkloadGUID, buildWorkloadGUID}, + )) + }).Should(Succeed()) + }) + }) + When("a buildpack isn't in the default ClusterBuilder", func() { BeforeEach(func() { buildpacks = append(buildpacks, "not/in-the-store")