From 88307deb3c392cec42e57eda6492ed92e30dc8c5 Mon Sep 17 00:00:00 2001 From: Dasha Komsa Date: Thu, 18 Jan 2024 16:40:41 +0000 Subject: [PATCH 1/4] set Ready condition after running ansible Signed-off-by: Dasha Komsa --- internal/controller/ansibleRun/ansibleRun.go | 44 +++++++++++++------- 1 file changed, 30 insertions(+), 14 deletions(-) diff --git a/internal/controller/ansibleRun/ansibleRun.go b/internal/controller/ansibleRun/ansibleRun.go index b0d1dc1..64c0950 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(ctx, 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,13 +472,11 @@ 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(ctx, desired); err != nil { + return managed.ExternalObservation{}, fmt.Errorf("running ansible: %w", err) } // The crossplane runtime is not aware of the external resource created by ansible content. @@ -492,6 +486,28 @@ func (c *external) handleLastApplied(ctx context.Context, lastParameters *v1alph return managed.ExternalObservation{ResourceExists: true, ResourceUpToDate: true}, nil } +func (c *external) runAnsible(ctx context.Context, 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 { From 5ebf7079aca1c23fb44d111e446690be39479251 Mon Sep 17 00:00:00 2001 From: Dasha Komsa Date: Thu, 18 Jan 2024 19:13:35 +0000 Subject: [PATCH 2/4] update unit tests for controller Signed-off-by: Dasha Komsa --- .../controller/ansibleRun/ansibleRun_test.go | 38 ++++++++++++++----- 1 file changed, 29 insertions(+), 9 deletions(-) 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) + } }) } } From b11dfd53cabcb762e5f5c10f650efe61a3c0fd87 Mon Sep 17 00:00:00 2001 From: Dasha Komsa Date: Thu, 18 Jan 2024 22:18:40 +0000 Subject: [PATCH 3/4] fixup - run ansible only when changed compared to last applied configuration Signed-off-by: Dasha Komsa --- internal/controller/ansibleRun/ansibleRun.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/internal/controller/ansibleRun/ansibleRun.go b/internal/controller/ansibleRun/ansibleRun.go index 64c0950..277d7f4 100644 --- a/internal/controller/ansibleRun/ansibleRun.go +++ b/internal/controller/ansibleRun/ansibleRun.go @@ -473,10 +473,9 @@ func (c *external) handleLastApplied(ctx context.Context, lastParameters *v1alph return managed.ExternalObservation{}, err } - } - - if err := c.runAnsible(ctx, desired); err != nil { - return managed.ExternalObservation{}, fmt.Errorf("running ansible: %w", err) + if err := c.runAnsible(ctx, desired); err != nil { + return managed.ExternalObservation{}, fmt.Errorf("running ansible: %w", err) + } } // The crossplane runtime is not aware of the external resource created by ansible content. From cb76ced5375bb56b8931fc4c6fb72e7585dfac32 Mon Sep 17 00:00:00 2001 From: Dasha Komsa Date: Mon, 29 Jan 2024 17:58:06 +0000 Subject: [PATCH 4/4] linting fix: remove unused ctx arg Signed-off-by: Dasha Komsa --- internal/controller/ansibleRun/ansibleRun.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/controller/ansibleRun/ansibleRun.go b/internal/controller/ansibleRun/ansibleRun.go index 277d7f4..d890567 100644 --- 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(ctx, cr); err != nil { + if err := c.runAnsible(cr); err != nil { return managed.ExternalUpdate{}, fmt.Errorf("running ansible: %w", err) } @@ -473,7 +473,7 @@ func (c *external) handleLastApplied(ctx context.Context, lastParameters *v1alph return managed.ExternalObservation{}, err } - if err := c.runAnsible(ctx, desired); err != nil { + if err := c.runAnsible(desired); err != nil { return managed.ExternalObservation{}, fmt.Errorf("running ansible: %w", err) } } @@ -485,7 +485,7 @@ func (c *external) handleLastApplied(ctx context.Context, lastParameters *v1alph return managed.ExternalObservation{ResourceExists: true, ResourceUpToDate: true}, nil } -func (c *external) runAnsible(ctx context.Context, cr *v1alpha1.AnsibleRun) error { +func (c *external) runAnsible(cr *v1alpha1.AnsibleRun) error { dc, _, err := c.runner.Run() if err != nil { return err