Skip to content

Commit 39901c5

Browse files
committed
Verify Toktonconfig state before making Build 'Ready'
retest Signed-off-by: Hasan Awad <hasan.m.awad94@gmail.com> sign Signed-off-by: Hasan Awad <hasan.m.awad94@gmail.com>
1 parent 7c692d8 commit 39901c5

File tree

5 files changed

+271
-15
lines changed

5 files changed

+271
-15
lines changed

controllers/buildstrategies_test.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ var _ = Describe("Install embedded build strategies", func() {
1919

2020
BeforeEach(func(ctx SpecContext) {
2121
setupTektonCRDs(ctx)
22+
createTektonConfig(ctx)
2223
build = createShipwrightBuild(ctx, "shipwright")
2324
test.CRDEventuallyExists(ctx, k8sClient, "clusterbuildstrategies.shipwright.io")
2425
})
@@ -43,6 +44,7 @@ var _ = Describe("Install embedded build strategies", func() {
4344

4445
AfterEach(func(ctx SpecContext) {
4546
deleteShipwrightBuild(ctx, build)
47+
deleteTektonConfig(ctx)
4648
})
4749

4850
})

controllers/default_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,12 +47,13 @@ var _ = g.Describe("Reconcile default ShipwrightBuild installation", func() {
4747
g.BeforeEach(func(ctx g.SpecContext) {
4848
// setting up the namespaces, where Shipwright Controller will be deployed
4949
setupTektonCRDs(ctx)
50+
createTektonConfig(ctx)
5051
build = createShipwrightBuild(ctx, targetNamespace)
5152
})
5253

5354
g.AfterEach(func(ctx g.SpecContext) {
5455
deleteShipwrightBuild(ctx, build)
55-
56+
deleteTektonConfig(ctx)
5657
g.By("checking that the shipwright-build-controller deployment has been removed")
5758
deployment := baseDeployment.DeepCopy()
5859
test.EventuallyRemoved(ctx, k8sClient, deployment)

controllers/shipwrightbuild_controller.go

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,73 @@ func deleteObjectsIfPresent(ctx context.Context, k8sClient client.Client, objs [
104104
return nil
105105
}
106106

107+
// fetchAndCheckTektonConfig fetches the "config" `TektonConfig` instance on the cluster, and checks if its "Ready" condition reports `True`.
108+
// Returns `true` if the TektonConfig object exists and is "Ready". Returns `false` otherwise.
109+
func (r *ShipwrightBuildReconciler) fetchAndCheckTektonConfig(ctx context.Context, logger logr.Logger, b *v1alpha1.ShipwrightBuild) (bool, error) {
110+
tektonConfig, err := r.TektonOperatorClient.TektonConfigs().Get(ctx, "config", metav1.GetOptions{})
111+
if err != nil {
112+
if errors.IsNotFound(err) {
113+
logger.Error(err, "TektonConfig not found")
114+
apimeta.SetStatusCondition(&b.Status.Conditions, metav1.Condition{
115+
Type: ConditionReady,
116+
Status: metav1.ConditionFalse,
117+
Reason: "TektonConfigMissing",
118+
Message: "TektonConfig is missing from the cluster",
119+
})
120+
if updateErr := r.Client.Status().Update(ctx, b); updateErr != nil {
121+
logger.Error(updateErr, "Failed to update ShipwrightBuild status after TektonConfig not found")
122+
return false, updateErr
123+
}
124+
return false, nil // handled
125+
}
126+
127+
logger.Error(err, "Failed to fetch TektonConfig due to unexpected error")
128+
apimeta.SetStatusCondition(&b.Status.Conditions, metav1.Condition{
129+
Type: ConditionReady,
130+
Status: metav1.ConditionFalse,
131+
Reason: "TektonConfigFetchError",
132+
Message: fmt.Sprintf("Unexpected error fetching TektonConfig: %v", err),
133+
})
134+
if updateErr := r.Client.Status().Update(ctx, b); updateErr != nil {
135+
logger.Error(updateErr, "Failed to update ShipwrightBuild status after TektonConfig fetch error")
136+
return false, updateErr
137+
}
138+
return false, err
139+
}
140+
141+
// Check if TektonConfig is marked Ready
142+
for _, condition := range tektonConfig.Status.Conditions {
143+
if condition.Type == ConditionReady && condition.Status == corev1.ConditionTrue {
144+
apimeta.SetStatusCondition(&b.Status.Conditions, metav1.Condition{
145+
Type: ConditionReady,
146+
Status: metav1.ConditionTrue,
147+
Reason: "TektonReady",
148+
Message: "TektonConfig is Ready",
149+
})
150+
if updateErr := r.Client.Status().Update(ctx, b); updateErr != nil {
151+
logger.Error(updateErr, "Failed to update ShipwrightBuild status when TektonConfig is ready")
152+
return false, updateErr
153+
}
154+
return true, nil
155+
}
156+
}
157+
158+
// TektonConfig is present but not ready
159+
logger.Info("TektonConfig is not ready yet")
160+
apimeta.SetStatusCondition(&b.Status.Conditions, metav1.Condition{
161+
Type: ConditionReady,
162+
Status: metav1.ConditionFalse,
163+
Reason: "TektonNotReady",
164+
Message: "TektonConfig exists but is not Ready",
165+
})
166+
if updateErr := r.Client.Status().Update(ctx, b); updateErr != nil {
167+
logger.Error(updateErr, "Failed to update ShipwrightBuild status when TektonConfig is not ready")
168+
return false, updateErr
169+
}
170+
171+
return false, nil
172+
}
173+
107174
// Reconcile performs the resource reconciliation steps to deploy or remove Shipwright Build
108175
// instances. When deletion-timestamp is found, the removal of the previously deploy resources is
109176
// executed, otherwise the regular deploy workflow takes place.
@@ -143,6 +210,15 @@ func (r *ShipwrightBuildReconciler) Reconcile(ctx context.Context, req ctrl.Requ
143210
}
144211
}
145212

213+
// Requeue incase TektonConfig is not ready
214+
ready, err := r.fetchAndCheckTektonConfig(ctx, logger, b)
215+
if err != nil {
216+
return RequeueWithError(err)
217+
}
218+
if !ready {
219+
return Requeue()
220+
}
221+
146222
// selecting the target namespace based on the CRD information, when not informed using the
147223
// default namespace instead
148224
targetNamespace := b.Spec.TargetNamespace

controllers/shipwrightbuild_controller_test.go

Lines changed: 131 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,23 +9,25 @@ import (
99

1010
o "github.com/onsi/gomega"
1111

12+
buildv1alpha1 "github.com/shipwright-io/build/pkg/apis/build/v1alpha1"
13+
"github.com/shipwright-io/operator/api/v1alpha1"
14+
tektonoperatorv1alpha1 "github.com/tektoncd/operator/pkg/apis/operator/v1alpha1"
15+
tektonoperatorv1alpha1client "github.com/tektoncd/operator/pkg/client/clientset/versioned/fake"
1216
appsv1 "k8s.io/api/apps/v1"
1317
corev1 "k8s.io/api/core/v1"
1418
rbacv1 "k8s.io/api/rbac/v1"
19+
crdv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
1520
crdclientv1 "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/fake"
1621
"k8s.io/apimachinery/pkg/api/errors"
1722
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1823
"k8s.io/apimachinery/pkg/runtime"
1924
"k8s.io/apimachinery/pkg/types"
25+
"knative.dev/pkg/apis"
26+
duckv1 "knative.dev/pkg/apis/duck/v1"
2027
"sigs.k8s.io/controller-runtime/pkg/client"
2128
"sigs.k8s.io/controller-runtime/pkg/client/fake"
2229
"sigs.k8s.io/controller-runtime/pkg/log/zap"
2330
"sigs.k8s.io/controller-runtime/pkg/reconcile"
24-
25-
"github.com/shipwright-io/operator/api/v1alpha1"
26-
tektonoperatorv1alpha1 "github.com/tektoncd/operator/pkg/apis/operator/v1alpha1"
27-
tektonoperatorv1alpha1client "github.com/tektoncd/operator/pkg/client/clientset/versioned/fake"
28-
crdv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
2931
)
3032

3133
// bootstrapShipwrightBuildReconciler start up a new instance of ShipwrightBuildReconciler which is
@@ -35,6 +37,7 @@ func bootstrapShipwrightBuildReconciler(
3537
b *v1alpha1.ShipwrightBuild,
3638
tcfg *tektonoperatorv1alpha1.TektonConfig,
3739
tcrds []*crdv1.CustomResourceDefinition,
40+
statusObjects ...client.Object,
3841
) (client.Client, *crdclientv1.Clientset, *tektonoperatorv1alpha1client.Clientset, *ShipwrightBuildReconciler) {
3942
g := o.NewGomegaWithT(t)
4043

@@ -44,10 +47,17 @@ func bootstrapShipwrightBuildReconciler(
4447
s.AddKnownTypes(v1alpha1.GroupVersion, &v1alpha1.ShipwrightBuild{})
4548
s.AddKnownTypes(rbacv1.SchemeGroupVersion, &rbacv1.ClusterRoleBinding{})
4649
s.AddKnownTypes(rbacv1.SchemeGroupVersion, &rbacv1.ClusterRole{})
50+
s.AddKnownTypes(tektonoperatorv1alpha1.SchemeGroupVersion, &tektonoperatorv1alpha1.TektonConfig{})
51+
s.AddKnownTypes(buildv1alpha1.SchemeGroupVersion, &buildv1alpha1.ClusterBuildStrategy{})
4752

4853
logger := zap.New()
54+
clientBuilder := fake.NewClientBuilder().WithScheme(s).WithObjects(b)
55+
if len(statusObjects) > 0 {
56+
// the fake client does not support the status subresource by default.
57+
clientBuilder = clientBuilder.WithStatusSubresource(statusObjects...)
58+
}
59+
c := clientBuilder.Build()
4960

50-
c := fake.NewClientBuilder().WithScheme(s).WithObjects(b).Build()
5161
var crdClient *crdclientv1.Clientset
5262
var toClient *tektonoperatorv1alpha1client.Clientset
5363
if len(tcrds) > 0 {
@@ -245,3 +255,118 @@ func TestShipwrightBuildReconciler_Reconcile(t *testing.T) {
245255
})
246256
}
247257
}
258+
259+
func TestShipwrightBuildReconciler_OperandReadiness(t *testing.T) {
260+
g := o.NewGomegaWithT(t)
261+
ctx := context.TODO()
262+
263+
// ShipwrightBuild object
264+
b := &v1alpha1.ShipwrightBuild{
265+
ObjectMeta: metav1.ObjectMeta{
266+
Name: "name",
267+
Namespace: "default",
268+
},
269+
Spec: v1alpha1.ShipwrightBuildSpec{
270+
TargetNamespace: "namespace",
271+
},
272+
Status: v1alpha1.ShipwrightBuildStatus{
273+
Conditions: []metav1.Condition{
274+
{
275+
Type: ConditionReady,
276+
Status: metav1.ConditionTrue,
277+
},
278+
},
279+
},
280+
}
281+
282+
// Mock TektonConfig to simulate Tekton Operator installation
283+
tektonConfig := &tektonoperatorv1alpha1.TektonConfig{
284+
ObjectMeta: metav1.ObjectMeta{
285+
Name: "config",
286+
},
287+
Status: tektonoperatorv1alpha1.TektonConfigStatus{
288+
Status: duckv1.Status{
289+
Conditions: duckv1.Conditions{
290+
{
291+
Type: ConditionReady,
292+
Status: corev1.ConditionFalse,
293+
Reason: "Installed",
294+
Message: "TektonConfig is not ready",
295+
},
296+
},
297+
},
298+
},
299+
}
300+
// Preload the fake client with a valid ClusterBuildStrategy to pass buildstrategy reconciliation
301+
cbs := &buildv1alpha1.ClusterBuildStrategy{
302+
ObjectMeta: metav1.ObjectMeta{
303+
Name: "buildah",
304+
},
305+
Spec: buildv1alpha1.BuildStrategySpec{
306+
BuildSteps: []buildv1alpha1.BuildStep{
307+
{
308+
Container: corev1.Container{
309+
Name: "build-step",
310+
Image: "quay.io/buildah/stable",
311+
},
312+
},
313+
},
314+
},
315+
}
316+
317+
// Prepare cr
318+
crd1 := &crdv1.CustomResourceDefinition{}
319+
crd1.Name = "taskruns.tekton.dev"
320+
crd2 := &crdv1.CustomResourceDefinition{}
321+
crd2.Name = "tektonconfigs.operator.tekton.dev"
322+
crd2.Labels = map[string]string{"operator.tekton.dev/release": common.TektonOpMinSupportedVersion}
323+
crd3 := &crdv1.CustomResourceDefinition{}
324+
crd3.Name = "clusterbuildstrategies.shipwright.io"
325+
crds := []*crdv1.CustomResourceDefinition{crd1, crd2, crd3}
326+
327+
// Bootstrap the reconciler with the mock objects
328+
c, _, _, r := bootstrapShipwrightBuildReconciler(t, b, nil, crds, &v1alpha1.ShipwrightBuild{})
329+
330+
// Inject a pre-created valid tektonconfig
331+
_, err := r.TektonOperatorClient.TektonConfigs().Create(ctx, tektonConfig, metav1.CreateOptions{})
332+
g.Expect(err).To(o.BeNil())
333+
334+
// Verify creation of tektonconfig
335+
cfg, err := r.TektonOperatorClient.TektonConfigs().Get(ctx, "config", metav1.GetOptions{})
336+
g.Expect(err).To(o.BeNil())
337+
g.Expect(cfg.Name).To(o.Equal("config"))
338+
g.Expect(cfg.Status.Conditions[0].Type).To(o.Equal(apis.ConditionReady))
339+
340+
// Simulate reconciliation
341+
namespacedName := types.NamespacedName{Namespace: "default", Name: "name"}
342+
req := reconcile.Request{NamespacedName: namespacedName}
343+
res, err := r.Reconcile(ctx, req)
344+
g.Expect(err).To(o.BeNil())
345+
g.Expect(res.Requeue).To(o.BeTrue(), "Reconciliation should requeue when TektonConfig is not ready")
346+
347+
// Verify that the ShipwrightBuild is marked as not ready
348+
updated := &v1alpha1.ShipwrightBuild{}
349+
err = c.Get(ctx, req.NamespacedName, updated)
350+
g.Expect(err).To(o.BeNil())
351+
g.Expect(updated.Status.IsReady()).To(o.BeFalse(), "ShipwrightBuild should not be ready when TektonConfig is not ready")
352+
353+
// Simulate TektonConfig becoming ready
354+
tektonConfig.Status.Conditions[0].Status = corev1.ConditionTrue
355+
tektonConfig.Status.Conditions[0].Reason = "Installed"
356+
tektonConfig.Status.Conditions[0].Message = "TektonConfig is now ready"
357+
_, err = r.TektonOperatorClient.TektonConfigs().Update(ctx, tektonConfig, metav1.UpdateOptions{})
358+
g.Expect(err).To(o.BeNil())
359+
360+
// Inject a pre-created valid ClusterBuildStrategy
361+
err = c.Create(ctx, cbs)
362+
g.Expect(err).To(o.BeNil())
363+
// Trigger reconciliation again
364+
res, err = r.Reconcile(ctx, req)
365+
g.Expect(err).To(o.BeNil())
366+
g.Expect(res.Requeue).To(o.BeFalse(), "Should not requeue after TektonConfig is ready")
367+
368+
// Fetch and verify ShipwrightBuild is now ready
369+
err = c.Get(ctx, req.NamespacedName, updated)
370+
g.Expect(err).To(o.BeNil())
371+
g.Expect(updated.Status.IsReady()).To(o.BeTrue(), "ShipwrightBuild should be ready when TektonConfig is ready")
372+
}

0 commit comments

Comments
 (0)