Skip to content
This repository has been archived by the owner on Nov 11, 2021. It is now read-only.

Debian: avoid misconfigured interfaces on up/down script errors #312

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

zajdee
Copy link

@zajdee zajdee commented May 5, 2020

In Debian, if you bring up dual-stack interfaces, the pre/post-up/down
scripts are run on the configuration of each stack. I.e. if you bring up
the IPv4 config on an interface, the ifupdown scripts launch scripts in
the post-up directory. These scripts are common for IPv4 and IPv6 in current
version of puppet-network. At the moment IPv6 is not yet configured, but
IPv6 commands are called - and fail. If an IPv6 command (e.g. ip -6 route add)
is the last one in the post-up script, the overall post-up script fails.

When the post-up script fails when bringing up the IPv4 configuration,
ifupdown no longer attempts to bring up the IPv6 configuration on that
interface and leaves the interface misconfigured (IPv4 is configured,
IPv6 is not).

This situation can be avoided by adding the exit 0 command at the very
end of the script files. It is okay if the IPv6 commands in the common script
fail when only IPv4 is brought up, as ifupdown will re-run them after
IPv6 is brought up.

Before submitting your PR

  1. Open an issue and refer to its number in your PR title
  2. If it's a bug and you have the solution, go on with the PR!
  3. If it's an enhancement, please wait for our feedback before starting to work on it
  4. Please run puppet-lint on your code and ensure it's compliant

After submitting your PR

  1. Verify Travis checks and eventually fix the errors
  2. Feel free to ping us if we don't reply promptly

In Debian, if you bring up dual-stack interfaces, the pre/post-up/down
scripts are run on the configuration of each stack. I.e. if you bring up
the IPv4 config on an interface, the ifupdown scripts launch scripts in
the post-up directory. These scripts are common for IPv4 and IPv6 in current
version of puppet-network. At the moment IPv6 is not yet configured, but
IPv6 commands are called - and fail. If an IPv6 command (e.g. `ip -6 route add`)
is the last one in the post-up script, the overall post-up script fails.

When the post-up script fails when bringing up the IPv4 configuration,
ifupdown no longer attempts to bring up the IPv6 configuration on that
interface and leaves the interface misconfigured (IPv4 is configured,
IPv6 is not).

This situation can be avoided by adding the `exit 0` command at the very
end of the script files. It is okay if the IPv6 commands in the common script
fail when only IPv4 is brought up, as ifupdown will re-run them after
IPv6 is brought up.
@@ -10,3 +10,5 @@ if [ "$IFACE" = "<%= @interface -%>" ] || [ "$IFACE" = "--all" ]; then
fi
<%- end -%>
fi
# non-zero exit code causes ifupdown to leave the interface partly configured, which may result in connectivity loss
Copy link
Member

Choose a reason for hiding this comment

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

Similarly to the above I'd prefer to add a parameter to the route define like $clean_exit_on_failure , undef by default. to allow users to decide the intended behaviour, then , possibly, in the define code set a variable like $real_clean_exit_on_failure than, in case the parameter is not set tries to do the sanest thing (ie set to true if ipv4 is not configured but ipv6 is) and then, if this $real_clean_exit_on_failure is true, then add the above 2 lines otherwise template remains as before

@@ -10,3 +10,5 @@ if [ "$IFACE" = "<%= @interface -%>" ] || [ "$IFACE" = "--all" ]; then
fi
<%- end -%>
fi
# non-zero exit code causes ifupdown to leave the interface partly configured, which may result in connectivity loss
Copy link
Member

Choose a reason for hiding this comment

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

Same as for the route define.

@alvagante
Copy link
Member

@zajdee thank you very much for the PR, I have added some comments to have changes on the files (and relevant network restarts) only when user wants, or in conditions which can't work, would you add these to the PR?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants