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

Pass a string to mount_point_exists? method #286

Merged
merged 1 commit into from
Oct 16, 2017

Conversation

carbonin
Copy link
Member

This method will return a false negative if passed a Pathname object, which is what we are doing currently.

example from an appliance:

irb(main):012:0> LinuxAdmin::LogicalVolume.mount_point_exists?(Pathname.new(ENV.fetch("APPLIANCE_PG_MOUNT_POINT")))
=> false
irb(main):013:0> LinuxAdmin::LogicalVolume.mount_point_exists?(Pathname.new(ENV.fetch("APPLIANCE_PG_MOUNT_POINT")).to_s)
=> true

Broken as of #277

Appliance builds which include this change will fail with the following output from journalctl -u appliance-initialize:

[root@localhost vmdb]# journalctl -u appliance-initialize
-- Logs begin at Mon 2017-10-16 08:53:57 EDT, end at Mon 2017-10-16 13:01:07 EDT. --
Oct 16 12:54:06 localhost.localdomain systemd[1]: Starting Initialize Appliance Database...
Oct 16 12:54:08 localhost.localdomain appliance-initialize.sh[1305]: TERM environment variable not set.
Oct 16 12:54:08 localhost.localdomain appliance-initialize.sh[1305]: create encryption key
Oct 16 12:54:08 localhost.localdomain appliance-initialize.sh[1305]: configuring internal database
Oct 16 12:54:08 localhost.localdomain appliance-initialize.sh[1305]: The disk for database must be a mount point
Oct 16 12:54:08 localhost.localdomain appliance-initialize.sh[1305]: Failed to configure internal database
Oct 16 12:54:08 localhost.localdomain systemctl[1576]: Removed symlink /etc/systemd/system/multi-user.target.wants/appliance-initialize.service.
Oct 16 12:54:08 localhost.localdomain systemd[1]: Started Initialize Appliance Database.

/cc @ailisp

This method will register a false negative if passed a Pathname
object, which is what we are doing currently.
@miq-bot
Copy link
Member

miq-bot commented Oct 16, 2017

Checked commit carbonin@eb7df40 with ruby 2.3.3, rubocop 0.47.1, and haml-lint 0.20.0
1 file checked, 0 offenses detected
Everything looks fine. 🍰

@Fryguy
Copy link
Member

Fryguy commented Oct 16, 2017

@carbonin I will merge as is, but we should also change LinuxAdmin to support both pathname and strings for the incoming parameter.

@Fryguy Fryguy merged commit 6f356fe into ManageIQ:master Oct 16, 2017
@Fryguy Fryguy added this to the Sprint 71 Ending Oct 16, 2017 milestone Oct 16, 2017
@carbonin carbonin deleted the pass_string_to_mount_point_exists branch October 16, 2017 20:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants