Skip to content

Commit

Permalink
Build workloads do not own custom kpack builders via ControllerReference
Browse files Browse the repository at this point in the history
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: #2645
  • Loading branch information
danail-branekov authored and davewalter committed Jun 29, 2023
1 parent 5275330 commit a6fff15
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
36 changes: 36 additions & 0 deletions kpack-image-builder/controllers/buildworkload_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down Expand Up @@ -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")
Expand Down

0 comments on commit a6fff15

Please sign in to comment.