Merge pull request #1346 from yast/huha-pkg-sp4

Ensure adding Storage Support Software Packages for MicroOS [SLE-15-SP4]

package/yast2-storage-ng.changes

@@ -1,3 +1,12 @@
+-------------------------------------------------------------------
+Wed Jul  5 13:42:12 UTC 2023 - Stefan Hundhammer <shundhammer@suse.com>
+
+- Ensure adding storage support software packages for MicroOS
+  which uses its custom partitions_proposal client, not the 
+  standard inst_disk_proposal client (bsc#1212452)
+  https://github.com/yast/yast-storage-ng/pull/1346
+- 4.4.45
+
 -------------------------------------------------------------------
 Thu Jun 29 11:42:21 UTC 2023 - Ancor Gonzalez Sosa <ancor@suse.com>
 

package/yast2-storage-ng.spec

@@ -16,7 +16,7 @@
 #
 
 Name:           yast2-storage-ng
-Version:        4.4.44
+Version:        4.4.45
 Release:        0
 Summary:        YaST2 - Storage Configuration
 License:        GPL-2.0-only OR GPL-3.0-only

src/lib/y2storage/clients/inst_disk_proposal.rb

@@ -162,23 +162,7 @@ def actions_after_partitioner(devicegraph, dialog_result)
       # Add storage-related software packages (filesystem tools etc.) to the
       # set of packages to be installed.
       def add_storage_packages
-        features = storage_manager.staging.used_features
-        required_features = storage_manager.staging.used_features(required_only: true)
-
-        required_packages = required_features.pkg_list
-        optional_packages = features.pkg_list - required_packages
-
-        set_proposal_packages(required_packages, false)
-        set_proposal_packages(optional_packages, true)
-      end
-
-      # @see #add_storage_packages
-      #
-      # @param pkgs [Array<String>] list of packages
-      # @param optional [Boolean] whether the packages in the list are optional
-      def set_proposal_packages(pkgs, optional)
-        pkg_handler = Y2Storage::PackageHandler.new(pkgs, optional: optional)
-        pkg_handler.set_proposal_packages
+        Y2Storage::PackageHandler.set_proposal_packages_for(storage_manager.staging)
       end
 
       # Save the list of filesystem to /etc/sysconfig/storage.

src/lib/y2storage/clients/partitions_proposal.rb

@@ -120,8 +120,9 @@ def update_state
           staging = storage_manager.staging
           actiongraph = staging ? staging.actiongraph : nil
           self.actions_presenter = ActionsPresenter.new(actiongraph)
-          Y2Storage::DumpManager.dump(staging)
-          Y2Storage::DumpManager.dump(actions_presenter)
+          DumpManager.dump(staging)
+          DumpManager.dump(actions_presenter)
+          PackageHandler.set_proposal_packages_for(staging)
         end
       end
 

src/lib/y2storage/devicegraph.rb

@@ -615,6 +615,23 @@ def used_features(required_only: false)
       StorageFeaturesList.from_bitfield(storage_used_features(type))
     end
 
+    # List of required (mandatory) storage features used by the devicegraph
+    #
+    # @return [StorageFeaturesList]
+    def required_used_features
+      used_features(required_only: true)
+    end
+
+    # List of optional storage features used by the devicegraph
+    #
+    # @return [StorageFeaturesList]
+    def optional_used_features
+      all = storage_used_features(Storage::UsedFeaturesDependencyType_SUGGESTED)
+      required = storage_used_features(Storage::UsedFeaturesDependencyType_REQUIRED)
+      # Using binary XOR in those bit fields to calculate the difference
+      StorageFeaturesList.from_bitfield(all ^ required)
+    end
+
     private
 
     # Copy of a device tree where hashes have been substituted by sorted

src/lib/y2storage/package_handler.rb

@@ -116,6 +116,21 @@ def set_proposal_packages
       success
     end
 
+    # Add the proposal packages for storage that are needed for the specified
+    # devicegraph's used features. This marks the packages for installation;
+    # it does not install them yet.
+    #
+    # @param devicegraph [Devicegraph] usually StorageManager.instance.staging
+    # @param optional
+    def self.set_proposal_packages_for(devicegraph, optional: true)
+      required_packages = devicegraph.required_used_features.pkg_list
+      PackageHandler.new(required_packages, optional: false).set_proposal_packages
+      return unless optional
+
+      optional_packages = devicegraph.optional_used_features.pkg_list
+      PackageHandler.new(optional_packages, optional: true).set_proposal_packages
+    end
+
     private
 
     # Whether the packages should be considered as optional when adding them to the

src/lib/y2storage/storage_feature.rb

@@ -124,6 +124,14 @@ def self.all
       end
     end
 
+    # Drop the cache of storage packages that are available.
+    #
+    # This is only ever needed if the available packages might have changed
+    # since the last use of this class.
+    def self.drop_cache
+      @all = nil
+    end
+
     # Constructor
     #
     # This looks up a constant in the ::Storage namespace to make sure the id

test/y2storage/devicegraph_test.rb

@@ -1331,4 +1331,54 @@ def with_sda2_deleted(initial_graph)
       end
     end
   end
+
+  describe "#required_used_features" do
+    before { fake_scenario(scenario) }
+
+    context "with local devices combining several filesystem types" do
+      let(:scenario) { "mixed_disks" }
+
+      it "returns only the features for mounted filesystems" do
+        features = fake_devicegraph.required_used_features
+        expect(features).to be_a Y2Storage::StorageFeaturesList
+        expect(features.map(&:id))
+          .to contain_exactly(:UF_BTRFS, :UF_XFS, :UF_SWAP)
+      end
+    end
+
+    context "with an empty disk" do
+      let(:scenario) { "empty_disks" }
+
+      it "Survives not having any storage features" do
+        features = fake_devicegraph.required_used_features
+        expect(features).to be_a Y2Storage::StorageFeaturesList
+        expect(features.map(&:id)).to be == []
+      end
+    end
+  end
+
+  describe "#optional_used_features" do
+    before { fake_scenario(scenario) }
+
+    context "with local devices combining several filesystem types" do
+      let(:scenario) { "mixed_disks" }
+
+      it "returns only the features for filesystems that are not mounted" do
+        features = fake_devicegraph.optional_used_features
+        expect(features).to be_a Y2Storage::StorageFeaturesList
+        expect(features.map(&:id))
+          .to contain_exactly(:UF_EXT4, :UF_NTFS)
+      end
+    end
+
+    context "with an empty disk" do
+      let(:scenario) { "empty_disks" }
+
+      it "Survives not having any storage features" do
+        features = fake_devicegraph.optional_used_features
+        expect(features).to be_a Y2Storage::StorageFeaturesList
+        expect(features.map(&:id)).to be == []
+      end
+    end
+  end
 end

test/y2storage/package_handler_test.rb

@@ -23,6 +23,7 @@
 
 require_relative "spec_helper"
 require "y2storage/package_handler"
+require "y2storage/storage_feature"
 
 describe Y2Storage::PackageHandler do
   let(:feature_pkg) { ["lvm2", "btrfsprogs", "e2fsprogs"] }
@@ -135,4 +136,47 @@
       end
     end
   end
+
+  describe ".set_proposal_packages_for" do
+    before do
+      fake_scenario(scenario)
+      Y2Storage::StorageFeature.drop_cache
+      allow(Yast::Mode).to receive(:installation).and_return(installation)
+      allow(Yast::Package).to receive(:Installed).and_return(false)
+      allow(Yast::Package).to receive(:Available).and_return(true)
+    end
+
+    context "with local devices combining several filesystem types" do
+      PROPOSAL_ID = "storage_proposal"
+      let(:scenario) { "mixed_disks" }
+      let(:installation) { true }
+
+      it "Adds the correct required and optional storage packages to the proposal" do
+        expect(Yast::PackagesProposal).to receive(:SetResolvables)
+          .with(PROPOSAL_ID, :package, ["btrfsprogs", "e2fsprogs", "xfsprogs"], optional: false)
+          .and_return true
+        expect(Yast::PackagesProposal).to receive(:SetResolvables)
+          .with(PROPOSAL_ID, :package, ["e2fsprogs", "ntfs-3g", "ntfsprogs"], optional: true)
+          .and_return true
+        described_class.set_proposal_packages_for(fake_devicegraph)
+      end
+
+      it "Adds only required storage packages to the proposal if 'optional' is 'false" do
+        expect(Yast::PackagesProposal).to receive(:SetResolvables)
+          .with(PROPOSAL_ID, :package, ["btrfsprogs", "e2fsprogs", "xfsprogs"], optional: false)
+          .and_return true
+        described_class.set_proposal_packages_for(fake_devicegraph, optional: false)
+      end
+    end
+
+    context "with empty disks" do
+      let(:scenario) { "empty_disks" }
+      let(:installation) { true }
+
+      it "Does not add any storage packages to the proposal" do
+        expect(Yast::PackagesProposal).not_to receive(:SetResolvables)
+        described_class.set_proposal_packages_for(fake_devicegraph, optional: true)
+      end
+    end
+  end
 end

test/y2storage/storage_features_list_test.rb

@@ -22,6 +22,7 @@
 # find current contact information at www.suse.com.
 
 require_relative "spec_helper"
+require "y2storage/storage_feature"
 require "y2storage/storage_features_list"
 
 describe Y2Storage::StorageFeaturesList do
@@ -84,6 +85,7 @@
       let(:bits) { Storage::UF_NTFS | Storage::UF_EXT3 }
 
       before do
+        Y2Storage::StorageFeature.drop_cache
         allow(Yast::Package).to receive(:Available).and_return false
         allow(Yast::Package).to receive(:Available).with("ntfsprogs").and_return true
       end