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.

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

Implement Ceph multiple pools and tests #167

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
7 changes: 5 additions & 2 deletions lib/fog/libvirt/models/compute/server.rb
Original file line number Diff line number Diff line change
Expand Up @@ -458,7 +458,7 @@

args = {}

valid_keys = ["monitor", "port", "libvirt_ceph_pools", "auth_username", "auth_uuid", "bus_type"]
valid_keys = ["monitor", "port", "libvirt_ceph_pools", "libvirt_ceph_pool", "auth_username", "auth_uuid", "bus_type"]

Check warning on line 461 in lib/fog/libvirt/models/compute/server.rb

View workflow job for this annotation

GitHub Actions / runner / rubocop

[rubocop] reported by reviewdog 🐶 Use `%w` or `%W` for an array of words. Raw Output: lib/fog/libvirt/models/compute/server.rb:461:24: C: Style/WordArray: Use `%w` or `%W` for an array of words.
array_values = ["monitor", "libvirt_ceph_pools"]

File.readlines(path).each do |line|
Expand All @@ -470,7 +470,10 @@
end
end

puts args
if args.has_key?("libvirt_ceph_pool")

Check warning on line 473 in lib/fog/libvirt/models/compute/server.rb

View workflow job for this annotation

GitHub Actions / runner / rubocop

[rubocop] reported by reviewdog 🐶 Use `Hash#key?` instead of `Hash#has_key?`. Raw Output: lib/fog/libvirt/models/compute/server.rb:473:19: C: Style/PreferredHashMethods: Use `Hash#key?` instead of `Hash#has_key?`.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd be tempted to only read libvirt_ceph_pool if libvirt_ceph_pools isn't set. The presence of libvirt_ceph_pools IMHO indicates a user has migrated to the newer format. Then only read libvirt_ceph_pool as a fallback. Would you agree?

Copy link
Author

Choose a reason for hiding this comment

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

Okay. I think it's more of a transitive way of managing both at the same time. But in the end, you're right.

args.has_key?("libvirt_ceph_pools") ? args["libvirt_ceph_pools"] << args["libvirt_ceph_pool"] : args["libvirt_ceph_pools"] = [args["libvirt_ceph_pool"]]

Check warning on line 474 in lib/fog/libvirt/models/compute/server.rb

View workflow job for this annotation

GitHub Actions / runner / rubocop

[rubocop] reported by reviewdog 🐶 Use `Hash#key?` instead of `Hash#has_key?`. Raw Output: lib/fog/libvirt/models/compute/server.rb:474:18: C: Style/PreferredHashMethods: Use `Hash#key?` instead of `Hash#has_key?`.
args.delete("libvirt_ceph_pool")
end

args
end
Expand Down
15 changes: 7 additions & 8 deletions tests/libvirt/models/compute/server_tests.rb
Original file line number Diff line number Diff line change
@@ -1,25 +1,22 @@
RealFile = File
class FakeFile < RealFile
def self.file?(path)
if path == '/etc/foreman/ceph.conf'
return true
else
return RealFile.file(path)
end
path == '/etc/foreman/ceph.conf' || RealFile.file(path)
end

def self.readlines(path)
if path == '/etc/foreman/ceph.conf'

Check warning on line 8 in tests/libvirt/models/compute/server_tests.rb

View workflow job for this annotation

GitHub Actions / runner / rubocop

[rubocop] reported by reviewdog 🐶 Use a guard clause (`return RealFile.readlines(path) unless path == '/etc/foreman/ceph.conf'`) instead of wrapping the code inside a conditional expression. Raw Output: tests/libvirt/models/compute/server_tests.rb:8:5: C: Style/GuardClause: Use a guard clause (`return RealFile.readlines(path) unless path == '/etc/foreman/ceph.conf'`) instead of wrapping the code inside a conditional expression.
return [

Check warning on line 9 in tests/libvirt/models/compute/server_tests.rb

View workflow job for this annotation

GitHub Actions / runner / rubocop

[rubocop] reported by reviewdog 🐶 Redundant `return` detected. Raw Output: tests/libvirt/models/compute/server_tests.rb:9:7: C: Style/RedundantReturn: Redundant `return` detected.
"monitor=mon001.example.com,mon002.example.com,mon003.example.com",
"port=6789",
"libvirt_ceph_pools=rbd_pool_name,second_rbd_pool_name",
"libvirt_ceph_pool=third_rbd_pool_name",
"auth_username=libvirt",
"auth_uuid=uuid_of_libvirt_secret",
"bus_type=virtio"
]
else
return RealFile.readlines(path)

Check warning on line 19 in tests/libvirt/models/compute/server_tests.rb

View workflow job for this annotation

GitHub Actions / runner / rubocop

[rubocop] reported by reviewdog 🐶 Redundant `return` detected. Raw Output: tests/libvirt/models/compute/server_tests.rb:19:7: C: Style/RedundantReturn: Redundant `return` detected.
end
end
end
Expand Down Expand Up @@ -134,20 +131,22 @@
{
:nics => [],
:volumes => [
Fog::Libvirt::Compute::Volume.new({ :path => "rbd_pool_name/block-1", :pool_name => "rbd_pool_name" })
Fog::Libvirt::Compute::Volume.new({ :path => "rbd_pool_name/block-1", :pool_name => "rbd_pool_name" }),
Fog::Libvirt::Compute::Volume.new({ :path => "third_rbd_pool_name/block-2", :pool_name => "third_rbd_pool_name" })
]
}
)

Check warning on line 139 in tests/libvirt/models/compute/server_tests.rb

View workflow job for this annotation

GitHub Actions / runner / rubocop

[rubocop] reported by reviewdog 🐶 Trailing whitespace detected. Raw Output: tests/libvirt/models/compute/server_tests.rb:139:1: C: Layout/TrailingWhitespace: Trailing whitespace detected.
xml = server.to_xml

network_disk = xml.match?(/<disk type="network" device="disk">/)
mon_host = xml.match?(%r{<host name="mon001.example.com" port="6789"/>})
source_rbd = xml.match?(%r{<source protocol="rbd" name="rbd_pool_name/block-1">})
source_block1_rbd = xml.match?(%r{<source protocol="rbd" name="rbd_pool_name/block-1">})
source_block2_rbd = xml.match?(%r{<source protocol="rbd" name="third_rbd_pool_name/block-2">})
auth_username = xml.match?(/<auth username="libvirt">/)
auth_secret = xml.match?(%r{<secret type="ceph" uuid="uuid_of_libvirt_secret"/>})

network_disk && mon_host && source_rbd && auth_username && auth_secret
network_disk && mon_host && source_block1_rbd && source_block2_rbd && auth_username && auth_secret
end
test("with q35 machine type on x86_64") { server.to_xml.match?(%r{<type arch="x86_64" machine="q35">hvm</type>}) }
end
Expand Down
Loading