Skip to content

Commit

Permalink
Validating the controller checks all targets are ready and have the s…
Browse files Browse the repository at this point in the history 
…ame version before proceeding with the promotion
  • Loading branch information
Luiz Filho committed Oct 19, 2023
1 parent be9adf0 commit 0a50efe
Show file tree
Hide file tree
Showing 2 changed files with 97 additions and 60 deletions.
20 changes: 17 additions & 3 deletions controllers/leveltriggered/controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -199,13 +199,17 @@ func (r *PipelineReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c

if !checkAllTargetsAreReady(pipeline.Status.Environments[firstEnv.Name]) {
// not all targets are ready, so we can't proceed
// if err := r.setPendingCondition(ctx, &pipeline, v1alpha1.TargetClusterNotReadyReason, "Waiting for all targets to be ready"); err != nil {
// return ctrl.Result{}, fmt.Errorf("error setting pending condition: %w", err)
// }
if err := r.setPendingCondition(ctx, pipeline, v1alpha1.TargetClusterNotReadyReason, "Waiting for all targets to be ready"); err != nil {
return ctrl.Result{}, fmt.Errorf("error setting pending condition: %w", err)
}

return ctrl.Result{}, nil
}

if err := r.removePendingCondition(ctx, pipeline); err != nil {
return ctrl.Result{}, fmt.Errorf("error removing pending condition: %w", err)
}

for _, env := range pipeline.Spec.Environments[1:] {
// if all targets run the latest revision and are ready, we can skip this environment
if checkAllTargetsRunRevision(pipeline.Status.Environments[env.Name], latestRevision) && checkAllTargetsAreReady(pipeline.Status.Environments[env.Name]) {
Expand Down Expand Up @@ -242,6 +246,16 @@ func (r *PipelineReconciler) setPendingCondition(ctx context.Context, pipeline v
return nil
}

func (r *PipelineReconciler) removePendingCondition(ctx context.Context, pipeline v1alpha1.Pipeline) error {
apimeta.RemoveStatusCondition(&pipeline.Status.Conditions, conditions.PromotionPendingCondition)

if err := r.patchStatus(ctx, client.ObjectKeyFromObject(&pipeline), pipeline.Status); err != nil {
return err
}

return nil
}

func (r *PipelineReconciler) promoteLatestRevision(ctx context.Context, pipeline v1alpha1.Pipeline, env v1alpha1.Environment, revision string) error {
// none of the current strategies are idepontent, using it now to keep the ball rolling, but we need to implement
// strategies that are.
Expand Down
137 changes: 80 additions & 57 deletions controllers/leveltriggered/controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func TestReconcile(t *testing.T) {
},
}})

checkReadyCondition(ctx, g, client.ObjectKeyFromObject(pipeline), metav1.ConditionFalse, v1alpha1.TargetNotReadableReason)
checkCondition(ctx, g, client.ObjectKeyFromObject(pipeline), meta.ReadyCondition, metav1.ConditionFalse, v1alpha1.TargetNotReadableReason)
g.Eventually(fetchEventsFor(ns.Name, name), time.Second, time.Millisecond*100).Should(Not(BeEmpty()))
events := fetchEventsFor(ns.Name, name)()
g.Expect(events).ToNot(BeEmpty())
Expand All @@ -54,7 +54,7 @@ func TestReconcile(t *testing.T) {

// make an unready cluster and see if it notices
testingutils.NewGitopsCluster(ctx, g, k8sClient, clusterName, ns.Name, kubeConfig)
checkReadyCondition(ctx, g, client.ObjectKeyFromObject(pipeline), metav1.ConditionFalse, v1alpha1.TargetNotReadableReason)
checkCondition(ctx, g, client.ObjectKeyFromObject(pipeline), meta.ReadyCondition, metav1.ConditionFalse, v1alpha1.TargetNotReadableReason)
})

