api for defining inventory collections #14022

durandom · 2017-02-22T16:24:33Z

This is supposed to replace InventoryCollectionDefaults with something more DSL like

DSL

A Persister can include ManagerRefresh::Inventory::AutomationManager to have access to all inventory collections an AutomationManager usually persists. Then we call the DSL has_inventory to define a method on the Persister that returns the InventoryCollection

Documentation

ManagerRefresh::Inventory::Core will have all CIs that are available for saving. For all Managers there is ManagerRefresh::Inventory::<Type>Manager which has all the CIs that are available for that Manager.

The developer can look at those files, or even better I'll add some introspection methods to query what is available for saving and what is required.

durandom · 2017-02-22T16:53:51Z

app/models/manager_refresh/inventory/core.rb

@@ -0,0 +1,41 @@
+module ManagerRefresh::Inventory::Core


maybe rename to Platform ?

hm, also this might go under automation and some under core? Since there will be shared ones and manager specific?

In here I would put everything that can be refreshed, i.e. really all models.
AutomationManager and CloudManager define what they need.

Not sure if I understand what you mean

Right, this will get huge soon, so that is why I started to divide it by managers. We could do that with modules, to have it under one class.

But then we will need different names for shared models, so like cloud_vms, infra_vms, cloud_miq_templates, infra_miq_templatesm etc. ? While now you would fetch a cloud.vms or infra.vms

I think it's ok to be huge, just like schema.rb

What is the benefit of having everything in Core and also in their respective managers? You could use modules to reduce the repetition but why bother?

shared definitions are handy, since e.g. all clouds will share like 90% of them

if this helps us remove a Helper or Utils class, then I'm all for it

What is the benefit of having everything in Core and also in their respective managers?

Core or Platform defines which CIs are available. A manager defines whats avail for that manager, which fields are used and which association is used.

I was thinking, we control whats going into core and then in the manager restrict it further.

But yeah, maybe this is overkill and just having it inside the Manager is enough

durandom · 2017-02-22T16:59:03Z

@Ladas @agrare @kbrock please have a look and let me know what you think

Ladas · 2017-02-22T18:05:56Z

app/models/manager_refresh/inventory_collection.rb

@@ -35,6 +36,7 @@ def initialize(model_class: nil, manager_ref: nil, association: nil, parent: nil
      @complete                         = complete.nil? ? true : complete
      @update_only                      = update_only.nil? ? false : update_only
      @builder_params                   = builder_params
+      @inventory_object_attributes      = inventory_object_attributes


so now it will fallback to the method missing? Maybe we should have there a check, if inventory_object_attributes is defined, we will raise exception on unknown?

so then you can choose auto/manual

Yeah, I had that check, but it was to buggy and I did not want to dig deeper.
I think right now only ansible uses the method access?!
So I actually think we could soon completely remove the method_missing version

hm, weird, should be enough to put there a one line with raise

I am still not convinced I want to necessarily list all attributes manually, when I can introspect them. I will need to see how it looks in action and the benefits I suppose. :-)

Ladas · 2017-02-22T18:17:26Z

app/models/manageiq/providers/ansible_tower/inventory/persister/automation_manager.rb

@@ -1,19 +1,18 @@
 class ManageIQ::Providers::AnsibleTower::Inventory::Persister::AutomationManager < ManagerRefresh::Inventory::Persister
+  include ManagerRefresh::Inventory::AutomationManager
+
+  has_inventory automation_manager.credentials


how will we set a :parent here?

e.g. as we have here:
https://github.com/Ladas/manageiq-providers-amazon/blob/80c25bac4a54198d45ce04a52e2bf2349a41c7ef/app/models/manageiq/providers/amazon/inventory/persister/network_manager.rb#L13-L13

also not sure about ICs that point to other ICs

https://github.com/Ladas/manageiq-providers-amazon/blob/80c25bac4a54198d45ce04a52e2bf2349a41c7ef/app/models/manageiq/providers/amazon/inventory/persister/cloud_manager.rb#L12-L12

has_inventory_collection?

the nifty yield here let's you do

has_inventory cloud_manager.vms do |collection, persister| collection.parent = persister.manager.parent_manager collection.dependency_attributes[: orchestration_stacks] = persister.orchestration_stacks end

but hopefully this would mostly be done in the core ManagerRefresh::Inventory::AutomationManager so provider developers dont have to deal with it.

right, so we will need an attr_accessor for all attributes? Yeah this could work. Some things can be done in core, some will be provider specific.

I don't think you want to actually want to call automation_manager.credentials (or cloud_manager.vms) at class instantiation here.

Think the automation_manager is determined at runtime - no?

this is probably a runtime call not a class instantiation call

Soo, has_inventory will create an instance method, that lazy instantiates an InventoryCollection.

automation_manager.credentials just returns the configuration for that InventoryCollection

The lambda's are needed for adding stuff to the configuration, that is only available at runtime.

It's a lot like Rails validations.

validates :password, confirmation: true, unless: Proc.new { |a| a.password.blank? }

But the automation_manager.credentials is interpreted at class load time, not at runtime.

I'd imagine that automation_manager needs to be determined at run time, and especially credentials

ok, just joined you all. automation_manager is hardcoded to this class. and configuration is a hash that is ~hardcoded to this class. It contains no instance data.

Think this looks good.
Just double checking:
Are we sure @automation_manager is different for each parent class?

@kbrock cool. But actually your initial confusion about this exhibits a problem. It should be obvious what this is. Maybe it's a naming problem.

Ladas · 2017-02-22T18:22:55Z

app/models/manager_refresh/inventory/automation_manager.rb

+  include Core
+
+  class Inventory < ManagerRefresh::Inventory::Core::Inventory
+    def credentials


so if we need to overwrite this for a specific manager, how would that look like?

It's up there in the persister

has_inventory automation_manager.credentials, :association => :something_else

is that what you mean?

hm, right now I have manager level overwrite and persister level, maybe it's enough to have one. But then a manager specific options will be duplicated

this will only work if credentials returns a lambda and looks up the approperiate automate_manager / credentials at runtime

miq-bot · 2017-02-22T18:26:40Z

This pull request is not mergeable. Please rebase and repush.

durandom · 2017-02-22T21:38:49Z

app/models/manager_refresh/inventory/automation_manager.rb

+                :model_class    => ManageIQ::Providers::AutomationManager::Authentication,
+                :association    => :credentials,
+                :builder_params => {
+                  :resource => ->(persister) { persister.manager }


@Ladas this is another way to access the persister at instance level. We could do this for all options passed

hm, that could work

yay lambda here

durandom · 2017-02-23T07:37:49Z

I am still not convinced I want to necessarily list all attributes manually, when I can introspect them. I will need to see how it looks in action and the benefits I suppose. :-)

Yeah, that's indeed debatable. Like ActiveRecord does not list attributes, but just uses the fields defined in the DB.

But imho this case is different.

We dont want every db field to be writable
Not every model will need / be allowed to fill every field (see e.g. Vm and Template in the same table)
Later we can have automated tests, if some fields dont get filled by refresh
We can have a list for documentation what fields are filled by which provider

Ladas · 2017-02-23T08:12:01Z

@durandom right, but then you would need to list the attributes per provider and also per base class, so e.g. cloud_vm and amazon_cloud_vm? And maybe per Persister, if the Persister will use parser that fills a subset of attrs?

durandom · 2017-02-23T08:52:56Z

you would need to list the attributes per provider and also per base class, so e.g. cloud_vm and amazon_cloud_vm?

Yes 😄 - although one could do with some clever :except => [:attribute, :another_attr] magic.

And maybe per Persister, if the Persister will use parser that fills a subset of attrs?

Yes, sounds like a lot of extra work, but I guess in most cases you want to provide everything thats possible, then it's just has_inventory cloud_manager.vms

kbrock

starting to get there. the introduction of lambdas is addressing delayed instantiation

kbrock · 2017-02-24T15:54:55Z

app/models/manageiq/providers/ansible_tower/inventory/persister/automation_manager.rb

@@ -1,19 +1,18 @@
 class ManageIQ::Providers::AnsibleTower::Inventory::Persister::AutomationManager < ManagerRefresh::Inventory::Persister
+  include ManagerRefresh::Inventory::AutomationManager
+
+  has_inventory automation_manager.credentials


I don't think you want to actually want to call automation_manager.credentials (or cloud_manager.vms) at class instantiation here.

Think the automation_manager is determined at runtime - no?

this is probably a runtime call not a class instantiation call

kbrock · 2017-02-24T15:57:06Z

app/models/manager_refresh/inventory/automation_manager.rb

+  include Core
+
+  class Inventory < ManagerRefresh::Inventory::Core::Inventory
+    def credentials


this will only work if credentials returns a lambda and looks up the approperiate automate_manager / credentials at runtime

kbrock · 2017-02-24T15:57:19Z

app/models/manager_refresh/inventory/automation_manager.rb

+                :model_class    => ManageIQ::Providers::AutomationManager::Authentication,
+                :association    => :credentials,
+                :builder_params => {
+                  :resource => ->(persister) { persister.manager }


yay lambda here

kbrock · 2017-02-24T16:00:40Z

app/models/manager_refresh/inventory/core.rb

@@ -0,0 +1,41 @@
+module ManagerRefresh::Inventory::Core


if this helps us remove a Helper or Utils class, then I'm all for it

durandom · 2017-02-24T16:45:50Z

@agrare @kbrock @Ladas so, are you generally ok with the kind of DSL and putting the configuration into app/models/manager_refresh/inventory/automation_manager.rb ?

If so, I could create a more complete PR to have it reviewed by Fryguy

Ladas · 2017-02-24T18:54:02Z

@agrare @durandom @kbrock so I have these https://github.com/Ladas/manageiq/tree/85ad5bf15b81d6de63ab3dfcda16c9bd9c485f16/app/models/manager_refresh/inventory_collection_default

Those are basically a hashes that you can just put to InventoryCollection.new and get an InventoryCollection for e.g. cloud vms.

Since these definitions are like 95% the same, for all clouds, I would like to have them on a shared place. But I prefer them being class methods, that you can call from anywhere. :-) Since my Persiters usually consists from ICs of more managers (e.g. for manager crosslinks). And I also fetch those definitions from specs. So it's really just a helper for me. :-)

Ladas · 2017-02-24T18:57:15Z

@durandom for the DSL, I think this could work for me, hopefully it will not be more verbose than I have it now. :-)

miq-bot · 2017-02-24T22:11:45Z

This pull request is not mergeable. Please rebase and repush.

kbrock · 2017-02-25T04:27:07Z

@durandom I'd prefer this be a module.

At first, runtime data has been slipping into these configurations (e.g. @parent) Looks like you've been eradicating these. thanks.

But with this current dsl, provider specific configuration data is getting into the InventoryObjectCollection definitions. I think moving these definitions into a module and called via def extended will fix our problems. This is based upon the assumption that automation_manager is vendor specific vs the same for every vendor and just a glorified constant/global.

Maybe we could do a quick call on Monday or chat room? I really like how this is going...
BUT, there are just 1 or 2 very minor points that have me concerned.

basically:

Rails class level dsls (e.g. belongs_to) use symbols (they call public_send later), lambdas (to be called later), and hardcoded values. But the dsls never have you invoke a method at definition time.

Then, these are executed either at runtime, or after the db schema has been loaded.
I notice there are a few places in our setup code that really should be delaying the execution of code. The example of introducing the lambdas / blocks do help, but a few things are still sneaking in like the automation_manager call.

durandom · 2017-03-01T11:35:41Z

app/models/manageiq/providers/ansible_tower/shared/inventory/persister/automation_manager.rb

+  include ManagerRefresh::Inventory::AutomationManager
+
+  included do
+    has_automation_manager_credentials


I have opted for dropping a _inventory suffix here.

So its either
has_inventory { :model_class ...
or
has_<inventory name>

Also note, that you can always overwrite any options like
has_automation_manager_credentials :builder_params => {...

durandom · 2017-03-01T11:39:53Z

@kbrock @Ladas @agrare I've updated the code reflecting the results of our conversation.
Please have a look and be really rigorous on the code style and naming, because if we go this route it will act as a copy paste template 😄

kbrock

kbrock · 2017-03-02T17:17:51Z

app/models/manager_refresh/inventory/persister.rb

+          end
+        end
+        yield(collection, self) if block_given?
+        collection


I think this is right.

But every time I read this I keep wondering if we want to return the block results instead:

(block_given? ? yield(collection, self) : nil) || collection

yeah, it's to hook into building the collection

has_inventory_cloud_manager_vms do |collection, persister| collection.parent = persister.manager.parent_manager collection.dependency_attributes[:orchestration_stacks] = persister.orchestration_stacks end

But I'm not even sure it'll work now, because I'm not passing the block down...
So, thanks, I'll drop this...

kbrock · 2017-03-02T17:20:06Z

I'm guessing we want to merge and then improve once we see where this breaks down in the amazon / dynamically created case.

kbrock

not sure what is breaking for the build. looks like you may need an unscoped somewhere or something.

but this code/style looks good to me

kbrock · 2017-03-02T21:37:42Z

app/models/manager_refresh/inventory_object.rb

      end
    end

    def self.define_data_reader(data_key)
      define_method(data_key) do
-        send(:[], data_key)
+        public_send(:[], data_key)


since we know the name of the method, can we just call it directly?

define_method(data_key) do self[data_key] end

kbrock · 2017-03-02T21:38:03Z

app/models/manager_refresh/inventory_object.rb

-      define_method(data_key) do |value|
-        send(:[]=, data_key.to_s.delete("=").to_sym, value)
+      define_method("#{data_key}=") do |value|
+        public_send(:[]=, data_key, value)


ditto for not using public_send

Ladas · 2017-03-03T08:06:57Z

app/models/manageiq/providers/ansible_tower/inventory_collection_default/automation_manager.rb

@@ -1,3 +0,0 @@
-class ManageIQ::Providers::AnsibleTower::InventoryCollectionDefault::AutomationManager < ManagerRefresh::InventoryCollectionDefault


don't we need this one? Or where is the shared module included? Not sure if we need a shared module in this case, since it's ansible specific?

nope, this is all done in app/models/manager_refresh/inventory/automation_manager.rb and app/models/manageiq/providers/ansible_tower/shared/inventory/persister/automation_manager.rb

Ladas · 2017-03-03T08:19:30Z

app/models/manager_refresh/inventory_object.rb

@@ -81,45 +81,22 @@ def dependency?
      true
    end

-    def allowed_writers
-      return [] unless model_class
-


could we still keep the automatic way, just put here

return inventory_collection.inventory_object_attributes if return inventory_collection.inventory_object_attributes

that should limit it to the explicitly defined set

I would deprecate the automatic way, once there is some gain from adding these attributes manually

TBH, if we keep the automatic way, people will not create the attribute lists in app/models/manager_refresh/inventory/core.rb - and this is something I really want, also for documentation purposes

you are right, I was planning to do exactly that :-D ok, make sense

Ladas · 2017-03-03T08:19:47Z

app/models/manager_refresh/inventory_object.rb

-
-    def allowed_readers
-      return [] unless model_class
-


return inventory_collection.inventory_object_attributes if return inventory_collection.inventory_object_attributes

Ladas · 2017-03-03T08:38:21Z

app/models/manager_refresh/inventory/core.rb

+      @provider_module ||= ManageIQ::Providers::Inflector.provider_module(self)
+    end
+
+    def has_authentications(options = {})


so can we put these definitions into a modules? So I can add the other managers and it's not all in one pile.

That being said, having it under one class will have a problem with conflicts. E.g. InfraManager vms vs. CloudManager vms, so that is partially the reason I went with the definitions I have now.

in here it's not scoped. It's only "helpers" for app/models/manager_refresh/inventory/automation_manager.rb

ah, I see, this became a module

so this is more ManagerRefresh::Inventory::Core::AutomationManager ? My point is that I want a file per manager, not 1 Core with 100 methods :-)

also you could do this a normal module and then extend instead of include? It would look nicer :-)

unfortunately then provider_module doesnt work anymore.

because provider_module is a class method created by Core and it's used at class level in the persister...

A `Persister` can `include ManagerRefresh::Inventory::AutomationManager` to have access to all inventory collections an AutomationManager usually persists. Then we call the DSL `has_inventory` to define a method on the `Persister` that returns the InventoryCollection Documentation `ManagerRefresh::Inventory::Core` will have all CIs that are available for saving. For all **Managers** there is `ManagerRefresh::Inventory::<Type>Manager` which has all the CIs that are available for that Manager.

miq-bot · 2017-03-03T09:37:25Z

Checked commits durandom/manageiq@2e46e5c~...6e14b58 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
6 files checked, 13 offenses detected

app/models/manager_refresh/inventory/automation_manager.rb

❗ - Line 16, Col 9 - Style/PredicateName - Rename has_automation_manager_configuration_script_payloads to automation_manager_configuration_script_payloads?.
❗ - Line 26, Col 9 - Style/PredicateName - Rename has_automation_manager_configuration_script_sources to automation_manager_configuration_script_sources?.
❗ - Line 36, Col 9 - Style/PredicateName - Rename has_automation_manager_configured_systems to automation_manager_configured_systems?.
❗ - Line 46, Col 9 - Style/PredicateName - Rename has_automation_manager_credentials to automation_manager_credentials?.
❗ - Line 56, Col 9 - Style/PredicateName - Rename has_automation_manager_inventory_root_groups to automation_manager_inventory_root_groups?.
❗ - Line 6, Col 9 - Style/PredicateName - Rename has_automation_manager_configuration_scripts to automation_manager_configuration_scripts?.

app/models/manager_refresh/inventory/core.rb

❗ - Line 17, Col 9 - Style/PredicateName - Rename has_configuration_scripts to configuration_scripts?.
❗ - Line 25, Col 9 - Style/PredicateName - Rename has_configuration_script_sources to configuration_script_sources?.
❗ - Line 33, Col 9 - Style/PredicateName - Rename has_configuration_script_payloads to configuration_script_payloads?.
❗ - Line 41, Col 9 - Style/PredicateName - Rename has_configured_systems to configured_systems?.
❗ - Line 49, Col 9 - Style/PredicateName - Rename has_ems_folders to ems_folders?.
❗ - Line 9, Col 9 - Style/PredicateName - Rename has_authentications to authentications?.

app/models/manager_refresh/inventory/persister.rb

❗ - Line 16, Col 12 - Style/PredicateName - Rename has_inventory to inventory?.

durandom · 2017-03-03T09:39:46Z

@miq-bot remove_label wip

Ladas · 2017-03-03T11:30:25Z

app/models/manager_refresh/inventory/automation_manager.rb

+
+  class_methods do
+    def has_automation_manager_configuration_scripts(options = {})
+      has_configuration_scripts({


right so with this approach, I would have has_cloud_manager_vms, has_infra_manager_vms or has_vms and I can pick any of them in Persistor?

Ladas · 2017-03-03T11:46:28Z

@kbrock ok, I think, I have just minor things we can solve later, so this looks good enough to merge for me :-)

miq-bot added enhancement refactoring wip providers/ansible_tower labels Feb 22, 2017

durandom force-pushed the inventory_object_attributes branch 3 times, most recently from 1c7bb7b to 4ad6a36 Compare February 22, 2017 16:48

durandom commented Feb 22, 2017

View reviewed changes

Ladas reviewed Feb 22, 2017

View reviewed changes

miq-bot added the unmergeable label Feb 22, 2017

durandom commented Feb 22, 2017

View reviewed changes

durandom force-pushed the inventory_object_attributes branch 2 times, most recently from 23efd3a to daa7cc0 Compare February 23, 2017 07:32

miq-bot removed the unmergeable label Feb 23, 2017

kbrock reviewed Feb 24, 2017

View reviewed changes

miq-bot added the unmergeable label Feb 24, 2017

chessbyte assigned blomquisg Feb 28, 2017

chessbyte requested a review from Fryguy February 28, 2017 14:59

durandom force-pushed the inventory_object_attributes branch from ad1b186 to b973b7b Compare March 1, 2017 11:33

durandom commented Mar 1, 2017

View reviewed changes

miq-bot removed the unmergeable label Mar 1, 2017

kbrock approved these changes Mar 2, 2017

View reviewed changes

durandom force-pushed the inventory_object_attributes branch from b973b7b to 2cd5c9c Compare March 2, 2017 20:55

kbrock approved these changes Mar 2, 2017

View reviewed changes

Ladas reviewed Mar 3, 2017

View reviewed changes

durandom force-pushed the inventory_object_attributes branch 2 times, most recently from 82ddffb to c9bee11 Compare March 3, 2017 08:42

durandom force-pushed the inventory_object_attributes branch from c9bee11 to 2e46e5c Compare March 3, 2017 09:25

inline define_data methods

6e14b58

durandom changed the title ~~[WIP] api for defining inventory collections~~ api for defining inventory collections Mar 3, 2017

miq-bot removed the wip label Mar 3, 2017

Ladas reviewed Mar 3, 2017

View reviewed changes

kbrock assigned kbrock and unassigned blomquisg Mar 3, 2017

kbrock added this to the Sprint 56 Ending Mar 13, 2017 milestone Mar 3, 2017

kbrock merged commit 9230b2c into ManageIQ:master Mar 3, 2017

durandom deleted the inventory_object_attributes branch March 3, 2017 20:19

durandom mentioned this pull request Mar 3, 2017

add vms via dsl in ansible automation manager #14180

Merged

Fryguy added the providers/automation label Jan 24, 2020

		@@ -1,3 +0,0 @@
		class ManageIQ::Providers::AnsibleTower::InventoryCollectionDefault::AutomationManager < ManagerRefresh::InventoryCollectionDefault

api for defining inventory collections #14022

api for defining inventory collections #14022

Conversation

durandom commented Feb 22, 2017 • edited Loading

DSL

Documentation

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ladas Feb 23, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

durandom commented Feb 22, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ladas Feb 22, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

durandom Feb 24, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kbrock Feb 27, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

miq-bot commented Feb 22, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

durandom commented Feb 23, 2017

Ladas commented Feb 23, 2017

durandom commented Feb 23, 2017

kbrock left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

durandom commented Feb 24, 2017

Ladas commented Feb 24, 2017 • edited Loading

Ladas commented Feb 24, 2017

miq-bot commented Feb 24, 2017

kbrock commented Feb 25, 2017

durandom Mar 1, 2017 • edited Loading

Choose a reason for hiding this comment

durandom commented Mar 1, 2017

kbrock left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

durandom Mar 2, 2017 • edited Loading

Choose a reason for hiding this comment

kbrock commented Mar 2, 2017

kbrock left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ladas Mar 3, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

durandom commented Feb 22, 2017 •

edited

Loading

Ladas Feb 23, 2017 •

edited

Loading

Ladas Feb 22, 2017 •

edited

Loading

durandom Feb 24, 2017 •

edited

Loading

kbrock Feb 27, 2017 •

edited

Loading

Ladas commented Feb 24, 2017 •

edited

Loading

durandom Mar 1, 2017 •

edited

Loading

durandom Mar 2, 2017 •

edited

Loading

Ladas Mar 3, 2017 •

edited

Loading