Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Save persistent volume claim to project relation #108

Merged
merged 1 commit into from
Sep 15, 2017
Merged

Save persistent volume claim to project relation #108

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions app/models/manageiq/providers/kubernetes/container_manager/refresh_parser.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,7 @@ def get_persistent_volume_claims(inventory)
process_collection(inventory["persistent_volume_claim"], key) { |n| parse_persistent_volume_claim(n) }
@data[key].each do |pvc|
@data_index.store_path(key, :by_namespace_and_name, pvc[:namespace], pvc[:name], pvc)
pvc[:project] = @data_index.fetch_path(path_for_entity("namespace"), :by_name, pvc[:namespace])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Much of the parser sets :project instead of :container_project for some reason. I want to fix it one day.
Is it easy to do :container_project here? Or do you think it's better to keep the pattern?

Copy link

@moolitayer moolitayer Sep 5, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Much of the parser sets :project instead of :container_project for some reason. I want to fix it one day.

it's been bugging me too :) never got around to fixing it.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

keep the pattern. change it in the next refactor.

end
end

Expand Down Expand Up @@ -371,6 +372,7 @@ def get_persistent_volume_claims_graph(inv)

inv["persistent_volume_claim"].each do |pvc|
h = parse_persistent_volume_claim(pvc)
h[:container_project] = lazy_find_project(:name => h[:namespace])
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cben so this has to be done in both refreshes since it involves data_index right? in cases where we change parsing (e.g add another textual field to an entity) we will have to update only the parse method?

Copy link
Contributor

@cben cben Sep 5, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah. A column on same table can be set once in parse_* method.

Links to other tables are done differently in 2 refreshes.

  • The old refresh does links directly by nested hashes. sometimes it's implicit, nesting comes from parse_* or directly from kubernetes. nesting across API requests is constructed via data_index.
    Then, in some cases depending on saving order, save_inventory needs to break nestings and set ids (this is the case add relation between container projects and persistent volume claims manageiq#15932).
  • Graph refresh does links within API request directly by linking InventoryObject, across API requests by lazy_find. (images & registries still unrefactored via @data_index.)
    Graph saving figures order automatically, no manual breaking needed.


collection.build(h)
end
Expand Down
2 changes: 2 additions & 0 deletions spec/models/manageiq/providers/kubernetes/container_manager/refresher_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -473,6 +473,8 @@ def assert_specific_persistent_volume_claim
:phase => "Bound",
:capacity => {:storage => 10.gigabytes},
)
expect(@bound_pvc.container_project.name).to eq("openshift-infra")

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)
Expand Down