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

Feature/adding fqdn #7

Merged
merged 2 commits into from
Oct 11, 2015
Merged

Feature/adding fqdn #7

merged 2 commits into from
Oct 11, 2015

Conversation

faja
Copy link
Contributor

@faja faja commented Oct 6, 2015

Hi,

the nodes provisioner is really great, but currently it doesn't support fqdn attribute.
This pull request simply adds automatic.fqdn attribute based on output of hostname -f command.

What do you think?

@@ -53,6 +57,19 @@ def ipaddress
state[:hostname]
end

def fqdn
state = create_state
Copy link
Owner

Choose a reason for hiding this comment

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

we definitely need to refactor ip_finder to handle other attributes. This would break windows.

@mwrock
Copy link
Owner

mwrock commented Oct 8, 2015

Thanks for this contribution! Before releasing, the transport stuff around what is now ip_finder needs to accommodate other node attributes to make shelling out to the test instance easy. If you want to tackle that, thats great but I totally understand if you dont have cycles/desire. If not, I'm happy to merge this in and then refactor that out.

@mwrock
Copy link
Owner

mwrock commented Oct 9, 2015

Just an FYI that I have moved forward with the refactorings.I'm also adding a kitchen.yml and test cookbook so this gem can have integration tests for linux and windows

@mwrock
Copy link
Owner

mwrock commented Oct 11, 2015

Any reason why you are filtering fqdn with [/(\w|\.)+/]?

This transforms a kitchn node's node1-ubuntu-1204 to node1

@mwrock mwrock merged commit 8c9b043 into mwrock:master Oct 11, 2015
@faja
Copy link
Contributor Author

faja commented Oct 12, 2015

Hi Matt,

thx for merging, and fixing my bad code:)
with [/(\w|\.)+/] I basically wanted to strip new line, .chomp is much better here.

Cheers

@mwrock
Copy link
Owner

mwrock commented Oct 12, 2015

Oh thats cool. Just wanted to make sure there was not a detail I was missing that would break you. Thanks a bunch for the PR!!

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.

2 participants