Skip to content

Commit

Permalink
Don't update k8s resource in Observe function
Browse files Browse the repository at this point in the history
Signed-off-by: Daniel Kozlowski <[email protected]>
  • Loading branch information
danielinclouds committed Oct 3, 2022
1 parent b019391 commit cf88e93
Show file tree
Hide file tree
Showing 4 changed files with 82 additions and 38 deletions.
75 changes: 75 additions & 0 deletions package/crds/dns.gcp.crossplane.io_managedzones.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,31 @@ spec:
name:
description: Name of the referenced object.
type: string
policy:
description: Policies for referencing.
properties:
resolution:
default: Required
description: Resolution specifies whether resolution of this
reference is required. The default is 'Required', which
means the reconcile will fail if the reference cannot be
resolved. 'Optional' means this reference will be a no-op
if it cannot be resolved.
enum:
- Required
- Optional
type: string
resolve:
description: Resolve specifies when this reference should
be resolved. The default is 'IfNotPresent', which will attempt
to resolve the reference only when the corresponding field
is not present. Use 'Always' to resolve the reference on
every reconcile.
enum:
- Always
- IfNotPresent
type: string
type: object
required:
- name
type: object
Expand All @@ -136,6 +161,31 @@ spec:
name:
description: Name of the referenced object.
type: string
policy:
description: Policies for referencing.
properties:
resolution:
default: Required
description: Resolution specifies whether resolution of this
reference is required. The default is 'Required', which
means the reconcile will fail if the reference cannot be
resolved. 'Optional' means this reference will be a no-op
if it cannot be resolved.
enum:
- Required
- Optional
type: string
resolve:
description: Resolve specifies when this reference should
be resolved. The default is 'IfNotPresent', which will attempt
to resolve the reference only when the corresponding field
is not present. Use 'Always' to resolve the reference on
every reconcile.
enum:
- Always
- IfNotPresent
type: string
type: object
required:
- name
type: object
Expand All @@ -155,6 +205,31 @@ spec:
name:
description: Name of the referenced object.
type: string
policy:
description: Policies for referencing.
properties:
resolution:
default: Required
description: Resolution specifies whether resolution of
this reference is required. The default is 'Required',
which means the reconcile will fail if the reference
cannot be resolved. 'Optional' means this reference
will be a no-op if it cannot be resolved.
enum:
- Required
- Optional
type: string
resolve:
description: Resolve specifies when this reference should
be resolved. The default is 'IfNotPresent', which will
attempt to resolve the reference only when the corresponding
field is not present. Use 'Always' to resolve the reference
on every reconcile.
enum:
- Always
- IfNotPresent
type: string
type: object
required:
- name
type: object
Expand Down
2 changes: 1 addition & 1 deletion pkg/clients/managedzone/managed_zone.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ func IsUpToDate(name string, spec *v1alpha1.ManagedZoneParameters, observed *dns

// GenerateManagedZoneObservation takes a dns.ManagedZone and returns
// *ManagedZoneObservation.
func GenerateManagedZoneObservation(observed *dns.ManagedZone) v1alpha1.ManagedZoneObservation {
func GenerateManagedZoneObservation(observed dns.ManagedZone) v1alpha1.ManagedZoneObservation {
return v1alpha1.ManagedZoneObservation{
CreationTime: observed.CreationTime,
ID: observed.Id,
Expand Down
12 changes: 4 additions & 8 deletions pkg/controller/dns/managed_zone.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ const (
errGetManagedZone = "cannot get the DNS ManagedZone"
errCreateManagedZone = "cannot create DNS ManagedZone"
errDeleteManagedZone = "cannot delete DNS ManagedZone"
errUpdateManagedZone = "cannot update DNS ManagedZone custom resource"
errUpToDateManagedZone = "cannot determine if ManagedZone is up to date"
)

Expand Down Expand Up @@ -109,7 +108,7 @@ func (e *managedZoneExternal) Observe(ctx context.Context, mg resource.Managed)
return managed.ExternalObservation{}, errors.New(errNotManagedZone)
}

mz, err := e.dns.Get(
observed, err := e.dns.Get(
e.projectID,
meta.GetExternalName(cr),
).Context(ctx).Do()
Expand All @@ -122,22 +121,19 @@ func (e *managedZoneExternal) Observe(ctx context.Context, mg resource.Managed)

lateInit := false
currentSpec := cr.Spec.ForProvider.DeepCopy()
mzclient.LateInitializeSpec(&cr.Spec.ForProvider, *mz)
mzclient.LateInitializeSpec(&cr.Spec.ForProvider, *observed)
if !cmp.Equal(currentSpec, &cr.Spec.ForProvider) {
if err := e.kube.Update(ctx, cr); err != nil {
return managed.ExternalObservation{}, errors.Wrap(err, errUpdateManagedZone)
}
lateInit = true
}

cr.Status.AtProvider = mzclient.GenerateManagedZoneObservation(mz)
cr.Status.AtProvider = mzclient.GenerateManagedZoneObservation(*observed)

cr.SetConditions(xpv1.Available())

upToDate, err := mzclient.IsUpToDate(
meta.GetExternalName(cr),
&cr.Spec.ForProvider,
mz)
observed)
if err != nil {
return managed.ExternalObservation{}, errors.Wrap(err, errUpToDateManagedZone)
}
Expand Down
31 changes: 2 additions & 29 deletions pkg/controller/dns/managed_zone_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,8 @@ const (
)

var (
nonManagedZone resource.Managed
errManagedZoneBoom = errors.New("boom")
managedZoneLabels = map[string]string{"foo": "bar"}
nonManagedZone resource.Managed
managedZoneLabels = map[string]string{"foo": "bar"}
)

type ManagedZoneOption func(*v1alpha1.ManagedZone)
Expand Down Expand Up @@ -141,32 +140,6 @@ func TestManagedZoneObserve(t *testing.T) {
}
}),
},
"UpdateResourceSpecFail": {
reason: "Should return an error if the internal update fails",
args: args{
mg: newManagedZone(),
},
want: want{
e: managed.ExternalObservation{},
err: errors.Wrap(errManagedZoneBoom, errUpdateManagedZone),
},
handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
_ = r.Body.Close()
if diff := cmp.Diff(http.MethodGet, r.Method); diff != "" {
t.Errorf("r: -want, +got:\n%s", diff)
}
w.WriteHeader(http.StatusOK)
cr := newManagedZone(withLabels(managedZoneLabels))
mz := &dns.ManagedZone{}
mzclient.GenerateManagedZone(meta.GetExternalName(cr), cr.Spec.ForProvider, mz)
if err := json.NewEncoder(w).Encode(mz); err != nil {
t.Error(err)
}
}),
kube: &test.MockClient{
MockUpdate: test.NewMockUpdateFn(errManagedZoneBoom),
},
},
"UpdateResourceSpecSuccess": {
reason: "Should not return an error if the internal update succeeds",
args: args{
Expand Down

0 comments on commit cf88e93

Please sign in to comment.