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 bridge IP source after NI modification #4258

Merged
merged 1 commit into from
Sep 16, 2024

Conversation

milan-zededa
Copy link
Contributor

There is a small bug in the master recently introduced, where the address source of the reported bridge IP changes from AddressSourceEVEInternal to AddressSourceUndefined when NI config is modified in any way (note that IP source is reported only internally between microservices, not to the controller).
While the reported source of the bridge IP is not important at all, we should still fix it to avoid any confusion.
Please note that this is already fixed in the backport PRs #4257 and #4256

There is a small bug in the master recently introduced, where
the address source of the reported bridge IP changes from
AddressSourceEVEInternal to AddressSourceUndefined when NI config
is modified in any way (note that IP source is reported only internally
between microservices, not to the controller).
While the reported source of the bridge IP is not important at all,
we should still fix it to avoid any confusion.

Signed-off-by: Milan Lenco <milan@zededa.com>
@OhmSpectator
Copy link
Member

How will it affect the backports of the original PR into the stable branches? Will you include the fix on top of the original commit, or will you change the commits? If the latest, could you please mention it in the commit messages,

@milan-zededa
Copy link
Contributor Author

How will it affect the backports of the original PR into the stable branches? Will you include the fix on top of the original commit, or will you change the commits? If the latest, could you please mention it in the commit messages,

I mentioned it in the PR description. Not sure if I want to link github PRs from the commit message.

@OhmSpectator
Copy link
Member

I mentioned it in the PR description. Not sure if I want to link github PRs from the commit message.

Sorrry, I overlooked it. I mean, it would be nice to reflect in the commit message of the cherry-picked commit the difference with the original commit. But without it is also fine.

Copy link
Contributor

@eriknordmark eriknordmark left a comment

Choose a reason for hiding this comment

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

LGTM

You said this is just to avoid confusion, but isn't there some code which looks at the source field?

@milan-zededa
Copy link
Contributor Author

milan-zededa commented Sep 16, 2024

LGTM

You said this is just to avoid confusion, but isn't there some code which looks at the source field?

Right now it is used only internally by LinuxCollector, but I added this to NetworkInstanceStatus for troubleshooting purposes.

@OhmSpectator OhmSpectator merged commit 4732410 into lf-edge:master Sep 16, 2024
43 of 44 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