fix ready condition not being set to true

crossplane-contrib · Feb 6, 2024 · 9f625cc · 9f625cc
1 parent c587baf
commit 9f625cc
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 14 deletions.
diff --git a/internal/controller/ansibleRun/ansibleRun.go b/internal/controller/ansibleRun/ansibleRun.go
@@ -395,7 +395,7 @@ func (c *external) Update(ctx context.Context, mg resource.Managed) (managed.Ext
 
 	// disable checkMode for real action
 	c.runner.EnableCheckMode(false)
-	if err := c.runAnsible(cr); err != nil {
+	if err := c.runAnsible(ctx, cr); err != nil {
 		return managed.ExternalUpdate{}, fmt.Errorf("running ansible: %w", err)
 	}
 
@@ -448,6 +448,10 @@ func (c *external) handleLastApplied(ctx context.Context, lastParameters *v1alph
 	isLastSyncOK := (desired.GetCondition(xpv1.TypeSynced).Status == v1.ConditionTrue)
 
 	if isUpToDate && isLastSyncOK {
+		desired.SetConditions(xpv1.Available())
+		if err := c.kube.Status().Update(ctx, desired); err != nil {
+			return managed.ExternalObservation{}, fmt.Errorf("updating status: %w", err)
+		}
 		// nothing to do for this run
 		return managed.ExternalObservation{ResourceExists: true, ResourceUpToDate: true}, nil
 	}
@@ -472,7 +476,7 @@ func (c *external) handleLastApplied(ctx context.Context, lastParameters *v1alph
 		return managed.ExternalObservation{}, err
 	}
 
-	if err := c.runAnsible(desired); err != nil {
+	if err := c.runAnsible(ctx, desired); err != nil {
 		return managed.ExternalObservation{}, fmt.Errorf("running ansible: %w", err)
 	}
 
@@ -483,7 +487,7 @@ func (c *external) handleLastApplied(ctx context.Context, lastParameters *v1alph
 	return managed.ExternalObservation{ResourceExists: true, ResourceUpToDate: true}, nil
 }
 
-func (c *external) runAnsible(cr *v1alpha1.AnsibleRun) error {
+func (c *external) runAnsible(ctx context.Context, cr *v1alpha1.AnsibleRun) error {
 	dc, _, err := c.runner.Run()
 	if err != nil {
 		return err
@@ -493,16 +497,15 @@ func (c *external) runAnsible(cr *v1alpha1.AnsibleRun) error {
 		cond := xpv1.Unavailable()
 		cond.Message = err.Error()
 		cr.SetConditions(cond)
-
-		return err
+	} else {
+		cr.SetConditions(xpv1.Available())
 	}
 
-	cr.SetConditions(xpv1.Available())
-
-	// no need to persist status update explicitly, cr modifications are in-place and will
-	// be persisted by crossplane-runtime
+	if err := c.kube.Status().Update(ctx, cr); err != nil {
+		return fmt.Errorf("updating status: %w", err)
+	}
 
-	return nil
+	return err
 }
 
 func addBehaviorVars(pc *v1alpha1.ProviderConfig) map[string]string {

diff --git a/internal/controller/ansibleRun/ansibleRun_test.go b/internal/controller/ansibleRun/ansibleRun_test.go
@@ -611,8 +611,9 @@ func TestObserve(t *testing.T) {
 			reason: "We should not run ansible when spec has not changed and last sync was successful",
 			fields: fields{
 				kube: &test.MockClient{
-					MockGet:    test.NewMockGetFn(nil),
-					MockUpdate: test.NewMockUpdateFn(nil),
+					MockGet:          test.NewMockGetFn(nil),
+					MockUpdate:       test.NewMockUpdateFn(nil),
+					MockStatusUpdate: test.NewMockSubResourceUpdateFn(nil),
 				},
 				runner: &MockRunner{
 					MockAnsibleRunPolicy: func() *ansible.RunPolicy {
@@ -639,8 +640,9 @@ func TestObserve(t *testing.T) {
 			reason: "We should run ansible when spec has not changed but last sync was unsuccessful",
 			fields: fields{
 				kube: &test.MockClient{
-					MockGet:    test.NewMockGetFn(nil),
-					MockUpdate: test.NewMockUpdateFn(nil),
+					MockGet:          test.NewMockGetFn(nil),
+					MockUpdate:       test.NewMockUpdateFn(nil),
+					MockStatusUpdate: test.NewMockSubResourceUpdateFn(nil),
 				},
 				runner: &MockRunner{
 					MockAnsibleRunPolicy: func() *ansible.RunPolicy {
@@ -769,6 +771,9 @@ func TestCreateOrUpdate(t *testing.T) {
 				mg: &v1alpha1.AnsibleRun{},
 			},
 			fields: fields{
+				kube: &test.MockClient{
+					MockStatusUpdate: test.NewMockSubResourceUpdateFn(nil),
+				},
 				runner: &MockRunner{
 					MockAnsibleRunPolicy: func() *ansible.RunPolicy {
 						return &ansible.RunPolicy{
@@ -816,6 +821,9 @@ func TestCreateOrUpdate(t *testing.T) {
 				mg: &v1alpha1.AnsibleRun{},
 			},
 			fields: fields{
+				kube: &test.MockClient{
+					MockStatusUpdate: test.NewMockSubResourceUpdateFn(nil),
+				},
 				runner: &MockRunner{
 					MockAnsibleRunPolicy: func() *ansible.RunPolicy {
 						return &ansible.RunPolicy{