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

fix: return an error if custom bridge name is not set successfully #100

Merged

Conversation

swagatbora90
Copy link
Contributor

Issue #, if available:
Addressing a comment from the PR #69 (comment)

Description of changes:

If the option to configure custom bridge name is set, finch-daemon will try to set the bridge name by updating the CNI conflist of the network. However, on failure, it will simply return a warning instead of failing. This change returns an explicit error if an error is encountered.

The created Network will be removed as part of a defer cleanup in Create.

Testing done:

  • I've reviewed the guidance in CONTRIBUTING.md

License Acceptance

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@swagatbora90 swagatbora90 force-pushed the return_err_on_set_bridgename branch from 5eed7a5 to d028402 Compare November 18, 2024 21:06
@swagatbora90 swagatbora90 changed the title return an error if custom bridge name is not set successfully fix: return an error if custom bridge name is not set successfully Nov 18, 2024
@swagatbora90 swagatbora90 force-pushed the return_err_on_set_bridgename branch from d028402 to 468acbc Compare November 18, 2024 23:08
Shubhranshu153
Shubhranshu153 previously approved these changes Nov 18, 2024
pendo324
pendo324 previously approved these changes Nov 18, 2024
@Shubhranshu153 Shubhranshu153 dismissed stale reviews from pendo324 and themself November 19, 2024 00:08

The merge-base changed after approval.

@swagatbora90 swagatbora90 merged commit 0469999 into runfinch:main Nov 19, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants