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

[3006.x] Minions check dns when re-connecting to a master #66422

Merged
merged 4 commits into from
Apr 25, 2024

Conversation

dwoz
Copy link
Contributor

@dwoz dwoz commented Apr 23, 2024

What does this PR do?

Make sure minions will check for a changed IP via DNS when the master_alive_interval check has failed.

What issues does this PR fix or reference?

Fixes: #63654

Merge requirements satisfied?

[NOTICE] Bug fixes or features added to Salt require tests.

dwoz added 2 commits April 23, 2024 03:25
Adding tests to validate we check for changing dns anytime we're
disconnected from the currently connected master
Check for a chainging dns record anytime a minion gets disconnected from
it's master. See github issue saltstack#63654 saltstack#61482.
@dwoz dwoz requested a review from a team as a code owner April 23, 2024 10:36
@dwoz dwoz requested review from Akm0d and removed request for a team April 23, 2024 10:36
@salt-project-bot-prod-environment salt-project-bot-prod-environment bot changed the title Minions check dns when re-connecting to a master [3006.x] Minions check dns when re-connecting to a master Apr 23, 2024
@dwoz dwoz changed the title [3006.x] Minions check dns when re-connecting to a master [wip] [3006.x] Minions check dns when re-connecting to a master Apr 23, 2024
Akm0d
Akm0d previously approved these changes Apr 23, 2024
Update docs to use master_alive_interval to detect master ip changes via
DNS.
@dwoz dwoz changed the title [wip] [3006.x] Minions check dns when re-connecting to a master [3006.x] Minions check dns when re-connecting to a master Apr 23, 2024
@dwoz dwoz added the test:full Run the full test suite label Apr 23, 2024
Comment on lines +2846 to +2853
try:
master, self.pub_channel = yield self.eval_master(
opts=self.opts,
failed=True,
failback=tag.startswith(master_event(type="failback")),
)
except SaltClientError:
pass
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
try:
master, self.pub_channel = yield self.eval_master(
opts=self.opts,
failed=True,
failback=tag.startswith(master_event(type="failback")),
)
except SaltClientError:
pass
with contextlib.suppress(SaltClientError):
master, self.pub_channel = yield self.eval_master(
opts=self.opts,
failed=True,
failback=tag.startswith(master_event(type="failback")),
)

https://docs.python.org/3/library/contextlib.html#contextlib.suppress

Although, not a blocker for merging the PR

Copy link
Contributor

@dmurphy18 dmurphy18 left a comment

Choose a reason for hiding this comment

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

Agree with Pedro change request for contextlib usage

@dwoz dwoz merged commit eedcf49 into saltstack:3006.x Apr 25, 2024
276 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test:full Run the full test suite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants