Skip to content

Conversation

@scottwulf
Copy link
Contributor

Rebase of PR #80
Only save the node when attribute(s) have changed
Do not fetch monitor secret if we're the master

Rebase of PR crowbar#80
Only save the node when attribute(s) have changed
Do not fetch monitor secret if we're the master
dirty = true
end
# if journal_device is nil, this will still work as expected
if node["ceph"]["osd_devices"][index]["journal"] != journal_device

Choose a reason for hiding this comment

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

Metrics/BlockNesting: Avoid more than 3 levels of block nesting. (https://github.com/bbatsov/ruby-style-guide#three-is-the-number-thou-shalt-count)

end
node.set["ceph"]["osd_devices"][index]["status"] = "deployed"
node.set["ceph"]["osd_devices"][index]["journal"] = journal_device unless journal_device.nil?
if node["ceph"]["osd_devices"][index]["status"] != "deployed"

Choose a reason for hiding this comment

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

Metrics/BlockNesting: Avoid more than 3 levels of block nesting. (https://github.com/bbatsov/ruby-style-guide#three-is-the-number-thou-shalt-count)


node.set["ceph"]["osd"]["use_ssd_for_journal"] = false unless has_ssds && has_hdds
use_ssd_for_journal = has_ssds && has_hdds
if node["ceph"]["osd"]["use_ssd_for_journal"] != use_ssd_for_journal

Choose a reason for hiding this comment

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

Metrics/BlockNesting: Avoid more than 3 levels of block nesting. (https://github.com/bbatsov/ruby-style-guide#three-is-the-number-thou-shalt-count)

if !node["ceph"]["osd"]["journal_devices"].empty?
node.set["ceph"]["osd"]["use_ssd_for_journal"] = false
# explicit comparison because we don't want a condition that uses nil
if node["ceph"]["osd"]["use_ssd_for_journal"] != false

Choose a reason for hiding this comment

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

Metrics/BlockNesting: Avoid more than 3 levels of block nesting. (https://github.com/bbatsov/ruby-style-guide#three-is-the-number-thou-shalt-count)

nicolasbock
nicolasbock previously approved these changes Aug 2, 2017
device["device"] = d.name
node.set["ceph"]["osd_devices"].push(device)
node.save
dirty = true
Copy link
Member

Choose a reason for hiding this comment

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

We were saving here explicitly for a reason, probably becasue we are using this values ["ceph"]["osd_devices"] immediately below.

Are we sure this is still working as before? I would say yes, as its only used in this recipe, but would like confirmation that this has been tested properly and the devices are being picked up in the recipe afterwards without saving the node.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, the array of devices is enumerable (i.e. usable) immediately below even after moving node.save to the end of the file. The node.save is only needed to persist the values for subsequent executions of the cookbook.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks!

Itxaka
Itxaka previously approved these changes Aug 2, 2017
Copy link
Member

@Itxaka Itxaka left a comment

Choose a reason for hiding this comment

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

Looks good, just a question and a nit

@Itxaka Itxaka dismissed their stale review August 2, 2017 11:08

Did not meant to approve, only comment

Copy link
Member

@Itxaka Itxaka left a comment

Choose a reason for hiding this comment

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

Looks good, just a question and a nit

@Itxaka Itxaka changed the title Decrease the amount of node saves [4.0] Decrease the amount of node saves Aug 2, 2017
dirty = false

node.set["ceph"]["osd_devices"] ||= []
node.set["ceph"]["osd_devices"] = [] if node["ceph"]["osd_devices"].nil?
Copy link
Contributor

Choose a reason for hiding this comment

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

Same question as is in #103. Why the change?

Fix multiline comment
Fix array initialization (was mistakenly initializing as a hash)
@scottwulf scottwulf force-pushed the decrease-node-save branch from 55455d5 to ad1fbcc Compare August 8, 2017 05:36
@crowbar crowbar deleted a comment from Itxaka Aug 8, 2017
@scottwulf scottwulf merged commit f44bf13 into crowbar:stable/4.0 Aug 9, 2017
@scottwulf scottwulf deleted the decrease-node-save branch August 9, 2017 14:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants