Skip to content

Commit

Permalink
Merge pull request #2710 from jackfrancis/async-createorupdate
Browse files Browse the repository at this point in the history 
refactor: rename CreateResource to CreateOrUpdateResource
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot authored Oct 14, 2022
2 parents 1d69efc + 84e9643 commit 94b3d74
Show file tree
Hide file tree
Showing 45 changed files with 203 additions and 196 deletions.
1 change: 1 addition & 0 deletions .codespellignore
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@ witht
geting
ot
intepreted
updat
2 changes: 1 addition & 1 deletion azure/services/agentpools/agentpools.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ func (s *Service) Reconcile(ctx context.Context) error {

var resultingErr error
if agentPoolSpec := s.scope.AgentPoolSpec(); agentPoolSpec != nil {
result, err := s.CreateResource(ctx, agentPoolSpec, serviceName)
result, err := s.CreateOrUpdateResource(ctx, agentPoolSpec, serviceName)
if err != nil {
resultingErr = err
} else {
Expand Down
6 changes: 3 additions & 3 deletions azure/services/agentpools/agentpools_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ func TestReconcileAgentPools(t *testing.T) {
expectedError: "",
expect: func(s *mock_agentpools.MockAgentPoolScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) {
s.AgentPoolSpec().Return(&fakeAgentPoolSpec)
r.CreateResource(gomockinternal.AContext(), &fakeAgentPoolSpec, serviceName).Return(fakeAgentPoolWithAutoscalingAndCount(true, 1), nil)
r.CreateOrUpdateResource(gomockinternal.AContext(), &fakeAgentPoolSpec, serviceName).Return(fakeAgentPoolWithAutoscalingAndCount(true, 1), nil)
s.SetCAPIMachinePoolAnnotation(azure.ReplicasManagedByAutoscalerAnnotation, "true")
s.SetCAPIMachinePoolReplicas(fakeAgentPoolWithAutoscalingAndCount(true, 1).Count)
s.UpdatePutStatus(infrav1.AgentPoolsReadyCondition, serviceName, nil)
Expand All @@ -81,7 +81,7 @@ func TestReconcileAgentPools(t *testing.T) {
expectedError: "",
expect: func(s *mock_agentpools.MockAgentPoolScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) {
s.AgentPoolSpec().Return(&fakeAgentPoolSpec)
r.CreateResource(gomockinternal.AContext(), &fakeAgentPoolSpec, serviceName).Return(fakeAgentPoolWithAutoscalingAndCount(false, 1), nil)
r.CreateOrUpdateResource(gomockinternal.AContext(), &fakeAgentPoolSpec, serviceName).Return(fakeAgentPoolWithAutoscalingAndCount(false, 1), nil)
s.RemoveCAPIMachinePoolAnnotation(azure.ReplicasManagedByAutoscalerAnnotation)

s.UpdatePutStatus(infrav1.AgentPoolsReadyCondition, serviceName, nil)
Expand All @@ -99,7 +99,7 @@ func TestReconcileAgentPools(t *testing.T) {
expectedError: internalError.Error(),
expect: func(s *mock_agentpools.MockAgentPoolScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) {
s.AgentPoolSpec().Return(&fakeAgentPoolSpec)
r.CreateResource(gomockinternal.AContext(), &fakeAgentPoolSpec, serviceName).Return(nil, internalError)
r.CreateOrUpdateResource(gomockinternal.AContext(), &fakeAgentPoolSpec, serviceName).Return(nil, internalError)
s.UpdatePutStatus(infrav1.AgentPoolsReadyCondition, serviceName, internalError)
},
},
Expand Down
20 changes: 13 additions & 7 deletions azure/services/async/async.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package async

import (
"context"
"fmt"
"net/http"
"strconv"
"time"
Expand Down Expand Up @@ -96,9 +97,9 @@ func processOngoingOperation(ctx context.Context, scope FutureScope, client Futu
return client.Result(ctx, sdkFuture, future.Type)
}

// CreateResource implements the logic for creating a resource Asynchronously.
func (s *Service) CreateResource(ctx context.Context, spec azure.ResourceSpecGetter, serviceName string) (result interface{}, err error) {
ctx, log, done := tele.StartSpanWithLogger(ctx, "async.Service.CreateResource")
// CreateOrUpdateResource implements the logic for creating a new, or updating an existing, resource Asynchronously.
func (s *Service) CreateOrUpdateResource(ctx context.Context, spec azure.ResourceSpecGetter, serviceName string) (result interface{}, err error) {
ctx, log, done := tele.StartSpanWithLogger(ctx, "async.Service.CreateOrUpdateResource")
defer done()

resourceName := spec.ResourceName()
Expand Down Expand Up @@ -132,20 +133,25 @@ func (s *Service) CreateResource(ctx context.Context, spec azure.ResourceSpecGet
}

// Create or update the resource with the desired parameters.
log.V(2).Info("creating resource", "service", serviceName, "resource", resourceName, "resourceGroup", rgName)
logMessageVerbPrefix := "creat"
if existingResource != nil {
logMessageVerbPrefix = "updat"
}
log.V(2).Info(fmt.Sprintf("%sing resource", logMessageVerbPrefix), "service", serviceName, "resource", resourceName, "resourceGroup", rgName)
result, sdkFuture, err := s.Creator.CreateOrUpdateAsync(ctx, spec, parameters)
errWrapped := errors.Wrapf(err, fmt.Sprintf("failed to %se resource %s/%s (service: %s)", logMessageVerbPrefix, rgName, resourceName, serviceName))
if sdkFuture != nil {
future, err := converters.SDKToFuture(sdkFuture, infrav1.PutFuture, serviceName, resourceName, rgName)
if err != nil {
return nil, errors.Wrapf(err, "failed to create resource %s/%s (service: %s)", rgName, resourceName, serviceName)
return nil, errWrapped
}
s.Scope.SetLongRunningOperationState(future)
return nil, azure.WithTransientError(azure.NewOperationNotDoneError(future), getRequeueAfterFromFuture(sdkFuture))
} else if err != nil {
return nil, errors.Wrapf(err, "failed to create resource %s/%s (service: %s)", rgName, resourceName, serviceName)
return nil, errWrapped
}

log.V(2).Info("successfully created resource", "service", serviceName, "resource", resourceName, "resourceGroup", rgName)
log.V(2).Info(fmt.Sprintf("successfully %sed resource", logMessageVerbPrefix), "service", serviceName, "resource", resourceName, "resourceGroup", rgName)
return result, nil
}

Expand Down
8 changes: 4 additions & 4 deletions azure/services/async/async_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -205,8 +205,8 @@ func TestProcessOngoingOperation(t *testing.T) {
}
}

// TestCreateResource tests the CreateResource function.
func TestCreateResource(t *testing.T) {
// TestCreateOrUpdateResource tests the CreateOrUpdateResource function.
func TestCreateOrUpdateResource(t *testing.T) {
testcases := []struct {
name string
serviceName string
Expand Down Expand Up @@ -291,7 +291,7 @@ func TestCreateResource(t *testing.T) {
},
{
name: "error occurs while running async create",
expectedError: "failed to create resource test-group/test-resource (service: test-service)",
expectedError: "failed to update resource test-group/test-resource (service: test-service)",
serviceName: "test-service",
expect: func(s *mock_async.MockFutureScopeMockRecorder, c *mock_async.MockCreatorMockRecorder, r *mock_azure.MockResourceSpecGetterMockRecorder) {
r.ResourceName().Return("test-resource")
Expand Down Expand Up @@ -333,7 +333,7 @@ func TestCreateResource(t *testing.T) {
tc.expect(scopeMock.EXPECT(), creatorMock.EXPECT(), specMock.EXPECT())

s := New(scopeMock, creatorMock, nil)
result, err := s.CreateResource(context.TODO(), specMock, tc.serviceName)
result, err := s.CreateOrUpdateResource(context.TODO(), specMock, tc.serviceName)
if tc.expectedError != "" {
g.Expect(err).To(HaveOccurred())
g.Expect(err.Error()).To(ContainSubstring(tc.expectedError))
Expand Down
2 changes: 1 addition & 1 deletion azure/services/async/interfaces.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,6 @@ type Deleter interface {

// Reconciler is a generic interface used to perform asynchronous reconciliation of Azure resources.
type Reconciler interface {
CreateResource(ctx context.Context, spec azure.ResourceSpecGetter, serviceName string) (result interface{}, err error)
CreateOrUpdateResource(ctx context.Context, spec azure.ResourceSpecGetter, serviceName string) (result interface{}, err error)
DeleteResource(ctx context.Context, spec azure.ResourceSpecGetter, serviceName string) (err error)
}
12 changes: 6 additions & 6 deletions azure/services/async/mock_async/async_mock.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions azure/services/availabilitysets/availabilitysets.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ func (s *Service) Name() string {
return serviceName
}

// Reconcile creates or updates availability sets.
// Reconcile idempotently creates or updates an availability set.
func (s *Service) Reconcile(ctx context.Context) error {
ctx, log, done := tele.StartSpanWithLogger(ctx, "availabilitysets.Service.Reconcile")
defer done()
Expand All @@ -72,7 +72,7 @@ func (s *Service) Reconcile(ctx context.Context) error {

var err error
if setSpec := s.Scope.AvailabilitySetSpec(); setSpec != nil {
_, err = s.CreateResource(ctx, setSpec, serviceName)
_, err = s.CreateOrUpdateResource(ctx, setSpec, serviceName)
} else {
log.V(2).Info("skip creation when no availability set spec is found")
return nil
Expand Down
6 changes: 3 additions & 3 deletions azure/services/availabilitysets/availabilitysets_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ func TestReconcileAvailabilitySets(t *testing.T) {
expectedError: "",
expect: func(s *mock_availabilitysets.MockAvailabilitySetScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) {
s.AvailabilitySetSpec().Return(&fakeSetSpec)
r.CreateResource(gomockinternal.AContext(), &fakeSetSpec, serviceName).Return(nil, nil)
r.CreateOrUpdateResource(gomockinternal.AContext(), &fakeSetSpec, serviceName).Return(nil, nil)
s.UpdatePutStatus(infrav1.AvailabilitySetReadyCondition, serviceName, nil)
},
},
Expand All @@ -100,7 +100,7 @@ func TestReconcileAvailabilitySets(t *testing.T) {
expectedError: "some error with parameters",
expect: func(s *mock_availabilitysets.MockAvailabilitySetScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) {
s.AvailabilitySetSpec().Return(&fakeSetSpecMissing)
r.CreateResource(gomockinternal.AContext(), &fakeSetSpecMissing, serviceName).Return(nil, parameterError)
r.CreateOrUpdateResource(gomockinternal.AContext(), &fakeSetSpecMissing, serviceName).Return(nil, parameterError)
s.UpdatePutStatus(infrav1.AvailabilitySetReadyCondition, serviceName, parameterError)
},
},
Expand All @@ -109,7 +109,7 @@ func TestReconcileAvailabilitySets(t *testing.T) {
expectedError: "#: Internal Server Error: StatusCode=500",
expect: func(s *mock_availabilitysets.MockAvailabilitySetScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) {
s.AvailabilitySetSpec().Return(&fakeSetSpec)
r.CreateResource(gomockinternal.AContext(), &fakeSetSpec, serviceName).Return(nil, internalError)
r.CreateOrUpdateResource(gomockinternal.AContext(), &fakeSetSpec, serviceName).Return(nil, internalError)
s.UpdatePutStatus(infrav1.AvailabilitySetReadyCondition, serviceName, internalError)
},
},
Expand Down
4 changes: 2 additions & 2 deletions azure/services/bastionhosts/bastionhosts.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ func (s *Service) Name() string {
return serviceName
}

// Reconcile gets/creates/updates a bastion host.
// Reconcile idempotently creates or updates a bastion host.
func (s *Service) Reconcile(ctx context.Context) error {
ctx, _, done := tele.StartSpanWithLogger(ctx, "bastionhosts.Service.Reconcile")
defer done()
Expand All @@ -65,7 +65,7 @@ func (s *Service) Reconcile(ctx context.Context) error {

var resultingErr error
if bastionSpec := s.Scope.AzureBastionSpec(); bastionSpec != nil {
_, resultingErr = s.CreateResource(ctx, bastionSpec, serviceName)
_, resultingErr = s.CreateOrUpdateResource(ctx, bastionSpec, serviceName)
} else {
return nil
}
Expand Down
4 changes: 2 additions & 2 deletions azure/services/bastionhosts/bastionhosts_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ func TestReconcileBastionHosts(t *testing.T) {
expectedError: "",
expect: func(s *mock_bastionhosts.MockBastionScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) {
s.AzureBastionSpec().Return(&fakeAzureBastionSpec)
r.CreateResource(gomockinternal.AContext(), &fakeAzureBastionSpec, serviceName).Return(nil, nil)
r.CreateOrUpdateResource(gomockinternal.AContext(), &fakeAzureBastionSpec, serviceName).Return(nil, nil)
s.UpdatePutStatus(infrav1.BastionHostReadyCondition, serviceName, nil)
},
},
Expand All @@ -76,7 +76,7 @@ func TestReconcileBastionHosts(t *testing.T) {
expectedError: internalError.Error(),
expect: func(s *mock_bastionhosts.MockBastionScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) {
s.AzureBastionSpec().Return(&fakeAzureBastionSpec)
r.CreateResource(gomockinternal.AContext(), &fakeAzureBastionSpec, serviceName).Return(nil, internalError)
r.CreateOrUpdateResource(gomockinternal.AContext(), &fakeAzureBastionSpec, serviceName).Return(nil, internalError)
s.UpdatePutStatus(infrav1.BastionHostReadyCondition, serviceName, internalError)
},
},
Expand Down
4 changes: 2 additions & 2 deletions azure/services/groups/groups.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ func (s *Service) Name() string {
return ServiceName
}

// Reconcile gets/creates/updates a resource group.
// Reconcile idempotently creates or updates a resource group.
func (s *Service) Reconcile(ctx context.Context) error {
ctx, _, done := tele.StartSpanWithLogger(ctx, "groups.Service.Reconcile")
defer done()
Expand All @@ -75,7 +75,7 @@ func (s *Service) Reconcile(ctx context.Context) error {
return nil
}

_, err := s.CreateResource(ctx, groupSpec, ServiceName)
_, err := s.CreateOrUpdateResource(ctx, groupSpec, ServiceName)
s.Scope.UpdatePutStatus(infrav1.ResourceGroupReadyCondition, ServiceName, err)
return err
}
Expand Down
4 changes: 2 additions & 2 deletions azure/services/groups/groups_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func TestReconcileGroups(t *testing.T) {
expectedError: "",
expect: func(s *mock_groups.MockGroupScopeMockRecorder, m *mock_groups.MockclientMockRecorder, r *mock_async.MockReconcilerMockRecorder) {
s.GroupSpec().Return(&fakeGroupSpec)
r.CreateResource(gomockinternal.AContext(), &fakeGroupSpec, ServiceName).Return(nil, nil)
r.CreateOrUpdateResource(gomockinternal.AContext(), &fakeGroupSpec, ServiceName).Return(nil, nil)
s.UpdatePutStatus(infrav1.ResourceGroupReadyCondition, ServiceName, nil)
},
},
Expand All @@ -82,7 +82,7 @@ func TestReconcileGroups(t *testing.T) {
expectedError: "#: Internal Server Error: StatusCode=500",
expect: func(s *mock_groups.MockGroupScopeMockRecorder, m *mock_groups.MockclientMockRecorder, r *mock_async.MockReconcilerMockRecorder) {
s.GroupSpec().Return(&fakeGroupSpec)
r.CreateResource(gomockinternal.AContext(), &fakeGroupSpec, ServiceName).Return(nil, internalError)
r.CreateOrUpdateResource(gomockinternal.AContext(), &fakeGroupSpec, ServiceName).Return(nil, internalError)
s.UpdatePutStatus(infrav1.ResourceGroupReadyCondition, ServiceName, internalError)
},
},
Expand Down
4 changes: 2 additions & 2 deletions azure/services/inboundnatrules/inboundnatrules.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func (s *Service) Name() string {
return serviceName
}

// Reconcile gets/creates/updates an inbound NAT rule.
// Reconcile idempotently creates or updates an inbound NAT rule.
func (s *Service) Reconcile(ctx context.Context) error {
ctx, log, done := tele.StartSpanWithLogger(ctx, "inboundnatrules.Service.Reconcile")
defer done()
Expand Down Expand Up @@ -107,7 +107,7 @@ func (s *Service) Reconcile(ctx context.Context) error {
natRule.SSHFrontendPort = &sshFrontendPort
// Add the SSH frontend port to the list of ports in use
portsInUse[sshFrontendPort] = struct{}{}
if _, err := s.CreateResource(ctx, natRule, serviceName); err != nil {
if _, err := s.CreateOrUpdateResource(ctx, natRule, serviceName); err != nil {
if !azure.IsOperationNotDoneError(err) || result == nil {
result = err
}
Expand Down
8 changes: 4 additions & 4 deletions azure/services/inboundnatrules/inboundnatrules_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,8 @@ func TestReconcileInboundNATRule(t *testing.T) {
m.List(gomockinternal.AContext(), fakeGroupName, fakeLBName).Return(noExistingRules, nil)
s.InboundNatSpecs().Return([]azure.ResourceSpecGetter{getFakeNatSpecWithoutPort(fakeNatSpec), getFakeNatSpecWithoutPort(fakeNatSpec2)})
gomock.InOrder(
r.CreateResource(gomockinternal.AContext(), getFakeNatSpecWithPort(fakeNatSpec, 22), serviceName).Return(nil, nil),
r.CreateResource(gomockinternal.AContext(), getFakeNatSpecWithPort(fakeNatSpec2, 2201), serviceName).Return(nil, nil),
r.CreateOrUpdateResource(gomockinternal.AContext(), getFakeNatSpecWithPort(fakeNatSpec, 22), serviceName).Return(nil, nil),
r.CreateOrUpdateResource(gomockinternal.AContext(), getFakeNatSpecWithPort(fakeNatSpec2, 2201), serviceName).Return(nil, nil),
s.UpdatePutStatus(infrav1.InboundNATRulesReadyCondition, serviceName, nil),
)
},
Expand All @@ -130,7 +130,7 @@ func TestReconcileInboundNATRule(t *testing.T) {
m.List(gomockinternal.AContext(), fakeGroupName, "my-lb").Return(fakeExistingRules, nil)
s.InboundNatSpecs().Return([]azure.ResourceSpecGetter{getFakeNatSpecWithoutPort(fakeNatSpec)})
gomock.InOrder(
r.CreateResource(gomockinternal.AContext(), getFakeNatSpecWithPort(fakeNatSpec, 2202), serviceName).Return(nil, nil),
r.CreateOrUpdateResource(gomockinternal.AContext(), getFakeNatSpecWithPort(fakeNatSpec, 2202), serviceName).Return(nil, nil),
s.UpdatePutStatus(infrav1.InboundNATRulesReadyCondition, serviceName, nil),
)
},
Expand Down Expand Up @@ -168,7 +168,7 @@ func TestReconcileInboundNATRule(t *testing.T) {
m.List(gomockinternal.AContext(), fakeGroupName, "my-lb").Return(fakeExistingRules, nil)
s.InboundNatSpecs().Return([]azure.ResourceSpecGetter{&fakeNatSpec})
gomock.InOrder(
r.CreateResource(gomockinternal.AContext(), getFakeNatSpecWithPort(fakeNatSpec, 2202), serviceName).Return(nil, internalError),
r.CreateOrUpdateResource(gomockinternal.AContext(), getFakeNatSpecWithPort(fakeNatSpec, 2202), serviceName).Return(nil, internalError),
s.UpdatePutStatus(infrav1.InboundNATRulesReadyCondition, serviceName, internalError),
)
},
Expand Down
4 changes: 2 additions & 2 deletions azure/services/loadbalancers/loadbalancers.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ func (s *Service) Name() string {
return serviceName
}

// Reconcile gets/creates/updates a load balancer.
// Reconcile idempotently creates or updates a load balancer.
func (s *Service) Reconcile(ctx context.Context) error {
ctx, _, done := tele.StartSpanWithLogger(ctx, "loadbalancers.Service.Reconcile")
defer done()
Expand All @@ -78,7 +78,7 @@ func (s *Service) Reconcile(ctx context.Context) error {
// Order of precedence (highest -> lowest) is: error that is not an operationNotDoneError (i.e. error creating) -> operationNotDoneError (i.e. creating in progress) -> no error (i.e. created)
var result error
for _, lbSpec := range specs {
if _, err := s.CreateResource(ctx, lbSpec, serviceName); err != nil {
if _, err := s.CreateOrUpdateResource(ctx, lbSpec, serviceName); err != nil {
if !azure.IsOperationNotDoneError(err) || result == nil {
result = err
}
Expand Down
Loading

0 comments on commit 94b3d74

Please sign in to comment.