Skip to content

add -f tag to config reload #388

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

Draft
wants to merge 21 commits into
base: master
Choose a base branch
from
Draft

add -f tag to config reload #388

wants to merge 21 commits into from

Conversation

Honigeintopf
Copy link
Contributor

@Honigeintopf Honigeintopf commented Mar 3, 2025

Description

When the swss is broken the config reload is not being run. The -f tag skips the check for it.

@Honigeintopf Honigeintopf requested a review from a team as a code owner March 3, 2025 14:20
@Honigeintopf Honigeintopf marked this pull request as draft March 3, 2025 14:20
@Honigeintopf Honigeintopf changed the title change order of notify add -f tag to config reload Mar 4, 2025
@@ -8,7 +8,7 @@
ansible.builtin.command: config load --yes

- name: config reload
ansible.builtin.command: config reload --yes
ansible.builtin.command: config reload --yes -f
Copy link
Contributor

@Gerrit91 Gerrit91 Mar 4, 2025

Choose a reason for hiding this comment

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

Shouldn't this be an optional behavior? Maybe sometimes people do not want to force this. 😅

Copy link
Contributor Author

Choose a reason for hiding this comment

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

One could argue that a config reload should always restart, even if the switch is broken, since it’s not a failure if the switch isn’t working anyway.

Copy link
Contributor

Choose a reason for hiding this comment

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

if I recall this correctly, a force config reload might result in some network disruption.
But correct me if I'm wrong

Copy link
Contributor

Choose a reason for hiding this comment

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

If I remember correctly a reload always interrupts the network. But maybe you do not want a restart if the SWSS is broken because the current state requires investigation before forcing a configuration change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes a config reload disrupts network, it disrupts it with or without the -f .

Copy link
Contributor Author

Choose a reason for hiding this comment

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

PR is still in draft, using this for certain testing functionalities right now.
When finished I will put it out.

Copy link
Contributor Author

@Honigeintopf Honigeintopf Mar 5, 2025

Choose a reason for hiding this comment

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

If I remember correctly a reload always interrupts the network. But maybe you do not want a restart if the SWSS is broken because the current state requires investigation before forcing a configuration change.

Also the problem is I think with config reload, it "restarts" it and isn't returning an error, even if the swss is broken.
I could be wrong tho.
That's the main reason I'm adding the -f tag.

Copy link
Contributor

Choose a reason for hiding this comment

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

Have you tried a force reload when the swss is down? I can't imagine a force reload fixing the situation where a regular reload is not possible. Usually it means there's something wrong with the config, which will instantly lead to a crash again after reload (and even after reboot).

@github-project-automation github-project-automation bot moved this to Review in Development Jun 5, 2025
@Gerrit91 Gerrit91 removed the status in Development Jun 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

4 participants