Merge pull request #74 from cben/graph-volumes

Volume-related graph refresh fixes & VCR specs

app/models/manageiq/providers/kubernetes/container_manager/inventory_collections.rb

@@ -313,11 +313,13 @@ def initialize_inventory_collections
     @collections[:persistent_volume_claims] =
       ::ManagerRefresh::InventoryCollection.new(
         shared_options.merge(
-          :model_class    => PersistentVolumeClaim,
-          :parent         => manager,
-          :builder_params => {:ems_id => manager.id},
-          :association    => :persistent_volume_claims,
-          :use_ar_object  => true # serialized :capacity attr
+          :model_class          => PersistentVolumeClaim,
+          :parent               => manager,
+          :builder_params       => {:ems_id => manager.id},
+          :association          => :persistent_volume_claims,
+          :use_ar_object        => true, # serialized :capacity attr
+          :secondary_refs       => {:by_namespace_and_name => [:namespace, :name]},
+          :attributes_blacklist => [:namespace],
         )
       )
   end

app/models/manageiq/providers/kubernetes/container_manager/refresh_parser.rb

@@ -129,6 +129,13 @@ def get_pods(inventory)
           path_for_entity("replication_controller"), :by_namespace_and_name,
           replicator_ref[:namespace], replicator_ref[:name]
         )
+        cg[:container_volumes].each do |cv|
+          pvc_ref = cv.delete(:persistent_volume_claim_ref)
+          cv[:persistent_volume_claim] = pvc_ref && @data_index.fetch_path(
+            path_for_entity("persistent_volume_claim"),
+            :by_namespace_and_name, pvc_ref[:namespace], pvc_ref[:name]
+          )
+        end
         # Note: save_container_groups_inventory also links build_pod by :build_pod_name.
 
         @data_index.store_path(key, :by_namespace_and_name,
@@ -365,8 +372,6 @@ def get_persistent_volume_claims_graph(inv)
       inv["persistent_volume_claim"].each do |pvc|
         h = parse_persistent_volume_claim(pvc)
 
-        h.except!(:namespace)
-
         collection.build(h)
       end
     end
@@ -379,11 +384,8 @@ def get_persistent_volumes_graph(inv)
 
         h.except!(:namespace) # TODO: project untested?
 
-        _pvc_ref = h.delete(:persistent_volume_claim_ref)
-        #h[:persistent_volume_claim] = pvc_ref && @data_index.fetch_path(
-        #                                path_for_entity("persistent_volume_claim"),
-        #                                :by_namespace_and_name, pvc_ref[:namespace], pvc_ref[:name]
-        #                              )
+        pvc_ref = h.delete(:persistent_volume_claim_ref)
+        h[:persistent_volume_claim] = lazy_find_persistent_volume_claim(pvc_ref)
         collection.build(h)
       end
     end
@@ -409,6 +411,7 @@ def get_pods_graph(inv)
 
         get_containers_graph(container_group, children[:containers])
         get_container_conditions_graph(container_group, children[:container_conditions])
+        get_container_volumes_graph(container_group, children[:container_volumes])
         get_custom_attributes_graph(container_group,
                                     :labels         => custom_attrs[:labels],
                                     # The actual section is "node_selectors"
@@ -419,9 +422,23 @@ def get_pods_graph(inv)
     # polymorphic, relation disambiguates parent
     def get_container_conditions_graph(parent, hashes)
       model_name = parent.inventory_collection.model_class.base_class.name
+      key = [:container_conditions_for, model_name]
+      collection = @inv_collections[key]
+      raise("can't save: missing @inv_collections[#{key}]") if collection.nil?
+
       hashes.to_a.each do |h|
         h = h.merge(:container_entity => parent)
-        @inv_collections[[:container_conditions_for, model_name]].build(h)
+        collection.build(h)
+      end
+    end
+
+    def get_container_volumes_graph(parent, hashes)
+      collection = @inv_collections[:container_volumes]
+      hashes.to_a.each do |h|
+        h = h.merge(:parent => parent)
+        pvc_ref = h.delete(:persistent_volume_claim_ref)
+        h[:persistent_volume_claim] = lazy_find_persistent_volume_claim(pvc_ref)
+        collection.build(h)
       end
     end
 
@@ -1229,14 +1246,17 @@ def parse_security_context(security_context)
 
     def parse_volumes(pod)
       pod.spec.volumes.to_a.collect do |volume|
-        {
-          :type                    => 'ContainerVolume',
-          :name                    => volume.name,
-          :persistent_volume_claim => @data_index.fetch_path(path_for_entity("persistent_volume_claim"),
-                                                             :by_namespace_and_name,
-                                                             pod.metadata.namespace,
-                                                             volume.persistentVolumeClaim.try(:claimName))
+        new_result = {
+          :type => 'ContainerVolume',
+          :name => volume.name,
         }.merge!(parse_volume_source(volume))
+        if volume.persistentVolumeClaim.try(:claimName)
+          new_result[:persistent_volume_claim_ref] = {
+            :namespace => pod.metadata.namespace,
+            :name      => volume.persistentVolumeClaim.claimName,
+          }
+        end
+        new_result
       end
     end
 
@@ -1321,5 +1341,10 @@ def lazy_find_build_pod(hash)
       return nil if hash.nil?
       @inv_collections[:container_build_pods].lazy_find_by(hash, :ref => :by_namespace_and_name)
     end
+
+    def lazy_find_persistent_volume_claim(hash)
+      return nil if hash.nil?
+      @inv_collections[:persistent_volume_claims].lazy_find_by(hash, :ref => :by_namespace_and_name)
+    end
   end
 end

spec/models/manageiq/providers/kubernetes/container_manager/refresher_spec.rb

@@ -53,7 +53,7 @@ def full_refresh_test
       assert_specific_container_limit
       assert_specific_container_image_and_registry
       assert_specific_container_component_status
-      assert_specific_persistent_volume
+      # Volumes, PVs, and PVCs are tested in _before_deletions VCR.
     end
   end
 
@@ -379,18 +379,6 @@ def assert_specific_container_component_status
     )
   end
 
-  def assert_specific_persistent_volume
-    @persistent_volume = PersistentVolume.find_by(:name => "pv0001")
-    expect(@persistent_volume).to have_attributes(
-      :type         => "PersistentVolume",
-      :common_path  => "/tmp/data01",
-      :status_phase => "Available",
-      :parent_type  => "ExtManagementSystem"
-    )
-    expect(@ems.persistent_volumes.count).to eq(1)
-    expect(@persistent_volume.parent.class).to eq(ManageIQ::Providers::Kubernetes::ContainerManager)
-  end
-
   def label_with_name_value(name, value)
     an_object_having_attributes(
       :section => 'labels', :source => 'kubernetes',
@@ -422,7 +410,73 @@ def tag_in_category_with_description(category, description)
       expect(ContainerQuotaItem.count).to eq(15)
       expect(ContainerLimit.count).to eq(3)
       expect(ContainerLimitItem.count).to eq(12)
+
+      expect(@ems.container_volumes.count).to eq(22)
+      expect(@ems.persistent_volumes.count).to eq(2)
+      expect(PersistentVolume.count).to eq(2)
+      expect(ContainerVolume.count).to eq(@ems.container_volumes.count +
+                                          @ems.persistent_volumes.count)
       expect(PersistentVolumeClaim.count).to eq(4)
+      assert_specific_container_volume
+      assert_specific_persistent_volume
+      assert_specific_persistent_volume_claim
+    end
+
+    def assert_specific_container_volume
+      # Not in template but typical.  TODO: add CV to template.
+      @container_volume = ContainerVolume.find_by(:name => "cassandra-data")
+      expect(@container_volume).to have_attributes(
+        :type       => "ContainerVolume",
+        :name       => "cassandra-data",
+        :claim_name => "metrics-cassandra-1",
+      )
+      expect(@container_volume.persistent_volume_claim).to eq(
+        PersistentVolumeClaim.find_by(:name => "metrics-cassandra-1")
+      )
+      expect(@container_volume.parent_type).to eq('ContainerGroup')
+      expect(@container_volume.parent.name).to match(/hawkular-cassandra-1-...../)
+    end
+
+    def assert_specific_persistent_volume
+      # Not in template but typical.  TODO: add PV to template.
+      @persistent_volume = PersistentVolume.find_by(:name => "metrics-volume")
+      expect(@persistent_volume).to have_attributes(
+        :type         => "PersistentVolume",
+        :common_path  => "/exports/metrics",
+        :status_phase => "Bound",
+        :capacity     => {:storage => 10.gigabytes},
+      )
+      expect(@persistent_volume.parent).to eq(@ems)
+      expect(@persistent_volume.persistent_volume_claim.name).to eq("metrics-cassandra-1")
+
+      # through shortcuts: PV -> PVC -> CVs -> ContainerGroups
+      expect(@persistent_volume.container_volumes).to eq(
+        [ContainerVolume.find_by(:name => "cassandra-data")]
+      )
+      expect(@persistent_volume.container_groups.size).to eq(1)
+      expect(@persistent_volume.container_groups[0].name).to match(/hawkular-cassandra-1-...../)
+    end
+
+    def assert_specific_persistent_volume_claim
+      # Pending PVC (in template):
+      @pending_pvc = PersistentVolumeClaim.find_by(:name => "my-persistentvolumeclaim-0")
+      expect(@pending_pvc).to have_attributes(
+        :phase    => "Pending",
+        :capacity => {}, # requested 8Gi but not bound to PV => no capacity
+      )
+      expect(@pending_pvc.container_volumes.count).to eq(0)
+      expect(@pending_pvc.persistent_volume).to eq(nil)
+
+      # Bound PVC (TODO: not in template but typical):
+      @bound_pvc = PersistentVolumeClaim.find_by(:name => "metrics-cassandra-1")
+      expect(@bound_pvc).to have_attributes(
+        :phase    => "Bound",
+        :capacity => {:storage => 10.gigabytes},
+      )
+      pv = PersistentVolume.find_by(:name => "metrics-volume")
+      cv = ContainerVolume.find_by(:name => "cassandra-data", :type => "ContainerVolume")
+      expect(@bound_pvc.container_volumes).to contain_exactly(pv, cv)
+      expect(@bound_pvc.persistent_volume).to eq(pv)
     end
 
     context "when refreshing non empty DB" do