Openstack production setup by pkr4711 · Pull Request #34 · cobaltcore-dev/openstack-nix

pkr4711 · 2026-09-14T09:27:01Z

This PR contains changes for a production setup of Openstack.

The default settings for the CI/CD setup should not be changed.
For the production or long living setup we introduces a new nix option:

openstack.production_setup = false

This option should be set to true if you use this code outside of a CI/CD pipeline.

Signed-off-by: Paul Kroeher <paul.kroeher@cyberus-technology.de> On-behalf-of: SAP paul.kroeher@sap.com

Copilot

🔵 Needs a closer look

Unresolved critical and moderate issues affect database initialization, storage provisioning, service gating, networking, and DNS integration.

Pull request overview

This pull request adds production-oriented OpenStack configuration, Designate/Knot DNS integration, and configurable controller, networking, and storage setup.

Changes:

Adds Designate, Infoblox, and Knot packages and backend integration.
Introduces openstack.production_setup and production uWSGI service paths.
Updates controller, compute, networking, host, and Cinder storage configuration.

File summaries

File	Reviewed change	Final review notes
`packages/python-openstackclient.nix`	Adds Designate client support.	No final findings.
`packages/infoblox-client.nix`	Adds Infoblox client packaging.	Nit, 1 vote (L41): disabled `stestr` checks leave the new dependency untested.
`packages/designate.nix`	Packages Designate with Knot backend support.	No final findings.
`packages/designate-knot3-backend.py`	Implements remote Knot zone management.	Nit, 1 vote (L31): the new backend is untested.
`packages/default.nix`	Registers new packages.	No final findings.
`modules/storage/cinder-storage-node.nix`	Adds production storage options and setup scripts.	Critical, 1 vote (L224): the setup hook is unsafe for LVM backends. Critical, 2 votes (L134, L136): setup is not fail-fast. Moderate, 2 votes (L140), 1 vote (L126): `storagePath` is inconsistent with mounts and exports. Moderate, 3 votes (L346): disabling the module can still enable NFS.
`modules/knot-designate.nix`	Configures an external Knot secondary service.	No final findings.
`modules/generic/global-options.nix`	Adds global production and networking options.	Nit, 1 vote (L12): production mode lacks integration coverage.
`modules/generic/controller-host-entry.nix`	Adds configurable controller hostname and IP defaults.	Moderate, 1 vote (L7): the configured hostname is not added to the hosts entry.
`modules/default.nix`	Exposes new modules and packages.	No final findings.
`modules/controller/placement.nix`	Adds production uWSGI support.	No final findings.
`modules/controller/openstack-controller.nix`	Adds setup scripts and production gating.	Critical, 3 votes (L98): the SQL string is unterminated. Moderate, 2 votes (L49, L52, L294): setup does not fail fast, default CI database reset behavior changed, and the Glance diagnostic checks the wrong unit. Nit, 1 vote (L342): production mode lacks integration coverage.
`modules/controller/nova.nix`	Makes Nova controller settings configurable.	No final findings.
`modules/controller/neutron.nix`	Updates controller networking configuration.	Moderate, 3 votes (L175): disabling Neutron leaves networking and support configuration active.
`modules/controller/keystone.nix`	Updates catalog endpoints and environment options.	Moderate, 2 votes (L48, L90): Designate endpoints are advertised even when Designate is disabled.
`modules/controller/horizon.nix`	Adds uWSGI environment configuration.	No final findings.
`modules/controller/glance.nix`	Updates Glance paths and environment configuration.	Nit, 1 vote (L70): the option description names the wrong vassal.
`modules/controller/designate.nix`	Adds Designate services and pool configuration.	Moderate, 3 votes (L77): the Knot target is not selected. Moderate, 1 vote (L216, L58): TCP 9001 is not opened, and default DNS names are not configured.
`modules/controller/cinder.nix`	Adds production Cinder API handling.	Moderate, 2 votes (L63): `$PATH` is passed literally to uWSGI.
`modules/compute/nova.nix`	Updates compute host configuration.	Moderate, 2 votes (L149): libvirt and support configuration remain active when Nova is disabled.
`modules/compute/neutron.nix`	Adds Open vSwitch preparation.	Moderate, 3 votes (L69, L113): existing provider bridges are not repaired when the provider port is missing.
`modules/compute/compute.nix`	Imports shared global options.	No final findings.
`flake.nix`	Passes `libredirect` to package definitions.	No final findings.

Review details

Suppressed comments (12)

modules/compute/neutron.nix:113

The unconditional module body still enables virtualisation.vswitch (and its related setup) even when neutron.enable = false; only the agent units were given an enable flag. Disabling this module therefore still starts Open vSwitch and configures Neutron users/files. Keep the module body behind the existing enable guard or gate every side effect.

  config = {

modules/controller/designate.nix:216

Designate API listens on 0.0.0.0:9001 and the Keystone catalog advertises that endpoint, but this module only opens TCP/UDP 5354. With the NixOS firewall enabled, API clients cannot reach Designate; include TCP 9001 in the allowed ports.

    networking.firewall.allowedTCPPorts = [ 5354 ];

modules/controller/designate.nix:62

The default ns_records publish osdns.openstack.test. and controller.openstack.test., but the modules configure neither hostname (the host entry only adds controller/controller.local, and the Knot address is an IP option). Enabling Designate with the default pools therefore advertises names that are not resolvable; make these names configurable or derive them from the configured hosts.

      ns_records:
        - hostname: osdns.openstack.test.
          priority: 2
        - hostname: controller.openstack.test.
          priority: 1

modules/controller/glance.nix:70

This option belongs to the Glance vassal, but its description says environment variables are passed to the Keystone vassal. That misleads users configuring the new option; update the service name in the description.

        Environment variables passed to the Keystone uWSGI vassal.

modules/controller/keystone.nix:90

Removing the outer cfg.enable guard makes this module enable nginx and the uWSGI emperor even when keystone.enable = false; only the Keystone vassal is conditional. That makes the module's disable option ineffective and can claim port 5000 for a disabled service. Keep the non-service setup behind the enable guard or gate these enables.

  config = {

modules/controller/openstack-controller.nix:342

The new production branch is only selected when openstack.production_setup = true, but the repository's NixOS integration tests import the modules without setting this option, so they exercise only the default false path. The changed service graph (disabled setup units and uWSGI/API behavior) has no integration coverage; add a production-mode test before relying on it for long-lived deployments.

    systemd.services.database-setup = lib.mkIf (!config.openstack.production_setup) {

modules/generic/controller-host-entry.nix:7

Adding controllerHostname does not update this module's hosts entry, which still emits only controller controller.local below. Nodes that rely on controller-host-entry.nix and override the new hostname will therefore generate configs referring to a name this module does not resolve unless separate DNS is guaranteed.

      default = "10.0.0.11";

modules/generic/global-options.nix:14

The new production_setup path is not exercised by either existing NixOS integration test: both leave the option at its default false. This path disables all initialization units and switches API serving to uWSGI, so add a test with openstack.production_setup = true that verifies the intended services and setup behavior.

  options.openstack.production_setup = lib.mkOption {
    type = lib.types.bool;
    default = false;

modules/storage/cinder-storage-node.nix:138

When the marker already exists, this branch only logs and then continues to mkfs.ext4 -F and remounts /dev/vdb, so rerunning the installed setup script destroys the existing volume despite the marker name saying not to delete it. Exit before formatting when the marker is present.

    if [ -e /exports/.cinder-volume-setup-done-dont-delete-me ]; then
      echo "cinder volume setup already done. Check content of this script."
    fi

modules/storage/cinder-storage-node.nix:126

storagePath is used for the generated Cinder nfs_shares entry, but the default /etc/exports content remains hard-coded to /exports. Overriding openstack.storagePath therefore points Cinder at a path that the NFS server does not export. Derive this default export path from the option.

    /exports 10.0.0.0/24(rw,no_root_squash,insecure)

packages/designate-knot3-backend.py:31

The backend is explicitly marked untested, and no tests were added for its SSH command construction, transactional conf-begin/conf-commit flow, or delete/purge behavior. The Designate package check does not exercise this new entry point, so failures in the production DNS synchronization path would go unnoticed.

    __backend_status__ = 'untested'

packages/infoblox-client.nix:43

This newly added dependency declares stestr but comments out its check phase, so the Infoblox client suite is not run. Designate also excludes all Infoblox backend tests, leaving the new backend dependency untested in the package build; either run a supported subset or explicitly document and disable the checks with a reason.

  # checkPhase = ''
  #   stestr run
  # '';

Files reviewed: 23/23 changed files
Comments generated: 14
Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+
+    system.activationScripts.openstack-setup-scripts.text = ''
+      install -d -m 0700 /root/os-setup
+      install -m 0700 ${cinderVolumeSetupScript} /root/os-setup/000-cinder-volume-setup.sh


  };

-  config = mkIf cfg.enable {
+  config = {


Copilot

🟡 Changes recommended

Multiple unresolved moderate and critical findings affect service gating, database setup, Designate/Knot operation, and persistent Cinder storage.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (19)

Previously missed (3) — in code that hasn't changed since the last review.

modules/compute/neutron.nix:112

Replacing the module-level mkIf cfg.enable with an unconditional config makes neutron.enable = false ineffective for host-level Open vSwitch configuration: virtualisation.vswitch.enable below remains true even though the Neutron agent is disabled. Keep the global configuration under the enable guard or guard each host-level setting.
modules/controller/keystone.nix:90
The former mkIf cfg.enable guard was removed, but this module still sets services.nginx.enable = true (and its own uWSGI instance) below. Setting keystone.enable = false therefore still starts Keystone's proxy infrastructure instead of disabling the module. Restore the enable guard around the module configuration.
modules/storage/cinder-storage-node.nix:140
When the marker exists, this branch only logs and then continues into setup. A rerun therefore reaches mkdir and fails instead of being idempotent, while leaving the destructive formatting path reachable. Exit successfully from the already-completed branch.

modules/compute/nova.nix:149

Making this module's config unconditional leaves virtualisation.libvirtd.enable = true below active even when nova.enable = false; previously the outer mkIf cfg.enable disabled libvirt with Nova. Gate libvirtd with the module option so disabling Nova does not still start the hypervisor daemon.

  config = {

modules/controller/cinder.nix:119

Because cfg.config is an overrideable configuration path, leaving this tmpfiles entry as L means an existing /etc/cinder/cinder.conf symlink is not replaced when the configured derivation changes. The new production/custom configuration can therefore be silently ignored after a rebuild; use the force-updating form used by the other updated service modules.

            argument = "${cfg.config}";

modules/controller/designate.nix:77

The default pool declares only a bind9 target, so the newly added Knot3Backend is never selected. This module does not configure a BIND service either, while the target requires rndc and /etc/bind/rndc.key; enabling Designate therefore cannot configure the external Knot zones and pool updates will fail unless BIND is provisioned separately. Make the pool target/options match Knot3, or add the missing BIND and Knot target configuration.

      targets:
        - type: bind9

modules/controller/designate.nix:110

In the non-production path every long-running Designate service Requires= this setup unit, but designate.service is a Type=oneshot unit without RemainAfterExit. After the setup script exits successfully, that required unit becomes inactive, which can deactivate the Designate daemons as well. Mark the setup unit RemainAfterExit = true (or use ordering without Requires) so the daemons remain running after initialization.

    ++ lib.optional (!config.openstack.production_setup) "designate.service";
    requires = lib.optional (!config.openstack.production_setup) "designate.service";

modules/controller/designate.nix:217

Designate's API is configured to listen on 0.0.0.0:9001 and the Keystone catalog advertises that endpoint, but this module only opens TCP/UDP 5354. With the normal NixOS firewall enabled, clients cannot reach the Designate API. Add TCP port 9001 to the allowed ports (UDP 9001 is not needed).

    networking.firewall.allowedTCPPorts = [ 5354 ];
    networking.firewall.allowedUDPPorts = [ 5354 ];

modules/controller/designate.nix:62

These default NS records are fixed to the CI/test names osdns.openstack.test. and controller.openstack.test. even though the pool's server addresses are configurable. Enabling Designate in a production deployment will publish incorrect authoritative NS names unless users replace the entire pools file; expose/configure these names or derive production-safe defaults.

      ns_records:
        - hostname: osdns.openstack.test.
          priority: 2
        - hostname: controller.openstack.test.
          priority: 1

modules/controller/keystone.nix:51

These catalog entries are unconditional even though config.designate.enable defaults to false. The default CI/CD controller will therefore advertise a DNS endpoint on port 9001 while no Designate API is enabled, causing clients to discover a dead service. Add this block only when Designate is enabled.

    catalog.RegionOne.dns.publicURL = http://${config.openstack.controllerHostname}:9001/
    catalog.RegionOne.dns.adminURL = http://${config.openstack.controllerHostname}:9001/
    catalog.RegionOne.dns.internalURL = http://${config.openstack.controllerHostname}:9001/
    catalog.RegionOne.dns.name = DNS Service

modules/controller/neutron.nix:175

As with the compute Neutron module, making this config unconditional leaves virtualisation.vswitch.enable = true active when neutron.enable = false. The option no longer disables all controller-Neutron host setup; retain the module-level enable guard or condition the virtual-switch settings.

  config = {

modules/controller/openstack-controller.nix:53

With production_setup = false, database-setup.service still uses this script, but the new cleanup script is only installed and never invoked. That removes the existing CI/CD reset-on-boot behavior the PR promises to preserve, so stale databases survive rebuilds. Invoke the cleanup script before the CREATE statements (or retain the old drop logic) in the non-production path.

    mariadb -N -e "CREATE DATABASE IF NOT EXISTS keystone;"

modules/controller/openstack-controller.nix:99

This SQL contains three consecutive single quotes after =, which MariaDB parses as an unterminated string. Because the generated script runs with set -e, database-setup.service will fail at this command; use an empty-string literal instead.

    mariadb -N -e "delete from mysql.user where user = ''';"

modules/controller/openstack-controller.nix:343

The existing NixOS integration tests exercise the default (production_setup = false) controller path, but no test configures production_setup = true. That branch changes service topology to uWSGI and disables all initialization units, so regressions such as missing vassals or unavailable endpoints will not be detected. Add a production-mode integration case covering the controller services.

    systemd.services.database-setup = lib.mkIf (!config.openstack.production_setup) {

modules/generic/global-options.nix:35

This new openstack.live_migration_inbound_addr option is not consumed anywhere in the modules; searching the repository finds only its declaration. Setting it therefore has no effect on Nova's live-migration configuration, despite the option description promising to configure the migration address. Wire it into the relevant Nova config or remove the option until it is supported.

  options.openstack.live_migration_inbound_addr = lib.mkOption {
    default = "10.100.100.1";
    type = types.str;
    description = ''
      My own ip address of the migration network. Usually our internal 100G link.
    '';

modules/storage/cinder-storage-node.nix:247

When the controller and storage modules are enabled on the same host, this condition suppresses the entire 20-cinder tmpfiles group. The storage config now requires /etc/cinder/rootwrap.conf, but the controller Cinder module does not create that file, so the volume service's rootwrap/sudo calls fail in the combined deployment. Keep the rootwrap symlink outside this conditional or have the controller module provide it.

      "20-cinder" = lib.mkIf (!(config ? cinder && builtins.isBool config.cinder.enable && config.cinder.enable == true)) {

modules/storage/cinder-storage-node.nix:222

The module-level mkIf cfg.enable guard was removed, so disabling cinder-storage-node.enable still applies global setup such as security.sudo.enable, the activation script, users, and tmpfiles. This makes the public enable option ineffective for those side effects; restore the guard or condition every global setting.

  config = {

modules/storage/cinder-storage-node.nix:142

The new storagePath option is documented as configurable, but mkdir only works when its parent directory already exists. A nested custom path therefore makes the setup script fail before formatting the disk; create the path recursively.

    mkdir ${config.openstack.storagePath}

packages/designate-knot3-backend.py:45

confdb_path is read into self.confdb but the only command-line use is commented out, so configuring a non-default configuration database has no effect. Either pass the option to the remote knotc invocation or remove the unused setting.

        self.confdb = self.options.get(
            'confdb_path', '/var/lib/knot/confdb')

Files reviewed: 23/23 changed files
Comments generated: 3
Review effort level: Lite

+        acl.designate-mdns = {
+          address = [ cfg.mdnsAddress ];
+          action = [ "notify" ];
+        };


+
+    mkdir ${config.openstack.storagePath}
+    mkfs.ext4 -F -m 0 -L cinder /dev/vdb
+    mount /dev/vdb ${config.openstack.storagePath}


Change enable logic of keystone from generation of configuration to running real service Signed-off-by: Paul Kroeher <paul.kroeher@cyberus-technology.de> On-behalf-of: SAP paul.kroeher@sap.com

Signed-off-by: Paul Kroeher <paul.kroeher@cyberus-technology.de> On-behalf-of: SAP paul.kroeher@sap.com

Copilot

🟡 Changes recommended

Unresolved critical and moderate findings affect service gating, database setup, DNS, storage safety, and endpoint availability.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (19)

modules/compute/neutron.nix:64

When br-provider already exists, the script never enables set -e, so failures from the subsequent ovs-vsctl --may-exist add-port are ignored and the agent can start without its provider port. Put the strict-mode handling around the probe and setup (while treating the expected br-exists nonzero result explicitly).

    ovs-vsctl br-exists br-provider >/dev/null
    status=$?

    if [[ $status -eq 0 ]]; then

modules/compute/nova.nix:149

Changing the outer guard to an unconditional configuration leaves virtualisation.libvirtd.enable = true active even when nova.enable = false. The previous mkIf cfg.enable prevented libvirt from being enabled with a disabled Nova compute module; retain the guard or gate all non-service configuration as well.

  config = {

modules/controller/cinder.nix:63

uWSGI receives env entries as literal VAR=value assignments; it does not run them through a shell. Consequently this sets PATH to a literal $PATH:/run/current-system/sw/bin value rather than preserving the inherited PATH, which can break subprocess/rootwrap lookups in Cinder API workers. Use an explicit path or configure PATH outside the vassal env list.

        "PATH=$PATH:/run/current-system/sw/bin"

modules/controller/designate.nix:78

The generated default pool contains only a bind9 target, so Designate never invokes the new knot3 backend to create or remove Knot zone entries. also_notifies alone cannot make Knot serve a zone that has not been added to its configuration; include a Knot target with its SSH options or generate a pool that uses the backend.

      targets:
        - type: bind9
          description: local bind server on controller

modules/controller/designate.nix:62

The default pool advertises osdns.openstack.test. and controller.openstack.test. as authoritative NS records even though the module exposes only an IP address for Knot and a configurable controller hostname. Enabling Designate without replacing pools publishes test-only names that do not describe the configured production DNS servers; derive these from options or require an explicit production pool file.

      ns_records:
        - hostname: osdns.openstack.test.
          priority: 2
        - hostname: controller.openstack.test.
          priority: 1

modules/controller/keystone.nix:51

These new catalog entries are unconditional, while designate.enable defaults to false and the Designate services are therefore absent by default. Keystone will advertise a DNS endpoint on port 9001 that returns connection failures in the default CI/CD setup; add these entries only when Designate is enabled.

    catalog.RegionOne.dns.publicURL = http://${config.openstack.controllerHostname}:9001/
    catalog.RegionOne.dns.adminURL = http://${config.openstack.controllerHostname}:9001/
    catalog.RegionOne.dns.internalURL = http://${config.openstack.controllerHostname}:9001/
    catalog.RegionOne.dns.name = DNS Service

modules/controller/neutron.nix:175

Removing the outer enable guard leaves virtualisation.vswitch.enable = true active even when the controller's neutron.enable is false. This changes the module's disable semantics and can start OVS for a disabled Neutron service; keep this configuration conditional on cfg.enable.

  config = {

modules/controller/openstack-controller.nix:354

Changing this to the idempotent setup script changes the default CI/CD behavior: databaseCleanupScript is only installed for manual use, so existing databases are no longer dropped before each database-setup.service run. Stale data and schemas can therefore survive between CI runs, contrary to the stated unchanged default. Invoke cleanup before setup here (or retain the old destructive path) when production_setup is false.

        ExecStart = "/root/os-setup/000-database-setup.sh";

modules/controller/openstack-controller.nix:99

This SQL literal has three consecutive single quotes after = (user = ''';), which leaves the empty-string literal unterminated. Because this script runs with set -e in the default database-setup service, the setup aborts here and dependent CI services cannot initialize; use valid SQL such as user = '';.

    mariadb -N -e "delete from mysql.user where user = ''';"

modules/controller/openstack-controller.nix:343

The existing NixOS integration tests exercise only the default production_setup = false path. No test sets this new flag to true to verify that database/CI setup services are absent while the production uWSGI services remain reachable, so the primary behavior of this PR is unprotected.

    systemd.services.database-setup = lib.mkIf (!config.openstack.production_setup) {

modules/generic/global-options.nix:18

The repository has NixOS integration coverage, but both existing scenarios use the default production_setup = false; no test exercises the new production branch that removes setup units and switches Placement/Cinder to uWSGI. Add a production-mode scenario that sets this option and verifies the setup services are absent and the production API endpoints are available, otherwise this path can regress unnoticed.

  options.openstack.production_setup = lib.mkOption {
    type = lib.types.bool;
    default = false;
    description = ''
      Whether the controller uses a production database setup. When enabled,
      the destructive database setup service is disabled.
    '';

modules/knot-designate.nix:19

mdnsAddress is a required option even though enable defaults to false. Importing the exported Knot module without enabling it will still fail NixOS evaluation for an unset address; make the address nullable/defaulted and validate it only when the module is enabled.

    mdnsAddress = lib.mkOption {
      type = lib.types.str;
      example = "10.0.0.39";
      description = "Address of the Designate MiniDNS server used for IXFR/AXFR.";
    };

modules/knot-designate.nix:57

The ACL permits only NOTIFY messages from MiniDNS, while this Knot instance is configured as a secondary and must also fetch IXFR/AXFR data. Without the transfer action, Knot can receive the notification but will reject the subsequent zone transfer.

        acl.designate-mdns = {
          address = [ cfg.mdnsAddress ];
          action = [ "notify" ];

modules/storage/cinder-storage-node.nix:126

The default exports file is fixed to /exports, while storagePath controls the filesystem that the setup script mounts and the path written to nfs_shares. Overriding openstack.storagePath therefore exports a different/nonexistent directory and makes the NFS backend unusable.

    /exports 10.0.0.0/24(rw,no_root_squash,insecure)

modules/storage/cinder-storage-node.nix:355

When production_setup is true, cinder-volume-group-setup.service is omitted, but the installed manual helper still formats and mounts /dev/vdb as the NFS export. With backend = "lvm", no cinder-volumes VG is created, so the production cinder-volume service cannot start; the production helper needs a backend-specific LVM path or the LVM setup must remain available.

    systemd.services.cinder-volume-group-setup = lib.mkIf (!config.openstack.production_setup) {

modules/storage/cinder-storage-node.nix:386

The configurable storagePath is also ignored by the non-production NFS setup branch, which still creates and mounts /exports. With a custom path, the setup service mounts one directory while nfs_shares, the exports file, and cinder configuration refer to another; use the same option in this branch.

      enable = cfg.enable;

modules/storage/cinder-storage-node.nix:303

Although storagePath is configurable and this generated nfs_shares entry now uses it, the default /etc/exports text and the non-production volume setup service still hard-code /exports. Setting this option therefore mounts/exports a different directory than Cinder consumes. Derive the default exports and setup commands from config.openstack.storagePath as well.

                  ${config.openstack.storageIP}:${config.openstack.storagePath}
                '';

packages/designate-knot3-backend.py:65

The confdb_path option is stored but never applied because the only --confdb argument is commented out. Any target using a non-default Knot configuration database will have conf-begin/conf-set operate on the wrong database (or fail), so this option does not match the backend’s behavior.

            # '--confdb=%s' % self.confdb,

packages/infoblox-client.nix:43

The package declares stestr but comments out its entire check phase, so the upstream Infoblox client tests are never run even though this new dependency is used by Designate. Enable the check phase (or document and narrowly exclude the failing tests) so packaging regressions are caught.

  # checkPhase = ''
  #   stestr run
  # '';

Files reviewed: 23/23 changed files
Comments generated: 9
Review effort level: Lite

+            rndc_host: 127.0.0.1
+            rndc_port: 953
+            rndc_key_file: /etc/bind/rndc.key
+            # set relative path so rootwrap works
+            rndc_bin_path: rndc


+      ];
+    };
+
+    networking.firewall.allowedTCPPorts = [ 5354 ];


  };

-  config = mkIf cfg.enable {
+  config = {


  };

-  config = mkIf cfg.enable {
+  config = {


  };

-  config = mkIf cfg.enable {
+  config = {


+        "PYTHONWARNINGS=ignore::DeprecationWarning"
+      ];
+      description = ''
+        Environment variables passed to the Keystone uWSGI vassal.


+      Whether the controller uses a production database setup. When enabled,
+      the destructive database setup service is disabled.


Copilot

🟡 Changes recommended

Critical Designate nameserver configuration and several DNS, storage-transfer, and reformatting issues remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (4)

modules/controller/designate.nix:78

The pool declares only a bind9 target, so Designate never selects the newly added knot3 backend and never calls its SSH-based zone configuration methods. The Knot server listed in nameservers will therefore not receive zones from Designate; add a knot3 target (with its connection options) to this pool.

      targets:
        - type: bind9
          description: local bind server on controller

modules/knot-designate.nix:57

The ACL for Designate MiniDNS only permits notify. Knot uses this ACL for the configured secondary template, so it can receive the NOTIFY but its subsequent AXFR/IXFR request is not authorized; zones will never transfer to the secondary. Permit the transfer action as well.

        acl.designate-mdns = {
          address = [ cfg.mdnsAddress ];
          action = [ "notify" ];

modules/storage/cinder-storage-node.nix:140

When the marker exists, this branch only logs a message and then continues to mkfs.ext4 -F and mounts /dev/vdb. Re-running the manually installed setup script therefore reformats the existing volume and destroys its contents, defeating the marker's safety purpose. Exit before the formatting steps when the marker is found (or fail and require explicit confirmation).

    if [ -e ${config.openstack.storagePath}/.cinder-volume-setup-done-dont-delete-me ]; then
      echo "cinder volume setup already done. Check content of this script."
    fi

modules/storage/cinder-storage-node.nix:172

The new storagePath option is used in Cinder's configuration and NFS share, but the default setup/export path remains hard-coded to /exports (and the CI setup script below also mounts /exports). Setting openstack.storagePath therefore exports and mounts a different directory than Cinder uses; derive the default exports content and setup commands from storagePath.

    storagePath = mkOption {
      type = types.str;
      default = "/exports";
      description = ''

Files reviewed: 23/23 changed files
Comments generated: 1
Review effort level: Lite

+      ns_records:
+        - hostname: osdns.openstack.test.
+          priority: 2
+        - hostname: controller.openstack.test.
+          priority: 1


Copilot

🟡 Changes recommended

Critical Designate service dependencies and several DNS, catalog, and destructive-storage setup issues remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (5)

modules/controller/designate.nix:78

The pool declares only a bind9 target, so Designate never invokes the newly added knot3 backend to create or remove zones on the external Knot server. Listing Knot under nameservers/also_notifies is not enough: Knot still has no zone configuration entry to apply its template, and the external secondary will not serve newly created zones. Add a knot3 target (with the SSH/Knot options required by the backend) to this pool, or remove the advertised Knot nameserver until it is actually managed.

      targets:
        - type: bind9
          description: local bind server on controller

modules/controller/keystone.nix:51

These DNS catalog entries are emitted unconditionally, even though designate.enable defaults to false and the Designate services are then absent. In the default configuration, Keystone advertises a DNS endpoint at port 9001 that cannot serve requests, changing the default setup and misleading service discovery. Add these entries only when Designate is enabled (or make the catalog configurable).

    catalog.RegionOne.dns.publicURL = http://${config.openstack.controllerHostname}:9001/
    catalog.RegionOne.dns.adminURL = http://${config.openstack.controllerHostname}:9001/
    catalog.RegionOne.dns.internalURL = http://${config.openstack.controllerHostname}:9001/
    catalog.RegionOne.dns.name = DNS Service

modules/knot-designate.nix:57

The Knot ACL is attached to the template used by secondary zones but permits only notify. Knot needs the transfer action as well to accept AXFR/IXFR from MiniDNS; with this ACL, NOTIFY may arrive but the subsequent zone transfer is denied and the secondary remains empty. Include the transfer permission in the ACL.

        acl.designate-mdns = {
          address = [ cfg.mdnsAddress ];
          action = [ "notify" ];

modules/storage/cinder-storage-node.nix:143

Finding the marker only prints a message and then continues, so rerunning /root/os-setup/000-cinder-volume-setup.sh still force-formats /dev/vdb and destroys the existing Cinder volume data. The marker guard must exit successfully (and the script should avoid mkfs when the filesystem is already initialized) before the destructive setup commands.

    if [ -e ${config.openstack.storagePath}/.cinder-volume-setup-done-dont-delete-me ]; then
      echo "cinder volume setup already done. Check content of this script."
    fi

    mkdir ${config.openstack.storagePath}
    mkfs.ext4 -F -m 0 -L cinder /dev/vdb

modules/storage/cinder-storage-node.nix:127

The exports-file default is hard-coded to /exports, but storagePath is configurable and the generated /etc/cinder/nfs_shares uses config.openstack.storagePath. Setting storagePath therefore makes Cinder advertise a path that the NFS server does not export; derive this default export from the configured storage path (and keep the network policy configurable if needed).

  cinderDefaultNFSexports = pkgs.writeText "exports" ''
    /exports 10.0.0.0/24(rw,no_root_squash,insecure)
  '';

Files reviewed: 24/24 changed files
Comments generated: 1
Review effort level: Lite

+    ++ lib.optional (!config.openstack.production_setup) "designate.service";
+    requires = lib.optional (!config.openstack.production_setup) "designate.service";


Copilot

🟡 Changes recommended

Critical configuration and authentication issues, plus several moderate setup and integration defects, remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (7)

modules/controller/designate.nix:78

The pool declares only a bind9 target, so Designate never selects the newly added knot3 backend and never calls its SSH-based create_zone/delete_zone methods. The Knot server is listed as a nameserver/notified host, but its zones will not be configured by this backend; add a knot3 target (with its backend options) to this pool.

      targets:
        - type: bind9
          description: local bind server on controller

modules/controller/openstack-controller.nix:354

The non-production database-setup service now runs only the idempotent CREATE/ALTER script, while the previous service dropped and recreated the databases on each CI/CD setup. The newly installed cleanup script is never invoked, so repeated default test setups retain old schemas and data, contrary to the stated unchanged CI/CD behavior. Keep the cleanup as a non-production-only prerequisite (for example, an ExecStartPre) rather than leaving it as an uncalled manual script.

    systemd.services.database-setup = lib.mkIf (!config.openstack.production_setup) {
      description = "OpenStack Database setup";
      after = [
        "mysql.service"
        "network.target"
        "uwsgi.service"
      ];
      wantedBy = [ "multi-user.target" ];
      path = [ pkgs.mariadb ];
      serviceConfig = {
        Type = "oneshot";
        ExecStart = "/root/os-setup/000-database-setup.sh";

modules/knot-designate.nix:57

This ACL permits only notify. As Knot is configured as a secondary, it must also be authorized to transfer zone data from MiniDNS; without the transfer action, the IXFR/AXFR requested after a notify is rejected and the secondary remains empty.

        acl.designate-mdns = {
          address = [ cfg.mdnsAddress ];
          action = [ "notify" ];

modules/storage/cinder-storage-node.nix:140

When the marker exists, this branch only logs and then continues into mkfs.ext4 -F, so rerunning the installed setup script reformats /dev/vdb and destroys the existing Cinder volume filesystem. Exit successfully (or otherwise skip the destructive setup) after detecting the marker.

    if [ -e ${config.openstack.storagePath}/.cinder-volume-setup-done-dont-delete-me ]; then
      echo "cinder volume setup already done. Check content of this script."
    fi

modules/storage/cinder-storage-node.nix:133

This manually installed script calls exportfs at line 145, but its PATH only adds util-linux; unlike the systemd setup service below, it does not include nfs-utils, so the NFS setup path fails with exportfs: command not found. Add pkgs.nfs-utils to this script's PATH (or invoke the absolute binary).

    export PATH=${
      lib.makeBinPath [
        pkgs.util-linux
      ]

modules/storage/cinder-storage-node.nix:127

The new storagePath option is used for the mounted volume and the generated Cinder share, but the default exports file remains hard-coded to /exports. Any user who changes openstack.storagePath will advertise a path that the NFS server does not export unless they also remember to override the unrelated exports option.

  cinderDefaultNFSexports = pkgs.writeText "exports" ''
    /exports 10.0.0.0/24(rw,no_root_squash,insecure)
  '';

packages/designate-knot3-backend.py:66

The configured confdb_path is not used when constructing the remote command; the commented-out argument means custom paths silently have no effect. Include --confdb=%s % self.confdb alongside the socket argument.

            self.knotc,
            # '--confdb=%s' % self.confdb,
            '--socket=%s' % self.socket,

Files reviewed: 25/25 changed files
Comments generated: 2
Review effort level: Lite

+    export PATH=${
+      lib.makeBinPath [
+        keystone
+        pkgs.openstackclient


Copilot

🟡 Changes recommended

Unresolved critical and moderate issues affect storage safety, CI/CD defaults, and Designate/Knot functionality.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (5)

modules/controller/designate.nix:78

The added Knot backend is never selected by this pool: the only target is bind9. Designate therefore never calls Knot3Backend.create_zone/delete_zone to create the zone in Knot's confdb, so the external Knot cannot serve newly created zones. Add a knot3 target with its backend options (or otherwise configure the pool to use this backend).

      targets:
        - type: bind9
          description: local bind server on controller

modules/controller/generic.nix:11

This unconditionally changes the MySQL default for every deployment, including the CI/CD configuration where openstack.production_setup remains false. That contradicts the PR requirement that CI/CD defaults stay unchanged; scope this override to production setup (or leave the existing default untouched).

      max_connections = 500;

modules/knot-designate.nix:91

This module provisions the remote account and its authorized keys, but it does not provision a private key or known-hosts entry for the Designate worker. The backend defaults to /etc/ssh/ssh_known_hosts and has no identity file (packages/designate-knot3-backend.py:36-42), so its first knotc SSH operation will fail host-key or public-key authentication unless users manually add unexposed SSH configuration.

    users.groups.${cfg.sshUser} = { };
    users.users.${cfg.sshUser} = {
      isSystemUser = true;
      group = cfg.sshUser;
      extraGroups = [ "knot" ];
      home = "/var/lib/${cfg.sshUser}";
      createHome = true;
      shell = pkgs.bashInteractive;
      openssh.authorizedKeys.keys = cfg.sshAuthorizedKeys;

modules/storage/cinder-storage-node.nix:140

The marker is only logged and execution continues, so rerunning the installed setup script reformats /dev/vdb and destroys the existing cinder volume filesystem before mounting it. Exit immediately when the marker exists.

    if [ -e ${config.openstack.storagePath}/.cinder-volume-setup-done-dont-delete-me ]; then
      echo "cinder volume setup already done. Check content of this script."
    fi

modules/storage/cinder-storage-node.nix:126

This default exports /exports regardless of the configurable storagePath, while the generated Cinder NFS share uses config.openstack.storagePath. Any deployment overriding storagePath will advertise an unexported path and the volume backend will fail.

    /exports 10.0.0.0/24(rw,no_root_squash,insecure)

Files reviewed: 25/25 changed files
Comments generated: 1
Review effort level: Lite

+    export PATH=${
+      lib.makeBinPath [
+        pkgs.util-linux
+      ]
+    }:$PATH


Use config option storagePath in CI/CD setup script and add guard for nfs-server. Signed-off-by: Paul Kroeher <paul.kroeher@cyberus-technology.de> On-behalf-of: SAP paul.kroeher@sap.com

Signed-off-by: Paul Kroeher <paul.kroeher@cyberus-technology.de> On-behalf-of: SAP paul.kroeher@sap.com

Copilot

🟡 Changes recommended

Critical missing-module and production-credential issues remain, along with storage and networking setup defects.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (2)

modules/compute/neutron.nix:64

This script does not reach the status check when br-provider is absent: ovs-vsctl br-exists exits nonzero, and the generated writeShellScript uses set -e by default, so the script terminates before the else can create the bridge. Use an if ovs-vsctl br-exists ...; then ... else ... fi conditional (or otherwise temporarily disable errexit) so first-time setup works.

    ovs-vsctl br-exists br-provider >/dev/null
    status=$?

    if [[ $status -eq 0 ]]; then

modules/storage/cinder-storage-node.nix:146

This one-time setup formats and mounts /dev/vdb, but it never creates a persistent mount (for example via fileSystems/an fstab or a mount unit). After a reboot /exports is no longer backed by the volume, so the marker check at the top cannot see the marker and rerunning the installed setup script will force-format /dev/vdb again, destroying existing Cinder data. Persist the mount and make the formatting step conditional on an explicitly unformatted device before exposing this script for production use.

    mkdir ${config.openstack.storagePath}
    mkfs.ext4 -F -m 0 -L cinder /dev/vdb
    mount /dev/vdb ${config.openstack.storagePath}

Files reviewed: 23/23 changed files
Comments generated: 2
Review effort level: Lite

+    mariadb -N -e "CREATE USER IF NOT EXISTS 'keystone'@'%' IDENTIFIED BY 'keystone';"
+    mariadb -N -e "ALTER USER 'keystone'@'%' IDENTIFIED BY 'keystone';"
+    mariadb -N -e "GRANT ALL PRIVILEGES ON keystone.* TO 'keystone'@'%';"



  storageModule = import ./storage/cinder-storage-node.nix { inherit (openstackPkgs) cinder; };

+  knotDesignateModule = import ./knot-designate.nix;


neutron-openvswitch-agent should start after network setup is completed by systemd-netword.service This fixes a runtime race condition during the reboot. Signed-off-by: Paul Kroeher <paul.kroeher@cyberus-technology.de> On-behalf-of: SAP paul.kroeher@sap.com

pkr4711 added 19 commits August 3, 2026 15:41

nix: add option to disable destructive database setup

a11b9ed

Signed-off-by: Paul Kroeher <paul.kroeher@cyberus-technology.de> On-behalf-of: SAP paul.kroeher@sap.com

nix: add self build openstack-client to system packages

c253407

Signed-off-by: Paul Kroeher <paul.kroeher@cyberus-technology.de> On-behalf-of: SAP paul.kroeher@sap.com

nix: move database setup into a separate script

628cbf8

Signed-off-by: Paul Kroeher <paul.kroeher@cyberus-technology.de> On-behalf-of: SAP paul.kroeher@sap.com

nix: move keystone setup into a separate script

2029a9c

Signed-off-by: Paul Kroeher <paul.kroeher@cyberus-technology.de> On-behalf-of: SAP paul.kroeher@sap.com

nix: move glance setup into a separate script

70c1dec

Signed-off-by: Paul Kroeher <paul.kroeher@cyberus-technology.de> On-behalf-of: SAP paul.kroeher@sap.com

nix: write default credentials into a separate script

c443595

Signed-off-by: Paul Kroeher <paul.kroeher@cyberus-technology.de> On-behalf-of: SAP paul.kroeher@sap.com

nix: move cinder setup into a separate script

c542c37

Signed-off-by: Paul Kroeher <paul.kroeher@cyberus-technology.de> On-behalf-of: SAP paul.kroeher@sap.com

nix: move placement setup into a separate script

9f885eb

Signed-off-by: Paul Kroeher <paul.kroeher@cyberus-technology.de> On-behalf-of: SAP paul.kroeher@sap.com

nix: move nova setup into a separate script

c484c7f

Signed-off-by: Paul Kroeher <paul.kroeher@cyberus-technology.de> On-behalf-of: SAP paul.kroeher@sap.com

nix: move neutron setup into a separate script

2f4006a

Signed-off-by: Paul Kroeher <paul.kroeher@cyberus-technology.de> On-behalf-of: SAP paul.kroeher@sap.com

nix: rename generated scripts

ac7e227

Signed-off-by: Paul Kroeher <paul.kroeher@cyberus-technology.de> On-behalf-of: SAP paul.kroeher@sap.com

nix: rename generated scripts

2b677f4

Signed-off-by: Paul Kroeher <paul.kroeher@cyberus-technology.de> On-behalf-of: SAP paul.kroeher@sap.com

nix: split database script into create and cleanup

f8ff827

Signed-off-by: Paul Kroeher <paul.kroeher@cyberus-technology.de> On-behalf-of: SAP paul.kroeher@sap.com

nix: disable services but create all configurations and systemd units

47dac0b

Signed-off-by: Paul Kroeher <paul.kroeher@cyberus-technology.de> On-behalf-of: SAP paul.kroeher@sap.com

nix: disable services but create all configurations and systemd units

46645a4

Signed-off-by: Paul Kroeher <paul.kroeher@cyberus-technology.de> On-behalf-of: SAP paul.kroeher@sap.com

nix: disable services but create all configurations and systemd units

1791381

Signed-off-by: Paul Kroeher <paul.kroeher@cyberus-technology.de> On-behalf-of: SAP paul.kroeher@sap.com

nix: rename mysql command and stop services before setup its database

6fbfa10

Signed-off-by: Paul Kroeher <paul.kroeher@cyberus-technology.de> On-behalf-of: SAP paul.kroeher@sap.com

nix: disable services but create all configurations and systemd units

3010c1f

Signed-off-by: Paul Kroeher <paul.kroeher@cyberus-technology.de> On-behalf-of: SAP paul.kroeher@sap.com

nix: fix mysql database permissions

d6c64f3

Signed-off-by: Paul Kroeher <paul.kroeher@cyberus-technology.de> On-behalf-of: SAP paul.kroeher@sap.com

Copilot AI lite review requested due to automatic review settings September 14, 2026 09:27

pkr4711 marked this pull request as draft September 14, 2026 09:27

Copilot started reviewing on behalf of pkr4711 September 14, 2026 09:27 View session

pkr4711 requested review from hertrste and tpressure September 14, 2026 09:33

Copilot AI reviewed Sep 14, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings September 15, 2026 07:51

Copilot started reviewing on behalf of pkr4711 September 15, 2026 07:52 View session

Copilot AI reviewed Sep 15, 2026

View reviewed changes

pkr4711 added 2 commits September 15, 2026 11:37

nix: keystone configuration

6e36c70

Change enable logic of keystone from generation of configuration to running real service Signed-off-by: Paul Kroeher <paul.kroeher@cyberus-technology.de> On-behalf-of: SAP paul.kroeher@sap.com

nix: change enabled logic of glance-api service

00cab96

Signed-off-by: Paul Kroeher <paul.kroeher@cyberus-technology.de> On-behalf-of: SAP paul.kroeher@sap.com

Copilot started reviewing on behalf of pkr4711 September 15, 2026 12:46 View session

Copilot AI reviewed Sep 15, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings September 17, 2026 10:40

Copilot started reviewing on behalf of pkr4711 September 17, 2026 10:40 View session

Copilot AI reviewed Sep 17, 2026

View reviewed changes

Comment thread modules/controller/designate.nix

Comment on lines +58 to +62

ns_records:

- hostname: osdns.openstack.test.

priority: 2

- hostname: controller.openstack.test.

priority: 1

Copilot AI review requested due to automatic review settings September 17, 2026 12:37

pkr4711 force-pushed the production-setup branch from 003d842 to bc08f82 Compare September 17, 2026 12:37

Copilot started reviewing on behalf of pkr4711 September 17, 2026 12:37 View session

Copilot AI reviewed Sep 17, 2026

View reviewed changes

Comment thread modules/controller/designate.nix

Comment on lines +109 to +110

++ lib.optional (!config.openstack.production_setup) "designate.service";

requires = lib.optional (!config.openstack.production_setup) "designate.service";

Copilot AI review requested due to automatic review settings September 17, 2026 14:54

Copilot started reviewing on behalf of pkr4711 September 17, 2026 14:54 View session

Copilot AI reviewed Sep 17, 2026

View reviewed changes

Comment thread modules/controller/openstack-controller.nix

export PATH=${

lib.makeBinPath [

keystone

pkgs.openstackclient

Comment thread modules/knot-designate.nix Outdated

Copilot AI review requested due to automatic review settings September 18, 2026 06:11

pkr4711 force-pushed the production-setup branch from ba6741a to 124e8a3 Compare September 18, 2026 06:11

Copilot started reviewing on behalf of pkr4711 September 18, 2026 06:12 View session

Copilot AI reviewed Sep 18, 2026

View reviewed changes

Comment thread modules/storage/cinder-storage-node.nix

Comment on lines +130 to +134

export PATH=${

lib.makeBinPath [

pkgs.util-linux

]

}:$PATH

pkr4711 added 9 commits September 18, 2026 09:02

nix: standardize nfs configuration

803ac1c

Use config option storagePath in CI/CD setup script and add guard for nfs-server. Signed-off-by: Paul Kroeher <paul.kroeher@cyberus-technology.de> On-behalf-of: SAP paul.kroeher@sap.com

nix: fix linting

b7ce1c6

Signed-off-by: Paul Kroeher <paul.kroeher@cyberus-technology.de> On-behalf-of: SAP paul.kroeher@sap.com

nix: enable caching in keystone

5314c68

Signed-off-by: Paul Kroeher <paul.kroeher@cyberus-technology.de> On-behalf-of: SAP paul.kroeher@sap.com

nix: fix keystone urls in all services

7d45eb5

Signed-off-by: Paul Kroeher <paul.kroeher@cyberus-technology.de> On-behalf-of: SAP paul.kroeher@sap.com

nix: set mariadb max_connections to 500

fe90f2e

Signed-off-by: Paul Kroeher <paul.kroeher@cyberus-technology.de> On-behalf-of: SAP paul.kroeher@sap.com

nix: removed knot3 backend

60dded9

Signed-off-by: Paul Kroeher <paul.kroeher@cyberus-technology.de> On-behalf-of: SAP paul.kroeher@sap.com

nix: run database cleanup in CI/CD

598f982

Signed-off-by: Paul Kroeher <paul.kroeher@cyberus-technology.de> On-behalf-of: SAP paul.kroeher@sap.com

nix: bugifxes storage node

dc73faa

Signed-off-by: Paul Kroeher <paul.kroeher@cyberus-technology.de> On-behalf-of: SAP paul.kroeher@sap.com

nix: add openstack packages into system path

1eb42ce

Signed-off-by: Paul Kroeher <paul.kroeher@cyberus-technology.de> On-behalf-of: SAP paul.kroeher@sap.com

Copilot AI review requested due to automatic review settings September 18, 2026 07:55

pkr4711 force-pushed the production-setup branch from 124e8a3 to 1eb42ce Compare September 18, 2026 07:55

Copilot started reviewing on behalf of pkr4711 September 18, 2026 07:55 View session

Copilot AI reviewed Sep 18, 2026

View reviewed changes

		Whether the controller uses a production database setup. When enabled,
		the destructive database setup service is disabled.

		++ lib.optional (!config.openstack.production_setup) "designate.service";
		requires = lib.optional (!config.openstack.production_setup) "designate.service";


		storageModule = import ./storage/cinder-storage-node.nix { inherit (openstackPkgs) cinder; };

		knotDesignateModule = import ./knot-designate.nix;

Conversation

pkr4711 commented Sep 14, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Suppressed comments (12)

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (19)

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (19)

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (4)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (5)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (7)

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (5)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (2)

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants