Merge pull request #204 from gossion/create_container_automatically

Create containers if they do not exist. Fix #195

docs/additional-information/migration.md

@@ -50,7 +50,7 @@ If you are customers of [Cloud Foundry on Azure Preview 2](https://azure.microso
 
     If this step is not done, you may hit the error [**#4**](#table_not_found).
 
-  3. Create a new storage account and containers in it manually.
+  3. Create a new storage account and containers in it.
 
     ```
     azure storage account create --location $location --type $account-type --resource-group $resource-group-name $storage-account-name
@@ -62,9 +62,9 @@ If you are customers of [Cloud Foundry on Azure Preview 2](https://azure.microso
     azure storage container create --account-name $storage-account-name --account-key $storage-account-key stemcell
     ```
 
-    If this step is not done, you may hit the error [**#5**](#storage_account_not_found).
+    Instead of manual operation, you can also use CPI to create a new storage account and containers automatically (refer to this [guide](../advanced/deploy-cloudfoundry-for-enterprise#multiple-storage-accounts)).
 
-  Please click [**HERE**](https://github.com/cloudfoundry-incubator/bosh-azure-cpi-release/blob/master/src/bosh_azure_cpi/README.md) to learn more new settings.
+  Please click [**HERE**](http://bosh.io/docs/azure-cpi.html) to learn more new settings.
 
 ## Error
 
@@ -111,10 +111,3 @@ If you are customers of [Cloud Foundry on Azure Preview 2](https://azure.microso
   ```
   Error 100: Unknown CPI error 'Unknown' with message 'TableNotFound (404): The table specified does not exist.
   ```
-
-<a name="storage_account_not_found"></a>
-5. Storage account not found
-
-  ```
-  Error 100: Unknown CPI error 'Bosh::AzureCloud::AzureError' with message 'http_post - error: 404 message: {"error":{"code":"ResourceNotFound","message":"The Resource 'Microsoft.Storage/storageAccounts/<your-storage-account>' under resource group '<your-resource-group>' was not found."}}'
-  ```

src/bosh_azure_cpi/lib/cloud/azure/blob_manager.rb

@@ -209,6 +209,16 @@ def has_container?(storage_account_name, container_name)
       end
     end
 
+    def prepare(storage_account_name, containers: [DISK_CONTAINER, STEMCELL_CONTAINER])
+      @logger.info("prepare(#{storage_account_name}, #{containers})")
+      containers.each do |container|
+        unless has_container?(storage_account_name, container)
+          @logger.debug("Creating the container `#{container}' in the storage account `#{storage_account_name}'")
+          create_container(storage_account_name, container)
+        end
+      end
+    end
+
     private
 
     def compute_chunks(file_size, max_chunk_size)

src/bosh_azure_cpi/lib/cloud/azure/cloud.rb

@@ -547,8 +547,7 @@ def create_storage_account(storage_account_name, resource_pool)
           end
           created = @azure_client2.create_storage_account(storage_account_name, location, resource_pool['storage_account_type'], {})
         end
-        @stemcell_manager.prepare(storage_account_name)
-        @disk_manager.prepare(storage_account_name)
+        @blob_manager.prepare(storage_account_name)
         true
       rescue => e
         error_msg = "create_storage_account - "

src/bosh_azure_cpi/lib/cloud/azure/disk_manager.rb

@@ -1,6 +1,5 @@
 module Bosh::AzureCloud
   class DiskManager
-    DISK_CONTAINER         = 'bosh'
     OS_DISK_PREFIX         = 'bosh-os'
     DATA_DISK_PREFIX       = 'bosh-data'
     EPHEMERAL_DISK_POSTFIX = 'ephemeral'
@@ -16,14 +15,6 @@ def initialize(azure_properties, blob_manager)
       @logger = Bosh::Clouds::Config.logger
     end
 
-    def prepare(storage_account_name)
-      @logger.info("prepare(#{storage_account_name})")
-      unless @blob_manager.has_container?(storage_account_name, DISK_CONTAINER)
-        @logger.debug("Prepare to create container #{DISK_CONTAINER} in #{storage_account_name}")
-        @blob_manager.create_container(storage_account_name, DISK_CONTAINER)
-      end
-    end
-
     def delete_disk(disk_name)
       @logger.info("delete_disk(#{disk_name})")
       storage_account_name = get_storage_account_name(disk_name)
@@ -227,4 +218,4 @@ def parse_snapshot_id(snapshot_id)
       return ret[1], ret[2]
     end
   end
-end
+end

src/bosh_azure_cpi/lib/cloud/azure/helpers.rb

@@ -47,6 +47,9 @@ module Helpers
 
     AZURE_MAX_RETRY_COUNT     = 10
 
+    DISK_CONTAINER            = 'bosh'
+    STEMCELL_CONTAINER        = 'stemcell'
+
     ##
     # Raises CloudError exception
     #

src/bosh_azure_cpi/lib/cloud/azure/stemcell_manager.rb

@@ -1,6 +1,5 @@
 module Bosh::AzureCloud
   class StemcellManager
-    STEMCELL_CONTAINER = 'stemcell'
     STEMCELL_PREFIX    = 'bosh-stemcell'
     STEMCELL_TABLE     = 'stemcells'
 
@@ -18,14 +17,6 @@ def initialize(azure_properties, blob_manager, table_manager)
       @default_storage_account_name = azure_properties['storage_account_name']
     end
 
-    def prepare(storage_account_name)
-      @logger.info("prepare(#{storage_account_name})")
-      unless @blob_manager.has_container?(storage_account_name, STEMCELL_CONTAINER)
-        @logger.debug("Prepare to create container #{STEMCELL_CONTAINER} in #{storage_account_name}")
-        @blob_manager.create_container(storage_account_name, STEMCELL_CONTAINER, {})
-      end
-    end
-
     def delete_stemcell(name)
       @logger.info("delete_stemcell(#{name})")
       if @table_manager.has_table?(STEMCELL_TABLE)
@@ -113,6 +104,11 @@ def handle_stemcell_in_different_storage_account(storage_account_name, name)
             return wait_stemcell_copy(storage_account_name, name, timeout = DEFAULT_COPY_STEMCELL_TIMEOUT)
           end
 
+          # Create containers if they are missing.
+          # Background: When users create a storage account without containers in it, and use that storage account for a resource pool,
+          #             CPI will try to create related containers when copying stemcell to that storage account.
+          @blob_manager.prepare(storage_account_name)
+
           @logger.info("Copy stemcell #{name} to #{storage_account_name}")
           source_blob_uri = get_stemcell_uri(@default_storage_account_name, name)
           @blob_manager.copy_blob(storage_account_name, STEMCELL_CONTAINER, "#{name}.vhd", source_blob_uri)
@@ -157,4 +153,4 @@ def wait_stemcell_copy(storage_account_name, name, timeout = DEFAULT_COPY_STEMCE
       end
     end
   end
-end
+end

src/bosh_azure_cpi/spec/unit/blob_manager_spec.rb

@@ -521,4 +521,59 @@
       end
     end
   end
+
+  describe "#prepare" do
+    let(:another_storage_account_name) { "another-storage-account-name" }
+    let(:another_storage_account) {
+      {
+        :id => "foo",
+        :name => another_storage_account_name,
+        :location => "bar",
+        :provisioning_state => "bar",
+        :account_type => "foo",
+        :storage_blob_host => "fake-blob-endpoint",
+        :storage_table_host => "fake-table-endpoint"
+      }
+    }
+    let(:blob) { instance_double(Azure::Storage::Blob::Blob) }
+    before do
+      allow(azure_client2).to receive(:get_storage_account_by_name).
+        with(another_storage_account_name).
+        and_return(another_storage_account)
+    end
+
+    context "when the container exists" do
+      let(:container) { instance_double(Azure::Storage::Blob::Container::Container) }
+      let(:container_properties) { "fake-properties" }
+
+      before do
+        allow(blob_service).to receive(:get_container_properties).
+          with(container_name).and_return(container)
+        allow(container).to receive(:properties).and_return(container_properties)
+      end
+
+      it "does not create the container" do
+        expect(blob_service).not_to receive(:create_container)
+
+        blob_manager.prepare(another_storage_account_name, containers: [container_name])
+      end
+    end
+
+    context "when the container does not exist" do
+      let(:options) { {} }
+
+      before do
+        allow(blob_service).to receive(:get_container_properties).
+          with(container_name).
+          and_raise("Error code: (404). This is a test!")
+      end
+
+      it "create the container" do
+        expect(blob_service).to receive(:create_container).
+          with(container_name, options)
+
+        blob_manager.prepare(another_storage_account_name, containers: [container_name])
+      end
+    end
+  end
 end

src/bosh_azure_cpi/spec/unit/cloud_spec.rb

@@ -314,8 +314,7 @@
             allow(client2).to receive(:check_storage_account_name_availability)
               .with(storage_account_name).and_return(result)
             allow(client2).to receive(:create_storage_account).and_return(true)
-            allow(stemcell_manager).to receive(:prepare).and_return(nil)
-            allow(disk_manager).to receive(:prepare).and_return(nil)
+            allow(blob_manager).to receive(:prepare).and_return(nil)
             allow(stemcell_manager).to receive(:has_stemcell?).
               with(storage_account_name, stemcell_id).
               and_return(true)
@@ -441,8 +440,7 @@
             allow(client2).to receive(:check_storage_account_name_availability)
               .with(storage_account_name).and_return(result)
 
-            allow(stemcell_manager).to receive(:prepare).and_return(nil)
-            allow(disk_manager).to receive(:prepare).and_return(nil)
+            allow(blob_manager).to receive(:prepare).and_return(nil)
             allow(stemcell_manager).to receive(:has_stemcell?).
               with(storage_account_name, stemcell_id).
               and_return(true)
@@ -483,8 +481,7 @@
             it 'should not raise any error' do
               expect(client2).not_to receive(:get_resource_group)
               expect(client2).not_to receive(:create_storage_account)
-              expect(stemcell_manager).to receive(:prepare)
-              expect(disk_manager).to receive(:prepare)
+              expect(blob_manager).to receive(:prepare)
 
               expect(
                 cloud.create_vm(
@@ -535,8 +532,7 @@
               .with(storage_account_name)
               .and_return(nil, creating_storage_account, storage_account)
 
-            allow(stemcell_manager).to receive(:prepare).and_return(nil)
-            allow(disk_manager).to receive(:prepare).and_return(nil)
+            allow(blob_manager).to receive(:prepare).and_return(nil)
             allow(stemcell_manager).to receive(:has_stemcell?).
               with(storage_account_name, stemcell_id).
               and_return(true)
@@ -547,8 +543,7 @@
 
           it 'should not raise any error' do
             expect(client2).to receive(:create_storage_account)
-            expect(stemcell_manager).to receive(:prepare)
-            expect(disk_manager).to receive(:prepare)
+            expect(blob_manager).to receive(:prepare)
 
             expect(
               cloud.create_vm(
@@ -615,30 +610,9 @@
             allow(client2).to receive(:create_storage_account).and_return(true)
           end
 
-          context 'when the container stemcell cannot be created' do
+          context 'when the containers cannot be created' do
             before do
-              allow(stemcell_manager).to receive(:prepare).and_raise(StandardError)
-            end
-
-            it 'should raise an error' do
-              expect(client2).to receive(:create_storage_account)
-              expect {
-                cloud.create_vm(
-                  agent_id,
-                  stemcell_id,
-                  resource_pool,
-                  networks_spec,
-                  disk_locality,
-                  environment
-                )
-              }.to raise_error(/create_storage_account - The storage account `fake-storage-account-name' is created successfully.\nBut it failed to create the containers bosh and stemcell.\nYou need to manually create them.\nError/)
-            end
-          end
-
-          context 'when the container bosh cannot be created' do
-            before do
-              allow(stemcell_manager).to receive(:prepare).and_return(nil)
-              allow(disk_manager).to receive(:prepare).and_raise(StandardError)
+              allow(blob_manager).to receive(:prepare).and_raise(StandardError)
             end
 
             it 'should raise an error' do

src/bosh_azure_cpi/spec/unit/disk_manager_spec.rb

@@ -10,35 +10,6 @@
   let(:data_disk_prefix) { "bosh-data" }
   let(:disk_name) { "#{data_disk_prefix}-#{storage_account_name}-#{SecureRandom.uuid}-None" }
 
-  describe "#prepare" do
-    context "when the container exists" do
-      before do
-        allow(blob_manager).to receive(:has_container?).
-          and_return(true)
-      end
-
-      it "does not create the container" do
-        expect(blob_manager).not_to receive(:create_container)
-
-        disk_manager.prepare(storage_account_name)
-      end
-    end
-
-    context "when the container does not exist" do
-      before do
-        allow(blob_manager).to receive(:has_container?).
-          and_return(false)
-      end
-
-      it "create the container" do
-        expect(blob_manager).to receive(:create_container).
-          with(storage_account_name, disk_container)
-
-        disk_manager.prepare(storage_account_name)
-      end
-    end
-  end
-
   describe "#delete_disk" do
     context "when the disk exists" do
       before do