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

ovn: fix more missing ovs binaries #378038

Merged
merged 1 commit into from
Jan 30, 2025
Merged

Conversation

positiveEV
Copy link
Contributor

Hi @adamcstephens,

Here are some additional changes for the ovn package:

  • Added --with-dbdir=/var/lib/openvswitch to configure OVS, as it is the traditional location for OVS DB. In the Open vSwitch module, it is set as /var/db/openvswitch. I guess that we can do the same for consistency, if you think it is better.
  • Moved the OVS binaries to $out/bin to match the file tree of the Open vSwitch package. This also helps with running the ovn-ctl and ovs-ctl scripts.
  • Added more OVS binaries needed by ovs-ctl.
  • We have to keep the system-id.conf file between reboots and package upgrades, so I set the sysconfig directory to var/lib because it seems to me that nixpkgs should not create files in /etc.

These changes allowed me to successfully set up Incus with a standalone OVN network. I was able to create an OVN network, attach a container to it, and ping the internet. To do so, I used this Nix module that I wrote: nixos-ovn-module. This module creates OVS services, as I unfortunately was not able to fix the OVS build in the OVN package to use the NixOS Open vSwitch module.
For reference the issue is the lack of git submodule awareness in nix-build, which does not seem to provide a way to set the variables we need to pass to the OVS autoconf. This results in multiple path issues in the OVS binaries that prevent me from using the Open vSwitch module with the OVN package

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 25.05 Release Notes (or backporting 24.11 and 25.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@github-actions github-actions bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1 10.rebuild-linux: 1-10 labels Jan 30, 2025
@nix-owners nix-owners bot requested a review from adamcstephens January 30, 2025 16:01
@adamcstephens adamcstephens merged commit 234a9c1 into NixOS:master Jan 30, 2025
24 of 27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 10.rebuild-linux: 1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants