From 8118c7306886508aa26a21a41003968ef36ae934 Mon Sep 17 00:00:00 2001 From: Dasha Komsa Date: Thu, 1 Feb 2024 01:05:14 -0500 Subject: [PATCH] set Ready condition after running ansible (#294) * set Ready condition after running ansible Signed-off-by: Dasha Komsa * update unit tests for controller Signed-off-by: Dasha Komsa * fixup - run ansible only when changed compared to last applied configuration Signed-off-by: Dasha Komsa * linting fix: remove unused ctx arg Signed-off-by: Dasha Komsa --------- Signed-off-by: Dasha Komsa Co-authored-by: Dasha Komsa --- internal/controller/ansibleRun/ansibleRun.go | 41 +++++++++++++------ .../controller/ansibleRun/ansibleRun_test.go | 38 +++++++++++++---- 2 files changed, 57 insertions(+), 22 deletions(-) diff --git a/internal/controller/ansibleRun/ansibleRun.go b/internal/controller/ansibleRun/ansibleRun.go index b0d1dc1..d890567 100644 --- 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 index 689a9a1..d94f37a 100644 --- 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) + } }) } }