From e8bb33764ac556d05eafd99b2f59395e81752496 Mon Sep 17 00:00:00 2001 From: Matthew Christopher Date: Thu, 14 Jan 2021 11:19:03 -0800 Subject: [PATCH 1/2] Azure SQL FailoverGroup improvements - Fix bug preventing reconciliation of updates after a FailoverGroup was created. - Fix bug where status of long running operation was not properly monitored. --- api/v1beta1/azuresqlfailovergroup_types.go | 11 + controllers/azuresql_combined_test.go | 63 +++-- pkg/errhelp/errhelp.go | 17 ++ .../azuresqldb/azuresqldb_reconcile.go | 6 +- .../azuresqlfailovergroup.go | 208 +++++++++++--- .../azuresqlfailovergroup_manager.go | 11 +- .../azuresqlfailovergroup_reconcile.go | 143 +++++----- .../azuresqlfailovergroup_test.go | 254 ++++++++++++++++++ .../azuresql/azuresqlshared/sqlproperties.go | 44 +-- pkg/resourcemanager/pollclient/pollclient.go | 51 ++++ 10 files changed, 638 insertions(+), 170 deletions(-) create mode 100644 pkg/resourcemanager/azuresql/azuresqlfailovergroup/azuresqlfailovergroup_test.go diff --git a/api/v1beta1/azuresqlfailovergroup_types.go b/api/v1beta1/azuresqlfailovergroup_types.go index 6a5210ecad8..0b3a9b75753 100644 --- a/api/v1beta1/azuresqlfailovergroup_types.go +++ b/api/v1beta1/azuresqlfailovergroup_types.go @@ -8,8 +8,17 @@ import ( ) // NOTE: json tags are required. Any new fields you add must have json tags for the fields to be serialized. + +// ReadWriteEndpointFailoverPolicy - wraps https://godoc.org/github.com/Azure/azure-sdk-for-go/services/preview/sql/mgmt/v3.0/sql#ReadWriteEndpointFailoverPolicy type ReadWriteEndpointFailoverPolicy string +const ( + // Automatic ... + FailoverPolicyAutomatic ReadWriteEndpointFailoverPolicy = "Automatic" + // Manual ... + FailoverPolicyManual ReadWriteEndpointFailoverPolicy = "Manual" +) + // AzureSqlFailoverGroupSpec defines the desired state of AzureSqlFailoverGroup type AzureSqlFailoverGroupSpec struct { // Important: Run "make" to regenerate code after modifying this file @@ -24,6 +33,8 @@ type AzureSqlFailoverGroupSpec struct { // +kubebuilder:validation:Required Server string `json:"server"` FailoverPolicy ReadWriteEndpointFailoverPolicy `json:"failoverPolicy"` + // TODO: This field should be a ptr as it must not be specified if the failover policy is Manual, + // TODO: but is required when the policy is Automatic FailoverGracePeriod int32 `json:"failoverGracePeriod"` SecondaryServer string `json:"secondaryServer"` SecondaryServerResourceGroup string `json:"secondaryServerResourceGroup"` diff --git a/controllers/azuresql_combined_test.go b/controllers/azuresql_combined_test.go index 9a2953fd056..d075aae7e10 100644 --- a/controllers/azuresql_combined_test.go +++ b/controllers/azuresql_combined_test.go @@ -11,6 +11,7 @@ import ( "strings" "testing" + sql "github.com/Azure/azure-sdk-for-go/services/preview/sql/mgmt/v3.0/sql" "github.com/stretchr/testify/assert" v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" @@ -21,8 +22,10 @@ import ( "github.com/Azure/azure-service-operator/api/v1beta1" "github.com/Azure/azure-service-operator/pkg/errhelp" helpers "github.com/Azure/azure-service-operator/pkg/helpers" + "github.com/Azure/azure-service-operator/pkg/resourcemanager/azuresql/azuresqlshared" "github.com/Azure/azure-service-operator/pkg/resourcemanager/config" kvsecrets "github.com/Azure/azure-service-operator/pkg/secrets/keyvault" + ) func TestAzureSqlServerCombinedHappyPath(t *testing.T) { @@ -50,6 +53,7 @@ func TestAzureSqlServerCombinedHappyPath(t *testing.T) { // create and wait RequireInstance(ctx, t, tc, sqlServerInstance) + RequireInstance(ctx, t, tc, sqlServerInstance2) //verify secret exists in k8s for server 1 --------------------------------- secret := &v1.Secret{} @@ -83,6 +87,22 @@ func TestAzureSqlServerCombinedHappyPath(t *testing.T) { var sqlFirewallRuleInstanceLocal *v1beta1.AzureSqlFirewallRule var sqlFirewallRuleInstanceRemote *v1beta1.AzureSqlFirewallRule + // Create the SqlDatabase object and expect the Reconcile to be created + sqlDatabaseInstance1 = &v1beta1.AzureSqlDatabase{ + ObjectMeta: metav1.ObjectMeta{ + Name: sqlDatabaseName1, + Namespace: "default", + }, + Spec: v1beta1.AzureSqlDatabaseSpec{ + Location: rgLocation, + ResourceGroup: rgName, + Server: sqlServerName, + Edition: 0, + }, + } + + EnsureInstance(ctx, t, tc, sqlDatabaseInstance1) + // run sub tests that require 1 sql server ---------------------------------- t.Run("group1", func(t *testing.T) { t.Run("sub test for actions", func(t *testing.T) { @@ -90,33 +110,6 @@ func TestAzureSqlServerCombinedHappyPath(t *testing.T) { RunSQLActionHappy(t, sqlServerName) }) - // Wait for 2nd sql server to resolve - t.Run("set up secondary server", func(t *testing.T) { - t.Parallel() - EnsureInstance(ctx, t, tc, sqlServerInstance2) - }) - - // Create a database in the new server - t.Run("set up database in primary server using edition", func(t *testing.T) { - t.Parallel() - - // Create the SqlDatabase object and expect the Reconcile to be created - sqlDatabaseInstance1 = &v1beta1.AzureSqlDatabase{ - ObjectMeta: metav1.ObjectMeta{ - Name: sqlDatabaseName1, - Namespace: "default", - }, - Spec: v1beta1.AzureSqlDatabaseSpec{ - Location: rgLocation, - ResourceGroup: rgName, - Server: sqlServerName, - Edition: 0, - }, - } - - EnsureInstance(ctx, t, tc, sqlDatabaseInstance1) - }) - t.Run("set up second database in primary server using sku with maxsizebytes, then update it to use a different SKU", func(t *testing.T) { t.Parallel() @@ -507,7 +500,7 @@ func TestAzureSqlServerCombinedHappyPath(t *testing.T) { Location: rgLocation, ResourceGroup: rgName, Server: sqlServerName, - FailoverPolicy: "automatic", + FailoverPolicy: v1beta1.FailoverPolicyAutomatic, FailoverGracePeriod: 30, SecondaryServer: sqlServerTwoName, SecondaryServerResourceGroup: rgName, @@ -524,6 +517,20 @@ func TestAzureSqlServerCombinedHappyPath(t *testing.T) { return strings.Contains(string(secrets["azureSqlPrimaryServer"]), sqlServerName) }, tc.timeout, tc.retry, "wait for secret store to show failovergroup server names ") + sqlFailoverGroupInstance.Spec.FailoverPolicy = v1beta1.FailoverPolicyManual + sqlFailoverGroupInstance.Spec.FailoverGracePeriod = 0 // GracePeriod cannot be set when policy is manual + + err = tc.k8sClient.Update(ctx, sqlFailoverGroupInstance) + assert.Equal(nil, err, "updating sql failover group in k8s") + + failoverGroupsClient, err := azuresqlshared.GetGoFailoverGroupsClient(config.GlobalCredentials()) + assert.Equal(nil, err, "getting failovergroup client") + + assert.Eventually(func() bool { + fog, err := failoverGroupsClient.Get(ctx, rgName, sqlServerName, sqlFailoverGroupName) + assert.Equal(nil, err, "err getting failover group from Azure") + return fog.ReadWriteEndpoint.FailoverPolicy == sql.Manual + }, tc.timeout, tc.retry, "wait for sql failover group failover policy to be updated in Azure") }) }) diff --git a/pkg/errhelp/errhelp.go b/pkg/errhelp/errhelp.go index 99a9482f675..8d9e1968d84 100644 --- a/pkg/errhelp/errhelp.go +++ b/pkg/errhelp/errhelp.go @@ -7,6 +7,8 @@ import ( "fmt" "regexp" "strings" + + "github.com/Azure/azure-service-operator/pkg/helpers" ) // ErrIdsRegex is used to find and remove uuids from errors @@ -40,3 +42,18 @@ func StripErrorTimes(err string) string { return ErrTimesRegex.ReplaceAllString(err, "") } + +func HandleEnsureError(err error, allowedErrorTypes []string, unrecoverableErrorTypes []string) (bool, error) { + azerr := NewAzureError(err) + if helpers.ContainsString(allowedErrorTypes, azerr.Type) { + return false, nil // false means the resource is not in a terminal state yet, keep trying to reconcile. + } + if helpers.ContainsString(unrecoverableErrorTypes, azerr.Type) { + // Unrecoverable error, so stop reconcilation + return true, nil + } + + // We don't know how to classify this error, so bubble it up in the operator logs but don't assume it's + // unrecoverable/terminal + return false, err +} diff --git a/pkg/resourcemanager/azuresql/azuresqldb/azuresqldb_reconcile.go b/pkg/resourcemanager/azuresql/azuresqldb/azuresqldb_reconcile.go index f9a878c7f26..c7fc3c9322b 100644 --- a/pkg/resourcemanager/azuresql/azuresqldb/azuresqldb_reconcile.go +++ b/pkg/resourcemanager/azuresql/azuresqldb/azuresqldb_reconcile.go @@ -115,10 +115,10 @@ func (db *AzureSqlDbManager) Ensure(ctx context.Context, obj runtime.Object, opt server, dbName, azuresqlshared.SQLDatabaseBackupLongTermRetentionPolicy{ - WeeklyRetention: instance.Spec.WeeklyRetention, + WeeklyRetention: instance.Spec.WeeklyRetention, MonthlyRetention: instance.Spec.MonthlyRetention, - YearlyRetention: instance.Spec.YearlyRetention, - WeekOfYear: instance.Spec.WeekOfYear, + YearlyRetention: instance.Spec.YearlyRetention, + WeekOfYear: instance.Spec.WeekOfYear, }) if err != nil { failureErrors := []string{ diff --git a/pkg/resourcemanager/azuresql/azuresqlfailovergroup/azuresqlfailovergroup.go b/pkg/resourcemanager/azuresql/azuresqlfailovergroup/azuresqlfailovergroup.go index fca7fe26276..07b18a1c7e0 100644 --- a/pkg/resourcemanager/azuresql/azuresqlfailovergroup/azuresqlfailovergroup.go +++ b/pkg/resourcemanager/azuresql/azuresqlfailovergroup/azuresqlfailovergroup.go @@ -7,15 +7,14 @@ import ( "context" "net/http" + sql "github.com/Azure/azure-sdk-for-go/services/preview/sql/mgmt/v3.0/sql" + "github.com/Azure/go-autorest/autorest" + "k8s.io/apimachinery/pkg/runtime" + "github.com/Azure/azure-service-operator/api/v1beta1" azuresqlshared "github.com/Azure/azure-service-operator/pkg/resourcemanager/azuresql/azuresqlshared" "github.com/Azure/azure-service-operator/pkg/resourcemanager/config" "github.com/Azure/azure-service-operator/pkg/secrets" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/types" - - sql "github.com/Azure/azure-sdk-for-go/services/preview/sql/mgmt/v3.0/sql" - "github.com/Azure/go-autorest/autorest" ) type AzureSqlFailoverGroupManager struct { @@ -61,6 +60,7 @@ func (m *AzureSqlFailoverGroupManager) GetDB(ctx context.Context, resourceGroupN ) } +// TODO: Delete this? // GetFailoverGroup retrieves a failover group func (m *AzureSqlFailoverGroupManager) GetFailoverGroup(ctx context.Context, resourceGroupName string, serverName string, failovergroupname string) (sql.FailoverGroup, error) { failoverGroupsClient, err := azuresqlshared.GetGoFailoverGroupsClient(m.Creds) @@ -89,17 +89,18 @@ func (m *AzureSqlFailoverGroupManager) DeleteFailoverGroup(ctx context.Context, if err != nil { return result, nil } - // check to see if the failover group exists, if it doesn't then short-circuit - _, err = m.GetFailoverGroup(ctx, resourceGroupName, serverName, failoverGroupName) - if err != nil { - return result, nil - } failoverGroupsClient, err := azuresqlshared.GetGoFailoverGroupsClient(m.Creds) if err != nil { return result, err } + // check to see if the failover group exists, if it doesn't then short-circuit + _, err = failoverGroupsClient.Get(ctx, resourceGroupName, serverName, failoverGroupName) + if err != nil { + return result, nil + } + future, err := failoverGroupsClient.Delete( ctx, resourceGroupName, @@ -113,22 +114,15 @@ func (m *AzureSqlFailoverGroupManager) DeleteFailoverGroup(ctx context.Context, return future.Result(failoverGroupsClient) } -// CreateOrUpdateFailoverGroup creates a failover group -func (m *AzureSqlFailoverGroupManager) CreateOrUpdateFailoverGroup(ctx context.Context, resourceGroupName string, serverName string, failovergroupname string, properties azuresqlshared.SQLFailoverGroupProperties) (result sql.FailoverGroupsCreateOrUpdateFuture, err error) { - failoverGroupsClient, err := azuresqlshared.GetGoFailoverGroupsClient(m.Creds) +// TransformToSQLFailoverGroup translates the Kubernetes shaped v1beta1.AzureSqlFailoverGroup into the Azure SDK sql.FailoverGroup. +// This function makes a number of remote calls and so should be called sparingly. +func (m *AzureSqlFailoverGroupManager) TransformToSQLFailoverGroup(ctx context.Context, instance *v1beta1.AzureSqlFailoverGroup) (sql.FailoverGroup, error) { + secondaryServer, err := m.GetServer(ctx, instance.Spec.SecondaryServerResourceGroup, instance.Spec.SecondaryServer) if err != nil { - return sql.FailoverGroupsCreateOrUpdateFuture{}, err - } - - // Construct a PartnerInfo object from the server name - // Get resource ID from the servername to use - - server, err := m.GetServer(ctx, properties.SecondaryServerResourceGroup, properties.SecondaryServer) - if err != nil { - return result, nil + return sql.FailoverGroup{}, err } - secServerResourceID := server.ID + secServerResourceID := secondaryServer.ID partnerServerInfo := sql.PartnerInfo{ ID: secServerResourceID, ReplicationRole: sql.Secondary, @@ -139,19 +133,30 @@ func (m *AzureSqlFailoverGroupManager) CreateOrUpdateFailoverGroup(ctx context.C var databaseIDArray []string // Parse the Databases in the Databaselist and form array of Resource IDs - for _, each := range properties.DatabaseList { - database, err := m.GetDB(ctx, resourceGroupName, serverName, each) + for _, each := range instance.Spec.DatabaseList { + database, err := m.GetDB(ctx, instance.Spec.ResourceGroup, instance.Spec.Server, each) if err != nil { - return result, err + return sql.FailoverGroup{}, err } databaseIDArray = append(databaseIDArray, *database.ID) } + failoverPolicy, err := azuresqlshared.TranslateFailoverPolicy(instance.Spec.FailoverPolicy) + if err != nil { + return sql.FailoverGroup{}, err + } + // Construct FailoverGroupProperties struct + failoverGracePeriod := &instance.Spec.FailoverGracePeriod + // TODO: This is a bit of a hack right now because the spec doesn't allow us to omit this field, but it + // TODO: cannot be specified when using Manual mode + if failoverPolicy == sql.Manual { + failoverGracePeriod = nil + } failoverGroupProperties := sql.FailoverGroupProperties{ ReadWriteEndpoint: &sql.FailoverGroupReadWriteEndpoint{ - FailoverPolicy: azuresqlshared.TranslateFailoverPolicy(properties.FailoverPolicy), - FailoverWithDataLossGracePeriodMinutes: &properties.FailoverGracePeriod, + FailoverPolicy: failoverPolicy, + FailoverWithDataLossGracePeriodMinutes: failoverGracePeriod, }, PartnerServers: &partnerServerInfoArray, Databases: &databaseIDArray, @@ -161,31 +166,146 @@ func (m *AzureSqlFailoverGroupManager) CreateOrUpdateFailoverGroup(ctx context.C FailoverGroupProperties: &failoverGroupProperties, } + return failoverGroup, nil +} + +// CreateOrUpdateFailoverGroup creates a failover group +func (m *AzureSqlFailoverGroupManager) CreateOrUpdateFailoverGroup( + ctx context.Context, + resourceGroup string, + server string, + failoverGroupName string, + failoverGroupProperties sql.FailoverGroup) (sql.FailoverGroupsCreateOrUpdateFuture, error) { + + failoverGroupsClient, err := azuresqlshared.GetGoFailoverGroupsClient(m.Creds) + if err != nil { + return sql.FailoverGroupsCreateOrUpdateFuture{}, err + } + return failoverGroupsClient.CreateOrUpdate( ctx, - resourceGroupName, - serverName, - failovergroupname, - failoverGroup) + resourceGroup, + server, + failoverGroupName, + failoverGroupProperties) } -func (m *AzureSqlFailoverGroupManager) GetOrPrepareSecret(ctx context.Context, instance *v1beta1.AzureSqlFailoverGroup) (map[string][]byte, error) { - failovergroupname := instance.ObjectMeta.Name - azuresqlprimaryserver := instance.Spec.Server - azuresqlsecondaryserver := instance.Spec.SecondaryServer +func (m *AzureSqlFailoverGroupManager) NewSecret(instance *v1beta1.AzureSqlFailoverGroup) map[string][]byte { + failoverGroupName := instance.ObjectMeta.Name + azureSQLPrimaryServer := instance.Spec.Server + azureSQLSecondaryServer := instance.Spec.SecondaryServer secret := map[string][]byte{} - key := types.NamespacedName{Name: failovergroupname, Namespace: instance.Namespace} + // TODO: In a future version we should consider moving these values to properties on status or something rather than + // TODO: indirecting them through KeyVault, since really none of these values are actually secrets + secret["azureSqlPrimaryServer"] = []byte(azureSQLPrimaryServer) + secret["readWriteListenerEndpoint"] = []byte(failoverGroupName + "." + config.Environment().SQLDatabaseDNSSuffix) + secret["azureSqlSecondaryServer"] = []byte(azureSQLSecondaryServer) + secret["readOnlyListenerEndpoint"] = []byte(failoverGroupName + ".secondary." + config.Environment().SQLDatabaseDNSSuffix) + + return secret +} + +func doReadWriteEndpointsMatch(expected *sql.FailoverGroupReadWriteEndpoint, actual *sql.FailoverGroupReadWriteEndpoint) bool { + if expected == nil && actual == nil { + return true + } + + if (expected == nil) != (actual == nil) { + return false + } + + if expected.FailoverPolicy != actual.FailoverPolicy { + return false + } - if stored, err := m.SecretClient.Get(ctx, key); err == nil { - return stored, nil + if expected.FailoverWithDataLossGracePeriodMinutes == nil && actual.FailoverWithDataLossGracePeriodMinutes != nil || + expected.FailoverWithDataLossGracePeriodMinutes != nil && actual.FailoverWithDataLossGracePeriodMinutes == nil { + return false } - secret["azureSqlPrimaryServer"] = []byte(azuresqlprimaryserver) - secret["readWriteListenerEndpoint"] = []byte(failovergroupname + "." + config.Environment().SQLDatabaseDNSSuffix) - secret["azureSqlSecondaryServer"] = []byte(azuresqlsecondaryserver) - secret["readOnlyListenerEndpoint"] = []byte(failovergroupname + ".secondary." + config.Environment().SQLDatabaseDNSSuffix) + if expected.FailoverWithDataLossGracePeriodMinutes == nil && actual.FailoverWithDataLossGracePeriodMinutes == nil { + return true + } + + return *expected.FailoverWithDataLossGracePeriodMinutes == *actual.FailoverWithDataLossGracePeriodMinutes +} + +func doDatabasesMatch(expectedDatabases *[]string, actualDatabases *[]string) bool { + if (expectedDatabases == nil) != (actualDatabases == nil) { + return false + } + if expectedDatabases == nil && actualDatabases == nil { + return true + } + expected := *expectedDatabases + actual := *actualDatabases + + if len(expected) != len(actual) { + return false + } + for i, v1 := range expected { + v2 := actual[i] + if v1 != v2 { + return false + } + } + + return true +} + +func doPartnerServersMatch(expectedPartnerServers *[]sql.PartnerInfo, actualPartnerServers *[]sql.PartnerInfo) bool { + if (expectedPartnerServers == nil) != (actualPartnerServers == nil) { + return false + } + if expectedPartnerServers != nil && actualPartnerServers != nil { + if len(*expectedPartnerServers) != len(*actualPartnerServers) { + return false + } + for i, v1 := range *expectedPartnerServers { + v2 := (*actualPartnerServers)[i] + if v1.ID == nil && v2.ID != nil || + v1.ID != nil && v2.ID == nil { + return false + } + if v1.ID != nil && v2.ID != nil { + if *v1.ID != *v2.ID { + return false + } + } + + } + } + + return true +} + +func DoesResourceMatchAzure(expected sql.FailoverGroup, actual sql.FailoverGroup) bool { + if len(expected.Tags) != len(actual.Tags) { + return false + } + for k, v := range expected.Tags { + if v != actual.Tags[k] { + return false + } + } + + if (expected.FailoverGroupProperties == nil) != (actual.FailoverGroupProperties == nil) { + return false + } + if expected.FailoverGroupProperties != nil && actual.FailoverGroupProperties != nil { + // We care about ReadWriteEndpoint, PartnerServers, and Databases + if !doReadWriteEndpointsMatch(expected.FailoverGroupProperties.ReadWriteEndpoint, actual.FailoverGroupProperties.ReadWriteEndpoint) { + return false + } + if !doDatabasesMatch(expected.FailoverGroupProperties.Databases, actual.FailoverGroupProperties.Databases) { + return false + } + if !doPartnerServersMatch(expected.FailoverGroupProperties.PartnerServers, actual.FailoverGroupProperties.PartnerServers) { + return false + } + } - return secret, nil + return true } diff --git a/pkg/resourcemanager/azuresql/azuresqlfailovergroup/azuresqlfailovergroup_manager.go b/pkg/resourcemanager/azuresql/azuresqlfailovergroup/azuresqlfailovergroup_manager.go index c0e83e1bbe1..1f2bd49551b 100644 --- a/pkg/resourcemanager/azuresql/azuresqlfailovergroup/azuresqlfailovergroup_manager.go +++ b/pkg/resourcemanager/azuresql/azuresqlfailovergroup/azuresqlfailovergroup_manager.go @@ -7,13 +7,18 @@ import ( "context" "github.com/Azure/azure-sdk-for-go/services/preview/sql/mgmt/v3.0/sql" - "github.com/Azure/azure-service-operator/pkg/resourcemanager" - azuresqlshared "github.com/Azure/azure-service-operator/pkg/resourcemanager/azuresql/azuresqlshared" "github.com/Azure/go-autorest/autorest" + + "github.com/Azure/azure-service-operator/pkg/resourcemanager" ) type SqlFailoverGroupManager interface { - CreateOrUpdateFailoverGroup(ctx context.Context, resourceGroupName string, serverName string, failovergroupname string, properties azuresqlshared.SQLFailoverGroupProperties) (result sql.FailoverGroupsCreateOrUpdateFuture, err error) + CreateOrUpdateFailoverGroup( + ctx context.Context, + resourceGroup string, + server string, + failoverGroupName string, + failoverGroupProperties sql.FailoverGroup) (sql.FailoverGroupsCreateOrUpdateFuture, error) DeleteFailoverGroup(ctx context.Context, resourceGroupName string, serverName string, failoverGroupName string) (result autorest.Response, err error) GetFailoverGroup(ctx context.Context, resourceGroupName string, serverName string, failovergroupname string) (sql.FailoverGroup, error) GetServer(ctx context.Context, resourceGroupName string, serverName string) (result sql.Server, err error) diff --git a/pkg/resourcemanager/azuresql/azuresqlfailovergroup/azuresqlfailovergroup_reconcile.go b/pkg/resourcemanager/azuresql/azuresqlfailovergroup/azuresqlfailovergroup_reconcile.go index 99b16aa1a7c..a21d5eb739e 100644 --- a/pkg/resourcemanager/azuresql/azuresqlfailovergroup/azuresqlfailovergroup_reconcile.go +++ b/pkg/resourcemanager/azuresql/azuresqlfailovergroup/azuresqlfailovergroup_reconcile.go @@ -7,16 +7,18 @@ import ( "context" "fmt" - "github.com/Azure/azure-service-operator/api/v1alpha1" + "github.com/pkg/errors" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + azurev1alpha1 "github.com/Azure/azure-service-operator/api/v1alpha1" "github.com/Azure/azure-service-operator/api/v1beta1" "github.com/Azure/azure-service-operator/pkg/errhelp" "github.com/Azure/azure-service-operator/pkg/helpers" "github.com/Azure/azure-service-operator/pkg/resourcemanager" - azuresqlshared "github.com/Azure/azure-service-operator/pkg/resourcemanager/azuresql/azuresqlshared" + "github.com/Azure/azure-service-operator/pkg/resourcemanager/azuresql/azuresqlshared" + "github.com/Azure/azure-service-operator/pkg/resourcemanager/pollclient" "github.com/Azure/azure-service-operator/pkg/secrets" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/types" ) // Ensure creates a sqlfailovergroup @@ -35,84 +37,101 @@ func (fg *AzureSqlFailoverGroupManager) Ensure(ctx context.Context, obj runtime. return false, err } - groupName := instance.Spec.ResourceGroup - serverName := instance.Spec.Server + pClient := pollclient.NewPollClient(fg.Creds) + lroPollResult, err := pClient.PollLongRunningOperationIfNeeded(ctx, &instance.Status) + if err != nil { + instance.Status.Message = err.Error() + return false, err + } + if lroPollResult == pollclient.PollResultTryAgainLater { + // Need to wait a bit before trying again + return false, nil + } + + failoverGroupsClient, err := azuresqlshared.GetGoFailoverGroupsClient(fg.Creds) + if err != nil { + return false, errors.Wrapf(err, "failed to create failovergroup client") + } + + notFoundErrorCodes := []string{ + errhelp.ParentNotFoundErrorCode, + errhelp.ResourceGroupNotFoundErrorCode, + errhelp.NotFoundErrorCode, + errhelp.ResourceNotFound, + } + failoverGroupName := instance.ObjectMeta.Name - sqlFailoverGroupProperties := azuresqlshared.SQLFailoverGroupProperties{ - FailoverPolicy: v1alpha1.ReadWriteEndpointFailoverPolicy(instance.Spec.FailoverPolicy), - FailoverGracePeriod: instance.Spec.FailoverGracePeriod, - SecondaryServer: instance.Spec.SecondaryServer, - SecondaryServerResourceGroup: instance.Spec.SecondaryServerResourceGroup, - DatabaseList: instance.Spec.DatabaseList, + azureFailoverGroup, err := failoverGroupsClient.Get(ctx, instance.Spec.ResourceGroup, instance.Spec.Server, failoverGroupName) + if err != nil { + azerr := errhelp.NewAzureError(err) + if azerr.Type != errhelp.ResourceNotFound { + instance.Status.Message = fmt.Sprintf("AzureSqlFailoverGroup Get error %s", err.Error()) + return errhelp.HandleEnsureError(err, notFoundErrorCodes, nil) + } } - resp, err := fg.GetFailoverGroup(ctx, groupName, serverName, failoverGroupName) - if err == nil { + failoverGroupProperties, err := fg.TransformToSQLFailoverGroup(ctx, instance) + if err != nil { + instance.Status.Message = err.Error() + return errhelp.HandleEnsureError(err, notFoundErrorCodes, nil) + } + + // We found a failover group, check to make sure that it matches what we have locally + resourceMatchesAzure := DoesResourceMatchAzure(failoverGroupProperties, azureFailoverGroup) - if *resp.ReplicationState == "SEEDING" { + if resourceMatchesAzure { + if *azureFailoverGroup.ReplicationState == "SEEDING" { + instance.Status.Message = "FailoverGroup replicationState is SEEDING" return false, nil } - instance.Status.Provisioning = false - instance.Status.Provisioned = true - instance.Status.Message = resourcemanager.SuccessMsg - instance.Status.ResourceId = *resp.ID + key := types.NamespacedName{Name: instance.ObjectMeta.Name, Namespace: instance.Namespace} + _, err := fg.SecretClient.Get(ctx, key) + // We make the same assumption many other places in the code make which is that if we cannot + // get the secret it must not exist. + if err != nil { + // Create a new secret + secret := fg.NewSecret(instance) + + // create or update the secret + err = fg.SecretClient.Upsert( + ctx, + key, + secret, + secrets.WithOwner(instance), + secrets.WithScheme(fg.Scheme), + ) + if err != nil { + instance.Status.Message = fmt.Sprintf("failed to update secret: %s", err.Error()) + return false, err + } + } + + instance.Status.SetProvisioned(resourcemanager.SuccessMsg) + instance.Status.ResourceId = *azureFailoverGroup.ID return true, nil } - instance.Status.Message = fmt.Sprintf("AzureSqlFailoverGroup Get error %s", err.Error()) - requeuErrors := []string{ - errhelp.ResourceGroupNotFoundErrorCode, - errhelp.ParentNotFoundErrorCode, - } - azerr := errhelp.NewAzureError(err) - if helpers.ContainsString(requeuErrors, azerr.Type) { - instance.Status.Provisioning = false - return false, nil - } - _, err = fg.CreateOrUpdateFailoverGroup(ctx, groupName, serverName, failoverGroupName, sqlFailoverGroupProperties) + // We need to reconcile as our state didn't match Azure + instance.Status.SetProvisioning("") + + future, err := fg.CreateOrUpdateFailoverGroup(ctx, instance.Spec.ResourceGroup, instance.Spec.Server, failoverGroupName, failoverGroupProperties) if err != nil { instance.Status.Message = err.Error() - catch := []string{ - errhelp.ParentNotFoundErrorCode, - errhelp.ResourceGroupNotFoundErrorCode, - errhelp.NotFoundErrorCode, + allowedErrors := []string{ errhelp.AsyncOpIncompleteError, - errhelp.ResourceNotFound, errhelp.AlreadyExists, errhelp.FailoverGroupBusy, } - catchUnrecoverableErrors := []string{ + unrecoverableErrors := []string{ errhelp.InvalidFailoverGroupRegion, } - azerr := errhelp.NewAzureError(err) - if helpers.ContainsString(catch, azerr.Type) { - return false, nil - } - if helpers.ContainsString(catchUnrecoverableErrors, azerr.Type) { - // Unrecoverable error, so stop reconcilation - instance.Status.Message = "Reconcilation hit unrecoverable error " + err.Error() - return true, nil - } - return false, err - } - - secret, _ := fg.GetOrPrepareSecret(ctx, instance) - - // create or update the secret - key := types.NamespacedName{Name: instance.ObjectMeta.Name, Namespace: instance.Namespace} - err = fg.SecretClient.Upsert( - ctx, - key, - secret, - secrets.WithOwner(instance), - secrets.WithScheme(fg.Scheme), - ) - if err != nil { - return false, err + return errhelp.HandleEnsureError(err, append(allowedErrors, notFoundErrorCodes...), unrecoverableErrors) } + instance.Status.SetProvisioning("Resource request successfully submitted to Azure") + instance.Status.PollingURL = future.PollingURL() - // create was received successfully but replication is not done + // Need to poll the polling URL, so not done yet! return false, nil } diff --git a/pkg/resourcemanager/azuresql/azuresqlfailovergroup/azuresqlfailovergroup_test.go b/pkg/resourcemanager/azuresql/azuresqlfailovergroup/azuresqlfailovergroup_test.go new file mode 100644 index 00000000000..116b32d6cfb --- /dev/null +++ b/pkg/resourcemanager/azuresql/azuresqlfailovergroup/azuresqlfailovergroup_test.go @@ -0,0 +1,254 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +package azuresqlfailovergroup_test + +import ( + "testing" + + sql "github.com/Azure/azure-sdk-for-go/services/preview/sql/mgmt/v3.0/sql" + "github.com/Azure/go-autorest/autorest/to" + "github.com/stretchr/testify/assert" + + "github.com/Azure/azure-service-operator/pkg/resourcemanager/azuresql/azuresqlfailovergroup" +) + +func toPartnerInfoSlicePtr(info []sql.PartnerInfo) *[]sql.PartnerInfo { + return &info +} + +func TestDoesResourceMatchAzure(t *testing.T) { + + cases := []struct { + caseName string + expected sql.FailoverGroup + actual sql.FailoverGroup + match bool + }{ + { + "differ only by readonly fields matches", + sql.FailoverGroup{ + Location: to.StringPtr("westus"), + ID: to.StringPtr("a"), + Name: to.StringPtr("a"), + }, + sql.FailoverGroup{ + Location: to.StringPtr("eastus"), + ID: to.StringPtr("b"), + Name: to.StringPtr("b"), + }, + true, + }, + { + "differ only by tag values do not match", + sql.FailoverGroup{ + Tags: map[string]*string{ + "a": to.StringPtr("b"), + }, + }, + sql.FailoverGroup{ + Tags: map[string]*string{ + "a": to.StringPtr("c"), + }, + }, + false, + }, + { + "differ only by tag keys do not match", + sql.FailoverGroup{ + Tags: map[string]*string{ + "a": to.StringPtr("a"), + }, + }, + sql.FailoverGroup{ + Tags: map[string]*string{ + "b": to.StringPtr("a"), + }, + }, + false, + }, + { + "different tag length do not match", + sql.FailoverGroup{ + Tags: map[string]*string{ + "a": to.StringPtr("a"), + }, + }, + sql.FailoverGroup{ + Tags: nil, + }, + false, + }, + { + "differ by failovergroup properties presence do not match", + sql.FailoverGroup{ + FailoverGroupProperties: &sql.FailoverGroupProperties{}, + }, + sql.FailoverGroup{ + FailoverGroupProperties: nil, + }, + false, + }, + { + // TODO: We don't support ReadOnlyEndpoint which is why we don't consider it -- should we? + "differ by readonly endpoint do match", + sql.FailoverGroup{ + FailoverGroupProperties: &sql.FailoverGroupProperties{ + ReadOnlyEndpoint: &sql.FailoverGroupReadOnlyEndpoint{ + FailoverPolicy: sql.ReadOnlyEndpointFailoverPolicyDisabled, + }, + }, + }, + sql.FailoverGroup{ + FailoverGroupProperties: &sql.FailoverGroupProperties{ + ReadOnlyEndpoint: &sql.FailoverGroupReadOnlyEndpoint{ + FailoverPolicy: sql.ReadOnlyEndpointFailoverPolicyEnabled, + }, + }, + }, + true, + }, + { + "differ by readwrite endpoint do not match", + sql.FailoverGroup{ + FailoverGroupProperties: &sql.FailoverGroupProperties{ + ReadWriteEndpoint: &sql.FailoverGroupReadWriteEndpoint{ + FailoverPolicy: sql.Automatic, + }, + }, + }, + sql.FailoverGroup{ + FailoverGroupProperties: &sql.FailoverGroupProperties{ + ReadWriteEndpoint: &sql.FailoverGroupReadWriteEndpoint{ + FailoverPolicy: sql.Manual, + }, + }, + }, + false, + }, + { + "differ by readwrite endpoint grace period do not match", + sql.FailoverGroup{ + FailoverGroupProperties: &sql.FailoverGroupProperties{ + ReadWriteEndpoint: &sql.FailoverGroupReadWriteEndpoint{ + FailoverWithDataLossGracePeriodMinutes: to.Int32Ptr(10), + }, + }, + }, + sql.FailoverGroup{ + FailoverGroupProperties: &sql.FailoverGroupProperties{ + ReadWriteEndpoint: &sql.FailoverGroupReadWriteEndpoint{ + FailoverWithDataLossGracePeriodMinutes: to.Int32Ptr(17), + }, + }, + }, + false, + }, + { + "differ by databases do not match", + sql.FailoverGroup{ + FailoverGroupProperties: &sql.FailoverGroupProperties{ + Databases: to.StringSlicePtr([]string{ + "a", "b", "c", + }), + }, + }, + sql.FailoverGroup{ + FailoverGroupProperties: &sql.FailoverGroupProperties{ + Databases: to.StringSlicePtr([]string{ + "a", "b", + }), + }, + }, + false, + }, + { + "same databases do match", + sql.FailoverGroup{ + FailoverGroupProperties: &sql.FailoverGroupProperties{ + Databases: to.StringSlicePtr([]string{ + "a", "b", "c", + }), + }, + }, + sql.FailoverGroup{ + FailoverGroupProperties: &sql.FailoverGroupProperties{ + Databases: to.StringSlicePtr([]string{ + "a", "b", "c", + }), + }, + }, + true, + }, + { + "differ by partnerServers do not match", + sql.FailoverGroup{ + FailoverGroupProperties: &sql.FailoverGroupProperties{ + PartnerServers: toPartnerInfoSlicePtr([]sql.PartnerInfo{ + { + ID: to.StringPtr("a"), + }, + }), + }, + }, + sql.FailoverGroup{ + FailoverGroupProperties: &sql.FailoverGroupProperties{ + PartnerServers: nil, + }, + }, + false, + }, + { + "differ by partnerServers do not match", + sql.FailoverGroup{ + FailoverGroupProperties: &sql.FailoverGroupProperties{ + PartnerServers: toPartnerInfoSlicePtr([]sql.PartnerInfo{ + { + ID: to.StringPtr("a"), + }, + }), + }, + }, + sql.FailoverGroup{ + FailoverGroupProperties: &sql.FailoverGroupProperties{ + PartnerServers: toPartnerInfoSlicePtr([]sql.PartnerInfo{ + { + ID: to.StringPtr("b"), + }, + }), + }, + }, + false, + }, + { + "same partnerServers do match", + sql.FailoverGroup{ + FailoverGroupProperties: &sql.FailoverGroupProperties{ + PartnerServers: toPartnerInfoSlicePtr([]sql.PartnerInfo{ + { + ID: to.StringPtr("a"), + }, + }), + }, + }, + sql.FailoverGroup{ + FailoverGroupProperties: &sql.FailoverGroupProperties{ + PartnerServers: toPartnerInfoSlicePtr([]sql.PartnerInfo{ + { + ID: to.StringPtr("a"), + }, + }), + }, + }, + true, + }, + } + + for _, c := range cases { + c := c + t.Run(c.caseName, func(t *testing.T) { + match := azuresqlfailovergroup.DoesResourceMatchAzure(c.expected, c.actual) + assert.Equal(t, c.match, match) + }) + } +} diff --git a/pkg/resourcemanager/azuresql/azuresqlshared/sqlproperties.go b/pkg/resourcemanager/azuresql/azuresqlshared/sqlproperties.go index 814682d9424..441afe88426 100644 --- a/pkg/resourcemanager/azuresql/azuresqlshared/sqlproperties.go +++ b/pkg/resourcemanager/azuresql/azuresqlshared/sqlproperties.go @@ -5,11 +5,13 @@ package azuresqlshared import ( "fmt" + "github.com/Azure/azure-sdk-for-go/services/preview/sql/mgmt/v3.0/sql" - "github.com/Azure/azure-service-operator/api/v1alpha1" - "github.com/Azure/azure-service-operator/api/v1beta1" "github.com/Azure/go-autorest/autorest/to" + "github.com/pkg/errors" "k8s.io/apimachinery/pkg/api/resource" + + "github.com/Azure/azure-service-operator/api/v1beta1" ) // SQLServerProperties contains values needed for adding / updating SQL servers, @@ -63,31 +65,11 @@ type Sku struct { Capacity *int32 } -// SQLFailoverGroupProperties contains values needed for adding / updating SQL failover groups, -// wraps: https://github.com/Azure/azure-sdk-for-go/blob/master/services/preview/sql/mgmt/2015-05-01-preview/sql/failovergroups.go#L53 -type SQLFailoverGroupProperties struct { - - // FailoverPolicy can be Automatic or Manual - FailoverPolicy v1alpha1.ReadWriteEndpointFailoverPolicy - - // Read/Write Grace Period in minutes - FailoverGracePeriod int32 - - // Secondary server to failover to (should be in a different region) - SecondaryServer string - - // Resource Group for the Secondary server - SecondaryServerResourceGroup string - - // Names of Databases to add to the failover group - DatabaseList []string -} - type SQLDatabaseBackupLongTermRetentionPolicy struct { - WeeklyRetention string + WeeklyRetention string MonthlyRetention string - YearlyRetention string - WeekOfYear int32 + YearlyRetention string + WeekOfYear int32 } // SQLServerPropertiesToServer translates SQLServerProperties to ServerProperties @@ -183,15 +165,17 @@ func MergeDBEditionAndSku(in v1beta1.DBEdition, sku *v1beta1.SqlDatabaseSku) (*S } // translateFailoverPolicy translates the enum -func TranslateFailoverPolicy(in v1alpha1.ReadWriteEndpointFailoverPolicy) (result sql.ReadWriteEndpointFailoverPolicy) { +func TranslateFailoverPolicy(in v1beta1.ReadWriteEndpointFailoverPolicy) (sql.ReadWriteEndpointFailoverPolicy, error) { + var result sql.ReadWriteEndpointFailoverPolicy + switch in { - case v1alpha1.FailoverPolicyAutomatic: + case v1beta1.FailoverPolicyAutomatic: result = sql.Automatic - case v1alpha1.FailoverPolicyManual: + case v1beta1.FailoverPolicyManual: result = sql.Manual default: - result = sql.Automatic + return result, errors.Errorf("couldn't translate failover policy %s", in) } - return result + return result, nil } diff --git a/pkg/resourcemanager/pollclient/pollclient.go b/pkg/resourcemanager/pollclient/pollclient.go index 2555465f6ff..1f81d48bd95 100644 --- a/pkg/resourcemanager/pollclient/pollclient.go +++ b/pkg/resourcemanager/pollclient/pollclient.go @@ -7,6 +7,8 @@ import ( "context" "net/http" + "github.com/Azure/azure-service-operator/api/v1beta1" + "github.com/Azure/azure-service-operator/pkg/errhelp" "github.com/Azure/azure-service-operator/pkg/resourcemanager/config" "github.com/Azure/azure-service-operator/pkg/resourcemanager/iam" "github.com/Azure/go-autorest/autorest" @@ -125,3 +127,52 @@ func (client PollClient) GetResponder(resp *http.Response) (result PollResponse, result.Response = autorest.Response{Response: resp} return } + +type LongRunningOperationPollResult string + +const ( + PollResultNoPollingNeeded = LongRunningOperationPollResult("noPollingNeeded") + PollResultCompletedSuccessfully = LongRunningOperationPollResult("completedSuccessfully") + PollResultTryAgainLater = LongRunningOperationPollResult("tryAgainLater") +) + +func (client PollClient) PollLongRunningOperationIfNeeded(ctx context.Context, status *v1beta1.ASOStatus) (LongRunningOperationPollResult, error) { + // Before we attempt to issue a new update, check if there is a previously ongoing update + if status.PollingURL == "" { + return PollResultNoPollingNeeded, nil + } + + res, err := client.Get(ctx, status.PollingURL) + pollErr := errhelp.NewAzureError(err) + if pollErr != nil { + if pollErr.Type == errhelp.OperationIdNotFound { + // Something happened to our OperationId, just clear things out and try again + status.PollingURL = "" + } + return PollResultTryAgainLater, err + } + + if res.Status == LongRunningOperationPollStatusFailed { + status.Message = res.Error.Error() + // There can be intermediate errors and various other things that cause requests to fail, so we need to try again. + status.PollingURL = "" // Clear URL to force retry + return PollResultTryAgainLater, nil + } + + // TODO: May need a notion of fatal error here too + + if res.Status == "InProgress" { + // We're waiting for an async op... keep waiting + return PollResultTryAgainLater, nil + } + + // Previous operation was a success, clear polling URL and continue + if res.Status == LongRunningOperationPollStatusSucceeded { + status.PollingURL = "" + return PollResultCompletedSuccessfully, nil + } + + // TODO: Unsure if this should be continue or tryagainlater. In th existing code it's continue + // TODO: which is why I've made it that here + return PollResultCompletedSuccessfully, nil +} From 2c418cc1333374f17f79d7a0cf05afe542883f74 Mon Sep 17 00:00:00 2001 From: Matthew Christopher Date: Thu, 14 Jan 2021 11:46:04 -0800 Subject: [PATCH 2/2] Add unit tests to CI --- Makefile | 13 ++++++------ api/v1beta1/zz_generated.deepcopy.go | 20 +++++++++++++++++++ azure-pipelines.yml | 12 +++++++++++ controllers/suite_test.go | 6 +++--- .../keyvaults/unittest/keyvault_test.go | 7 ++++--- 5 files changed, 45 insertions(+), 13 deletions(-) diff --git a/Makefile b/Makefile index 6bb1ca57f48..9cd662efef0 100644 --- a/Makefile +++ b/Makefile @@ -55,8 +55,7 @@ generate-test-certs: .PHONY: test-integration-controllers test-integration-controllers: generate fmt vet manifests TEST_RESOURCE_PREFIX=$(TEST_RESOURCE_PREFIX) TEST_USE_EXISTING_CLUSTER=false REQUEUE_AFTER=20 \ - go test -v -tags "$(BUILD_TAGS)" -coverprofile=reports/integration-controllers-coverage-output.txt -coverpkg=./... -covermode count -parallel 4 -timeout 45m \ - ./controllers/... + go test -v -tags "$(BUILD_TAGS)" -coverprofile=reports/integration-controllers-coverage-output.txt -coverpkg=./... -covermode count -parallel 4 -timeout 45m ./controllers/... #2>&1 | tee reports/integration-controllers-output.txt #go-junit-report < reports/integration-controllers-output.txt > reports/integration-controllers-report.xml @@ -81,13 +80,13 @@ test-integration-managers: generate fmt vet manifests .PHONY: test-unit test-unit: generate fmt vet manifests TEST_USE_EXISTING_CLUSTER=false REQUEUE_AFTER=20 \ - go test -v -tags "$(BUILD_TAGS)" -coverprofile=coverage-unit.txt -covermode count -parallel 4 -timeout 10m \ - ./api/... \ - ./pkg/secrets/... + go test -v -tags "$(BUILD_TAGS)" -coverprofile=reports/unittest-coverage-ouput.txt -covermode count -parallel 4 -timeout 10m \ ./pkg/resourcemanager/keyvaults/unittest/ \ + ./pkg/resourcemanager/azuresql/azuresqlfailovergroup + # The below folders are commented out because the tests in them fail... + # ./api/... \ + # ./pkg/secrets/... \ #2>&1 | tee testlogs.txt - #go-junit-report < testlogs.txt > report-unit.xml - go tool cover -html=coverage/coverage.txt -o cover-unit.html # Merge all the available test coverage results and publish a single report .PHONY: test-process-coverage diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index 0c0b3711977..7c5d2fa1a7d 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -106,6 +106,11 @@ func (in *AzureSqlDatabaseSpec) DeepCopyInto(out *AzureSqlDatabaseSpec) { x := (*in).DeepCopy() *out = &x } + if in.ShortTermRetentionPolicy != nil { + in, out := &in.ShortTermRetentionPolicy, &out.ShortTermRetentionPolicy + *out = new(SQLDatabaseShortTermRetentionPolicy) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AzureSqlDatabaseSpec. @@ -376,6 +381,21 @@ func (in *GenericSpec) DeepCopy() *GenericSpec { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *SQLDatabaseShortTermRetentionPolicy) DeepCopyInto(out *SQLDatabaseShortTermRetentionPolicy) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SQLDatabaseShortTermRetentionPolicy. +func (in *SQLDatabaseShortTermRetentionPolicy) DeepCopy() *SQLDatabaseShortTermRetentionPolicy { + if in == nil { + return nil + } + out := new(SQLDatabaseShortTermRetentionPolicy) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *SqlDatabaseSku) DeepCopyInto(out *SqlDatabaseSku) { *out = *in diff --git a/azure-pipelines.yml b/azure-pipelines.yml index c0bce33425e..de0e9964407 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -103,6 +103,18 @@ steps: make generate-test-certs workingDirectory: '$(System.DefaultWorkingDirectory)' + - script: | + set -e + export PATH=$PATH:$(go env GOPATH)/bin + make test-unit + displayName: Run unit tests + condition: eq(variables['check_changes.SOURCE_CODE_CHANGED'], 'true') + continueOnError: 'false' + env: + GO111MODULE: on + BUILD_ID: $(Build.BuildId) + workingDirectory: '$(System.DefaultWorkingDirectory)' + - script: | set -e export PATH=$PATH:$(go env GOPATH)/bin:$(go env GOPATH)/kubebuilder/bin diff --git a/controllers/suite_test.go b/controllers/suite_test.go index ad6c4b76986..5f4f52ef8b5 100644 --- a/controllers/suite_test.go +++ b/controllers/suite_test.go @@ -153,9 +153,9 @@ func setup() error { // +kubebuilder:scaffold:scheme k8sManager, err = ctrl.NewManager(cfg, ctrl.Options{ - Scheme: scheme.Scheme, - CertDir: testEnv.WebhookInstallOptions.LocalServingCertDir, - Port: testEnv.WebhookInstallOptions.LocalServingPort, + Scheme: scheme.Scheme, + CertDir: testEnv.WebhookInstallOptions.LocalServingCertDir, + Port: testEnv.WebhookInstallOptions.LocalServingPort, }) if err != nil { return err diff --git a/pkg/resourcemanager/keyvaults/unittest/keyvault_test.go b/pkg/resourcemanager/keyvaults/unittest/keyvault_test.go index 61434bd916e..bca094ffd4b 100644 --- a/pkg/resourcemanager/keyvaults/unittest/keyvault_test.go +++ b/pkg/resourcemanager/keyvaults/unittest/keyvault_test.go @@ -8,12 +8,13 @@ import ( "testing" keyvault "github.com/Azure/azure-sdk-for-go/services/keyvault/mgmt/2018-02-14/keyvault" - v1alpha1 "github.com/Azure/azure-service-operator/api/v1alpha1" - "github.com/Azure/azure-service-operator/pkg/resourcemanager/config" - azurekeyvault "github.com/Azure/azure-service-operator/pkg/resourcemanager/keyvaults" "github.com/google/go-cmp/cmp" uuid "github.com/satori/go.uuid" "github.com/stretchr/testify/assert" + + v1alpha1 "github.com/Azure/azure-service-operator/api/v1alpha1" + "github.com/Azure/azure-service-operator/pkg/resourcemanager/config" + azurekeyvault "github.com/Azure/azure-service-operator/pkg/resourcemanager/keyvaults" ) func TestParseAccessPoliciesInvalid(t *testing.T) {