Skip to content

Commit

Permalink
Controller should use HEAD if GET is not available (#3530)
Browse files Browse the repository at this point in the history
  • Loading branch information
matthchr authored Nov 19, 2023
1 parent cf4ed83 commit 69eb381
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -559,18 +559,35 @@ func (r *azureDeploymentReconcilerInstance) getStatus(ctx context.Context, id st
}

// Get the resource
retryAfter, err := r.ARMConnection.Client().GetByID(ctx, id, apiVersion, armStatus)
if err != nil {
return nil, retryAfter, errors.Wrapf(err, "getting resource with ID: %q", id)
}
if genruntime.ResourceOperationGet.IsSupportedBy(r.Obj) {
var retryAfter time.Duration
retryAfter, err = r.ARMConnection.Client().GetByID(ctx, id, apiVersion, armStatus)
if err != nil {
return nil, retryAfter, errors.Wrapf(err, "getting resource with ID: %q", id)
}

if r.Log.V(Debug).Enabled() {
statusBytes, marshalErr := json.Marshal(armStatus)
if marshalErr != nil {
return nil, zeroDuration, errors.Wrapf(marshalErr, "serializing ARM status to JSON for debugging")
}

if r.Log.V(Debug).Enabled() {
statusBytes, marshalErr := json.Marshal(armStatus)
if marshalErr != nil {
return nil, zeroDuration, errors.Wrapf(marshalErr, "serializing ARM status to JSON for debugging")
r.Log.V(Debug).Info("Got ARM status", "status", string(statusBytes))
}
} else if genruntime.ResourceOperationHead.IsSupportedBy(r.Obj) {
var retryAfter time.Duration
var exists bool
exists, retryAfter, err = r.ARMConnection.Client().CheckExistenceByID(ctx, id, apiVersion)
if err != nil {
return nil, retryAfter, errors.Wrapf(err, "getting resource with ID: %q", id)
}

r.Log.V(Debug).Info("Got ARM status", "status", string(statusBytes))
// We expect the resource to exist
if !exists {
return nil, retryAfter, errors.Wrapf(err, "getting resource with ID: %q", id)
}
} else {
return nil, zeroDuration, errors.Errorf("resource must support one of GET or HEAD, but it supports neither")
}

// Convert the ARM shape to the Kube shape
Expand Down
10 changes: 10 additions & 0 deletions v2/pkg/genruntime/base_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,16 @@ const (
ResourceOperationDelete = ResourceOperation("DELETE")
)

func (o ResourceOperation) IsSupportedBy(obj SupportedResourceOperations) bool {
for _, op := range obj.GetSupportedOperations() {
if op == o {
return true
}
}

return false
}

// TODO: It's weird that this is isn't with the other annotations
// TODO: Should we move them all here (so they're exported?) Or shold we move them
// TODO: to serviceoperator-internal.azure.com to signify they are internal?
Expand Down
10 changes: 7 additions & 3 deletions v2/pkg/genruntime/kubernetes_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,17 @@ type ARMOwned interface {
Owner() *ResourceReference
}

type SupportedResourceOperations interface {

// GetSupportedOperations gets the set of supported resource operations
GetSupportedOperations() []ResourceOperation
}

// KubernetesResource is an Azure resource. This interface contains the common set of
// methods that apply to all ASO ARM resources.
type KubernetesResource interface {
ARMOwned
SupportedResourceOperations

// TODO: I think we need this?
// KnownOwner() *KnownResourceReference
Expand All @@ -36,9 +43,6 @@ type KubernetesResource interface {
// GetResourceScope returns the ResourceScope of the resource.
GetResourceScope() ResourceScope

// GetSupportedOperations gets the set of supported resource operations
GetSupportedOperations() []ResourceOperation

// Some types, but not all, have a corresponding:
// SetAzureName(name string)
// They do not if the name must be a fixed value (like 'default').
Expand Down

0 comments on commit 69eb381

Please sign in to comment.