Chargeback for container images #10677

zeari · 2016-08-22T18:55:30Z

Chargeback based on the number of hours container images are running.
For this we need the following:
~~- [ ] Disconnect ContainerImages or keep them otherwise #10872~~ we might not need this

Attach ChargebackRates to specific image tags Added rate assigning by tags to container images #11015
Keep image tags along with Container metrics Add image tag names for Containers in vim performance state. #11016
Add code to ChargebackContainerImage according to the above changes (This PR)
testing (This PR)

~~Full functionality depends on: #10120 #10671~~
~~With just this patch, all image hours will cost the same.~~
cc @simon3z

zeari · 2016-08-22T18:57:15Z

@miq-bot add_label providers/containers, chargeback, wip

simon3z · 2016-08-23T09:41:45Z

@miq-bot assign zeari

miq-bot · 2016-08-23T14:57:14Z

<pr_mergeability_checker />This pull request is not mergeable. Please rebase and repush.

miq-bot · 2016-09-07T16:21:38Z

<pr_mergeability_checker />This pull request is not mergeable. Please rebase and repush.

miq-bot · 2016-09-13T13:07:00Z

<pr_mergeability_checker />This pull request is not mergeable. Please rebase and repush.

zeari · 2016-09-22T10:51:17Z

app/models/container_definition.rb

@@ -12,7 +12,6 @@ def disconnect_inv
    _log.info "Disconnecting Container definition [#{name}] id [#{id}]"
    self.container.try(:disconnect_inv)
    self.deleted_on = Time.now.utc
-    self.container_group = nil


@simon3z In order to know which project a container belonged to, i need to keep this relationship and use

has_one :old_container_project, :through :container_group

I think this involved @moolitayer and @Fryguy in the past (they should be in the loop if they have time to look into this).

Let me export this part to another PR and we can discuss it there.

zeari · 2016-09-22T13:33:54Z

app/models/chargeback.rb

@@ -86,17 +86,20 @@ def get_rates(perf)
    @rates ||= {}
    @enterprise ||= MiqEnterprise.my_enterprise

+    state = perf.resource.vim_performance_state_for_ts(perf.timestamp)


TODO: See if we can make this happen only once

this is still bad performance. itll run vim_performance_state_for_ts once per unique timestamp+image meaning if we have metrics for one week from 4 containers with different images then vim_performance_state_for_ts will be called (7days) * (24 hours) * (4 number of different images) which is still a lot.

miq-bot · 2016-09-22T16:02:49Z

<pr_mergeability_checker />This pull request is not mergeable. Please rebase and repush.

zeari · 2016-09-26T15:06:23Z

@miq-bot add_label euwe/yes

miq-bot · 2016-10-04T12:57:09Z

<pr_mergeability_checker />This pull request is not mergeable. Please rebase and repush.

yaacov · 2016-10-06T11:10:34Z

app/models/chargeback_container_project.rb

-  def self.report_name_field
-    "project_name"
+  def self.report_static_cols
+    ["project_name"]


i think using %w(...) is nicer ?

Changed where relevent.

gtanzillo · 2016-10-06T13:34:22Z

app/models/chargeback_container_project.rb

@@ -102,4 +102,9 @@ def self.report_col_options
  def tags
    ContainerProject.includes(:tags).find_by_ems_ref(project_uid).try(:tags).to_a
  end
+
+  def get_rate_parents(perf)


Should add this method to the base Chargeback class and have it raise an error indicating that get_rate_parents must be implemented in child class.

gtanzillo · 2016-10-06T13:35:17Z

app/controllers/report_controller/reports/editor.rb

@@ -1030,20 +1031,27 @@ def set_record_vars(rpt)
    # Add in the chargeback static fields
    if Chargeback.db_is_chargeback?(rpt.db) # For chargeback, add in static fields
      rpt.cols = %w(start_date display_range)
-      name_col = @edit[:new][:model].constantize.report_name_field
-      tag_col = @edit[:new][:model].constantize.report_tag_field
+


As we discussed, move to Chargeback modesl.

gtanzillo · 2016-10-06T13:36:31Z

app/models/chargeback_container_image.rb

+    # get rates from image tags only
+    []
+  end
+end # class Chargeback


Fix comment to match class name

gtanzillo · 2016-10-06T13:42:25Z

spec/models/chargeback_container_image_spec.rb

+                                                        :tag_names                => "",
+                                                        :resource_name            => @project.name,
+                                                        :resource_id              => @project.id)
+        state = VimPerformanceState.capture(@container)


Should use a factory for VimPerformanceState

~~Im going to create another PR for this~~
Done

zeari · 2016-10-06T16:07:22Z

@gtanzillo All comments addressed

miq-bot · 2016-10-06T16:13:08Z

Checked commits zeari/manageiq@3462cd1~...6cdb5ce with ruby 2.2.5, rubocop 0.37.2, and haml-lint 0.16.1
9 files checked, 29 offenses detected

app/models/chargeback.rb

❗ - Line 249, Col 12 - Style/AccessorMethodName - Do not prefix reader method names with get_.
❕ - Line 108, Col 121 - Metrics/LineLength - Line is too long. [148/120]
❕ - Line 112, Col 121 - Metrics/LineLength - Line is too long. [134/120]
❕ - Line 253, Col 3 - Metrics/AbcSize - Assignment Branch Condition size for set_chargeback_report_options is too high. [47.68/20]
❕ - Line 253, Col 3 - Metrics/MethodLength - Method has too many lines. [35/25]
❕ - Line 253, Col 3 - Metrics/PerceivedComplexity - Perceived complexity for set_chargeback_report_options is too high. [8/7]

app/models/chargeback_container_image.rb

❗ - Line 29, Col 12 - Style/MethodName - Use snake_case for method names.
❕ - Line 29, Col 3 - Metrics/AbcSize - Assignment Branch Condition size for build_results_for_report_ChargebackContainerImage is too high. [23.58/20]
❕ - Line 29, Col 3 - Metrics/CyclomaticComplexity - Cyclomatic complexity for build_results_for_report_ChargebackContainerImage is too high. [7/6]
❕ - Line 29, Col 3 - Metrics/PerceivedComplexity - Perceived complexity for build_results_for_report_ChargebackContainerImage is too high. [8/7]
❕ - Line 53, Col 121 - Metrics/LineLength - Line is too long. [161/120]
❕ - Line 68, Col 3 - Metrics/AbcSize - Assignment Branch Condition size for get_keys_and_extra_fields is too high. [20.62/20]

app/models/chargeback_vm.rb

❕ - Line 172, Col 121 - Metrics/LineLength - Line is too long. [126/120]

app/views/report/_form_filter_chargeback.html.haml

⚠️ - Line 113 - Prefer to_s over string interpolation.
⚠️ - Line 113 - Prefer to_s over string interpolation.
⚠️ - Line 114 - Prefer to_s over string interpolation.
⚠️ - Line 115 - Prefer to_s over string interpolation.

spec/factories/vim_performance_state.rb

❗ - Line 4, Col 17 - Style/SpaceInsideBlockBraces - Space missing inside {.
❗ - Line 4, Col 19 - Style/SpaceInsideBlockBraces - Space missing inside }.

spec/models/chargeback_container_image_spec.rb

gtanzillo · 2016-10-10T20:17:16Z

LGTM 👍

Chargeback for container images (cherry picked from commit 3b2860f)

chessbyte · 2016-10-13T20:09:55Z

Euwe Backport details:

$ git log
commit 18cbc4604dc10b0f838333542b6e27d080b2a78b
Author: Gregg Tanzillo <gtanzill@redhat.com>
Date:   Mon Oct 10 16:17:29 2016 -0400

    Merge pull request #10677 from zeari/chargeback_container_image

    Chargeback for container images
    (cherry picked from commit 3b2860f0878228c593575b4ce121c51b3d988c28)

miq-bot added wip providers/containers core/chargeback labels Aug 22, 2016

miq-bot assigned zeari Aug 23, 2016

This was referenced Sep 5, 2016

Added rate assigning by tags to container images #11015

Merged

Add image tag names for Containers in vim performance state. #11016

Merged

zeari force-pushed the chargeback_container_image branch 6 times, most recently from d8352d2 to eadcc6e Compare September 5, 2016 22:22

zeari force-pushed the chargeback_container_image branch 2 times, most recently from 21a6338 to c9a89c6 Compare September 12, 2016 09:59

zeari force-pushed the chargeback_container_image branch from c9a89c6 to 2373231 Compare September 22, 2016 10:48

zeari commented Sep 22, 2016

View reviewed changes

zeari force-pushed the chargeback_container_image branch 4 times, most recently from aa85f77 to 23371ed Compare September 26, 2016 14:54

miq-bot added the euwe/yes label Sep 26, 2016

zeari force-pushed the chargeback_container_image branch from 23371ed to 126589d Compare September 26, 2016 15:19

zeari changed the title ~~[WIP] Chargeback for container images~~ Chargeback for container images Sep 26, 2016

zeari force-pushed the chargeback_container_image branch 8 times, most recently from bd1cf74 to 99c5d77 Compare September 29, 2016 13:21

zeari force-pushed the chargeback_container_image branch 2 times, most recently from 3bc013c to 0e687a4 Compare October 6, 2016 11:07

yaacov reviewed Oct 6, 2016

View reviewed changes

zeari force-pushed the chargeback_container_image branch from 0e687a4 to e2c8539 Compare October 6, 2016 11:34

gtanzillo suggested changes Oct 6, 2016

View reviewed changes

Added chargeback based on container images

3462cd1

zeari force-pushed the chargeback_container_image branch from e2c8539 to 3462cd1 Compare October 6, 2016 15:51

Add VimPerformanceState factory

6cdb5ce

gtanzillo approved these changes Oct 10, 2016

View reviewed changes

gtanzillo added this to the Sprint 48 Ending Oct 24, 2016 milestone Oct 10, 2016

gtanzillo added the enhancement label Oct 10, 2016

gtanzillo merged commit 3b2860f into ManageIQ:master Oct 10, 2016

chessbyte pushed a commit that referenced this pull request Oct 13, 2016

Merge pull request #10677 from zeari/chargeback_container_image

18cbc46

Chargeback for container images (cherry picked from commit 3b2860f)

chessbyte added euwe/backported and removed euwe/yes labels Oct 13, 2016

zeari deleted the chargeback_container_image branch August 24, 2017 07:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Chargeback for container images #10677

Chargeback for container images #10677

zeari commented Aug 22, 2016 •

edited

Loading

zeari commented Aug 22, 2016

simon3z commented Aug 23, 2016

miq-bot commented Aug 23, 2016

miq-bot commented Sep 7, 2016

miq-bot commented Sep 13, 2016

zeari Sep 22, 2016 •

edited

Loading

simon3z Sep 22, 2016

zeari Sep 22, 2016

zeari Sep 22, 2016

zeari Sep 22, 2016

zeari Sep 26, 2016 •

edited

Loading

miq-bot commented Sep 22, 2016

zeari commented Sep 26, 2016

miq-bot commented Oct 4, 2016

yaacov Oct 6, 2016

zeari Oct 6, 2016

gtanzillo Oct 6, 2016 •

edited

Loading

zeari Oct 6, 2016

gtanzillo Oct 6, 2016

zeari Oct 6, 2016

gtanzillo Oct 6, 2016

zeari Oct 6, 2016

gtanzillo Oct 6, 2016

zeari Oct 6, 2016 •

edited

Loading

zeari commented Oct 6, 2016

miq-bot commented Oct 6, 2016

gtanzillo commented Oct 10, 2016

chessbyte commented Oct 13, 2016

Chargeback for container images #10677

Chargeback for container images #10677

Conversation

zeari commented Aug 22, 2016 • edited Loading

zeari commented Aug 22, 2016

simon3z commented Aug 23, 2016

miq-bot commented Aug 23, 2016

miq-bot commented Sep 7, 2016

miq-bot commented Sep 13, 2016

zeari Sep 22, 2016 • 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

zeari Sep 26, 2016 • edited Loading

Choose a reason for hiding this comment

miq-bot commented Sep 22, 2016

zeari commented Sep 26, 2016

miq-bot commented Oct 4, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gtanzillo Oct 6, 2016 • 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

zeari Oct 6, 2016 • edited Loading

Choose a reason for hiding this comment

zeari commented Oct 6, 2016

miq-bot commented Oct 6, 2016

gtanzillo commented Oct 10, 2016

chessbyte commented Oct 13, 2016

zeari commented Aug 22, 2016 •

edited

Loading

zeari Sep 22, 2016 •

edited

Loading

zeari Sep 26, 2016 •

edited

Loading

gtanzillo Oct 6, 2016 •

edited

Loading

zeari Oct 6, 2016 •

edited

Loading