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

[WIP] Add a foreman subnet type #111

Closed
wants to merge 1 commit into from

Conversation

ekohl
Copy link
Member

@ekohl ekohl commented Sep 13, 2013

By adding a foreman subnet type, you can ensure a subnet exists in your
foreman instance.

@ekohl
Copy link
Member Author

ekohl commented Sep 13, 2013

This is still a very WIP, untested, will eat your dog and cat so be warned. Currently I'm just pushing new commits when I've added some code and will squash when done, including tests. And feedback is welcome.

end

def exists?
id != nil
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick use .nil? or just !!subnet

Copy link
Member Author

Choose a reason for hiding this comment

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

Please do nitpick. I think I copied this from the smartproxy provider and am still very new to ruby.

@domcleal
Copy link
Contributor

Looks sane to me.

end

newproperty(:dhcp_proxy) do
desc 'The DHCP proxy that serves this subnet. Should be the name of the smartproxy.'
Copy link
Contributor

Choose a reason for hiding this comment

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

We could also accept the foreman_smartproxy registration resource here.

Copy link
Member Author

Choose a reason for hiding this comment

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

I was thinking exactly the same thing. Maybe it would be even better to only accept that resource to abstract away the mapping. Same thing for domains, since you need to domain to be present as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

True, that would work well. The user doesn't need to do the auto-registration either, they could just define a foreman_smartproxy resource without the ensure property.

By adding a foreman subnet type, you can ensure a subnet exists in your
foreman instance.

TODO:

[ ] Allow clearing of domains
[ ] Flush all values at once
@ekohl
Copy link
Member Author

ekohl commented Feb 4, 2014

Rebased, done some testing. Still work in progress, but the basics work.

@ohadlevy
Copy link
Member

ohadlevy commented Apr 2, 2014

@ekohl what would it take to use create_resource (or the like) to create the subnet in foreman once the installer is running with dhcp? bonus points for setting up the relationship between the subnet and the proxy. thanks!

@ekohl
Copy link
Member Author

ekohl commented Apr 2, 2014

@ohadlevy that works. What needs to be done is flushing the resource so it does one update API call instead of one per parameter. This fixes the bug where you can't assign from and to on an existing subnet because that needs to be done in a single API call due to constraints. It would also make it more efficient.

@ekohl
Copy link
Member Author

ekohl commented Apr 2, 2014

@ohadlevy and I also see in the commit message that clearing the related domain does not work.

@ares
Copy link
Member

ares commented Apr 2, 2014

This seems as part of something we are trying to create in https://github.com/theforeman/staypuft-installer (see hooks/lib/provisioning_seeder.rb). Does it make sense to somehow merge our efforts? Is it a benefit to have this as puppet type?

@ekohl
Copy link
Member Author

ekohl commented Apr 2, 2014

@ares I'd say so. We already have the smartproxy type so it'd make sense to have all foreman types.

@ekohl ekohl mentioned this pull request Jan 31, 2015
2 tasks
@ekohl
Copy link
Member Author

ekohl commented Jan 31, 2015

Replaced by #280

@ekohl ekohl closed this Jan 31, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants