Merge pull request #386 from SUSE/faster_pubcloud_registration

Faster pubcloud registration

lib/suse/connect/client.rb

@@ -34,11 +34,11 @@ def register!
         log.info 'Successfully registered system.'
       end
 
-      # Activate the product, add the service and install the relase package
+      # Activate the product, add the service and install the release package
       def register_product(product, install_release_package = true)
         service = activate_product(product, @config.email)
 
-        System.add_service(service)
+        System.add_service(service, !@config.no_zypper_refs)
         Zypper.install_release_package(product.identifier) if install_release_package
 
         print_success_message(product)

lib/suse/connect/system.rb

@@ -37,9 +37,9 @@ def cleanup!
           Zypper.remove_all_suse_services
         end
 
-        def add_service(service)
+        def add_service(service, refresh_zypper_service = true)
           raise ArgumentError, 'only Remote::Service accepted' unless service.is_a? Remote::Service
-          Zypper.add_service(service.url, service.name)
+          Zypper.add_service(service.url, service.name, refresh_zypper_service)
           service
         end
 

lib/suse/connect/zypper.rb

@@ -90,14 +90,14 @@ def repositories
         #
         # @see https://doc.opensuse.org/projects/libzypp/HEAD/zypp-services.html#services-remote ZYpp Services Documentation
         # @todo TODO: introduce Product class
-        def add_service(service_url, service_name)
+        def add_service(service_url, service_name, refresh_zypper_service = true)
           # INFO: Remove old service which could be modified by a customer
           remove_service(service_name)
           call("--non-interactive addservice -t ris #{Shellwords.escape(service_url)} '#{Shellwords.escape(service_name)}'")
           enable_service_autorefresh(service_name)
           write_service_credentials(service_name)
 
-          refresh_service(service_name)
+          refresh_service(service_name) if refresh_zypper_service
         end
 
         # @param service_name [String] Alias-mnemonic with which zypper should remove this service
@@ -154,7 +154,9 @@ def services
         end
 
         def install_release_package(identifier)
-          call("--no-refresh --non-interactive install --no-recommends --auto-agree-with-product-licenses -t product #{identifier}") if identifier
+          return unless identifier
+          _, _, status = execute_raw("rpm -q #{identifier}-release")
+          call("--no-refresh --non-interactive install --no-recommends --auto-agree-with-product-licenses -t product #{identifier}") unless (status == 0)
         end
 
         def remove_release_package(identifier)

lib/suse/toolkit/system_calls.rb

@@ -6,11 +6,17 @@ module Toolkit
     module SystemCalls
       include Connect::Logger
 
-      def execute(cmd, quiet = true, valid_exit_codes = [0]) # rubocop:disable CyclomaticComplexity
-        log.debug("Executing: '#{cmd}' Quiet: #{quiet}")
+      def execute_raw(cmd)
+        log.debug("Executing raw: '#{cmd}'")
         output, error, status = Open3.capture3({ 'LC_ALL' => 'C' }, cmd) { |_stdin, stdout, _stderr, _wait_thr| stdout.read }
         log.debug("Output: '#{output.strip}'") unless output.empty?
         log.debug("Error: '#{error.strip}'") unless error.empty?
+        [output, error, status]
+      end
+
+      def execute(cmd, quiet = true, valid_exit_codes = [0]) # rubocop:disable CyclomaticComplexity
+        log.debug("Executing: '#{cmd}' Quiet: #{quiet}")
+        output, error, status = execute_raw(cmd)
 
         # Catching interactive failures of zypper. --non-interactive always returns with exit code 0 here
         if !valid_exit_codes.include?(status.exitstatus) || error.include?('ABORT request')

package/SUSEConnect.changes

@@ -4,7 +4,8 @@ Wed Sep  5 12:21:39 UTC 2018 - tmuntaner@suse.com
 - Update to 0.3.12
   - Detect if system is in cloud provider (AWS/Google/Azure)
     (fate#320935)
-- Don't fail when trying to parse an empty body. Fixes bsc#1098220
+  - Don't fail when trying to parse an empty body. Fixes bsc#1098220
+  - Don't install release packages if they are already present
 
 -------------------------------------------------------------------
 Tue Jun 19 15:58:46 UTC 2018 - tschmidt@suse.com

package/SUSEConnect.example

@@ -9,3 +9,6 @@
 
 ## Do not verify SSL certificates when using https (default: false)
 # insecure: false
+
+## Do not refresh zypper service when registering (default: false)
+# no_zypper_refs: false

spec/connect/client_spec.rb

@@ -373,22 +373,60 @@
       SUSE::Connect::GlobalLogger.instance.log = default_logger
     end
 
-    it 'should activate the product, add service file and install release package' do
-      expect(subject).to receive(:activate_product).with(product, fake_email).and_return service_stub
-      expect(System).to receive(:add_service).with(service_stub)
+    context 'when no_zypper_refs is false' do
+      context 'when install_release_package is true' do
+        it 'activates the product, add the service, refreshes the service and installs release package' do
+          expect(subject).to receive(:activate_product).with(product, fake_email).and_return service_stub
+          expect(System).to receive(:add_service).with(service_stub, true)
 
-      expect(Zypper).to receive(:install_release_package).with(product.identifier)
+          expect(Zypper).to receive(:install_release_package).with(product.identifier)
 
-      subject.register_product(product)
+          subject.register_product(product)
+        end
+      end
+
+      context 'when install_release_package is false' do
+        it "refreshes the service, doesn't install the release package" do
+          expect(subject).to receive(:activate_product).with(product, fake_email).and_return service_stub
+          expect(System).to receive(:add_service).with(service_stub, true)
+
+          expect(Zypper).not_to receive(:install_release_package)
+
+          subject.register_product(product, false)
+        end
+      end
     end
 
-    it 'should not install the release package if install_release_package is false' do
-      expect(subject).to receive(:activate_product).with(product, fake_email).and_return service_stub
-      expect(System).to receive(:add_service).with(service_stub)
+    context 'when no_zypper_refs is true' do
+      let(:config) do
+        SUSE::Connect::Config.new.merge!({ 'no_zypper_refs' => true })
+      end
+
+      before do
+        allow(SUSE::Connect::Config).to receive(:new).and_return(config)
+      end
+
+      context 'when install_release_package is true' do
+        it "activates the product, adds service file, doesn't refresh the service and installs release package" do
+          expect(subject).to receive(:activate_product).with(product, fake_email).and_return service_stub
+          expect(System).to receive(:add_service).with(service_stub, false)
+
+          expect(Zypper).to receive(:install_release_package).with(product.identifier)
+
+          subject.register_product(product)
+        end
+      end
+
+      context 'when install_release_package is false' do
+        it "doesn't refresh the service, doesn't not install the release package" do
+          expect(subject).to receive(:activate_product).with(product, fake_email).and_return service_stub
+          expect(System).to receive(:add_service).with(service_stub, false)
 
-      expect(Zypper).not_to receive(:install_release_package)
+          expect(Zypper).not_to receive(:install_release_package)
 
-      subject.register_product(product, false)
+          subject.register_product(product, false)
+        end
+      end
     end
 
     it 'informs the user about progress' do

spec/connect/system_spec.rb

@@ -72,9 +72,18 @@
   end
 
   describe '.add_service' do
-    it 'adds zypper service to the system' do
-      expect(Zypper).to receive(:add_service).with('furl', 'JiYoKo')
-      subject.add_service service
+    context 'when refresh_zypper_service is true' do
+      it 'adds zypper service to the system and refreshes it' do
+        expect(Zypper).to receive(:add_service).with('furl', 'JiYoKo', true)
+        subject.add_service service
+      end
+    end
+
+    context 'when refresh_zypper_service is false' do
+      it "adds zypper service to the system and doesn't refresh it" do
+        expect(Zypper).to receive(:add_service).with('furl', 'JiYoKo', false)
+        subject.add_service(service, false)
+      end
     end
 
     context 'with wrong argument' do

spec/connect/zypper_spec.rb

@@ -121,21 +121,35 @@
     let(:service_url)  { 'http://example.com' }
     let(:args) { "zypper --non-interactive addservice -t ris #{service_url} '#{service_name}'" }
 
-    before :each do
+    before do
       allow(Zypper).to receive(:remove_service).with(service_name)
       allow(Zypper).to receive(:enable_service_autorefresh).with(service_name)
       allow(Zypper).to receive(:write_service_credentials).with(service_name)
       allow(Zypper).to receive(:refresh_service)
     end
 
-    it 'adds service' do
-      expect(Zypper).to receive(:remove_service).with(service_name)
-      expect(Open3).to receive(:capture3).with(shared_env_hash, args).and_return(['', '', status])
-      expect(Zypper).to receive(:enable_service_autorefresh).with(service_name)
-      expect(Zypper).to receive(:write_service_credentials).with(service_name)
-      expect(Zypper).to receive(:refresh_service).with(service_name)
+    context 'when refresh_zypper_service is false' do
+      it "adds a service, but doesn't refresh it" do
+        expect(Zypper).to receive(:remove_service).with(service_name)
+        expect(Open3).to receive(:capture3).with(shared_env_hash, args).and_return(['', '', status])
+        expect(Zypper).to receive(:enable_service_autorefresh).with(service_name)
+        expect(Zypper).to receive(:write_service_credentials).with(service_name)
+        expect(Zypper).not_to receive(:refresh_service)
 
-      subject.add_service(service_url, service_name)
+        subject.add_service(service_url, service_name, false)
+      end
+    end
+
+    context 'when refresh_zypper_service is true' do
+      it 'adds service and refreshes it' do
+        expect(Zypper).to receive(:remove_service).with(service_name)
+        expect(Open3).to receive(:capture3).with(shared_env_hash, args).and_return(['', '', status])
+        expect(Zypper).to receive(:enable_service_autorefresh).with(service_name)
+        expect(Zypper).to receive(:write_service_credentials).with(service_name)
+        expect(Zypper).to receive(:refresh_service).with(service_name)
+
+        subject.add_service(service_url, service_name)
+      end
     end
 
     it 'sets autorefresh flag' do
@@ -391,10 +405,25 @@
   end
 
   describe '.install_release_package' do
-    it 'calls the command' do
-      expect(Open3).to receive(:capture3).with(shared_env_hash, 'zypper --no-refresh --non-interactive install --no-recommends --auto-agree-with-product-licenses -t product opensuse') # rubocop:disable LineLength
-                                         .and_return(['', '', status])
-      subject.install_release_package('opensuse')
+    context 'when the release package is not yet installed' do
+      it 'calls zypper install' do
+        expect(Open3).to receive(:capture3).with(shared_env_hash, 'rpm -q opensuse-release')
+                           .and_return(['', '', 1])
+
+        expect(Open3).to receive(:capture3).with(shared_env_hash, 'zypper --no-refresh --non-interactive install --no-recommends --auto-agree-with-product-licenses -t product opensuse') # rubocop:disable LineLength
+                                           .and_return(['', '', status])
+        subject.install_release_package('opensuse')
+      end
+    end
+
+    context 'when the release package is already installed' do
+      it "doesn't call zypper install" do
+        expect(Open3).to receive(:capture3).with(shared_env_hash, 'rpm -q opensuse-release')
+                           .and_return(['', '', 0])
+
+        expect(Open3).not_to receive(:capture3)
+        subject.install_release_package('opensuse')
+      end
     end
   end
 

spec/toolkit/system_calls_spec.rb

@@ -23,6 +23,7 @@
 
     it 'produces debug log output' do
       expect(SUSE::Connect::GlobalLogger.instance.log).to receive(:debug).with(/Executing:/)
+      expect(SUSE::Connect::GlobalLogger.instance.log).to receive(:debug).with(/Executing raw:/)
       expect(SUSE::Connect::GlobalLogger.instance.log).to receive(:debug).with(/Output:/)
       expect(subject).to receive(:capture3).with(shared_env_hash, 'date').and_return([date, '', success])
       expect(execute('date', false)).to eql date