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
Fixes #16439 - Don't fail recreate if DNS is ok #3812
Conversation
@@ -35,7 +35,7 @@ def dns_feasible?(type) | |||
end | |||
|
|||
def recreate_dns_record(type) | |||
set_dns_record(type) unless dns_record(type).nil? || dns_record(type).valid? | |||
dns_record(type).nil? || dns_record(type).valid? ? true : set_dns_record(type) |
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.
I think, a guard clause would make this more readable.
def recreate_dns_record(type)
return true if dns_record(type).nil? || dns_record(type).valid?
set_dns_record(type)
end
@agx: Thanks for the patch. It looks good to me. Could you just add a tiny unit test for this? |
|
||
context 'dns not feasible' do | ||
test 'should not fail dns rebuild' do | ||
Nic::Managed.any_instance.stubs(:dns_feasible?).returns(false) |
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.
Use 2 (not 3) spaces for indentation.
@timogoebel test added and code changed as suggested |
If rebuilding a host and DNS is not "feasible" we don't want to fail that host but rather recreate what's currently feasible. If a DNS record is still valid we don't want to fail that host but rather take the still valid entry.
[test foreman] |
@agx: Looks good, thank you. Test results are unrelated. |
If rebuilding a host and DNS is not "feasible" we don't want to fail
that host but rather recreate what's currently feasible.
If a DNS record is still valid we don't want to fail that host but
rather take the still valid entry.