Recover from default bridge init failure#49292
Merged
thaJeztah merged 1 commit intomoby:masterfrom Jan 18, 2025
Merged
Conversation
2b69b86 to
3aac22d
Compare
Signed-off-by: Rob Murray <[email protected]>
3aac22d to
22c0221
Compare
thaJeztah
approved these changes
Jan 17, 2025
Comment on lines
+846
to
+855
| // SetEnvVar updates the set of extra env variables for the daemon, to take | ||
| // effect on the next start/restart. | ||
| func (d *Daemon) SetEnvVar(name, val string) { | ||
| prefix := name + "=" | ||
| if idx := slices.IndexFunc(d.extraEnv, func(ev string) bool { return strings.HasPrefix(ev, prefix) }); idx > 0 { | ||
| d.extraEnv[idx] = prefix + val | ||
| return | ||
| } | ||
| d.extraEnv = append(d.extraEnv, prefix+val) | ||
| } |
Member
There was a problem hiding this comment.
This looks sane for now, but I went looking "didn't we already have an options for this?", but that can only be used on daemon.New();
d := daemon.New(t, daemon.WithEnvVars(...)and daemon.Start() doesn't take options as argument; perhaps we should consider making it accept Options, so that we can customise (some at least) daemon settings between restarts without having to create a new daemon.
Or perhaps a StartWithOptions() if we don't want to replace the existing one (which takes command-line arguments only)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
- What I did
If a bridge network can't be restored from the store on startup, then it's deleted, the daemon and bridge driver get out of step. If that happens to the default bridge, the daemon can't recover, it won't start again.
See the issue for more detailed description/analysis.
The ipvlan/macvlan/overlay network drivers don't seem to be affected by an equivalent issue.
This is a very long standing issue, but perhaps provoked by the checks on kernel module loading added in 27.x. (The version label on the PR is arbitrary, just something-old.)
- How I did it
When deleting a network the bridge driver doesn't know about, make sure it's not still lurking in the data store.
- How to verify it
New tests.
- Description for the changelog
- Fixed an issue that could persistently prevent daemon startup after failure to initialize the default bridge.