Bring back pr cloudfoundry#1558

This reverts commit d1cf638, reversing
changes made to 39952cf.

Author: Aakash Shah

lib/cloud_controller/opi/stager_client.rb

@@ -10,20 +10,54 @@ class StagerClient < BaseClient
     def stage(staging_guid, staging_details)
       logger.info('stage.request', staging_guid: staging_guid)
 
-      if staging_details.lifecycle.type == VCAP::CloudController::Lifecycles::DOCKER
-        complete_staging(staging_guid, staging_details)
-      elsif staging_details.lifecycle.type == VCAP::CloudController::Lifecycles::BUILDPACK
-        staging_request = to_request(staging_guid, staging_details)
-        start_staging(staging_guid, staging_request)
-      else
+      unless [VCAP::CloudController::Lifecycles::DOCKER, VCAP::CloudController::Lifecycles::BUILDPACK].include?(staging_details.lifecycle.type)
         raise("lifecycle type `#{staging_details.lifecycle.type}` is invalid")
       end
+
+      request = to_request(staging_guid, staging_details)
+      start_staging(staging_guid, request)
     end
 
     def stop_staging(staging_guid); end
 
     private
 
+    class BuildpackLifecycle
+      def initialize(action_builder, staging_guid, cc_uploader_url)
+        @action_builder = action_builder
+        @staging_guid = staging_guid
+        @cc_uploader_url = cc_uploader_url
+      end
+
+      def to_hash
+        lifecycle_data = @action_builder.lifecycle_data
+        droplet_upload_uri = "#{@cc_uploader_url}/v1/droplet/#{@staging_guid}?cc-droplet-upload-uri=#{lifecycle_data[:droplet_upload_uri]}"
+        {
+          buildpack_lifecycle: {
+              droplet_upload_uri: droplet_upload_uri,
+              app_bits_download_uri: lifecycle_data[:app_bits_download_uri],
+              buildpacks: lifecycle_data[:buildpacks]
+          }
+        }
+      end
+    end
+
+    class DockerLifecycle
+      def initialize(staging_details)
+        @staging_details = staging_details
+      end
+
+      def to_hash
+        {
+          docker_lifecycle: {
+            image: @staging_details.package.image,
+            registry_username: @staging_details.package.docker_username,
+            registry_password: @staging_details.package.docker_password
+          }
+        }
+      end
+    end
+
     def start_staging(staging_guid, staging_request)
       payload = MultiJson.dump(staging_request)
       response = client.post("/stage/#{staging_guid}", body: payload)
@@ -34,32 +68,11 @@ def start_staging(staging_guid, staging_request)
       end
     end
 
-    def complete_staging(staging_guid, staging_details)
-      build = VCAP::CloudController::BuildModel.find(guid: staging_guid)
-      raise CloudController::Errors::ApiError.new_from_details('ResourceNotFound', 'Build not found') if build.nil?
-
-      completion_handler = VCAP::CloudController::Diego::Docker::StagingCompletionHandler.new(build)
-      payload = {
-        result: {
-          lifecycle_type: 'docker',
-          lifecycle_metadata: {
-            docker_image: staging_details.package.image
-          },
-          process_types: { web: '' },
-          execution_metadata: '{\"cmd\":[],\"ports\":[{\"Port\":8080,\"Protocol\":\"tcp\"}]}'
-        }
-      }
-      completion_handler.staging_complete(payload, staging_details.start_after_staging)
-    end
-
     def to_request(staging_guid, staging_details)
       lifecycle_type = staging_details.lifecycle.type
       action_builder = VCAP::CloudController::Diego::LifecycleProtocol.protocol_for_type(lifecycle_type).staging_action_builder(config, staging_details)
-      lifecycle_data = action_builder.lifecycle_data
-
-      cc_uploader_url = config.get(:opi, :cc_uploader_url)
-      droplet_upload_uri = "#{cc_uploader_url}/v1/droplet/#{staging_guid}?cc-droplet-upload-uri=#{lifecycle_data[:droplet_upload_uri]}"
 
+      lifecycle = get_lifecycle(staging_details, staging_guid, action_builder)
       {
           app_guid: staging_details.package.app_guid,
           app_name: staging_details.package.app.name,
@@ -68,19 +81,24 @@ def to_request(staging_guid, staging_details)
           org_guid: staging_details.package.app.organization.guid,
           space_name: staging_details.package.app.space.name,
           space_guid: staging_details.package.app.space.guid,
-          environment: build_env(staging_details.environment_variables) + action_builder.task_environment_variables,
+          environment: build_env(staging_details.environment_variables) + action_builder.task_environment_variables.to_a,
           completion_callback: staging_completion_callback(staging_details),
-          lifecycle_data: {
-              droplet_upload_uri: droplet_upload_uri,
-              app_bits_download_uri: lifecycle_data[:app_bits_download_uri],
-              buildpacks: lifecycle_data[:buildpacks]
-          },
+          lifecycle: lifecycle,
           cpu_weight: VCAP::CloudController::Diego::STAGING_TASK_CPU_WEIGHT,
           disk_mb: staging_details.staging_disk_in_mb,
           memory_mb: staging_details.staging_memory_in_mb
       }
     end
 
+    def get_lifecycle(staging_details, staging_guid, action_builder)
+      if staging_details.lifecycle.type == VCAP::CloudController::Lifecycles::DOCKER
+        DockerLifecycle.new(staging_details)
+      else
+        cc_uploader_url = config.get(:opi, :cc_uploader_url)
+        BuildpackLifecycle.new(action_builder, staging_guid, cc_uploader_url)
+      end
+    end
+
     def staging_completion_callback(staging_details)
       port   = config.get(:tls_port)
       scheme = 'https'

spec/unit/lib/cloud_controller/opi/stager_client_spec.rb

@@ -46,14 +46,17 @@
 
   context 'when staging an app' do
     before do
-      allow(VCAP::CloudController::Diego::Buildpack::LifecycleProtocol).to receive(:new).and_return(lifecycle_protocol)
-
       stub_request(:post, "#{eirini_url}/stage/#{staging_guid}").
         to_return(status: 202)
     end
 
     context 'when lifecycle type is buildpack' do
       let(:lifecycle_type) { VCAP::CloudController::Lifecycles::BUILDPACK }
+
+      before do
+        allow(VCAP::CloudController::Diego::Buildpack::LifecycleProtocol).to receive(:new).and_return(lifecycle_protocol)
+      end
+
       it 'should send the expected request' do
         stager_client.stage(staging_guid, staging_details)
         expect(WebMock).to have_requested(:post, "#{eirini_url}/stage/#{staging_guid}").with(body: {
@@ -68,9 +71,12 @@
                         { name: 'MEMORY_LIMIT', value: '256m' },
                         { name: 'VCAP_SERVICES', value: '{}' }],
           completion_callback: 'https://internal_user:internal_password@api.internal.cf:8182/internal/v3/staging//build_completed?start=',
-          lifecycle_data: { droplet_upload_uri: "http://cc-uploader.service.cf.internal:9091/v1/droplet/#{staging_guid}?cc-droplet-upload-uri=http://upload.me",
-                            app_bits_download_uri: 'http://download.me',
-                            buildpacks: [{ name: 'ruby', key: 'idk', url: 'www.com', skip_detect: false }]
+          lifecycle: {
+            buildpack_lifecycle: {
+              droplet_upload_uri: "http://cc-uploader.service.cf.internal:9091/v1/droplet/#{staging_guid}?cc-droplet-upload-uri=http://upload.me",
+                              app_bits_download_uri: 'http://download.me',
+                              buildpacks: [{ name: 'ruby', key: 'idk', url: 'www.com', skip_detect: false }]
+            }
           },
           cpu_weight: VCAP::CloudController::Diego::STAGING_TASK_CPU_WEIGHT,
           disk_mb: 100,
@@ -99,9 +105,12 @@
                           { name: 'MEMORY_LIMIT', value: '256m' },
                           { name: 'VCAP_SERVICES', value: '{}' }],
             completion_callback: 'https://internal_user:internal_password@api.internal.cf:8182/internal/v3/staging//build_completed?start=',
-            lifecycle_data: { droplet_upload_uri: "http://cc-uploader.service.cf.internal:9091/v1/droplet/#{staging_guid}?cc-droplet-upload-uri=http://upload.me",
-                              app_bits_download_uri: 'http://download.me',
-                              buildpacks: [{ name: 'ruby', key: 'idk', url: 'www.com', skip_detect: false }]
+            lifecycle: {
+              buildpack_lifecycle: {
+                droplet_upload_uri: "http://cc-uploader.service.cf.internal:9091/v1/droplet/#{staging_guid}?cc-droplet-upload-uri=http://upload.me",
+                                app_bits_download_uri: 'http://download.me',
+                                buildpacks: [{ name: 'ruby', key: 'idk', url: 'www.com', skip_detect: false }]
+              }
             },
             cpu_weight: VCAP::CloudController::Diego::STAGING_TASK_CPU_WEIGHT,
             disk_mb: 100,
@@ -125,57 +134,60 @@
 
     context 'when lifecycle type is docker' do
       let(:lifecycle_type) { VCAP::CloudController::Lifecycles::DOCKER }
-      let(:staging_completion_handler) { instance_double(VCAP::CloudController::Diego::Docker::StagingCompletionHandler) }
-      let(:build_model) { instance_double(VCAP::CloudController::BuildModel) }
-      let(:payload) {
-        {
-        result: {
-          lifecycle_type: 'docker',
-          lifecycle_metadata: {
-            docker_image: 'docker.io/some/image'
-          },
-          process_types: { web: '' },
-          execution_metadata: '{\"cmd\":[],\"ports\":[{\"Port\":8080,\"Protocol\":\"tcp\"}]}'
-        }
-      }
-      }
-
-      it 'should not make any http calls to eirini' do
-        allow(VCAP::CloudController::BuildModel).to receive(:find).and_return(build_model)
-        allow(VCAP::CloudController::Diego::Docker::StagingCompletionHandler).to receive(:new).and_return(staging_completion_handler)
-        allow(staging_completion_handler).to receive(:staging_complete)
 
-        stager_client.stage(staging_guid, staging_details)
-        expect(WebMock).not_to have_requested(:any, "#{eirini_url}/stage/#{staging_guid}")
+      let(:staging_action_builder) do
+        instance_double(VCAP::CloudController::Diego::Docker::StagingActionBuilder,
+          task_environment_variables: lifecycle_environment_variables,
+        )
       end
 
-      it 'should mark staging as completed' do
-        expect(VCAP::CloudController::BuildModel).to receive(:find).with(guid: staging_guid).and_return(build_model)
-        expect(VCAP::CloudController::Diego::Docker::StagingCompletionHandler).to receive(:new).with(build_model).and_return(staging_completion_handler)
-        expect(staging_completion_handler).to receive(:staging_complete).with(payload, true)
+      let(:lifecycle_protocol) do
+        instance_double(VCAP::CloudController::Diego::Docker::LifecycleProtocol,
+          staging_action_builder: staging_action_builder
+        )
+      end
 
-        staging_details.start_after_staging = true
-        stager_client.stage(staging_guid, staging_details)
+      before do
+        allow(VCAP::CloudController::Diego::Docker::LifecycleProtocol).to receive(:new).and_return(lifecycle_protocol)
       end
 
-      context 'when build is not found' do
-        it 'should raise an error' do
-          expect(VCAP::CloudController::BuildModel).to receive(:find).with(guid: staging_guid).and_return(nil)
-          expect {
-            stager_client.stage(staging_guid, staging_details)
-          }.to raise_error(CloudController::Errors::ApiError, 'Build not found')
-        end
+      it 'should set a docker lifecycle' do
+        stager_client.stage(staging_guid, staging_details)
+        expect(WebMock).to have_requested(:post, "#{eirini_url}/stage/#{staging_guid}").with(body: {
+          app_guid: 'thor',
+          app_name: 'the_thor',
+          staging_guid: staging_guid,
+          org_name: 'some-org',
+          org_guid: 'some-org-guid',
+          space_name: 'outer',
+          space_guid: 'outer-guid',
+          environment: [{ name: 'VCAP_APPLICATION', value: '{"wow":"pants"}' },
+                        { name: 'MEMORY_LIMIT', value: '256m' },
+                        { name: 'VCAP_SERVICES', value: '{}' }],
+          completion_callback: 'https://internal_user:internal_password@api.internal.cf:8182/internal/v3/staging//build_completed?start=',
+          lifecycle: {
+            docker_lifecycle: {
+              image: 'docker.io/some/image',
+              registry_username: 'theone',
+              registry_password: 'notone'
+            }
+          },
+          cpu_weight: VCAP::CloudController::Diego::STAGING_TASK_CPU_WEIGHT,
+          disk_mb: 100,
+          memory_mb: 200
+        }.to_json
+        )
       end
     end
+  end
 
-    context 'when lifecycle type is invalid' do
-      let(:lifecycle_type) { 'dockerpack' }
+  context 'when lifecycle type is invalid' do
+    let(:lifecycle_type) { 'dockerpack' }
 
-      it 'should raise an error' do
-        expect {
-          stager_client.stage(staging_guid, staging_details)
-        }.to raise_error(RuntimeError, 'lifecycle type `dockerpack` is invalid')
-      end
+    it 'should raise an error' do
+      expect {
+        stager_client.stage(staging_guid, staging_details)
+      }.to raise_error(RuntimeError, 'lifecycle type `dockerpack` is invalid')
     end
   end
 
@@ -193,6 +205,8 @@ def stub_staging_details(lifecycle_type)
     package_model = VCAP::CloudController::PackageModel.make(
       type: 'docker',
       docker_image: 'docker.io/some/image',
+      docker_username: 'theone',
+      docker_password: 'notone',
       app: app_model)
 
     staging_details                                 = VCAP::CloudController::Diego::StagingDetails.new