From 8038529c3fa16d7e26d8f32f2ea1bc7698241857 Mon Sep 17 00:00:00 2001 From: jpflueger Date: Mon, 13 Apr 2020 17:36:16 -0600 Subject: [PATCH 01/21] saving cosmosdb keys as secrets --- api/v1alpha1/cosmosdb_types.go | 9 +-- config/samples/azure_v1alpha1_cosmosdb.yaml | 4 ++ controllers/cosmosdb_controller_test.go | 25 ++++++-- controllers/suite_test.go | 2 +- main.go | 5 +- pkg/resourcemanager/cosmosdbs/cosmosdb.go | 23 ++++++- .../cosmosdbs/cosmosdb_manager.go | 8 ++- .../cosmosdbs/cosmosdb_reconcile.go | 60 ++++++++++++++++++- 8 files changed, 122 insertions(+), 14 deletions(-) diff --git a/api/v1alpha1/cosmosdb_types.go b/api/v1alpha1/cosmosdb_types.go index 34fea0935fc..36796e27ea0 100644 --- a/api/v1alpha1/cosmosdb_types.go +++ b/api/v1alpha1/cosmosdb_types.go @@ -17,10 +17,11 @@ type CosmosDBSpec struct { // +kubebuilder:validation:MinLength=0 - Location string `json:"location,omitempty"` - ResourceGroup string `json:"resourceGroup"` - Kind CosmosDBKind `json:"kind,omitempty"` - Properties CosmosDBProperties `json:"properties,omitempty"` + Location string `json:"location,omitempty"` + ResourceGroup string `json:"resourceGroup"` + Kind CosmosDBKind `json:"kind,omitempty"` + Properties CosmosDBProperties `json:"properties,omitempty"` + KeyVaultToStoreSecrets string `json:"keyVaultToStoreSecrets,omitempty"` } // CosmosDBKind enumerates the values for kind. diff --git a/config/samples/azure_v1alpha1_cosmosdb.yaml b/config/samples/azure_v1alpha1_cosmosdb.yaml index d3efa498467..3d84aab4dea 100644 --- a/config/samples/azure_v1alpha1_cosmosdb.yaml +++ b/config/samples/azure_v1alpha1_cosmosdb.yaml @@ -8,3 +8,7 @@ spec: resourceGroup: resourcegroup-azure-operators properties: databaseAccountOfferType: Standard + + # Use the field below to optionally specify a different keyvault + # to store the connectiong string secrets in + #keyVaultToStoreSecrets: asoSecretKeyVault \ No newline at end of file diff --git a/controllers/cosmosdb_controller_test.go b/controllers/cosmosdb_controller_test.go index f9ca7f45df0..2fd0ccdb935 100644 --- a/controllers/cosmosdb_controller_test.go +++ b/controllers/cosmosdb_controller_test.go @@ -10,22 +10,26 @@ import ( "testing" "github.com/Azure/azure-service-operator/api/v1alpha1" + "github.com/stretchr/testify/assert" + v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" ) func TestCosmosDBHappyPath(t *testing.T) { t.Parallel() defer PanicRecover(t) ctx := context.Background() + assert := assert.New(t) - cosmosDBAccountName := GenerateTestResourceNameWithRandom("cosmosdb", 8) - cosmosDBNamespace := "default" + name := GenerateTestResourceNameWithRandom("cosmosdb", 8) + namespace := "default" dbInstance := &v1alpha1.CosmosDB{ ObjectMeta: metav1.ObjectMeta{ - Name: cosmosDBAccountName, - Namespace: cosmosDBNamespace, + Name: name, + Namespace: namespace, }, Spec: v1alpha1.CosmosDBSpec{ Location: "westus", @@ -37,8 +41,21 @@ func TestCosmosDBHappyPath(t *testing.T) { }, } + key := types.NamespacedName{Name: name, Namespace: namespace} + secret := &v1.Secret{} + EnsureInstance(ctx, t, tc, dbInstance) + assert.Eventually(func() bool { + err := tc.k8sClient.Get(ctx, key, secret) + return err == nil && secret.ObjectMeta.Name == name && secret.ObjectMeta.Namespace == namespace + }, tc.timeoutFast, tc.retry, "wait for cosmosdb to have secret") + EnsureDelete(ctx, t, tc, dbInstance) + assert.Eventually(func() bool { + err := tc.k8sClient.Get(ctx, key, secret) + return err != nil + }, tc.timeoutFast, tc.retry, "wait for cosmosdb to delete secret") + } diff --git a/controllers/suite_test.go b/controllers/suite_test.go index 3eb278353f0..f617cd3fa26 100644 --- a/controllers/suite_test.go +++ b/controllers/suite_test.go @@ -154,7 +154,7 @@ func setup() error { secretClient, scheme.Scheme, ) - cosmosDbManager = resourcemanagercosmosdb.NewAzureCosmosDBManager() + cosmosDbManager = resourcemanagercosmosdb.NewAzureCosmosDBManager(secretClient) apiMgmtManager = resourcemanagerapimgmt.NewManager() resourceGroupManager = resourcegroupsresourcemanager.NewAzureResourceGroupManager() eventHubManagers = resourcemanagereventhub.AzureEventHubManagers diff --git a/main.go b/main.go index 101141afdce..da81dbe5bcb 100644 --- a/main.go +++ b/main.go @@ -126,6 +126,9 @@ func main() { ) eventhubNamespaceClient := resourcemanagereventhub.NewEventHubNamespaceClient() consumerGroupClient := resourcemanagereventhub.NewConsumerGroupClient() + cosmosDBClient := resourcemanagercosmosdb.NewAzureCosmosDBManager( + secretClient, + ) storageManagers := resourcemanagerstorage.AzureStorageManagers keyVaultManager := resourcemanagerkeyvault.NewAzureKeyVaultManager(mgr.GetScheme()) keyVaultKeyManager := &resourcemanagerkeyvault.KeyvaultKeyClient{ @@ -171,7 +174,7 @@ func main() { err = (&controllers.CosmosDBReconciler{ Reconciler: &controllers.AsyncReconciler{ Client: mgr.GetClient(), - AzureClient: resourcemanagercosmosdb.NewAzureCosmosDBManager(), + AzureClient: cosmosDBClient, Telemetry: telemetry.InitializeTelemetryDefault( "CosmosDB", ctrl.Log.WithName("controllers").WithName("CosmosDB"), diff --git a/pkg/resourcemanager/cosmosdbs/cosmosdb.go b/pkg/resourcemanager/cosmosdbs/cosmosdb.go index 0d22bb85e85..8f7f8ada4d1 100644 --- a/pkg/resourcemanager/cosmosdbs/cosmosdb.go +++ b/pkg/resourcemanager/cosmosdbs/cosmosdb.go @@ -13,12 +13,15 @@ import ( "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/azure-service-operator/pkg/secrets" "github.com/Azure/go-autorest/autorest" "github.com/Azure/go-autorest/autorest/to" ) // AzureCosmosDBManager is the struct which contains helper functions for resource groups -type AzureCosmosDBManager struct{} +type AzureCosmosDBManager struct { + SecretClient secrets.SecretClient +} func getCosmosDBClient() (documentdb.DatabaseAccountsClient, error) { cosmosDBClient := documentdb.NewDatabaseAccountsClientWithBaseURI(config.BaseURI(), config.SubscriptionID()) @@ -164,3 +167,21 @@ func (*AzureCosmosDBManager) DeleteCosmosDB( } return &ar, nil } + +// ListKeys lists the read & write keys for a database account +func (*AzureCosmosDBManager) ListKeys( + ctx context.Context, + groupName string, + accountName string) (*documentdb.DatabaseAccountListKeysResult, error) { + client, err := getCosmosDBClient() + if err != nil { + return nil, errhelp.NewAzureErrorAzureError(err) + } + + result, err := client.ListKeys(ctx, groupName, accountName) + if err != nil { + return nil, errhelp.NewAzureErrorAzureError(err) + } + + return &result, nil +} diff --git a/pkg/resourcemanager/cosmosdbs/cosmosdb_manager.go b/pkg/resourcemanager/cosmosdbs/cosmosdb_manager.go index f916e70fd54..e67ec2c5471 100644 --- a/pkg/resourcemanager/cosmosdbs/cosmosdb_manager.go +++ b/pkg/resourcemanager/cosmosdbs/cosmosdb_manager.go @@ -10,12 +10,13 @@ import ( "github.com/Azure/azure-service-operator/api/v1alpha1" "github.com/Azure/azure-service-operator/pkg/errhelp" "github.com/Azure/azure-service-operator/pkg/resourcemanager" + "github.com/Azure/azure-service-operator/pkg/secrets" "github.com/Azure/go-autorest/autorest" ) // NewAzureCosmosDBManager creates a new cosmos db client -func NewAzureCosmosDBManager() *AzureCosmosDBManager { - return &AzureCosmosDBManager{} +func NewAzureCosmosDBManager(secretClient secrets.SecretClient) *AzureCosmosDBManager { + return &AzureCosmosDBManager{secretClient} } // CosmosDBManager client functions @@ -32,5 +33,8 @@ type CosmosDBManager interface { // CheckNameExistsCosmosDB check if the account name already exists globally CheckNameExistsCosmosDB(ctx context.Context, accountName string) (bool, *errhelp.AzureError) + // ListKeys lists the read & write keys for a database account + ListKeys(ctx context.Context, groupName string, accountName string) (*documentdb.DatabaseAccountListKeysResult, error) + resourcemanager.ARMClient } diff --git a/pkg/resourcemanager/cosmosdbs/cosmosdb_reconcile.go b/pkg/resourcemanager/cosmosdbs/cosmosdb_reconcile.go index 4eb8f6b0a0f..680612c3a48 100644 --- a/pkg/resourcemanager/cosmosdbs/cosmosdb_reconcile.go +++ b/pkg/resourcemanager/cosmosdbs/cosmosdb_reconcile.go @@ -18,6 +18,15 @@ import ( // Ensure ensures that cosmosdb is provisioned as specified func (m *AzureCosmosDBManager) Ensure(ctx context.Context, obj runtime.Object, opts ...resourcemanager.ConfigOption) (bool, error) { + options := &resourcemanager.Options{} + for _, opt := range opts { + opt(options) + } + + if options.SecretClient != nil { + m.SecretClient = options.SecretClient + } + instance, err := m.convert(obj) if err != nil { return false, err @@ -44,6 +53,12 @@ func (m *AzureCosmosDBManager) Ensure(ctx context.Context, obj runtime.Object, o } if instance.Status.State == "Succeeded" { + // provisioning is complete, update the secrets + if err = m.createOrUpdateAccountKeysSecret(ctx, instance); err != nil { + instance.Status.Message = err.Error() + return false, err + } + instance.Status.Message = resourcemanager.SuccessMsg instance.Status.Provisioning = false instance.Status.Provisioned = true @@ -120,6 +135,15 @@ func (m *AzureCosmosDBManager) Ensure(ctx context.Context, obj runtime.Object, o // Delete drops cosmosdb func (m *AzureCosmosDBManager) Delete(ctx context.Context, obj runtime.Object, opts ...resourcemanager.ConfigOption) (bool, error) { + options := &resourcemanager.Options{} + for _, opt := range opts { + opt(options) + } + + if options.SecretClient != nil { + m.SecretClient = options.SecretClient + } + instance, err := m.convert(obj) if err != nil { return false, err @@ -160,8 +184,14 @@ func (m *AzureCosmosDBManager) Delete(ctx context.Context, obj runtime.Object, o return true, nil } - // try to delete the cosmosdb instance + // create must succeed before delete succeeds + if instance.Status.State == "Creating" { + return true, nil + } + + // try to delete the cosmosdb instance & secrets _, azerr = m.DeleteCosmosDB(ctx, groupName, accountName) + m.deleteAccountKeysSecret(ctx, instance) if azerr != nil { // this is likely to happen on first try due to not waiting for the future to complete if azerr.Type == errhelp.AsyncOpIncompleteError { @@ -217,3 +247,31 @@ func (m *AzureCosmosDBManager) convert(obj runtime.Object) (*v1alpha1.CosmosDB, } return db, nil } + +func (m *AzureCosmosDBManager) createOrUpdateAccountKeysSecret(ctx context.Context, instance *v1alpha1.CosmosDB) error { + result, err := m.ListKeys(ctx, instance.Spec.ResourceGroup, instance.ObjectMeta.Name) + if err != nil { + return err + } + + secretKey := types.NamespacedName{ + Name: instance.Name, + Namespace: instance.Namespace, + } + secretData := map[string][]byte{ + "primaryConnectionString": []byte(*result.PrimaryMasterKey), + "secondaryConnectionString": []byte(*result.SecondaryMasterKey), + "primaryReadonlyMasterKey": []byte(*result.PrimaryReadonlyMasterKey), + "secondaryReadonlyMasterKey": []byte(*result.SecondaryReadonlyMasterKey), + } + + return m.SecretClient.Upsert(ctx, secretKey, secretData) +} + +func (m *AzureCosmosDBManager) deleteAccountKeysSecret(ctx context.Context, instance *v1alpha1.CosmosDB) error { + secretKey := types.NamespacedName{ + Name: instance.Name, + Namespace: instance.Namespace, + } + return m.SecretClient.Delete(ctx, secretKey) +} From 49560ce4a0f6560a0e4f0aa76c11e7ee403b5e1b Mon Sep 17 00:00:00 2001 From: jpflueger Date: Tue, 14 Apr 2020 15:22:24 -0600 Subject: [PATCH 02/21] testing with secret client --- controllers/cosmosdb_controller_test.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/controllers/cosmosdb_controller_test.go b/controllers/cosmosdb_controller_test.go index 2fd0ccdb935..c0a47d398a0 100644 --- a/controllers/cosmosdb_controller_test.go +++ b/controllers/cosmosdb_controller_test.go @@ -12,7 +12,6 @@ import ( "github.com/Azure/azure-service-operator/api/v1alpha1" "github.com/stretchr/testify/assert" - v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" ) @@ -42,19 +41,18 @@ func TestCosmosDBHappyPath(t *testing.T) { } key := types.NamespacedName{Name: name, Namespace: namespace} - secret := &v1.Secret{} EnsureInstance(ctx, t, tc, dbInstance) assert.Eventually(func() bool { - err := tc.k8sClient.Get(ctx, key, secret) - return err == nil && secret.ObjectMeta.Name == name && secret.ObjectMeta.Namespace == namespace + secret, err := tc.secretClient.Get(ctx, key) + return err == nil && len(secret) > 0 }, tc.timeoutFast, tc.retry, "wait for cosmosdb to have secret") EnsureDelete(ctx, t, tc, dbInstance) assert.Eventually(func() bool { - err := tc.k8sClient.Get(ctx, key, secret) + _, err := tc.secretClient.Get(ctx, key) return err != nil }, tc.timeoutFast, tc.retry, "wait for cosmosdb to delete secret") From 2ccd809aa02c6752129fb0ce4c6fc843f82d8828 Mon Sep 17 00:00:00 2001 From: jpflueger Date: Tue, 14 Apr 2020 15:30:10 -0600 Subject: [PATCH 03/21] returning error from cosmosdb resource manager --- pkg/resourcemanager/cosmosdbs/cosmosdb.go | 35 +++++++++---------- .../cosmosdbs/cosmosdb_manager.go | 9 +++-- .../cosmosdbs/cosmosdb_reconcile.go | 21 ++++++----- 3 files changed, 34 insertions(+), 31 deletions(-) diff --git a/pkg/resourcemanager/cosmosdbs/cosmosdb.go b/pkg/resourcemanager/cosmosdbs/cosmosdb.go index 8f7f8ada4d1..a79573e4976 100644 --- a/pkg/resourcemanager/cosmosdbs/cosmosdb.go +++ b/pkg/resourcemanager/cosmosdbs/cosmosdb.go @@ -10,7 +10,6 @@ import ( "github.com/Azure/azure-sdk-for-go/services/cosmos-db/mgmt/2015-04-08/documentdb" "github.com/Azure/azure-service-operator/api/v1alpha1" - "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/azure-service-operator/pkg/secrets" @@ -45,10 +44,10 @@ func (*AzureCosmosDBManager) CreateOrUpdateCosmosDB( location string, kind v1alpha1.CosmosDBKind, dbType v1alpha1.CosmosDBDatabaseAccountOfferType, - tags map[string]*string) (*documentdb.DatabaseAccount, *errhelp.AzureError) { + tags map[string]*string) (*documentdb.DatabaseAccount, error) { cosmosDBClient, err := getCosmosDBClient() if err != nil { - return nil, errhelp.NewAzureErrorAzureError(err) + return nil, err } dbKind := documentdb.DatabaseAccountKind(kind) @@ -94,13 +93,13 @@ func (*AzureCosmosDBManager) CreateOrUpdateCosmosDB( if err != nil { // initial create request failed, wrap error - return nil, errhelp.NewAzureErrorAzureError(err) + return nil, err } result, err := createUpdateFuture.Result(cosmosDBClient) if err != nil { // there is no immediate result, wrap error - return &result, errhelp.NewAzureErrorAzureError(err) + return &result, err } return &result, nil } @@ -109,15 +108,15 @@ func (*AzureCosmosDBManager) CreateOrUpdateCosmosDB( func (*AzureCosmosDBManager) GetCosmosDB( ctx context.Context, groupName string, - cosmosDBName string) (*documentdb.DatabaseAccount, *errhelp.AzureError) { + cosmosDBName string) (*documentdb.DatabaseAccount, error) { cosmosDBClient, err := getCosmosDBClient() if err != nil { - return nil, errhelp.NewAzureErrorAzureError(err) + return nil, err } result, err := cosmosDBClient.Get(ctx, groupName, cosmosDBName) if err != nil { - return &result, errhelp.NewAzureErrorAzureError(err) + return &result, err } return &result, nil } @@ -125,15 +124,15 @@ func (*AzureCosmosDBManager) GetCosmosDB( // CheckNameExistsCosmosDB checks if the global account name already exists func (*AzureCosmosDBManager) CheckNameExistsCosmosDB( ctx context.Context, - accountName string) (bool, *errhelp.AzureError) { + accountName string) (bool, error) { cosmosDBClient, err := getCosmosDBClient() if err != nil { - return false, errhelp.NewAzureErrorAzureError(err) + return false, err } response, err := cosmosDBClient.CheckNameExists(ctx, accountName) if err != nil { - return false, errhelp.NewAzureErrorAzureError(err) + return false, err } switch response.StatusCode { @@ -142,7 +141,7 @@ func (*AzureCosmosDBManager) CheckNameExistsCosmosDB( case http.StatusOK: return true, nil default: - return false, errhelp.NewAzureErrorAzureError(fmt.Errorf("unhandled status code for CheckNameExists")) + return false, fmt.Errorf("unhandled status code for CheckNameExists") } } @@ -150,20 +149,20 @@ func (*AzureCosmosDBManager) CheckNameExistsCosmosDB( func (*AzureCosmosDBManager) DeleteCosmosDB( ctx context.Context, groupName string, - cosmosDBName string) (*autorest.Response, *errhelp.AzureError) { + cosmosDBName string) (*autorest.Response, error) { cosmosDBClient, err := getCosmosDBClient() if err != nil { - return nil, errhelp.NewAzureErrorAzureError(err) + return nil, err } deleteFuture, err := cosmosDBClient.Delete(ctx, groupName, cosmosDBName) if err != nil { - return nil, errhelp.NewAzureErrorAzureError(err) + return nil, err } ar, err := deleteFuture.Result(cosmosDBClient) if err != nil { - return nil, errhelp.NewAzureErrorAzureError(err) + return nil, err } return &ar, nil } @@ -175,12 +174,12 @@ func (*AzureCosmosDBManager) ListKeys( accountName string) (*documentdb.DatabaseAccountListKeysResult, error) { client, err := getCosmosDBClient() if err != nil { - return nil, errhelp.NewAzureErrorAzureError(err) + return nil, err } result, err := client.ListKeys(ctx, groupName, accountName) if err != nil { - return nil, errhelp.NewAzureErrorAzureError(err) + return nil, err } return &result, nil diff --git a/pkg/resourcemanager/cosmosdbs/cosmosdb_manager.go b/pkg/resourcemanager/cosmosdbs/cosmosdb_manager.go index e67ec2c5471..9ce839c076d 100644 --- a/pkg/resourcemanager/cosmosdbs/cosmosdb_manager.go +++ b/pkg/resourcemanager/cosmosdbs/cosmosdb_manager.go @@ -8,7 +8,6 @@ import ( "github.com/Azure/azure-sdk-for-go/services/cosmos-db/mgmt/2015-04-08/documentdb" "github.com/Azure/azure-service-operator/api/v1alpha1" - "github.com/Azure/azure-service-operator/pkg/errhelp" "github.com/Azure/azure-service-operator/pkg/resourcemanager" "github.com/Azure/azure-service-operator/pkg/secrets" "github.com/Azure/go-autorest/autorest" @@ -22,16 +21,16 @@ func NewAzureCosmosDBManager(secretClient secrets.SecretClient) *AzureCosmosDBMa // CosmosDBManager client functions type CosmosDBManager interface { // CreateOrUpdateCosmosDB creates a new cosmos database account - CreateOrUpdateCosmosDB(ctx context.Context, groupName string, cosmosDBName string, location string, kind v1alpha1.CosmosDBKind, dbType v1alpha1.CosmosDBDatabaseAccountOfferType, tags map[string]*string) (*documentdb.DatabaseAccount, *errhelp.AzureError) + CreateOrUpdateCosmosDB(ctx context.Context, groupName string, cosmosDBName string, location string, kind v1alpha1.CosmosDBKind, dbType v1alpha1.CosmosDBDatabaseAccountOfferType, tags map[string]*string) (*documentdb.DatabaseAccount, error) // GetCosmosDB gets a cosmos database account - GetCosmosDB(ctx context.Context, groupName string, cosmosDBName string) (*documentdb.DatabaseAccount, *errhelp.AzureError) + GetCosmosDB(ctx context.Context, groupName string, cosmosDBName string) (*documentdb.DatabaseAccount, error) // DeleteCosmosDB removes the cosmos database account - DeleteCosmosDB(ctx context.Context, groupName string, cosmosDBName string) (*autorest.Response, *errhelp.AzureError) + DeleteCosmosDB(ctx context.Context, groupName string, cosmosDBName string) (*autorest.Response, error) // CheckNameExistsCosmosDB check if the account name already exists globally - CheckNameExistsCosmosDB(ctx context.Context, accountName string) (bool, *errhelp.AzureError) + CheckNameExistsCosmosDB(ctx context.Context, accountName string) (bool, error) // ListKeys lists the read & write keys for a database account ListKeys(ctx context.Context, groupName string, accountName string) (*documentdb.DatabaseAccountListKeysResult, error) diff --git a/pkg/resourcemanager/cosmosdbs/cosmosdb_reconcile.go b/pkg/resourcemanager/cosmosdbs/cosmosdb_reconcile.go index 680612c3a48..e746619f4ff 100644 --- a/pkg/resourcemanager/cosmosdbs/cosmosdb_reconcile.go +++ b/pkg/resourcemanager/cosmosdbs/cosmosdb_reconcile.go @@ -42,7 +42,8 @@ func (m *AzureCosmosDBManager) Ensure(ctx context.Context, obj runtime.Object, o return true, nil } else if instance.Status.Provisioning { // get the instance and update status - db, azerr := m.GetCosmosDB(ctx, instance.Spec.ResourceGroup, instance.Name) + db, err := m.GetCosmosDB(ctx, instance.Spec.ResourceGroup, instance.Name) + azerr := errhelp.NewAzureErrorAzureError(err) if azerr == nil { instance.Status.ResourceId = *db.ID instance.Status.State = *db.ProvisioningState @@ -106,10 +107,10 @@ func (m *AzureCosmosDBManager) Ensure(ctx context.Context, obj runtime.Object, o kind := instance.Spec.Kind dbType := instance.Spec.Properties.DatabaseAccountOfferType - db, azerr := m.CreateOrUpdateCosmosDB(ctx, groupName, accountName, location, kind, dbType, tags) + db, err := m.CreateOrUpdateCosmosDB(ctx, groupName, accountName, location, kind, dbType, tags) // everything is in a created/updated state - if azerr == nil { + if err == nil { instance.Status.Provisioned = true instance.Status.Provisioning = false instance.Status.Message = resourcemanager.SuccessMsg @@ -118,7 +119,7 @@ func (m *AzureCosmosDBManager) Ensure(ctx context.Context, obj runtime.Object, o return true, nil } - switch azerr.Type { + switch azerr := errhelp.NewAzureErrorAzureError(err); azerr.Type { case errhelp.AsyncOpIncompleteError: instance.Status.Message = "Resource request successfully submitted to Azure" @@ -165,8 +166,10 @@ func (m *AzureCosmosDBManager) Delete(ctx context.Context, obj runtime.Object, o } // fetch the latest to inspect provisioning state - cosmosDB, azerr := m.GetCosmosDB(ctx, groupName, accountName) - if azerr != nil { + cosmosDB, err := m.GetCosmosDB(ctx, groupName, accountName) + if err != nil { + azerr := errhelp.NewAzureErrorAzureError(err) + // deletion finished if helpers.ContainsString(notFoundErrors, azerr.Type) { return false, nil @@ -190,9 +193,11 @@ func (m *AzureCosmosDBManager) Delete(ctx context.Context, obj runtime.Object, o } // try to delete the cosmosdb instance & secrets - _, azerr = m.DeleteCosmosDB(ctx, groupName, accountName) + _, err = m.DeleteCosmosDB(ctx, groupName, accountName) m.deleteAccountKeysSecret(ctx, instance) - if azerr != nil { + if err != nil { + azerr := errhelp.NewAzureErrorAzureError(err) + // this is likely to happen on first try due to not waiting for the future to complete if azerr.Type == errhelp.AsyncOpIncompleteError { instance.Status.Message = "Deletion request submitted successfully" From ba70604777389a66bf1ceb48d1c4e75114f7c7e2 Mon Sep 17 00:00:00 2001 From: jpflueger Date: Wed, 15 Apr 2020 11:17:39 -0600 Subject: [PATCH 04/21] simplifying ensure --- pkg/resourcemanager/cosmosdbs/cosmosdb.go | 2 +- .../cosmosdbs/cosmosdb_reconcile.go | 208 ++++++++---------- 2 files changed, 90 insertions(+), 120 deletions(-) diff --git a/pkg/resourcemanager/cosmosdbs/cosmosdb.go b/pkg/resourcemanager/cosmosdbs/cosmosdb.go index a79573e4976..ad83a5577a7 100644 --- a/pkg/resourcemanager/cosmosdbs/cosmosdb.go +++ b/pkg/resourcemanager/cosmosdbs/cosmosdb.go @@ -30,9 +30,9 @@ func getCosmosDBClient() (documentdb.DatabaseAccountsClient, error) { cosmosDBClient = documentdb.DatabaseAccountsClient{} } else { cosmosDBClient.Authorizer = a - cosmosDBClient.AddToUserAgent(config.UserAgent()) } + err = cosmosDBClient.AddToUserAgent(config.UserAgent()) return cosmosDBClient, err } diff --git a/pkg/resourcemanager/cosmosdbs/cosmosdb_reconcile.go b/pkg/resourcemanager/cosmosdbs/cosmosdb_reconcile.go index e746619f4ff..24ff5838558 100644 --- a/pkg/resourcemanager/cosmosdbs/cosmosdb_reconcile.go +++ b/pkg/resourcemanager/cosmosdbs/cosmosdb_reconcile.go @@ -33,72 +33,59 @@ func (m *AzureCosmosDBManager) Ensure(ctx context.Context, obj runtime.Object, o } hash := helpers.Hash256(instance.Spec) - if instance.Status.SpecHash != hash { - // need to push a create or update - instance.Status.SpecHash = hash - } else if instance.Status.Provisioned { - // provisioned and no changes needed + + if instance.Status.SpecHash == hash && instance.Status.Provisioned { instance.Status.RequestedAt = nil return true, nil - } else if instance.Status.Provisioning { - // get the instance and update status - db, err := m.GetCosmosDB(ctx, instance.Spec.ResourceGroup, instance.Name) - azerr := errhelp.NewAzureErrorAzureError(err) - if azerr == nil { - instance.Status.ResourceId = *db.ID - instance.Status.State = *db.ProvisioningState - - if instance.Status.State == "Creating" { - instance.Status.Message = "Waiting for resource to finish creation" - return false, nil - } - - if instance.Status.State == "Succeeded" { - // provisioning is complete, update the secrets - if err = m.createOrUpdateAccountKeysSecret(ctx, instance); err != nil { - instance.Status.Message = err.Error() - return false, err - } + } + instance.Status.Provisioned = false - instance.Status.Message = resourcemanager.SuccessMsg - instance.Status.Provisioning = false - instance.Status.Provisioned = true - return true, nil - } + // get the instance and update status + db, err := m.GetCosmosDB(ctx, instance.Spec.ResourceGroup, instance.Name) + if err != nil { + azerr := errhelp.NewAzureErrorAzureError(err) - if instance.Status.State == "Failed" { - instance.Status.Message = "Failed to provision CosmosDB" - instance.Status.Provisioning = false - return true, nil - } - } else if azerr.Type == errhelp.ResourceGroupNotFoundErrorCode { + switch azerr.Type { + case errhelp.ResourceGroupNotFoundErrorCode, errhelp.ParentNotFoundErrorCode: instance.Status.Provisioning = false - instance.Status.Message = fmt.Sprintf("Waiting for resource group '%s' to be available", instance.Spec.ResourceGroup) + instance.Status.Message = azerr.Reason instance.Status.State = "Waiting" return false, nil - } else if azerr.Type == errhelp.ResourceNotFound { - exists, azerr := m.CheckNameExistsCosmosDB(ctx, instance.Name) - if azerr != nil { - instance.Status.Provisioning = false - instance.Status.Message = "Unexpected error occurred during resource request" - instance.Status.State = "Failed" - return false, err - } else if exists { - // get request returned resource not found and the name already exists - // so it must exist in a different resource group, user must fix it - instance.Status.Provisioning = false - instance.Status.Message = "CosmosDB name already exists" - instance.Status.State = "Failed" - return true, nil - } - } else { - instance.Status.Provisioning = false - instance.Status.Message = azerr.Error() - return false, azerr.Original + case errhelp.ResourceNotFound: + //NO-OP, try to create + default: + instance.Status.Message = fmt.Sprintf("Unhandled error after Get %v", azerr.Reason) } + + } else { + instance.Status.ResourceId = *db.ID + instance.Status.State = *db.ProvisioningState } - instance.Status.Provisioning = true + if instance.Status.State == "Creating" { + // avoid multiple CreateOrUpdate requests while resource is already creating + return false, nil + } + + if instance.Status.State == "Succeeded" && instance.Status.SpecHash == hash { + // provisioning is complete, update the secrets + if err = m.createOrUpdateAccountKeysSecret(ctx, instance); err != nil { + instance.Status.Message = err.Error() + return false, err + } + + instance.Status.Message = resourcemanager.SuccessMsg + instance.Status.Provisioning = false + instance.Status.Provisioned = true + return true, nil + } + + if instance.Status.State == "Failed" { + instance.Status.Message = "Failed to provision CosmosDB" + instance.Status.Provisioning = false + instance.Status.Provisioned = false + return true, nil + } tags := helpers.LabelsToTags(instance.GetLabels()) accountName := instance.ObjectMeta.Name @@ -107,30 +94,44 @@ func (m *AzureCosmosDBManager) Ensure(ctx context.Context, obj runtime.Object, o kind := instance.Spec.Kind dbType := instance.Spec.Properties.DatabaseAccountOfferType - db, err := m.CreateOrUpdateCosmosDB(ctx, groupName, accountName, location, kind, dbType, tags) + db, err = m.CreateOrUpdateCosmosDB(ctx, groupName, accountName, location, kind, dbType, tags) + if err != nil { + azerr := errhelp.NewAzureErrorAzureError(err) - // everything is in a created/updated state - if err == nil { - instance.Status.Provisioned = true - instance.Status.Provisioning = false - instance.Status.Message = resourcemanager.SuccessMsg - instance.Status.State = "Succeeded" - instance.Status.ResourceId = *db.ID - return true, nil + switch azerr.Type { + case errhelp.AsyncOpIncompleteError: + instance.Status.State = "Creating" + instance.Status.Message = "Resource request successfully submitted to Azure" + return false, nil + case errhelp.InvalidResourceLocation, errhelp.LocationNotAvailableForResourceType: + instance.Status.Provisioning = false + instance.Status.Message = azerr.Reason + return true, nil + case errhelp.ResourceGroupNotFoundErrorCode, errhelp.ParentNotFoundErrorCode: + instance.Status.Provisioning = false + instance.Status.Message = azerr.Reason + return false, nil + case errhelp.NotFoundErrorCode: + if nameExists, err := m.CheckNameExistsCosmosDB(ctx, accountName); err != nil { + instance.Status.Message = err.Error() + return false, err + } else if nameExists { + instance.Status.Provisioning = false + instance.Status.Message = "CosmosDB Account name already exists" + return true, nil + } + default: + instance.Status.Message = azerr.Reason + return false, nil + } } - switch azerr := errhelp.NewAzureErrorAzureError(err); azerr.Type { - - case errhelp.AsyncOpIncompleteError: - instance.Status.Message = "Resource request successfully submitted to Azure" - instance.Status.State = "Creating" - - case errhelp.InvalidResourceLocation: - instance.Status.Provisioning = false - instance.Status.Message = azerr.Reason - return true, nil - - } + instance.Status.SpecHash = hash + instance.Status.ResourceId = *db.ID + instance.Status.State = *db.ProvisioningState + instance.Status.Provisioned = true + instance.Status.Provisioning = false + instance.Status.Message = resourcemanager.SuccessMsg return false, nil } @@ -150,51 +151,17 @@ func (m *AzureCosmosDBManager) Delete(ctx context.Context, obj runtime.Object, o return false, err } - accountName := instance.ObjectMeta.Name - groupName := instance.Spec.ResourceGroup - // if the resource is in a failed state it was never created or could never be verified // so we skip attempting to delete the resrouce from Azure if instance.Status.FailedProvisioning { return false, nil } - notFoundErrors := []string{ - errhelp.NotFoundErrorCode, // happens on first request after deletion succeeds - errhelp.ResourceNotFound, // happens on subsequent requests after deletion succeeds - errhelp.ResourceGroupNotFoundErrorCode, // database doesn't exist in this resource group but the name exists globally - } - - // fetch the latest to inspect provisioning state - cosmosDB, err := m.GetCosmosDB(ctx, groupName, accountName) - if err != nil { - azerr := errhelp.NewAzureErrorAzureError(err) - - // deletion finished - if helpers.ContainsString(notFoundErrors, azerr.Type) { - return false, nil - } - - //TODO: are there other errors that need handling here? - instance.Status.Message = azerr.Error() - return true, azerr.Original - } - - instance.Status.State = *cosmosDB.ProvisioningState - - // already deleting the resource, try again later - if instance.Status.State == "Deleting" { - return true, nil - } - - // create must succeed before delete succeeds - if instance.Status.State == "Creating" { - return true, nil - } + groupName := instance.Spec.ResourceGroup + accountName := instance.ObjectMeta.Name // try to delete the cosmosdb instance & secrets _, err = m.DeleteCosmosDB(ctx, groupName, accountName) - m.deleteAccountKeysSecret(ctx, instance) if err != nil { azerr := errhelp.NewAzureErrorAzureError(err) @@ -204,18 +171,21 @@ func (m *AzureCosmosDBManager) Delete(ctx context.Context, obj runtime.Object, o return true, nil } - // already deleted + notFoundErrors := []string{ + errhelp.NotFoundErrorCode, // happens on first request after deletion succeeds + errhelp.ResourceNotFound, // happens on subsequent requests after deletion succeeds + errhelp.ResourceGroupNotFoundErrorCode, // database doesn't exist in this resource group but the name exists globally + } if helpers.ContainsString(notFoundErrors, azerr.Type) { - return false, nil + return false, m.deleteAccountKeysSecret(ctx, instance) } // unhandled error - instance.Status.Message = azerr.Error() - return false, azerr.Original + instance.Status.Message = azerr.Reason + return false, err } - // second delete calls succeed immediately - return false, nil + return false, m.deleteAccountKeysSecret(ctx, instance) } // GetParents returns the parents of cosmosdb From 7a53105f6bfcae4e89e0cec845c2a8bd1da6e1e7 Mon Sep 17 00:00:00 2001 From: jpflueger Date: Wed, 15 Apr 2020 11:24:19 -0600 Subject: [PATCH 05/21] using Error() instead of Reason --- pkg/resourcemanager/cosmosdbs/cosmosdb_reconcile.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/resourcemanager/cosmosdbs/cosmosdb_reconcile.go b/pkg/resourcemanager/cosmosdbs/cosmosdb_reconcile.go index 24ff5838558..8125fd8d079 100644 --- a/pkg/resourcemanager/cosmosdbs/cosmosdb_reconcile.go +++ b/pkg/resourcemanager/cosmosdbs/cosmosdb_reconcile.go @@ -48,13 +48,13 @@ func (m *AzureCosmosDBManager) Ensure(ctx context.Context, obj runtime.Object, o switch azerr.Type { case errhelp.ResourceGroupNotFoundErrorCode, errhelp.ParentNotFoundErrorCode: instance.Status.Provisioning = false - instance.Status.Message = azerr.Reason + instance.Status.Message = azerr.Error() instance.Status.State = "Waiting" return false, nil case errhelp.ResourceNotFound: //NO-OP, try to create default: - instance.Status.Message = fmt.Sprintf("Unhandled error after Get %v", azerr.Reason) + instance.Status.Message = fmt.Sprintf("Unhandled error after Get %v", azerr.Error()) } } else { @@ -105,11 +105,11 @@ func (m *AzureCosmosDBManager) Ensure(ctx context.Context, obj runtime.Object, o return false, nil case errhelp.InvalidResourceLocation, errhelp.LocationNotAvailableForResourceType: instance.Status.Provisioning = false - instance.Status.Message = azerr.Reason + instance.Status.Message = azerr.Error() return true, nil case errhelp.ResourceGroupNotFoundErrorCode, errhelp.ParentNotFoundErrorCode: instance.Status.Provisioning = false - instance.Status.Message = azerr.Reason + instance.Status.Message = azerr.Error() return false, nil case errhelp.NotFoundErrorCode: if nameExists, err := m.CheckNameExistsCosmosDB(ctx, accountName); err != nil { @@ -121,7 +121,7 @@ func (m *AzureCosmosDBManager) Ensure(ctx context.Context, obj runtime.Object, o return true, nil } default: - instance.Status.Message = azerr.Reason + instance.Status.Message = azerr.Error() return false, nil } } @@ -181,7 +181,7 @@ func (m *AzureCosmosDBManager) Delete(ctx context.Context, obj runtime.Object, o } // unhandled error - instance.Status.Message = azerr.Reason + instance.Status.Message = azerr.Error() return false, err } From c8156b9cb54d9775c930a88b52b3cfe6d7699403 Mon Sep 17 00:00:00 2001 From: jpflueger Date: Wed, 15 Apr 2020 16:03:11 -0600 Subject: [PATCH 06/21] fixing several issues from feedback --- pkg/errhelp/errors.go | 1 + .../cosmosdbs/cosmosdb_reconcile.go | 25 ++++++++++--------- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/pkg/errhelp/errors.go b/pkg/errhelp/errors.go index 4318fde13e5..f27f90f41f2 100644 --- a/pkg/errhelp/errors.go +++ b/pkg/errhelp/errors.go @@ -42,6 +42,7 @@ const ( NotFoundErrorCode = "NotFound" NoSuchHost = "no such host" ParentNotFoundErrorCode = "ParentResourceNotFound" + PreconditionFailed = "PreconditionFailed" QuotaExceeded = "QuotaExceeded" ResourceGroupNotFoundErrorCode = "ResourceGroupNotFound" RegionDoesNotAllowProvisioning = "RegionDoesNotAllowProvisioning" diff --git a/pkg/resourcemanager/cosmosdbs/cosmosdb_reconcile.go b/pkg/resourcemanager/cosmosdbs/cosmosdb_reconcile.go index 8125fd8d079..659bc37f079 100644 --- a/pkg/resourcemanager/cosmosdbs/cosmosdb_reconcile.go +++ b/pkg/resourcemanager/cosmosdbs/cosmosdb_reconcile.go @@ -6,6 +6,7 @@ package cosmosdbs import ( "context" "fmt" + "strings" "github.com/Azure/azure-service-operator/api/v1alpha1" "github.com/Azure/azure-service-operator/pkg/errhelp" @@ -45,16 +46,13 @@ func (m *AzureCosmosDBManager) Ensure(ctx context.Context, obj runtime.Object, o if err != nil { azerr := errhelp.NewAzureErrorAzureError(err) + instance.Status.Message = err.Error() + switch azerr.Type { case errhelp.ResourceGroupNotFoundErrorCode, errhelp.ParentNotFoundErrorCode: instance.Status.Provisioning = false - instance.Status.Message = azerr.Error() instance.Status.State = "Waiting" return false, nil - case errhelp.ResourceNotFound: - //NO-OP, try to create - default: - instance.Status.Message = fmt.Sprintf("Unhandled error after Get %v", azerr.Error()) } } else { @@ -87,6 +85,8 @@ func (m *AzureCosmosDBManager) Ensure(ctx context.Context, obj runtime.Object, o return true, nil } + instance.Status.Provisioning = true + tags := helpers.LabelsToTags(instance.GetLabels()) accountName := instance.ObjectMeta.Name groupName := instance.Spec.ResourceGroup @@ -165,18 +165,19 @@ func (m *AzureCosmosDBManager) Delete(ctx context.Context, obj runtime.Object, o if err != nil { azerr := errhelp.NewAzureErrorAzureError(err) - // this is likely to happen on first try due to not waiting for the future to complete - if azerr.Type == errhelp.AsyncOpIncompleteError { + // request submitted or already in progress + if azerr.Type == errhelp.AsyncOpIncompleteError || (azerr.Type == errhelp.PreconditionFailed && strings.Contains(azerr.Reason, "operation in progress")) { + instance.Status.State = "Deleting" instance.Status.Message = "Deletion request submitted successfully" return true, nil } - notFoundErrors := []string{ - errhelp.NotFoundErrorCode, // happens on first request after deletion succeeds - errhelp.ResourceNotFound, // happens on subsequent requests after deletion succeeds - errhelp.ResourceGroupNotFoundErrorCode, // database doesn't exist in this resource group but the name exists globally + notFound := []string{ + errhelp.NotFoundErrorCode, + errhelp.ResourceNotFound, + errhelp.ResourceGroupNotFoundErrorCode, } - if helpers.ContainsString(notFoundErrors, azerr.Type) { + if helpers.ContainsString(notFound, azerr.Type) { return false, m.deleteAccountKeysSecret(ctx, instance) } From 799e654c499dfec2519debf57c1ee9e7c42d8a3c Mon Sep 17 00:00:00 2001 From: jpflueger Date: Wed, 15 Apr 2020 16:15:50 -0600 Subject: [PATCH 07/21] setting hash for op incomplete --- .../cosmosdbs/cosmosdb_reconcile.go | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/pkg/resourcemanager/cosmosdbs/cosmosdb_reconcile.go b/pkg/resourcemanager/cosmosdbs/cosmosdb_reconcile.go index 659bc37f079..9533331f459 100644 --- a/pkg/resourcemanager/cosmosdbs/cosmosdb_reconcile.go +++ b/pkg/resourcemanager/cosmosdbs/cosmosdb_reconcile.go @@ -97,33 +97,32 @@ func (m *AzureCosmosDBManager) Ensure(ctx context.Context, obj runtime.Object, o db, err = m.CreateOrUpdateCosmosDB(ctx, groupName, accountName, location, kind, dbType, tags) if err != nil { azerr := errhelp.NewAzureErrorAzureError(err) + instance.Status.Message = err.Error() switch azerr.Type { case errhelp.AsyncOpIncompleteError: instance.Status.State = "Creating" instance.Status.Message = "Resource request successfully submitted to Azure" - return false, nil + instance.Status.SpecHash = hash case errhelp.InvalidResourceLocation, errhelp.LocationNotAvailableForResourceType: instance.Status.Provisioning = false instance.Status.Message = azerr.Error() return true, nil case errhelp.ResourceGroupNotFoundErrorCode, errhelp.ParentNotFoundErrorCode: instance.Status.Provisioning = false - instance.Status.Message = azerr.Error() - return false, nil case errhelp.NotFoundErrorCode: - if nameExists, err := m.CheckNameExistsCosmosDB(ctx, accountName); err != nil { + nameExists, err := m.CheckNameExistsCosmosDB(ctx, accountName) + if err != nil { instance.Status.Message = err.Error() - return false, err - } else if nameExists { + } + if nameExists { instance.Status.Provisioning = false instance.Status.Message = "CosmosDB Account name already exists" return true, nil } - default: - instance.Status.Message = azerr.Error() - return false, nil } + + return false, nil } instance.Status.SpecHash = hash From 65576bcf0f8f02564c7df8bdc06c64a1ed1d44ff Mon Sep 17 00:00:00 2001 From: Melanie Rush Date: Thu, 16 Apr 2020 14:21:57 -0600 Subject: [PATCH 08/21] location to create --- api/v1alpha1/cosmosdb_types.go | 4 +++- config/samples/azure_v1alpha1_cosmosdb.yaml | 3 ++- pkg/resourcemanager/cosmosdbs/cosmosdb.go | 5 +++-- pkg/resourcemanager/cosmosdbs/cosmosdb_manager.go | 2 +- pkg/resourcemanager/cosmosdbs/cosmosdb_reconcile.go | 3 ++- 5 files changed, 11 insertions(+), 6 deletions(-) diff --git a/api/v1alpha1/cosmosdb_types.go b/api/v1alpha1/cosmosdb_types.go index 34fea0935fc..4b328a531b8 100644 --- a/api/v1alpha1/cosmosdb_types.go +++ b/api/v1alpha1/cosmosdb_types.go @@ -40,12 +40,14 @@ const ( // CosmosDBProperties the CosmosDBProperties of CosmosDB. type CosmosDBProperties struct { // CosmosDBDatabaseAccountOfferType - The offer type for the Cosmos DB database account. - DatabaseAccountOfferType CosmosDBDatabaseAccountOfferType `json:"databaseAccountOfferType,omitempty"` + DatabaseAccountOfferType CosmosDBDatabaseAccountOfferType `json:"databaseAccountOfferType,omitempty"` + EnableMultipleWriteLocations CosmosDBEnableMultipleWriteLocations `json:"enableMultipleWriteLocations,omitempty"` //Locations []CosmosDBLocation `json:"locations,omitempty"` } // +kubebuilder:validation:Enum=Standard type CosmosDBDatabaseAccountOfferType string +type CosmosDBEnableMultipleWriteLocations bool const ( // CosmosDBDatabaseAccountOfferTypeStandard string constant describing standard account offer type diff --git a/config/samples/azure_v1alpha1_cosmosdb.yaml b/config/samples/azure_v1alpha1_cosmosdb.yaml index d3efa498467..0dceae2222d 100644 --- a/config/samples/azure_v1alpha1_cosmosdb.yaml +++ b/config/samples/azure_v1alpha1_cosmosdb.yaml @@ -1,10 +1,11 @@ apiVersion: azure.microsoft.com/v1alpha1 kind: CosmosDB metadata: - name: cosmosdb-sample1908xyzkj + name: cosmosdb-sample-1 spec: kind: GlobalDocumentDB location: westus resourceGroup: resourcegroup-azure-operators properties: databaseAccountOfferType: Standard + enableMultipleWriteLocations: false diff --git a/pkg/resourcemanager/cosmosdbs/cosmosdb.go b/pkg/resourcemanager/cosmosdbs/cosmosdb.go index 0d22bb85e85..a2cbf1744b5 100644 --- a/pkg/resourcemanager/cosmosdbs/cosmosdb.go +++ b/pkg/resourcemanager/cosmosdbs/cosmosdb.go @@ -42,6 +42,7 @@ func (*AzureCosmosDBManager) CreateOrUpdateCosmosDB( location string, kind v1alpha1.CosmosDBKind, dbType v1alpha1.CosmosDBDatabaseAccountOfferType, + enableWriteLocations v1alpha1.CosmosDBEnableMultipleWriteLocations, tags map[string]*string) (*documentdb.DatabaseAccount, *errhelp.AzureError) { cosmosDBClient, err := getCosmosDBClient() if err != nil { @@ -50,7 +51,7 @@ func (*AzureCosmosDBManager) CreateOrUpdateCosmosDB( dbKind := documentdb.DatabaseAccountKind(kind) sDBType := string(dbType) - + bWriteLocal := bool(enableWriteLocations) /* * Current state of Locations and CosmosDB properties: * Creating a Database account with CosmosDB requires @@ -81,7 +82,7 @@ func (*AzureCosmosDBManager) CreateOrUpdateCosmosDB( ID: &cosmosDBName, DatabaseAccountCreateUpdateProperties: &documentdb.DatabaseAccountCreateUpdateProperties{ DatabaseAccountOfferType: &sDBType, - EnableMultipleWriteLocations: to.BoolPtr(false), + EnableMultipleWriteLocations: &bWriteLocal, IsVirtualNetworkFilterEnabled: to.BoolPtr(false), Locations: &locationsArray, }, diff --git a/pkg/resourcemanager/cosmosdbs/cosmosdb_manager.go b/pkg/resourcemanager/cosmosdbs/cosmosdb_manager.go index f916e70fd54..09d4134f72f 100644 --- a/pkg/resourcemanager/cosmosdbs/cosmosdb_manager.go +++ b/pkg/resourcemanager/cosmosdbs/cosmosdb_manager.go @@ -21,7 +21,7 @@ func NewAzureCosmosDBManager() *AzureCosmosDBManager { // CosmosDBManager client functions type CosmosDBManager interface { // CreateOrUpdateCosmosDB creates a new cosmos database account - CreateOrUpdateCosmosDB(ctx context.Context, groupName string, cosmosDBName string, location string, kind v1alpha1.CosmosDBKind, dbType v1alpha1.CosmosDBDatabaseAccountOfferType, tags map[string]*string) (*documentdb.DatabaseAccount, *errhelp.AzureError) + CreateOrUpdateCosmosDB(ctx context.Context, groupName string, cosmosDBName string, location string, kind v1alpha1.CosmosDBKind, dbType v1alpha1.CosmosDBDatabaseAccountOfferType, enableWriteLocations v1alpha1.CosmosDBEnableMultipleWriteLocations, tags map[string]*string) (*documentdb.DatabaseAccount, *errhelp.AzureError) // GetCosmosDB gets a cosmos database account GetCosmosDB(ctx context.Context, groupName string, cosmosDBName string) (*documentdb.DatabaseAccount, *errhelp.AzureError) diff --git a/pkg/resourcemanager/cosmosdbs/cosmosdb_reconcile.go b/pkg/resourcemanager/cosmosdbs/cosmosdb_reconcile.go index 4eb8f6b0a0f..4f5ff0d4568 100644 --- a/pkg/resourcemanager/cosmosdbs/cosmosdb_reconcile.go +++ b/pkg/resourcemanager/cosmosdbs/cosmosdb_reconcile.go @@ -90,8 +90,9 @@ func (m *AzureCosmosDBManager) Ensure(ctx context.Context, obj runtime.Object, o location := instance.Spec.Location kind := instance.Spec.Kind dbType := instance.Spec.Properties.DatabaseAccountOfferType + enableWriteLocations := instance.Spec.Properties.EnableMultipleWriteLocations - db, azerr := m.CreateOrUpdateCosmosDB(ctx, groupName, accountName, location, kind, dbType, tags) + db, azerr := m.CreateOrUpdateCosmosDB(ctx, groupName, accountName, location, kind, dbType, enableWriteLocations, tags) // everything is in a created/updated state if azerr == nil { From 9519962f6c9b6fddfc9346fb8446c72fc73aa8db Mon Sep 17 00:00:00 2001 From: Melanie Rush Date: Thu, 16 Apr 2020 16:38:29 -0600 Subject: [PATCH 09/21] adjusting method to properties --- api/v1alpha1/cosmosdb_types.go | 6 ++---- pkg/resourcemanager/cosmosdbs/cosmosdb.go | 8 ++++---- pkg/resourcemanager/cosmosdbs/cosmosdb_manager.go | 3 ++- pkg/resourcemanager/cosmosdbs/cosmosdb_reconcile.go | 10 +++++++--- 4 files changed, 15 insertions(+), 12 deletions(-) diff --git a/api/v1alpha1/cosmosdb_types.go b/api/v1alpha1/cosmosdb_types.go index 4b328a531b8..0a291437f63 100644 --- a/api/v1alpha1/cosmosdb_types.go +++ b/api/v1alpha1/cosmosdb_types.go @@ -40,14 +40,12 @@ const ( // CosmosDBProperties the CosmosDBProperties of CosmosDB. type CosmosDBProperties struct { // CosmosDBDatabaseAccountOfferType - The offer type for the Cosmos DB database account. - DatabaseAccountOfferType CosmosDBDatabaseAccountOfferType `json:"databaseAccountOfferType,omitempty"` - EnableMultipleWriteLocations CosmosDBEnableMultipleWriteLocations `json:"enableMultipleWriteLocations,omitempty"` - //Locations []CosmosDBLocation `json:"locations,omitempty"` + DatabaseAccountOfferType CosmosDBDatabaseAccountOfferType `json:"databaseAccountOfferType,omitempty"` + EnableMultipleWriteLocations bool `json:"enableMultipleWriteLocations,omitempty"` } // +kubebuilder:validation:Enum=Standard type CosmosDBDatabaseAccountOfferType string -type CosmosDBEnableMultipleWriteLocations bool const ( // CosmosDBDatabaseAccountOfferTypeStandard string constant describing standard account offer type diff --git a/pkg/resourcemanager/cosmosdbs/cosmosdb.go b/pkg/resourcemanager/cosmosdbs/cosmosdb.go index a2cbf1744b5..8952ed049c6 100644 --- a/pkg/resourcemanager/cosmosdbs/cosmosdb.go +++ b/pkg/resourcemanager/cosmosdbs/cosmosdb.go @@ -10,6 +10,7 @@ import ( "github.com/Azure/azure-sdk-for-go/services/cosmos-db/mgmt/2015-04-08/documentdb" "github.com/Azure/azure-service-operator/api/v1alpha1" + azurev1alpha1 "github.com/Azure/azure-service-operator/api/v1alpha1" "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" @@ -41,8 +42,7 @@ func (*AzureCosmosDBManager) CreateOrUpdateCosmosDB( cosmosDBName string, location string, kind v1alpha1.CosmosDBKind, - dbType v1alpha1.CosmosDBDatabaseAccountOfferType, - enableWriteLocations v1alpha1.CosmosDBEnableMultipleWriteLocations, + properties azurev1alpha1.CosmosDBProperties, tags map[string]*string) (*documentdb.DatabaseAccount, *errhelp.AzureError) { cosmosDBClient, err := getCosmosDBClient() if err != nil { @@ -50,8 +50,8 @@ func (*AzureCosmosDBManager) CreateOrUpdateCosmosDB( } dbKind := documentdb.DatabaseAccountKind(kind) - sDBType := string(dbType) - bWriteLocal := bool(enableWriteLocations) + sDBType := string(properties.DatabaseAccountOfferType) + bWriteLocal := bool(properties.EnableMultipleWriteLocations) /* * Current state of Locations and CosmosDB properties: * Creating a Database account with CosmosDB requires diff --git a/pkg/resourcemanager/cosmosdbs/cosmosdb_manager.go b/pkg/resourcemanager/cosmosdbs/cosmosdb_manager.go index 09d4134f72f..5f117fc6554 100644 --- a/pkg/resourcemanager/cosmosdbs/cosmosdb_manager.go +++ b/pkg/resourcemanager/cosmosdbs/cosmosdb_manager.go @@ -8,6 +8,7 @@ import ( "github.com/Azure/azure-sdk-for-go/services/cosmos-db/mgmt/2015-04-08/documentdb" "github.com/Azure/azure-service-operator/api/v1alpha1" + azurev1alpha1 "github.com/Azure/azure-service-operator/api/v1alpha1" "github.com/Azure/azure-service-operator/pkg/errhelp" "github.com/Azure/azure-service-operator/pkg/resourcemanager" "github.com/Azure/go-autorest/autorest" @@ -21,7 +22,7 @@ func NewAzureCosmosDBManager() *AzureCosmosDBManager { // CosmosDBManager client functions type CosmosDBManager interface { // CreateOrUpdateCosmosDB creates a new cosmos database account - CreateOrUpdateCosmosDB(ctx context.Context, groupName string, cosmosDBName string, location string, kind v1alpha1.CosmosDBKind, dbType v1alpha1.CosmosDBDatabaseAccountOfferType, enableWriteLocations v1alpha1.CosmosDBEnableMultipleWriteLocations, tags map[string]*string) (*documentdb.DatabaseAccount, *errhelp.AzureError) + CreateOrUpdateCosmosDB(ctx context.Context, groupName string, cosmosDBName string, location string, kind v1alpha1.CosmosDBKind, properties azurev1alpha1.CosmosDBProperties, tags map[string]*string) (*documentdb.DatabaseAccount, *errhelp.AzureError) // GetCosmosDB gets a cosmos database account GetCosmosDB(ctx context.Context, groupName string, cosmosDBName string) (*documentdb.DatabaseAccount, *errhelp.AzureError) diff --git a/pkg/resourcemanager/cosmosdbs/cosmosdb_reconcile.go b/pkg/resourcemanager/cosmosdbs/cosmosdb_reconcile.go index 4f5ff0d4568..c4ad59cad98 100644 --- a/pkg/resourcemanager/cosmosdbs/cosmosdb_reconcile.go +++ b/pkg/resourcemanager/cosmosdbs/cosmosdb_reconcile.go @@ -8,6 +8,7 @@ import ( "fmt" "github.com/Azure/azure-service-operator/api/v1alpha1" + azurev1alpha1 "github.com/Azure/azure-service-operator/api/v1alpha1" "github.com/Azure/azure-service-operator/pkg/errhelp" "github.com/Azure/azure-service-operator/pkg/helpers" "github.com/Azure/azure-service-operator/pkg/resourcemanager" @@ -89,10 +90,13 @@ func (m *AzureCosmosDBManager) Ensure(ctx context.Context, obj runtime.Object, o groupName := instance.Spec.ResourceGroup location := instance.Spec.Location kind := instance.Spec.Kind - dbType := instance.Spec.Properties.DatabaseAccountOfferType - enableWriteLocations := instance.Spec.Properties.EnableMultipleWriteLocations - db, azerr := m.CreateOrUpdateCosmosDB(ctx, groupName, accountName, location, kind, dbType, enableWriteLocations, tags) + cosmosDBProperties := azurev1alpha1.CosmosDBProperties{ + DatabaseAccountOfferType: instance.Spec.Properties.DatabaseAccountOfferType, + EnableMultipleWriteLocations: instance.Spec.Properties.EnableMultipleWriteLocations, + } + + db, azerr := m.CreateOrUpdateCosmosDB(ctx, groupName, accountName, location, kind, cosmosDBProperties, tags) // everything is in a created/updated state if azerr == nil { From 4c83c1ef17625db8bfd4ca08697426892904e9a4 Mon Sep 17 00:00:00 2001 From: jpflueger Date: Thu, 16 Apr 2020 17:01:31 -0600 Subject: [PATCH 10/21] adjusting no resource group test --- controllers/cosmosdb_controller_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/controllers/cosmosdb_controller_test.go b/controllers/cosmosdb_controller_test.go index 7c0b4ff255e..31ad06cb5e7 100644 --- a/controllers/cosmosdb_controller_test.go +++ b/controllers/cosmosdb_controller_test.go @@ -10,6 +10,8 @@ import ( "testing" "github.com/Azure/azure-service-operator/api/v1alpha1" + "github.com/Azure/azure-service-operator/pkg/errhelp" + "github.com/stretchr/testify/assert" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -84,10 +86,8 @@ func TestCosmosDBControllerNoResourceGroup(t *testing.T) { }, }, } - //the expected error meessage to be shown - errMessage := "Waiting for resource group '" + resourceGroupName + "' to be available" - EnsureInstanceWithResult(ctx, t, tc, dbInstance1, errMessage, false) + EnsureInstanceWithResult(ctx, t, tc, dbInstance1, errhelp.ResourceGroupNotFoundErrorCode, false) EnsureDelete(ctx, t, tc, dbInstance1) } From 4fc8814f19901d8c5d51622ee9330dd940c253c4 Mon Sep 17 00:00:00 2001 From: jpflueger Date: Fri, 17 Apr 2020 16:09:54 -0600 Subject: [PATCH 11/21] adding temporary logging to diagnose CI test failures --- .../cosmosdbs/cosmosdb_reconcile.go | 26 ++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/pkg/resourcemanager/cosmosdbs/cosmosdb_reconcile.go b/pkg/resourcemanager/cosmosdbs/cosmosdb_reconcile.go index 9533331f459..13efe1dd368 100644 --- a/pkg/resourcemanager/cosmosdbs/cosmosdb_reconcile.go +++ b/pkg/resourcemanager/cosmosdbs/cosmosdb_reconcile.go @@ -6,6 +6,7 @@ package cosmosdbs import ( "context" "fmt" + "log" "strings" "github.com/Azure/azure-service-operator/api/v1alpha1" @@ -226,6 +227,15 @@ func (m *AzureCosmosDBManager) convert(obj runtime.Object) (*v1alpha1.CosmosDB, func (m *AzureCosmosDBManager) createOrUpdateAccountKeysSecret(ctx context.Context, instance *v1alpha1.CosmosDB) error { result, err := m.ListKeys(ctx, instance.Spec.ResourceGroup, instance.ObjectMeta.Name) if err != nil { + //TODO: remove before completing pull request + log.Printf( + `Failed to list CosmosDB Account Keys: + Status: %v + Error: %v + `, + instance.Status, + err, + ) return err } @@ -240,7 +250,21 @@ func (m *AzureCosmosDBManager) createOrUpdateAccountKeysSecret(ctx context.Conte "secondaryReadonlyMasterKey": []byte(*result.SecondaryReadonlyMasterKey), } - return m.SecretClient.Upsert(ctx, secretKey, secretData) + err = m.SecretClient.Upsert(ctx, secretKey, secretData) + if err != nil { + //TODO: remove before completing pull request + log.Printf( + `Failed to upsert CosmosDB Account Keys: + Status: %v + Error: %v + `, + instance.Status, + err, + ) + return err + } + + return nil } func (m *AzureCosmosDBManager) deleteAccountKeysSecret(ctx context.Context, instance *v1alpha1.CosmosDB) error { From a83bff64aac046e44456595b8c31b672c8f32938 Mon Sep 17 00:00:00 2001 From: jpflueger Date: Fri, 17 Apr 2020 16:56:47 -0600 Subject: [PATCH 12/21] adding more temporary logging --- controllers/cosmosdb_controller_test.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/controllers/cosmosdb_controller_test.go b/controllers/cosmosdb_controller_test.go index 31ad06cb5e7..2e43c40123e 100644 --- a/controllers/cosmosdb_controller_test.go +++ b/controllers/cosmosdb_controller_test.go @@ -7,6 +7,7 @@ package controllers import ( "context" + "log" "testing" "github.com/Azure/azure-service-operator/api/v1alpha1" @@ -48,6 +49,12 @@ func TestCosmosDBHappyPath(t *testing.T) { assert.Eventually(func() bool { secret, err := tc.secretClient.Get(ctx, key) + if err != nil { + log.Printf("CosmosDB Get Secret Failed: %v\n", err) + } + if len(secret) <= 0 { + log.Printf("CosmosDB Length Secret Failed: %v\n", secret) + } return err == nil && len(secret) > 0 }, tc.timeoutFast, tc.retry, "wait for cosmosdb to have secret") From b7b151623074ade5cfc16318a03b72820d9e633c Mon Sep 17 00:00:00 2001 From: jpflueger Date: Fri, 17 Apr 2020 17:07:37 -0600 Subject: [PATCH 13/21] adding more diagnostic logging --- controllers/cosmosdb_controller_test.go | 23 +++++++++++++++---- .../cosmosdbs/cosmosdb_reconcile.go | 2 ++ 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/controllers/cosmosdb_controller_test.go b/controllers/cosmosdb_controller_test.go index 2e43c40123e..7b5aaa31c7a 100644 --- a/controllers/cosmosdb_controller_test.go +++ b/controllers/cosmosdb_controller_test.go @@ -49,11 +49,24 @@ func TestCosmosDBHappyPath(t *testing.T) { assert.Eventually(func() bool { secret, err := tc.secretClient.Get(ctx, key) - if err != nil { - log.Printf("CosmosDB Get Secret Failed: %v\n", err) - } - if len(secret) <= 0 { - log.Printf("CosmosDB Length Secret Failed: %v\n", secret) + cond := err == nil && len(secret) > 0 + if !cond { + //TODO: remove before completing pull request + tc.k8sClient.Get(ctx, key, dbInstance) + log.Printf( + `----- COSMOSDB ----- + Secret: %v + Assertion: %v + Error: %v + Secret: %v + Status: %v + `, + key, + cond, + err, + secret, + dbInstance.Status, + ) } return err == nil && len(secret) > 0 }, tc.timeoutFast, tc.retry, "wait for cosmosdb to have secret") diff --git a/pkg/resourcemanager/cosmosdbs/cosmosdb_reconcile.go b/pkg/resourcemanager/cosmosdbs/cosmosdb_reconcile.go index 13efe1dd368..26d6c4f280d 100644 --- a/pkg/resourcemanager/cosmosdbs/cosmosdb_reconcile.go +++ b/pkg/resourcemanager/cosmosdbs/cosmosdb_reconcile.go @@ -264,6 +264,8 @@ func (m *AzureCosmosDBManager) createOrUpdateAccountKeysSecret(ctx context.Conte return err } + //TODO: remove before completing pull request + log.Printf("Successfully upserted CosmosDB Account Keys: %v\n", secretKey) return nil } From 348f49f295989549ce22503fdd3b18c9e9fee1ec Mon Sep 17 00:00:00 2001 From: jpflueger Date: Fri, 17 Apr 2020 19:49:03 -0600 Subject: [PATCH 14/21] adding more logging and moving secret creation --- .../cosmosdbs/cosmosdb_reconcile.go | 25 ++++++++++++++----- 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/pkg/resourcemanager/cosmosdbs/cosmosdb_reconcile.go b/pkg/resourcemanager/cosmosdbs/cosmosdb_reconcile.go index 26d6c4f280d..18ec817e172 100644 --- a/pkg/resourcemanager/cosmosdbs/cosmosdb_reconcile.go +++ b/pkg/resourcemanager/cosmosdbs/cosmosdb_reconcile.go @@ -66,17 +66,22 @@ func (m *AzureCosmosDBManager) Ensure(ctx context.Context, obj runtime.Object, o return false, nil } - if instance.Status.State == "Succeeded" && instance.Status.SpecHash == hash { + if instance.Status.State == "Succeeded" { + log.Println("Hit Get success path") + // provisioning is complete, update the secrets if err = m.createOrUpdateAccountKeysSecret(ctx, instance); err != nil { instance.Status.Message = err.Error() return false, err } - instance.Status.Message = resourcemanager.SuccessMsg - instance.Status.Provisioning = false - instance.Status.Provisioned = true - return true, nil + if instance.Status.SpecHash == hash { + log.Println("Hit Get + SpecHash success path") + instance.Status.Message = resourcemanager.SuccessMsg + instance.Status.Provisioning = false + instance.Status.Provisioned = true + return true, nil + } } if instance.Status.State == "Failed" { @@ -105,6 +110,7 @@ func (m *AzureCosmosDBManager) Ensure(ctx context.Context, obj runtime.Object, o instance.Status.State = "Creating" instance.Status.Message = "Resource request successfully submitted to Azure" instance.Status.SpecHash = hash + return false, nil case errhelp.InvalidResourceLocation, errhelp.LocationNotAvailableForResourceType: instance.Status.Provisioning = false instance.Status.Message = azerr.Error() @@ -123,9 +129,16 @@ func (m *AzureCosmosDBManager) Ensure(ctx context.Context, obj runtime.Object, o } } - return false, nil + log.Printf("Hit unhandled error case: %v\n", err) + return false, err + } + + if err = m.createOrUpdateAccountKeysSecret(ctx, instance); err != nil { + instance.Status.Message = err.Error() + return false, err } + log.Println("Hit terminal success case.") instance.Status.SpecHash = hash instance.Status.ResourceId = *db.ID instance.Status.State = *db.ProvisioningState From 421942eab99055b2c1da3efc62368d4f654bc3c1 Mon Sep 17 00:00:00 2001 From: jpflueger Date: Sun, 19 Apr 2020 23:52:27 -0600 Subject: [PATCH 15/21] removing diagnostic logging --- controllers/cosmosdb_controller_test.go | 20 -------------- .../cosmosdbs/cosmosdb_reconcile.go | 26 ------------------- 2 files changed, 46 deletions(-) diff --git a/controllers/cosmosdb_controller_test.go b/controllers/cosmosdb_controller_test.go index 7b5aaa31c7a..31ad06cb5e7 100644 --- a/controllers/cosmosdb_controller_test.go +++ b/controllers/cosmosdb_controller_test.go @@ -7,7 +7,6 @@ package controllers import ( "context" - "log" "testing" "github.com/Azure/azure-service-operator/api/v1alpha1" @@ -49,25 +48,6 @@ func TestCosmosDBHappyPath(t *testing.T) { assert.Eventually(func() bool { secret, err := tc.secretClient.Get(ctx, key) - cond := err == nil && len(secret) > 0 - if !cond { - //TODO: remove before completing pull request - tc.k8sClient.Get(ctx, key, dbInstance) - log.Printf( - `----- COSMOSDB ----- - Secret: %v - Assertion: %v - Error: %v - Secret: %v - Status: %v - `, - key, - cond, - err, - secret, - dbInstance.Status, - ) - } return err == nil && len(secret) > 0 }, tc.timeoutFast, tc.retry, "wait for cosmosdb to have secret") diff --git a/pkg/resourcemanager/cosmosdbs/cosmosdb_reconcile.go b/pkg/resourcemanager/cosmosdbs/cosmosdb_reconcile.go index 18ec817e172..bd44b45a448 100644 --- a/pkg/resourcemanager/cosmosdbs/cosmosdb_reconcile.go +++ b/pkg/resourcemanager/cosmosdbs/cosmosdb_reconcile.go @@ -6,7 +6,6 @@ package cosmosdbs import ( "context" "fmt" - "log" "strings" "github.com/Azure/azure-service-operator/api/v1alpha1" @@ -67,8 +66,6 @@ func (m *AzureCosmosDBManager) Ensure(ctx context.Context, obj runtime.Object, o } if instance.Status.State == "Succeeded" { - log.Println("Hit Get success path") - // provisioning is complete, update the secrets if err = m.createOrUpdateAccountKeysSecret(ctx, instance); err != nil { instance.Status.Message = err.Error() @@ -76,7 +73,6 @@ func (m *AzureCosmosDBManager) Ensure(ctx context.Context, obj runtime.Object, o } if instance.Status.SpecHash == hash { - log.Println("Hit Get + SpecHash success path") instance.Status.Message = resourcemanager.SuccessMsg instance.Status.Provisioning = false instance.Status.Provisioned = true @@ -129,7 +125,6 @@ func (m *AzureCosmosDBManager) Ensure(ctx context.Context, obj runtime.Object, o } } - log.Printf("Hit unhandled error case: %v\n", err) return false, err } @@ -138,7 +133,6 @@ func (m *AzureCosmosDBManager) Ensure(ctx context.Context, obj runtime.Object, o return false, err } - log.Println("Hit terminal success case.") instance.Status.SpecHash = hash instance.Status.ResourceId = *db.ID instance.Status.State = *db.ProvisioningState @@ -240,15 +234,6 @@ func (m *AzureCosmosDBManager) convert(obj runtime.Object) (*v1alpha1.CosmosDB, func (m *AzureCosmosDBManager) createOrUpdateAccountKeysSecret(ctx context.Context, instance *v1alpha1.CosmosDB) error { result, err := m.ListKeys(ctx, instance.Spec.ResourceGroup, instance.ObjectMeta.Name) if err != nil { - //TODO: remove before completing pull request - log.Printf( - `Failed to list CosmosDB Account Keys: - Status: %v - Error: %v - `, - instance.Status, - err, - ) return err } @@ -265,20 +250,9 @@ func (m *AzureCosmosDBManager) createOrUpdateAccountKeysSecret(ctx context.Conte err = m.SecretClient.Upsert(ctx, secretKey, secretData) if err != nil { - //TODO: remove before completing pull request - log.Printf( - `Failed to upsert CosmosDB Account Keys: - Status: %v - Error: %v - `, - instance.Status, - err, - ) return err } - //TODO: remove before completing pull request - log.Printf("Successfully upserted CosmosDB Account Keys: %v\n", secretKey) return nil } From d53a8323225f79c72fffc1516223f966eb2d1e7d Mon Sep 17 00:00:00 2001 From: jpflueger Date: Mon, 20 Apr 2020 12:59:26 -0600 Subject: [PATCH 16/21] adding mongodb version to cosmosdb --- api/v1alpha1/cosmosdb_types.go | 1 + config/samples/azure_v1alpha1_cosmosdb.yaml | 5 ++++- pkg/resourcemanager/cosmosdbs/cosmosdb.go | 9 +++++++++ pkg/resourcemanager/cosmosdbs/cosmosdb_manager.go | 2 +- pkg/resourcemanager/cosmosdbs/cosmosdb_reconcile.go | 3 ++- 5 files changed, 17 insertions(+), 3 deletions(-) diff --git a/api/v1alpha1/cosmosdb_types.go b/api/v1alpha1/cosmosdb_types.go index 36796e27ea0..a037f4e02f1 100644 --- a/api/v1alpha1/cosmosdb_types.go +++ b/api/v1alpha1/cosmosdb_types.go @@ -43,6 +43,7 @@ type CosmosDBProperties struct { // CosmosDBDatabaseAccountOfferType - The offer type for the Cosmos DB database account. DatabaseAccountOfferType CosmosDBDatabaseAccountOfferType `json:"databaseAccountOfferType,omitempty"` //Locations []CosmosDBLocation `json:"locations,omitempty"` + MongoDBVersion string `json:"mongoDBVersion,omitempty"` } // +kubebuilder:validation:Enum=Standard diff --git a/config/samples/azure_v1alpha1_cosmosdb.yaml b/config/samples/azure_v1alpha1_cosmosdb.yaml index 3d84aab4dea..8580c570468 100644 --- a/config/samples/azure_v1alpha1_cosmosdb.yaml +++ b/config/samples/azure_v1alpha1_cosmosdb.yaml @@ -8,7 +8,10 @@ spec: resourceGroup: resourcegroup-azure-operators properties: databaseAccountOfferType: Standard + # optionally set the mongoDBVersion to "3.2" or "3.6", if omitted the default is "3.2" + # NOTE: kind must be set to MongoDB for this to take effect + #mongoDBVersion: "3.6" - # Use the field below to optionally specify a different keyvault + # Use the field below to optionally specify a different keyvault # to store the connectiong string secrets in #keyVaultToStoreSecrets: asoSecretKeyVault \ No newline at end of file diff --git a/pkg/resourcemanager/cosmosdbs/cosmosdb.go b/pkg/resourcemanager/cosmosdbs/cosmosdb.go index ad83a5577a7..1813aab22ce 100644 --- a/pkg/resourcemanager/cosmosdbs/cosmosdb.go +++ b/pkg/resourcemanager/cosmosdbs/cosmosdb.go @@ -44,6 +44,7 @@ func (*AzureCosmosDBManager) CreateOrUpdateCosmosDB( location string, kind v1alpha1.CosmosDBKind, dbType v1alpha1.CosmosDBDatabaseAccountOfferType, + dbVersion string, tags map[string]*string) (*documentdb.DatabaseAccount, error) { cosmosDBClient, err := getCosmosDBClient() if err != nil { @@ -53,6 +54,13 @@ func (*AzureCosmosDBManager) CreateOrUpdateCosmosDB( dbKind := documentdb.DatabaseAccountKind(kind) sDBType := string(dbType) + var capabilities []documentdb.Capability + if dbKind == documentdb.MongoDB && dbVersion == "3.6" { + capabilities = []documentdb.Capability{ + {Name: to.StringPtr("EnableMongo")}, + } + } + /* * Current state of Locations and CosmosDB properties: * Creating a Database account with CosmosDB requires @@ -86,6 +94,7 @@ func (*AzureCosmosDBManager) CreateOrUpdateCosmosDB( EnableMultipleWriteLocations: to.BoolPtr(false), IsVirtualNetworkFilterEnabled: to.BoolPtr(false), Locations: &locationsArray, + Capabilities: &capabilities, }, } createUpdateFuture, err := cosmosDBClient.CreateOrUpdate( diff --git a/pkg/resourcemanager/cosmosdbs/cosmosdb_manager.go b/pkg/resourcemanager/cosmosdbs/cosmosdb_manager.go index 9ce839c076d..88e42e8dc31 100644 --- a/pkg/resourcemanager/cosmosdbs/cosmosdb_manager.go +++ b/pkg/resourcemanager/cosmosdbs/cosmosdb_manager.go @@ -21,7 +21,7 @@ func NewAzureCosmosDBManager(secretClient secrets.SecretClient) *AzureCosmosDBMa // CosmosDBManager client functions type CosmosDBManager interface { // CreateOrUpdateCosmosDB creates a new cosmos database account - CreateOrUpdateCosmosDB(ctx context.Context, groupName string, cosmosDBName string, location string, kind v1alpha1.CosmosDBKind, dbType v1alpha1.CosmosDBDatabaseAccountOfferType, tags map[string]*string) (*documentdb.DatabaseAccount, error) + CreateOrUpdateCosmosDB(ctx context.Context, groupName string, cosmosDBName string, location string, kind v1alpha1.CosmosDBKind, dbType v1alpha1.CosmosDBDatabaseAccountOfferType, dbVersion string, tags map[string]*string) (*documentdb.DatabaseAccount, error) // GetCosmosDB gets a cosmos database account GetCosmosDB(ctx context.Context, groupName string, cosmosDBName string) (*documentdb.DatabaseAccount, error) diff --git a/pkg/resourcemanager/cosmosdbs/cosmosdb_reconcile.go b/pkg/resourcemanager/cosmosdbs/cosmosdb_reconcile.go index bd44b45a448..7885ff5958a 100644 --- a/pkg/resourcemanager/cosmosdbs/cosmosdb_reconcile.go +++ b/pkg/resourcemanager/cosmosdbs/cosmosdb_reconcile.go @@ -95,8 +95,9 @@ func (m *AzureCosmosDBManager) Ensure(ctx context.Context, obj runtime.Object, o location := instance.Spec.Location kind := instance.Spec.Kind dbType := instance.Spec.Properties.DatabaseAccountOfferType + dbVersion := instance.Spec.Properties.MongoDBVersion - db, err = m.CreateOrUpdateCosmosDB(ctx, groupName, accountName, location, kind, dbType, tags) + db, err = m.CreateOrUpdateCosmosDB(ctx, groupName, accountName, location, kind, dbType, dbVersion, tags) if err != nil { azerr := errhelp.NewAzureErrorAzureError(err) instance.Status.Message = err.Error() From 89963d0a7aee4481f1f66129990a43cb75e3c4b4 Mon Sep 17 00:00:00 2001 From: jpflueger Date: Mon, 20 Apr 2020 13:48:21 -0600 Subject: [PATCH 17/21] add empty capabilities array --- pkg/resourcemanager/cosmosdbs/cosmosdb.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/resourcemanager/cosmosdbs/cosmosdb.go b/pkg/resourcemanager/cosmosdbs/cosmosdb.go index 1813aab22ce..aa405dac93a 100644 --- a/pkg/resourcemanager/cosmosdbs/cosmosdb.go +++ b/pkg/resourcemanager/cosmosdbs/cosmosdb.go @@ -59,6 +59,8 @@ func (*AzureCosmosDBManager) CreateOrUpdateCosmosDB( capabilities = []documentdb.Capability{ {Name: to.StringPtr("EnableMongo")}, } + } else { + capabilities = make([]documentdb.Capability, 0) } /* From 67fabde80751679dbed3378453c28c3270c417fb Mon Sep 17 00:00:00 2001 From: Erin Corson Date: Fri, 17 Apr 2020 13:58:57 -0600 Subject: [PATCH 18/21] improve message output for storage account and comment optional vals in sample --- .../samples/azure_v1alpha1_storageaccount.yaml | 16 ++++++++-------- .../storageaccount/storageaccount_reconcile.go | 8 +++----- 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/config/samples/azure_v1alpha1_storageaccount.yaml b/config/samples/azure_v1alpha1_storageaccount.yaml index 39b590fe276..29cd0fe5cdf 100644 --- a/config/samples/azure_v1alpha1_storageaccount.yaml +++ b/config/samples/azure_v1alpha1_storageaccount.yaml @@ -11,11 +11,11 @@ spec: accessTier: Hot supportsHttpsTrafficOnly: true # Optional: networkRule - networkRule: - bypass: AzureServices # Possible values are AzureServices, Metrics, None, Logging - defaultAction: Deny # Possible values are Allow, Deny - virtualNetworkRules: - - subnetId: /subscriptions/{subscription}/resourceGroups/{resourcegroup}/providers/Microsoft.Network/virtualNetworks/{vnet}/subnets/{subnet} - ipRules: #could be an ip range or a ip address - - ipAddressOrRange: 2.2.0.0/24 - - ipAddressOrRange: 2.2.2.1 + # networkRule: + # bypass: AzureServices # Possible values are AzureServices, Metrics, None, Logging + # defaultAction: Deny # Possible values are Allow, Deny + # virtualNetworkRules: + # - subnetId: /subscriptions/{subscription}/resourceGroups/{resourcegroup}/providers/Microsoft.Network/virtualNetworks/{vnet}/subnets/{subnet} + # ipRules: #could be an ip range or a ip address + # - ipAddressOrRange: 2.2.0.0/24 + # - ipAddressOrRange: 2.2.2.1 diff --git a/pkg/resourcemanager/storages/storageaccount/storageaccount_reconcile.go b/pkg/resourcemanager/storages/storageaccount/storageaccount_reconcile.go index 45efda9e48c..b21e2e2b682 100644 --- a/pkg/resourcemanager/storages/storageaccount/storageaccount_reconcile.go +++ b/pkg/resourcemanager/storages/storageaccount/storageaccount_reconcile.go @@ -42,11 +42,7 @@ func (sa *azureStorageManager) Ensure(ctx context.Context, obj runtime.Object, o networkAcls = ParseNetworkPolicy(instance.Spec.NetworkRule) } // convert kube labels to expected tag format - labels := map[string]*string{} - for k, v := range instance.GetLabels() { - value := v - labels[k] = &value - } + labels := helpers.LabelsToTags(instance.GetLabels()) hash := "" stor, err := sa.GetStorage(ctx, groupName, name) @@ -132,6 +128,8 @@ func (sa *azureStorageManager) Ensure(ctx context.Context, obj runtime.Object, o instance.Status.Message = "Storage Account Already exists somewhere else" return true, nil } + + instance.Status.Message = "Storage Account already exists and should be available shortly" instance.Status.Provisioning = true } From a212e9c4fbfc169b76e2c74cd666389f18c8d8b5 Mon Sep 17 00:00:00 2001 From: Erin Corson Date: Fri, 17 Apr 2020 17:12:54 -0600 Subject: [PATCH 19/21] attempt to prevent failedProvisioning due to race conditions in keyvault --- pkg/resourcemanager/keyvaults/keyvault.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/pkg/resourcemanager/keyvaults/keyvault.go b/pkg/resourcemanager/keyvaults/keyvault.go index 2522fe0f3af..c50eefc9818 100644 --- a/pkg/resourcemanager/keyvaults/keyvault.go +++ b/pkg/resourcemanager/keyvaults/keyvault.go @@ -7,6 +7,7 @@ import ( "context" "fmt" "strings" + "time" auth "github.com/Azure/azure-sdk-for-go/services/graphrbac/1.6/graphrbac" "github.com/Azure/azure-sdk-for-go/services/keyvault/mgmt/2018-02-14/keyvault" @@ -21,6 +22,7 @@ import ( "github.com/Azure/go-autorest/autorest" "github.com/Azure/go-autorest/autorest/to" uuid "github.com/satori/go.uuid" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" ) @@ -447,6 +449,14 @@ func (k *azureKeyVaultManager) Ensure(ctx context.Context, obj runtime.Object, o } if helpers.ContainsString(catchUnrecoverableErrors, azerr.Type) { // Unrecoverable error, so stop reconcilation + switch azerr.Type { + case errhelp.AlreadyExists: + timeNow := metav1.NewTime(time.Now()) + if timeNow.Sub(instance.Status.RequestedAt.Time) < (30 * time.Second) { + return false, nil + } + + } instance.Status.Message = "Reconcilation hit unrecoverable error " + err.Error() return true, nil } From 4b0b17e35d7187e4c7e15df625af8bbf72610be6 Mon Sep 17 00:00:00 2001 From: Erin Corson Date: Fri, 17 Apr 2020 17:17:55 -0600 Subject: [PATCH 20/21] don't reset RequestedAt until after the 30 seconds --- pkg/resourcemanager/keyvaults/keyvault.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/resourcemanager/keyvaults/keyvault.go b/pkg/resourcemanager/keyvaults/keyvault.go index c50eefc9818..17fc2e07edd 100644 --- a/pkg/resourcemanager/keyvaults/keyvault.go +++ b/pkg/resourcemanager/keyvaults/keyvault.go @@ -453,6 +453,7 @@ func (k *azureKeyVaultManager) Ensure(ctx context.Context, obj runtime.Object, o case errhelp.AlreadyExists: timeNow := metav1.NewTime(time.Now()) if timeNow.Sub(instance.Status.RequestedAt.Time) < (30 * time.Second) { + instance.Status.Provisioning = true return false, nil } From 95002d726126f92fb95470e2a0cc609dd7eff0ee Mon Sep 17 00:00:00 2001 From: jpflueger Date: Mon, 20 Apr 2020 16:31:54 -0600 Subject: [PATCH 21/21] fixing merge branch 'master' into cosmoslocation vet --- api/v1alpha1/cosmosdb_types.go | 2 +- pkg/resourcemanager/cosmosdbs/cosmosdb.go | 1 - pkg/resourcemanager/cosmosdbs/cosmosdb_manager.go | 1 - pkg/resourcemanager/cosmosdbs/cosmosdb_reconcile.go | 4 ++-- 4 files changed, 3 insertions(+), 5 deletions(-) diff --git a/api/v1alpha1/cosmosdb_types.go b/api/v1alpha1/cosmosdb_types.go index adeab99d032..2481a4d1a2f 100644 --- a/api/v1alpha1/cosmosdb_types.go +++ b/api/v1alpha1/cosmosdb_types.go @@ -43,7 +43,7 @@ type CosmosDBProperties struct { // CosmosDBDatabaseAccountOfferType - The offer type for the Cosmos DB database account. DatabaseAccountOfferType CosmosDBDatabaseAccountOfferType `json:"databaseAccountOfferType,omitempty"` EnableMultipleWriteLocations bool `json:"enableMultipleWriteLocations,omitempty"` - MongoDBVersion string `json:"mongoDBVersion,omitempty"` + MongoDBVersion string `json:"mongoDBVersion,omitempty"` } // +kubebuilder:validation:Enum=Standard diff --git a/pkg/resourcemanager/cosmosdbs/cosmosdb.go b/pkg/resourcemanager/cosmosdbs/cosmosdb.go index 497ec177c48..1e76e65b73f 100644 --- a/pkg/resourcemanager/cosmosdbs/cosmosdb.go +++ b/pkg/resourcemanager/cosmosdbs/cosmosdb.go @@ -10,7 +10,6 @@ import ( "github.com/Azure/azure-sdk-for-go/services/cosmos-db/mgmt/2015-04-08/documentdb" "github.com/Azure/azure-service-operator/api/v1alpha1" - "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/azure-service-operator/pkg/secrets" diff --git a/pkg/resourcemanager/cosmosdbs/cosmosdb_manager.go b/pkg/resourcemanager/cosmosdbs/cosmosdb_manager.go index f2656bedfa7..87f6bfd8345 100644 --- a/pkg/resourcemanager/cosmosdbs/cosmosdb_manager.go +++ b/pkg/resourcemanager/cosmosdbs/cosmosdb_manager.go @@ -8,7 +8,6 @@ import ( "github.com/Azure/azure-sdk-for-go/services/cosmos-db/mgmt/2015-04-08/documentdb" "github.com/Azure/azure-service-operator/api/v1alpha1" - "github.com/Azure/azure-service-operator/pkg/errhelp" "github.com/Azure/azure-service-operator/pkg/resourcemanager" "github.com/Azure/azure-service-operator/pkg/secrets" "github.com/Azure/go-autorest/autorest" diff --git a/pkg/resourcemanager/cosmosdbs/cosmosdb_reconcile.go b/pkg/resourcemanager/cosmosdbs/cosmosdb_reconcile.go index 83cf6e1ebb1..4173f42fdc8 100644 --- a/pkg/resourcemanager/cosmosdbs/cosmosdb_reconcile.go +++ b/pkg/resourcemanager/cosmosdbs/cosmosdb_reconcile.go @@ -98,10 +98,10 @@ func (m *AzureCosmosDBManager) Ensure(ctx context.Context, obj runtime.Object, o cosmosDBProperties := v1alpha1.CosmosDBProperties{ DatabaseAccountOfferType: instance.Spec.Properties.DatabaseAccountOfferType, EnableMultipleWriteLocations: instance.Spec.Properties.EnableMultipleWriteLocations, - MongoDBVersion: instance.Spec.Properties.MongoDBVersion, + MongoDBVersion: instance.Spec.Properties.MongoDBVersion, } - db, err := m.CreateOrUpdateCosmosDB(ctx, groupName, accountName, location, kind, cosmosDBProperties, tags) + db, err = m.CreateOrUpdateCosmosDB(ctx, groupName, accountName, location, kind, cosmosDBProperties, tags) if err != nil { azerr := errhelp.NewAzureErrorAzureError(err) instance.Status.Message = err.Error()