t.Run("sets reconciliation succeeded condition for remote cluster", func(t *testing.T) {
Expand All @@ -69,7 +69,7 @@ func TestReconcile(t *testing.T) {

pipeline := newPipeline(ctx, g, name, ns.Name, []*clusterctrlv1alpha1.GitopsCluster{gc})

checkReadyCondition(ctx, g, client.ObjectKeyFromObject(pipeline), metav1.ConditionTrue, v1alpha1.ReconciliationSucceededReason)
checkCondition(ctx, g, client.ObjectKeyFromObject(pipeline), meta.ReadyCondition, metav1.ConditionTrue, v1alpha1.ReconciliationSucceededReason)

g.Eventually(fetchEventsFor(ns.Name, name), time.Second, time.Millisecond*100).Should(Not(BeEmpty()))

Expand All @@ -87,7 +87,7 @@ func TestReconcile(t *testing.T) {

pipeline := newPipeline(ctx, g, name, ns.Name, nil)

checkReadyCondition(ctx, g, client.ObjectKeyFromObject(pipeline), metav1.ConditionTrue, v1alpha1.ReconciliationSucceededReason)
checkCondition(ctx, g, client.ObjectKeyFromObject(pipeline), meta.ReadyCondition, metav1.ConditionTrue, v1alpha1.ReconciliationSucceededReason)

g.Eventually(fetchEventsFor(ns.Name, name), time.Second, time.Millisecond*100).Should(Not(BeEmpty()))

Expand All @@ -101,10 +101,10 @@ func TestReconcile(t *testing.T) {
name := "pipeline-" + rand.String(5)
ns := testingutils.NewNamespace(ctx, g, k8sClient)
pipeline := newPipeline(ctx, g, name, ns.Name, nil)
checkReadyCondition(ctx, g, client.ObjectKeyFromObject(pipeline), metav1.ConditionFalse, v1alpha1.TargetNotReadableReason)
checkCondition(ctx, g, client.ObjectKeyFromObject(pipeline), meta.ReadyCondition, metav1.ConditionFalse, v1alpha1.TargetNotReadableReason)

hr := newApp(ctx, g, name, ns.Name) // the name of the pipeline is also used as the name in the appRef, in newPipeline(...)
checkReadyCondition(ctx, g, client.ObjectKeyFromObject(pipeline), metav1.ConditionTrue, v1alpha1.ReconciliationSucceededReason)
checkCondition(ctx, g, client.ObjectKeyFromObject(pipeline), meta.ReadyCondition, metav1.ConditionTrue, v1alpha1.ReconciliationSucceededReason)

// we didn't set the app to be ready, so we should expect the target to be reported as unready
p := getPipeline(ctx, g, client.ObjectKeyFromObject(pipeline))
Expand Down Expand Up @@ -176,16 +176,16 @@ func TestReconcile(t *testing.T) {
}

devApp := newApp(ctx, g, name, devNs.Name)
setAppRevision(ctx, g, devApp, "v1.0.0")
setAppRevisionAndReadyStatus(ctx, g, devApp, "v1.0.0")

stagingApp := newApp(ctx, g, name, stagingNs.Name)
setAppRevision(ctx, g, stagingApp, "v1.0.0")
setAppRevisionAndReadyStatus(ctx, g, stagingApp, "v1.0.0")

prodApp := newApp(ctx, g, name, prodNs.Name)
setAppRevision(ctx, g, prodApp, "v1.0.0")
setAppRevisionAndReadyStatus(ctx, g, prodApp, "v1.0.0")

g.Expect(k8sClient.Create(ctx, pipeline)).To(Succeed())
checkReadyCondition(ctx, g, client.ObjectKeyFromObject(pipeline), metav1.ConditionTrue, v1alpha1.ReconciliationSucceededReason)
checkCondition(ctx, g, client.ObjectKeyFromObject(pipeline), meta.ReadyCondition, metav1.ConditionTrue, v1alpha1.ReconciliationSucceededReason)

versionToPromote := "v1.0.1"

Expand All @@ -202,7 +202,7 @@ func TestReconcile(t *testing.T) {
})

// Bumping dev revision to trigger the promotion
setAppRevision(ctx, g, devApp, versionToPromote)
setAppRevisionAndReadyStatus(ctx, g, devApp, versionToPromote)

// checks if the revision of all target status is v1.0.1
g.Eventually(func() bool {
Expand All @@ -221,7 +221,7 @@ func TestReconcile(t *testing.T) {

t.Run("triggers another promotion if the app is updated again", func(t *testing.T) {
// Bumping dev revision to trigger the promotion
setAppRevision(ctx, g, devApp, "v1.0.2")
setAppRevisionAndReadyStatus(ctx, g, devApp, "v1.0.2")

// checks if the revision of all target status is v1.0.2
g.Eventually(func() bool {
Expand Down Expand Up @@ -286,53 +286,40 @@ func TestReconcile(t *testing.T) {
}

devApp := newApp(ctx, g, name, devNs.Name)
setAppRevision(ctx, g, devApp, "v1.0.0")
setAppRevisionAndReadyStatus(ctx, g, devApp, "v1.0.0")

_ = newApp(ctx, g, name, devNs2.Name)
devApp2 := newApp(ctx, g, name, devNs2.Name)

stagingApp := newApp(ctx, g, name, stagingNs.Name)
setAppRevision(ctx, g, stagingApp, "v1.0.0")
setAppRevisionAndReadyStatus(ctx, g, stagingApp, "v1.0.0")

g.Expect(k8sClient.Create(ctx, pipeline)).To(Succeed())
checkReadyCondition(ctx, g, client.ObjectKeyFromObject(pipeline), metav1.ConditionTrue, v1alpha1.ReconciliationSucceededReason)

checkPendingCondition(ctx, g, client.ObjectKeyFromObject(pipeline), metav1.ConditionTrue, v1alpha1.TargetClusterDifferentRevisionReason)

// versionToPromote := "v1.0.1"

// mockStrategy.EXPECT().
// Promote(gomock.Any(), *pipeline.Spec.Promotion, gomock.Any()).
// AnyTimes().
// Do(func(ctx context.Context, p v1alpha1.Promotion, prom strategy.Promotion) {
// switch prom.Environment.Name {
// case "staging":
// setAppRevision(ctx, g, stagingApp, prom.Version)
// }
// })

// // Bumping dev revision to trigger the promotion
// setAppRevision(ctx, g, devApp, versionToPromote)

// // checks if the revision of all target status is v1.0.1
// g.Eventually(func() bool {
// p := getPipeline(ctx, g, client.ObjectKeyFromObject(pipeline))

// p.Status.Conditions = conditions.RemoveStatusCondition(p.Status.Conditions, meta.ReadyCondition)
// for _, env := range p.Status.Environments {
// for _, target := range env.Targets {
// if target.Revision != versionToPromote {
// return false
// }
// }
// }

// return true
// }, "5s", "0.2s").Should(BeTrue())
checkCondition(ctx, g, client.ObjectKeyFromObject(pipeline), meta.ReadyCondition, metav1.ConditionTrue, v1alpha1.ReconciliationSucceededReason)
checkCondition(ctx, g, client.ObjectKeyFromObject(pipeline), pipelineconditions.PromotionPendingCondition, metav1.ConditionTrue, v1alpha1.TargetClusterDifferentRevisionReason)

setAppRevision(ctx, g, devApp2, "v1.0.0")
checkCondition(ctx, g, client.ObjectKeyFromObject(pipeline), pipelineconditions.PromotionPendingCondition, metav1.ConditionTrue, v1alpha1.TargetClusterNotReadyReason)

setAppStatusReadyCondition(ctx, g, devApp2)

g.Eventually(func() bool {
p := getPipeline(ctx, g, client.ObjectKeyFromObject(pipeline))
return apimeta.FindStatusCondition(p.Status.Conditions, pipelineconditions.PromotionPendingCondition) == nil
}).Should(BeTrue())
})
}

func setAppRevisionAndReadyStatus(ctx context.Context, g Gomega, hr *helmv2.HelmRelease, revision string) {
setAppRevision(ctx, g, hr, revision)
setAppStatusReadyCondition(ctx, g, hr)
}

func setAppRevision(ctx context.Context, g Gomega, hr *helmv2.HelmRelease, revision string) {
hr.Status.LastAppliedRevision = revision
g.Expect(k8sClient.Status().Update(ctx, hr)).To(Succeed())
}

func setAppStatusReadyCondition(ctx context.Context, g Gomega, hr *helmv2.HelmRelease) {
apimeta.SetStatusCondition(&hr.Status.Conditions, metav1.Condition{Type: "Ready", Status: metav1.ConditionTrue, Reason: "test"})
g.Expect(k8sClient.Status().Update(ctx, hr)).To(Succeed())
}
Expand Down Expand Up @@ -364,33 +351,69 @@ func getTargetStatus(g Gomega, pipeline *v1alpha1.Pipeline, envName string, targ
return env.Targets[target]
}

func checkReadyCondition(ctx context.Context, g Gomega, n types.NamespacedName, status metav1.ConditionStatus, reason string) {
func checkCondition(ctx context.Context, g Gomega, n types.NamespacedName, conditionType string, status metav1.ConditionStatus, reason string) {
pipeline := &v1alpha1.Pipeline{}
assrt := g.Eventually(func() []metav1.Condition {
assrt := g.Eventually(func() metav1.Condition {
err := k8sClient.Get(ctx, n, pipeline)
if err != nil {
return nil
return metav1.Condition{}
}

cond := apimeta.FindStatusCondition(pipeline.Status.Conditions, conditionType)
if cond == nil {
return metav1.Condition{}
}
return pipeline.Status.Conditions

return *cond
}, defaultTimeout, defaultInterval)

cond := metav1.Condition{
Type: meta.ReadyCondition,
Type: conditionType,
Status: status,
Reason: reason,
}

assrt.Should(conditions.MatchCondition(cond))
}

// func checkCondition(ctx context.Context, g Gomega, n meta.ReadyCondition, types.NamespacedName, status metav1.ConditionStatus, reason string) {
// pipeline := &v1alpha1.Pipeline{}
// assrt := g.Eventually(func() metav1.Condition {
// err := k8sClient.Get(ctx, n, pipeline)
// if err != nil {
// return metav1.Condition{}
// }
// cond := apimeta.FindStatusCondition(pipeline.Status.Conditions, meta.ReadyCondition)
// if cond == nil {
// return metav1.Condition{}
// }

// return *cond
// }, defaultTimeout, defaultInterval)

// cond := metav1.Condition{
// Type: meta.ReadyCondition,
// Status: status,
// Reason: reason,
// }

// assrt.Should(conditions.MatchCondition(cond))
// }

func checkPendingCondition(ctx context.Context, g Gomega, n types.NamespacedName, status metav1.ConditionStatus, reason string) {

Check failure on line 403 in controllers/leveltriggered/controller_test.go

View workflow job for this annotation

GitHub Actions / lint 

func `checkPendingCondition` is unused (unused)
pipeline := &v1alpha1.Pipeline{}
assrt := g.Eventually(func() []metav1.Condition {
assrt := g.Eventually(func() metav1.Condition {
err := k8sClient.Get(ctx, n, pipeline)
if err != nil {
return nil
return metav1.Condition{}
}

cond := apimeta.FindStatusCondition(pipeline.Status.Conditions, pipelineconditions.PromotionPendingCondition)
if cond == nil {
return metav1.Condition{}
}
return pipeline.Status.Conditions

return *cond
}, defaultTimeout, defaultInterval)

cond := metav1.Condition{
Expand Down

0 comments on commit 0a50efe

Please sign in to comment.