Skip to content

OCPBUGS-56169: [release-4.16] Update virtualmachines service to armcompute/v5 SDK #147

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: release-4.16
Choose a base branch
from
Open

OCPBUGS-56169: [release-4.16] Update virtualmachines service to armcompute/v5 SDK #147

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
04ba80e
Update virtualmachines service to armcompute/v5 SDK
mdbooth May 7, 2025
73b1ae4
go mod
mdbooth May 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 16 additions & 15 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ toolchain go1.21.0

require (
github.com/Azure/azure-sdk-for-go v68.0.0+incompatible
github.com/Azure/azure-sdk-for-go-extensions v0.1.8
github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5 v5.7.0
github.com/Azure/go-autorest/autorest v0.11.29
github.com/Azure/go-autorest/autorest/to v0.4.0
github.com/ghodss/yaml v1.0.0
Expand All @@ -20,7 +22,7 @@ require (
github.com/openshift/machine-api-operator v0.2.1-0.20240125175440-c9de8bda0dd1
github.com/pkg/errors v0.9.1
github.com/spf13/cobra v1.8.0
golang.org/x/crypto v0.18.0
golang.org/x/crypto v0.24.0

// kube 1.29
k8s.io/api v0.29.0
Expand All @@ -35,23 +37,23 @@ require (
)

require (
github.com/Azure/azure-sdk-for-go/sdk/azcore v1.9.1
github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.5.1
github.com/Azure/azure-sdk-for-go/sdk/azcore v1.12.0
github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.6.0
github.com/jongio/azidext/go/azidext v0.5.0
github.com/openshift/client-go v0.0.0-20240115204758-e6bf7d631d5e
github.com/openshift/library-go v0.0.0-20240116081341-964bcb3f545c
)

require (
github.com/Azure/azure-sdk-for-go/sdk/internal v1.5.1 // indirect
github.com/Azure/azure-sdk-for-go/sdk/internal v1.9.0 // indirect
github.com/Azure/go-ansiterm v0.0.0-20210617225240-d185dfc1b5a1 // indirect
github.com/Azure/go-autorest v14.2.0+incompatible // indirect
github.com/Azure/go-autorest/autorest/adal v0.9.23 // indirect
github.com/Azure/go-autorest/autorest/date v0.3.0 // indirect
github.com/Azure/go-autorest/autorest/validation v0.3.1 // indirect
github.com/Azure/go-autorest/logger v0.2.1 // indirect
github.com/Azure/go-autorest/tracing v0.6.0 // indirect
github.com/AzureAD/microsoft-authentication-library-for-go v1.2.1 // indirect
github.com/AzureAD/microsoft-authentication-library-for-go v1.2.2 // indirect
github.com/MakeNowJust/heredoc v1.0.0 // indirect
github.com/beorn7/perks v1.0.1 // indirect
github.com/blang/semver/v4 v4.0.0 // indirect
Expand All @@ -73,15 +75,15 @@ require (
github.com/gobuffalo/flect v1.0.2 // indirect
github.com/gogo/protobuf v1.3.2 // indirect
github.com/golang-jwt/jwt/v4 v4.5.0 // indirect
github.com/golang-jwt/jwt/v5 v5.2.0 // indirect
github.com/golang-jwt/jwt/v5 v5.2.1 // indirect
github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
github.com/golang/protobuf v1.5.3 // indirect
github.com/google/btree v1.0.1 // indirect
github.com/google/gnostic-models v0.6.8 // indirect
github.com/google/gofuzz v1.2.0 // indirect
github.com/google/pprof v0.0.0-20230926050212-f7f687d19a98 // indirect
github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 // indirect
github.com/google/uuid v1.5.0 // indirect
github.com/google/uuid v1.6.0 // indirect
github.com/gorilla/websocket v1.5.0 // indirect
github.com/gregjones/httpcache v0.0.0-20180305231024-9cad4c3443a7 // indirect
github.com/imdario/mergo v0.3.13 // indirect
Expand Down Expand Up @@ -109,7 +111,6 @@ require (
github.com/prometheus/common v0.45.0 // indirect
github.com/prometheus/procfs v0.12.0 // indirect
github.com/robfig/cron v1.2.0 // indirect
github.com/rogpeppe/go-internal v1.11.0 // indirect
github.com/russross/blackfriday/v2 v2.1.0 // indirect
github.com/spf13/afero v1.10.0 // indirect
github.com/spf13/pflag v1.0.5 // indirect
Expand All @@ -119,15 +120,15 @@ require (
go.uber.org/multierr v1.11.0 // indirect
go.uber.org/zap v1.26.0 // indirect
golang.org/x/exp v0.0.0-20230905200255-921286631fa9 // indirect
golang.org/x/mod v0.14.0 // indirect
golang.org/x/net v0.20.0 // indirect
golang.org/x/mod v0.17.0 // indirect
golang.org/x/net v0.26.0 // indirect
golang.org/x/oauth2 v0.12.0 // indirect
golang.org/x/sync v0.5.0 // indirect
golang.org/x/sys v0.16.0 // indirect
golang.org/x/term v0.16.0 // indirect
golang.org/x/text v0.14.0 // indirect
golang.org/x/sync v0.7.0 // indirect
golang.org/x/sys v0.21.0 // indirect
golang.org/x/term v0.21.0 // indirect
golang.org/x/text v0.16.0 // indirect
golang.org/x/time v0.3.0 // indirect
golang.org/x/tools v0.16.1 // indirect
golang.org/x/tools v0.21.1-0.20240508182429-e35e4ccd0d2d // indirect
gomodules.xyz/jsonpatch/v2 v2.4.0 // indirect
google.golang.org/appengine v1.6.7 // indirect
google.golang.org/protobuf v1.31.0 // indirect
Expand Down
74 changes: 40 additions & 34 deletions go.sum
View file
Open in desktop

Large diffs are not rendered by default.

35 changes: 35 additions & 0 deletions pkg/cloud/azure/actuators/clients.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,47 @@ limitations under the License.
package actuators

import (
"net/http"

"github.com/Azure/azure-sdk-for-go/sdk/azcore"
"github.com/Azure/azure-sdk-for-go/sdk/azcore/arm"
"github.com/Azure/azure-sdk-for-go/sdk/azcore/cloud"
"github.com/Azure/azure-sdk-for-go/sdk/azcore/policy"
"github.com/Azure/go-autorest/autorest"

"github.com/openshift/machine-api-provider-azure/pkg/cloud/azure"
)

// AzureClients contains all the Azure clients used by the scopes.
type AzureClients struct {
SubscriptionID string
Authorizer autorest.Authorizer
ResourceManagerEndpoint string

// For SDK v2
Token azcore.TokenCredential
Cloud cloud.Configuration
}

// ARMClientOptions returns default ARM client options for CAPZ SDK v2 requests.
func (c *AzureClients) ARMClientOptions() *arm.ClientOptions {
opts := &arm.ClientOptions{}
opts.Cloud = c.Cloud
opts.PerCallPolicies = []policy.Policy{
userAgentPolicy{},
}
opts.Retry.MaxRetries = -1 // Less than zero means one try and no retries.

return opts
}

// userAgentPolicy extends the "User-Agent" header on requests.
// It implements the policy.Policy interface.
type userAgentPolicy struct{}

// Do extends the "User-Agent" header of a request by appending CAPZ's user agent.
func (p userAgentPolicy) Do(req *policy.Request) (*http.Response, error) {
// FIXME: Ought to include a version after our UA string if we have one
req.Raw().Header.Set("User-Agent", req.Raw().UserAgent()+" "+azure.UserAgent)
return req.Next()
}
10 changes: 4 additions & 6 deletions pkg/cloud/azure/actuators/machine/actuator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import (
"fmt"
"time"

"github.com/Azure/go-autorest/autorest"
azureerrors "github.com/Azure/azure-sdk-for-go-extensions/pkg/errors"
machinev1 "github.com/openshift/api/machine/v1beta1"
machineapierrors "github.com/openshift/machine-api-operator/pkg/controller/machine"
"github.com/openshift/machine-api-provider-azure/pkg/cloud/azure"
Expand Down Expand Up @@ -106,15 +106,13 @@ func (a *Actuator) Create(ctx context.Context, machine *machinev1.Machine) error
klog.Errorf("Error storing machine info: %v", err)
}

var detailedError autorest.DetailedError
if errors.As(err, &detailedError) {
statusCode, ok := detailedError.StatusCode.(int)
if azErr := azureerrors.IsResponseError(err); azErr != nil {
// Any 4xx error that isn't invalid credentials should be a terminal failure.
// Invalid Credentials implies that the credentials expired between the scope creation and API calls,
// this may happen when CCO is refreshing credentials simultaneously.
// In this case we should retry as the credentials should be updated in the secret.
if ok && statusCode >= 400 && statusCode < 500 && !azure.InvalidCredentials(err) {
return a.handleMachineError(machine, machineapierrors.InvalidMachineConfiguration("failed to reconcile machine %q: %v", machine.Name, detailedError), createEventAction)
if azErr.StatusCode >= 400 && azErr.StatusCode < 500 && !azure.InvalidCredentials(err) {
return a.handleMachineError(machine, machineapierrors.InvalidMachineConfiguration("failed to reconcile machine %q: %v", machine.Name, err), createEventAction)
}
}

Expand Down
59 changes: 30 additions & 29 deletions pkg/cloud/azure/actuators/machine/actuator_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,9 @@ import (
"strings"
"testing"

"github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2021-11-01/compute"
"github.com/Azure/azure-sdk-for-go/sdk/azcore"
"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5"
"github.com/Azure/azure-sdk-for-go/services/network/mgmt/2021-02-01/network"
"github.com/Azure/go-autorest/autorest"
"github.com/Azure/go-autorest/autorest/to"
"github.com/ghodss/yaml"
"github.com/golang/mock/gomock"
Expand Down Expand Up @@ -181,21 +181,22 @@ type FakeVMService struct {
// Get returns fake success.
func (s *FakeVMService) Get(ctx context.Context, spec azure.Spec) (interface{}, error) {
s.GetCallCount++
return compute.VirtualMachine{
vm := armcompute.VirtualMachine{
ID: to.StringPtr(s.ID),
Name: to.StringPtr(s.Name),
VirtualMachineProperties: &compute.VirtualMachineProperties{
Properties: &armcompute.VirtualMachineProperties{
ProvisioningState: to.StringPtr(s.ProvisioningState),
NetworkProfile: &compute.NetworkProfile{
NetworkInterfaces: &[]compute.NetworkInterfaceReference{
NetworkProfile: &armcompute.NetworkProfile{
NetworkInterfaces: []*armcompute.NetworkInterfaceReference{
{
ID: to.StringPtr("machine-test-nic"),
NetworkInterfaceReferenceProperties: &compute.NetworkInterfaceReferenceProperties{},
ID: to.StringPtr("machine-test-nic"),
Properties: &armcompute.NetworkInterfaceReferenceProperties{},
},
},
},
},
}, nil
}
return armcompute.VirtualMachinesClientGetResponse{VirtualMachine: vm}, nil
}

// CreateOrUpdate returns fake success.
Expand Down Expand Up @@ -393,9 +394,10 @@ func (s *FakeVMCheckZonesService) Get(ctx context.Context, spec azure.Spec) (int
return nil, errors.New("vm not found")
}

return &compute.VirtualMachine{
VirtualMachineProperties: &compute.VirtualMachineProperties{},
}, nil
vm := armcompute.VirtualMachine{
Properties: &armcompute.VirtualMachineProperties{},
}
return armcompute.VirtualMachinesClientGetResponse{VirtualMachine: vm}, nil
}

// CreateOrUpdate returns fake success.
Expand Down Expand Up @@ -731,19 +733,20 @@ func TestMachineEvents(t *testing.T) {
})

networkSvc.EXPECT().CreateOrUpdate(gomock.Any(), gomock.Any()).Return(nil).AnyTimes()
vmSvc.EXPECT().Get(gomock.Any(), gomock.Any()).Return(compute.VirtualMachine{
vm := armcompute.VirtualMachine{
ID: ptr.To[string]("vm-id"),
Name: ptr.To[string]("vm-name"),
VirtualMachineProperties: &compute.VirtualMachineProperties{
Properties: &armcompute.VirtualMachineProperties{
ProvisioningState: ptr.To[string]("Succeeded"),
HardwareProfile: &compute.HardwareProfile{
VMSize: compute.VirtualMachineSizeTypesStandardB2ms,
HardwareProfile: &armcompute.HardwareProfile{
VMSize: ptr.To(armcompute.VirtualMachineSizeTypesStandardB2Ms),
},
OsProfile: &compute.OSProfile{
OSProfile: &armcompute.OSProfile{
ComputerName: ptr.To[string]("vm-name"),
},
},
}, nil).AnyTimes()
}
vmSvc.EXPECT().Get(gomock.Any(), gomock.Any()).Return(armcompute.VirtualMachinesClientGetResponse{VirtualMachine: vm}, nil).AnyTimes()
vmSvc.EXPECT().Delete(gomock.Any(), gomock.Any()).Return(nil).AnyTimes()
disksSvc.EXPECT().Delete(gomock.Any(), gomock.Any()).Return(nil).AnyTimes()
networkSvc.EXPECT().Delete(gomock.Any(), gomock.Any()).Return(nil).AnyTimes()
Expand Down Expand Up @@ -788,25 +791,21 @@ func TestStatusCodeBasedCreationErrors(t *testing.T) {
cases := []struct {
name string
operation func(actuator *Actuator, machine *machinev1.Machine)
event string
statusCode interface{}
statusCode int
requeable bool
}{
{
name: "InvalidConfig",
event: "Warning FailedCreate InvalidConfiguration: failed to reconcile machine \"azure-actuator-testing-machine\": compute.VirtualMachinesClient#CreateOrUpdate: MOCK: StatusCode=400",
statusCode: 400,
requeable: false,
},
{
name: "CreateMachine",
event: "Warning FailedCreate CreateError: failed to reconcile machine \"azure-actuator-testing-machine\"s: failed to create vm azure-actuator-testing-machine: failed to create VM: failed to create or get machine: compute.VirtualMachinesClient#CreateOrUpdate: MOCK: StatusCode=300",
statusCode: 300,
requeable: true,
},
{
name: "CreateMachine",
event: "Warning FailedCreate CreateError: failed to reconcile machine \"azure-actuator-testing-machine\"s: failed to create vm azure-actuator-testing-machine: failed to create VM: failed to create or get machine: compute.VirtualMachinesClient#CreateOrUpdate: MOCK: StatusCode=401",
statusCode: 401,
requeable: true,
},
Expand Down Expand Up @@ -850,11 +849,13 @@ func TestStatusCodeBasedCreationErrors(t *testing.T) {
})

networkSvc.EXPECT().CreateOrUpdate(gomock.Any(), gomock.Any()).Return(nil).Times(1)
azureErr := autorest.NewError("compute.VirtualMachinesClient", "CreateOrUpdate", "MOCK")
azureErr.StatusCode = tc.statusCode
var azureErr error = &azcore.ResponseError{
ErrorCode: fmt.Sprintf("StatusCode=%d", tc.statusCode),
StatusCode: tc.statusCode,
}
wrapErr := fmt.Errorf("failed to create or get machine: %w", azureErr)
vmSvc.EXPECT().CreateOrUpdate(gomock.Any(), gomock.Any()).Return(wrapErr).Times(1)
vmSvc.EXPECT().Get(gomock.Any(), gomock.Any()).Return(nil, autorest.NewError("compute.VirtualMachinesClient", "Get", "MOCK")).Times(1)
vmSvc.EXPECT().Get(gomock.Any(), gomock.Any()).Return(nil, fmt.Errorf("mock compute.VirtualMachinesClient Get error")).Times(1)
availabilityZonesSvc.EXPECT().Get(gomock.Any(), gomock.Any()).Return([]string{"testzone"}, nil).Times(1)

_, ok := machineActuator.Create(context.TODO(), machine).(*machineapierrors.RequeueAfterError)
Expand All @@ -867,11 +868,11 @@ func TestStatusCodeBasedCreationErrors(t *testing.T) {

select {
case event := <-eventsChannel:
if event != tc.event {
t.Errorf("Expected %q event, got %q", tc.event, event)
if !strings.Contains(event, azureErr.Error()) {
t.Errorf("Expected event to contain error %q, got %q", azureErr.Error(), event)
}
default:
t.Errorf("Expected %q event, got none", tc.event)
t.Errorf("Expected an event containing error %q, got none", azureErr.Error())
}
})
}
Expand Down
Loading