set Ready condition after running ansible (#294)

* set Ready condition after running ansible Signed-off-by: Dasha Komsa <[email protected]> * update unit tests for controller Signed-off-by: Dasha Komsa <[email protected]> * fixup - run ansible only when changed compared to last applied configuration Signed-off-by: Dasha Komsa <[email protected]> * linting fix: remove unused ctx arg Signed-off-by: Dasha Komsa <[email protected]> --------- Signed-off-by: Dasha Komsa <[email protected]> Co-authored-by: Dasha Komsa <[email protected]>
crossplane-contrib · Feb 1, 2024 · 8118c73 · 8118c73
1 parent 5e0d992
commit 8118c73
Show file tree

Hide file tree

Showing 2 changed files with 57 additions and 22 deletions.
diff --git a/internal/controller/ansibleRun/ansibleRun.go b/internal/controller/ansibleRun/ansibleRun.go
@@ -388,19 +388,15 @@ func (c *external) Create(ctx context.Context, mg resource.Managed) (managed.Ext
 }
 
 func (c *external) Update(ctx context.Context, mg resource.Managed) (managed.ExternalUpdate, error) {
-	_, ok := mg.(*v1alpha1.AnsibleRun)
+	cr, ok := mg.(*v1alpha1.AnsibleRun)
 	if !ok {
 		return managed.ExternalUpdate{}, errors.New(errNotAnsibleRun)
 	}
 
 	// disable checkMode for real action
 	c.runner.EnableCheckMode(false)
-	dc, _, err := c.runner.Run()
-	if err != nil {
-		return managed.ExternalUpdate{}, err
-	}
-	if err = dc.Wait(); err != nil {
-		return managed.ExternalUpdate{}, err
+	if err := c.runAnsible(cr); err != nil {
+		return managed.ExternalUpdate{}, fmt.Errorf("running ansible: %w", err)
 	}
 
 	// TODO handle ConnectionDetails https://github.com/multicloudlab/crossplane-provider-ansible/pull/74#discussion_r888467991
@@ -476,12 +472,9 @@ func (c *external) handleLastApplied(ctx context.Context, lastParameters *v1alph
 		if err := c.runner.WriteExtraVar(nestedMap); err != nil {
 			return managed.ExternalObservation{}, err
 		}
-		dc, _, err := c.runner.Run()
-		if err != nil {
-			return managed.ExternalObservation{}, err
-		}
-		if err = dc.Wait(); err != nil {
-			return managed.ExternalObservation{}, err
+
+		if err := c.runAnsible(desired); err != nil {
+			return managed.ExternalObservation{}, fmt.Errorf("running ansible: %w", err)
 		}
 	}
 
@@ -492,6 +485,28 @@ 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 {
+	dc, _, err := c.runner.Run()
+	if err != nil {
+		return err
+	}
+
+	if err = dc.Wait(); err != nil {
+		cond := xpv1.Unavailable()
+		cond.Message = err.Error()
+		cr.SetConditions(cond)
+
+		return err
+	}
+
+	cr.SetConditions(xpv1.Available())
+
+	// no need to persist status update explicitly, cr modifications are in-place and will
+	// be persisted by crossplane-runtime
+
+	return nil
+}
+
 func addBehaviorVars(pc *v1alpha1.ProviderConfig) map[string]string {
 	behaviorVars := make(map[string]string, len(pc.Spec.Vars))
 	for _, v := range pc.Spec.Vars {

diff --git a/internal/controller/ansibleRun/ansibleRun_test.go b/internal/controller/ansibleRun/ansibleRun_test.go
@@ -28,6 +28,7 @@ import (
 	"io"
 
 	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-cmp/cmp/cmpopts"
 	"github.com/spf13/afero"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
@@ -531,8 +532,9 @@ func TestObserve(t *testing.T) {
 	}
 
 	type want struct {
-		o   managed.ExternalObservation
-		err error
+		o          managed.ExternalObservation
+		err        error
+		conditions []xpv1.Condition
 	}
 
 	cases := map[string]struct {
@@ -580,7 +582,8 @@ func TestObserve(t *testing.T) {
 				mg: &v1alpha1.AnsibleRun{},
 			},
 			want: want{
-				err: fmt.Errorf("%s: %w", errGetAnsibleRun, errBoom),
+				err:        fmt.Errorf("%s: %w", errGetAnsibleRun, errBoom),
+				conditions: []xpv1.Condition{xpv1.Unavailable()},
 			},
 		},
 		"GetObservedErrorWhenCheckWhenObservePolicy": {
@@ -640,8 +643,9 @@ func TestCreateOrUpdate(t *testing.T) {
 	}
 
 	type want struct {
-		o   managed.ExternalCreation
-		err error
+		o          managed.ExternalCreation
+		err        error
+		conditions []xpv1.Condition
 	}
 
 	cases := map[string]struct {
@@ -678,7 +682,7 @@ func TestCreateOrUpdate(t *testing.T) {
 				},
 			},
 			want: want{
-				err: errBoom,
+				err: fmt.Errorf("running ansible: %w", errBoom),
 			},
 		},
 		"SuccessObserveAndDelete": {
@@ -702,7 +706,9 @@ func TestCreateOrUpdate(t *testing.T) {
 					},
 				},
 			},
-			want: want{},
+			want: want{
+				conditions: []xpv1.Condition{xpv1.Available()},
+			},
 		},
 		"RunErrorWithCheckWhenObservePolicy": {
 			reason: "We should return any error we encounter when running the runner",
@@ -723,7 +729,7 @@ func TestCreateOrUpdate(t *testing.T) {
 				},
 			},
 			want: want{
-				err: errBoom,
+				err: fmt.Errorf("running ansible: %w", errBoom),
 			},
 		},
 		"SuccessCheckWhenObserve": {
@@ -747,7 +753,9 @@ func TestCreateOrUpdate(t *testing.T) {
 					},
 				},
 			},
-			want: want{},
+			want: want{
+				conditions: []xpv1.Condition{xpv1.Available()},
+			},
 		},
 	}
 
@@ -761,6 +769,18 @@ func TestCreateOrUpdate(t *testing.T) {
 			if diff := cmp.Diff(tc.want.o, got); diff != "" {
 				t.Errorf("\n%s\ne.Observe(...): -want, +got:\n%s\n", tc.reason, diff)
 			}
+
+			if tc.args.mg == nil {
+				return
+			}
+
+			if diff := cmp.Diff(
+				tc.want.conditions,
+				tc.args.mg.(*v1alpha1.AnsibleRun).Status.Conditions,
+				cmpopts.IgnoreFields(xpv1.Condition{}, "LastTransitionTime"),
+			); diff != "" {
+				t.Errorf("ansiblerun conditions: (-want +got):\n%s", diff)
+			}
 		})
 	}
 }