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 --domain option #63

Merged
merged 1 commit into from
Dec 8, 2022
Merged

Implement --domain option #63

merged 1 commit into from
Dec 8, 2022

Conversation

bastelfreak
Copy link
Member

Previously, the domain example.com was hardcoded for the beaker instances. This is now configureable.

@bastelfreak bastelfreak added the enhancement New feature or request label Dec 8, 2022
@bastelfreak bastelfreak requested a review from ekohl December 8, 2022 12:12
@bastelfreak bastelfreak self-assigned this Dec 8, 2022
@bastelfreak
Copy link
Member Author

bastelfreak commented Dec 8, 2022

This is related to voxpupuli/beaker-hostgenerator#261 (comment).

This now adds a bunch of example.com strings into the method headers and I'm not really happy about it. Should we get rid of this default value?

@bastelfreak bastelfreak force-pushed the fqdn branch 3 times, most recently from ed4768e to 7662bfb Compare December 8, 2022 12:18
Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

Looking at this I'm wondering if we should make domain an Optional[String] and drop use_fqdn. This could map nicely to where we specify the domain as an option to beaker-hostgenerator and stop overriding the hostname.

For compatibility I'd say that it should append domain if it's set, otherwise fall back to use_fqdn. So roughly

def os_release_to_setfile(os, release, use_fqdn: false, pidfile_workaround: false, domain: nil)
  domain ||= 'example.com' if use_fqdn
  options = {}
  options[:hostname] = "#{name}.#{domain}" if domain
end

@bastelfreak
Copy link
Member Author

okay it was clearly too early when I wrote that code.

@bastelfreak bastelfreak force-pushed the fqdn branch 2 times, most recently from 59500f6 to 93d260c Compare December 8, 2022 12:50
@bastelfreak
Copy link
Member Author

@ekohl I implemented your suggestion

@bastelfreak bastelfreak force-pushed the fqdn branch 2 times, most recently from 97772d4 to 76da92f Compare December 8, 2022 12:58
lib/puppet_metadata/beaker.rb Outdated Show resolved Hide resolved
lib/puppet_metadata/github_actions.rb Outdated Show resolved Hide resolved
lib/puppet_metadata/metadata.rb Outdated Show resolved Hide resolved
lib/puppet_metadata/metadata.rb Outdated Show resolved Hide resolved
@bastelfreak bastelfreak force-pushed the fqdn branch 3 times, most recently from 2be03fc to 3f12367 Compare December 8, 2022 13:09
Previously, the domain `example.com` was hardcoded for the beaker
instances. This is now configureable.
@bastelfreak
Copy link
Member Author

@ekohl, this looks good to me. Are you fine with merging it?

@ekohl ekohl merged commit d118c7d into voxpupuli:master Dec 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants