-
Notifications
You must be signed in to change notification settings - Fork 242
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
feat (dns) : Add host.crc.testing
to /etc/hosts
(#4410)
#4471
Conversation
Hi @rohanKanojia. Thanks for your PR. I'm waiting for a crc-org member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
host.crc.testing
to /etc/hosts (#4410)host.crc.testing
to /etc/hosts
(#4410)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apart from this naming issue, looks good to me!
pkg/crc/machine/bundle/metadata.go
Outdated
@@ -112,6 +112,10 @@ func (bundle *CrcBundleInfo) GetAPIHostname() string { | |||
return fmt.Sprintf("api.%s.%s", bundle.ClusterInfo.ClusterName, bundle.ClusterInfo.BaseDomain) | |||
} | |||
|
|||
func (bundle *CrcBundleInfo) GetHostname() string { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The correct (ie consistent with GetAPIHostname
and GetAppHostname
) name for this is GetHostHostname
, which is awkward
Maybe add a more generic GetFQDN(shortName string)
(or GetHostname
- which is consistent with GetAppHostname
) which could be used for example as return GetFQDN("api")
?
+ Add another method `GetFQDN()` to CrcBundleInfo struct in order to provide host domain + Use abovementioned method in `dns.addOpenShiftHosts` in order to add another dns entry for `host.crc.testing` Signed-off-by: Rohan Kumar <[email protected]>
5c91517
to
19205a6
Compare
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cfergeau The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Fixes: Issue #4410
Relates to: Issue #4410
Solution/Idea
Add another entry in
/etc/hosts/
forhost.crc.testing
domain as suggested in #4410 (comment)Proposed changes
GetFQDN(string)
to CrcBundleInfo struct in order to provide host domaindns.addOpenShiftHosts
in order to add another dns entry forhost.crc.testing
Testing
After doing
crc start
you'd notice an additional entry in/etc/hosts
forhost.crc.testing
Old state of
/etc/hosts
New state of
/etc/hosts
(after these changes):