add support to configure shard_duration to retention_policy resource #170

kjschnei001 · 2018-09-26T17:46:48Z

I ran the integration tests across 2 different versions of chef and the changes work. I don't think the cookbook in general is written to support chef v14 as I found issues unrelated to things I changed:

Chef 12

       influxdb
         User "influxdb"
           should exist
         Service "influxdb"
           should be running
         Port "8086"
           should be listening
         test_database
           exists
           default retention policy
             exists
             its retention is set for 1 week
             its shard duration is set for 24 hours
             is set for 1 replica
             is the default
           test continuous queries
             is exists
         test_user
           exists
           is not an admin
         test_admin
           exists
           is an admin
       
       Finished in 0.20375 seconds (files took 0.31744 seconds to load)
       14 examples, 0 failures
       
       Finished verifying <default-centos-73-chef1219> (0m5.72s).

Chef 13

       influxdb
         User "influxdb"
           should exist
         Service "influxdb"
           should be running
         Port "8086"
           should be listening
         test_database
           exists
           default retention policy
             exists
             its retention is set for 1 week
             its shard duration is set for 24 hours
             is set for 1 replica
             is the default
           test continuous queries
             is exists
         test_user
           exists
           is not an admin
         test_admin
           exists
           is an admin
       
       Finished in 0.21215 seconds (files took 0.30468 seconds to load)
       14 examples, 0 failures
       
       Finished verifying <default-centos-73-chef139> (0m6.37s).

Chef 14

           NoMethodError
           -------------
           undefined method `updated_by_last_action' for #<#<Class:0x0000000003ea5ff8>:0x0000000003983250>
           
           Cookbook Trace:
           ---------------
           /tmp/kitchen/cache/cookbooks/influxdb/resources/retention_policy.rb:29:in `block in class_from_file'

resources/install.rb

Rakefile

recipes/client.rb

resources/install.rb

recipes/default.rb

test/integration/default/serverspec/default_spec.rb

bdangit · 2018-09-27T02:02:23Z

Thanks @kjschnei001 for the PR. I’ll take a look at this soon and should receive feedback this week.

bdangit · 2018-09-28T05:00:16Z

resources/retention_policy.rb

-      client.alter_retention_policy(new_resource.policy_name, new_resource.database, new_resource.duration, new_resource.replication, new_resource.default)
+    if current_policy['duration'] != new_resource.duration || current_policy['replicaN'] != new_resource.replication || current_policy['default'] != new_resource.default || current_policy['shard_duration'] != new_resource.shard_duration
+      client.alter_retention_policy(new_resource.policy_name, new_resource.database, new_resource.duration, new_resource.replication, new_resource.default, shard_duration: new_resource.shard_duration)
+      updated_by_last_action true


to be chef14 compliant, you'll need to change this line to new_resource.updated_by_last_action true

bdangit · 2018-09-28T05:00:30Z

resources/retention_policy.rb

    end
  else
-    client.create_retention_policy(new_resource.policy_name, new_resource.database, new_resource.duration, new_resource.replication, new_resource.default)
+    client.create_retention_policy(new_resource.policy_name, new_resource.database, new_resource.duration, new_resource.replication, new_resource.default, shard_duration: new_resource.shard_duration)
+    updated_by_last_action true


to be chef14 compliant, you'll need to change this line to new_resource.updated_by_last_action true

bdangit · 2018-09-28T05:04:10Z

@kjschnei001 I ran through the kitchen setup just for chef14 clients. Check out my review comments to get it to work.

$ kitchen test "default-ubuntu-1404-chef14.*"
...
    Recipe: influxdb-test::default
         * influxdb_database[test_database] action create (up to date)
         * influxdb_user[test_user] action create (up to date)
         * influxdb_admin[test_admin] action create (up to date)
         * influxdb_retention_policy[test_policy] action create

         * influxdb_continuous_query[test_cq] action create (up to date)
       Recipe: influxdb::default
         * service[influxdb] action restart
           - restart service service[influxdb]
...
  test_database
           exists
           default retention policy
             exists
             its retention is set for 1 week
             its shard duration is set for 24 hours
             is set for 1 replica
             is the default
           test continuous queries
...

bdangit · 2018-10-06T00:11:33Z

Hi @kjschnei001, just checking in to see if you needed any additional help with this PR.

kjschnei001 · 2018-10-09T15:46:22Z

Sorry about the delay, @bdangit . Your fixes did the trick!:

       influxdb
         User "influxdb"
           should exist
         Service "influxdb"
           should be running
         Port "8086"
           should be listening
         test_database
           exists
           default retention policy
             exists
             its retention is set for 1 week
             its shard duration is set for 24 hours
             is set for 1 replica
             is the default
           test continuous queries
             is exists
         test_user
           exists
           is not an admin
         test_admin
           exists
           is an admin
       
       Finished in 0.30415 seconds (files took 0.35817 seconds to load)
       14 examples, 0 failures
       
       Finished verifying <default-centos-73-chef142> (0m11.22s).

kjschnei001 · 2018-10-09T15:50:21Z

It looks like foodcritic is complaining about that syntax though:

FC085: Resource using new_resource.updated_by_last_action to converge resource: repo/bdangit/chef-influxdb/resources/retention_policy.rb:25
FC085: Resource using new_resource.updated_by_last_action to converge resource: repo/bdangit/chef-influxdb/resources/retention_policy.rb:29

kjschnei001 · 2018-10-09T16:14:34Z

@bdangit The continuous integeration tests are failing, but I can't see the page to know why. Can I get your help on this one?

kjschnei001 · 2018-10-09T16:19:57Z

It looks like the testing is just executing rake test:quick which I am able to do locally without issue, so I'm really blind to the issue here:

$ chef exec "rake test:quick"
Running RuboCop...
Inspecting 27 files
...........................

27 files inspected, no offenses detected
Starting Foodcritic linting...
Checking 16 files
................
Completed!

bdangit · 2018-10-11T23:41:32Z

hey @kjschnei001 I'm so sorry about the lack of build visibility feedback you are getting. I have created an issue #171 to use something more open source friendly.

W.r.t to the issue at hand this is what I see:

export WERCKER_STEP_ROOT="/pipeline/script-d062f272-8cec-4c8d-a693-211bc8b3187d"
export WERCKER_STEP_ID="script-d062f272-8cec-4c8d-a693-211bc8b3187d"
export WERCKER_STEP_OWNER="wercker"
export WERCKER_STEP_NAME="script"
export WERCKER_REPORT_NUMBERS_FILE="/report/script-d062f272-8cec-4c8d-a693-211bc8b3187d/numbers.ini"
export WERCKER_REPORT_MESSAGE_FILE="/report/script-d062f272-8cec-4c8d-a693-211bc8b3187d/message.txt"
export WERCKER_REPORT_ARTIFACTS_DIR="/report/script-d062f272-8cec-4c8d-a693-211bc8b3187d/artifacts"
source "/pipeline/script-d062f272-8cec-4c8d-a693-211bc8b3187d/run.sh" < /dev/null
RuboCop failed!
Running RuboCop...
Inspecting 27 files
...............C...........

Offenses:

resources/install.rb:14:1: C: Style/MixinUsage: include is used at the top level. Use inside class or module.
include InfluxdbCookbook::Helpers
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

27 files inspected, 1 offense detected

This error looks like something you didn't change. So I suspect that chefdk tool suite has been upgraded within this docker image chef/chefdk What I would do is the following:

docker run -it -rm -v $(pwd):/work chef/chefdk bash
cd /work
chef exec "rake test:quick"

Note: I haven't tested above, but hopefully you get the gist of what I am recommending.

kjschnei001 · 2018-10-12T04:25:48Z

@bdangit Thanks Ben! Running the tests in the docker environment gave me the insight I needed to get the tests passing.

bdangit

Thanks @kjschnei001! Before I give the thumbsup, can you please squash your commits?

kjschnei001 · 2018-10-25T14:02:14Z

@bdangit Sorry for the delay. I have squashed the commits. Are we good to go now?

bdangit · 2018-10-25T15:51:40Z

awesome! thanks for your PR @kjschnei001!

kjschnei001 · 2018-10-25T17:59:18Z

@bdangit would you mind cutting a 6.3.0 release?

bdangit · 2018-10-26T13:30:38Z

@kjschnei001 done!

kjschnei001 commented Sep 26, 2018

View reviewed changes

bdangit requested changes Sep 28, 2018

View reviewed changes

bdangit requested changes Oct 13, 2018

View reviewed changes

add support to configure shard_duration to retention_policy resource

aa65f8b

kjschnei001 force-pushed the shard_duration branch from 636d1dc to aa65f8b Compare October 25, 2018 14:01

bdangit approved these changes Oct 25, 2018

View reviewed changes

bdangit merged commit c9d930b into bdangit:master Oct 25, 2018

kjschnei001 deleted the shard_duration branch October 25, 2018 17:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add support to configure shard_duration to retention_policy resource #170

add support to configure shard_duration to retention_policy resource #170

kjschnei001 commented Sep 26, 2018 •

edited

Loading

bdangit commented Sep 27, 2018

bdangit Sep 28, 2018

bdangit Sep 28, 2018

bdangit commented Sep 28, 2018

bdangit commented Oct 6, 2018

kjschnei001 commented Oct 9, 2018

kjschnei001 commented Oct 9, 2018 •

edited

Loading

kjschnei001 commented Oct 9, 2018

kjschnei001 commented Oct 9, 2018

bdangit commented Oct 11, 2018

kjschnei001 commented Oct 12, 2018

bdangit left a comment

kjschnei001 commented Oct 25, 2018

bdangit commented Oct 25, 2018

kjschnei001 commented Oct 25, 2018

bdangit commented Oct 26, 2018

add support to configure shard_duration to retention_policy resource #170

add support to configure shard_duration to retention_policy resource #170

Conversation

kjschnei001 commented Sep 26, 2018 • edited Loading

bdangit commented Sep 27, 2018

bdangit Sep 28, 2018

Choose a reason for hiding this comment

bdangit Sep 28, 2018

Choose a reason for hiding this comment

bdangit commented Sep 28, 2018

bdangit commented Oct 6, 2018

kjschnei001 commented Oct 9, 2018

kjschnei001 commented Oct 9, 2018 • edited Loading

kjschnei001 commented Oct 9, 2018

kjschnei001 commented Oct 9, 2018

bdangit commented Oct 11, 2018

kjschnei001 commented Oct 12, 2018

bdangit left a comment

Choose a reason for hiding this comment

kjschnei001 commented Oct 25, 2018

bdangit commented Oct 25, 2018

kjschnei001 commented Oct 25, 2018

bdangit commented Oct 26, 2018

kjschnei001 commented Sep 26, 2018 •

edited

Loading

kjschnei001 commented Oct 9, 2018 •

edited

Loading