Not all config file changes take effect after initial deployment

[Matthias247]

For some config files like resource pools, editing the file after the initial deployment will cause no effect. The initialization logic will determine that an entry already exists in the database and will not change it.

This is confusing for users. NICo should instead stick to one of the following patterns:

• The new configuration takes effect (if possible)
• The service rejects the new config and does not start
• The service shows errors that show the config drift, and provides alarms for it

[ianderson-nvidia]

Some use cases that have come up

NetworkSegments - Updating the config file does not update database

FYI, just updating the configuration file won’t help. You also either need to:

update the DB with the correct values for each “network_segment”
Use the CLI to remove all 4 network segments.
This requires no machine interfaces on the network segments, which means you probably temporarily need to disable DHCP or have a while :; loop to keep removing machine interfaces :slightly_smiling_face:

Ask me how I learned that? :smile:
[7:15 PM]If you delete the network segments, carbide recreates them on next restart.

[nvaprado]

@SiamakNa @ajf @ianderson-nvidia @deepak-poornachandra ; I heard this needs clarifications, how can I help to clarify ?

[ianderson-nvidia]

@nvaprado I spoke with @daluz on some use cases (the one above). If you have any other use cases, I am working on a config drift feature that will surface configuration drift to the user. (Value in file is different than database as an example)

[Matthias247]

on the very high level, the issue applies at least for the following:

• for every "resource pool" entry that is declared in the config file, the values will not be touched after the pool has been initially been created. That applies to things like DPU loopback IPs, various VNIs, IB pkey pools, and probably more
• for every "network segment" that is defined in the config file, the values will not be touched after the segment has been initially created. That applies to underlay and admin network segments

For both of these, we should be able to enumerate the exact amount of settings by auditing the file.

They might need to have different strategies for dealing with the drift. for resource pools we can always

• add additional ranges during startup
• remove no longer referenced and unused values during startup
• keep no longer referenced and used values - but monitor and alarm on it

Network segments are trickier, because we can't poke holes into them and remove just some of the values.

[ajf]

We can also just remove 'database seeding' from the main config file and have a separate file for initial seeds.

[ianderson-nvidia]

The ask from @daluz was just to notify users that the configuration in the file and what exists in the database (or vice versa if we want to make sure file and database are always equal) have drifted. Meaning someone has changed a file, expecting those settings to persist in the database. Right now (as Matthias noted), any changes to ResourcePools and NetworkSegments in the configuration file have no effect. No changes, no alerting, nothing. Rather than attempting to remediate the configuration drift, the first step was to notify users of the drift.

[ajf]

Yeah, that seems like a lot of work, when just making it clearer that some of the stuff in the config file is a one-time database-load (like a migration) and you can just say that the seeding was already performed. If the file was "initial-objects.toml" it'd probably be clearer that changing that file wouldn't do anything - can even record that the initial-db-seed was done and that file was being ignored from here on out.

Some alternatives:

• Remove anything that updates the database from the config file at all and make whoever is setting up the site do it externally (kind of like how expected machines works).
• Move the source of truth to the config file itself (likely harder because of referential integrity)

But if what you're working on isn't terribly difficult, then I guess notification is better than nothing. I just don't know how someone would fix it if they ran into the situation.

[Matthias247]

We could handle it like this. But the question then will likely become whether the config for the service should be declarative or imperative/API driven. I think at some point in time declarative was the preferred pattern.

If we move towards just imperative APIs, then anyone who wants to use declarative configs would need to implement the reconciliation loop (everything discussed in this ticket) somewhere else.

[ianderson-nvidia]

I am running into two sources of truth with ResourcePools.

1. carbide-api-site-config.toml which, upon first start of carbide-api creates and populates the resource_pools
2. admin_grow_resource_pool, which takes a GrowResourcePoolRequest, which takes a TOML file describing the updated resource pool. The changes to the pool are applied to the database but carbide-api-site-config.toml stays the same. So when attempting to compare &api.runtime_config.pools with pool definitions from the database, the configuration has technically drifted. Which one is authoritative?

What is also interesting is that the contents of the config file do not match what we populate() into the database. e.g.

[pools.vpc-vni]
type = "integer"
ranges = [{ start = "0", end = "100" }]

That pool gets expanded in the resource_pool table as:

    172 | vpc-vni                 | 11             | 2024-06-01 14:02:29.440412+00 |                               | {"state": "free"}                                                                                                       | V1-T1717250549462955 | integer    | t
     228 | vpc-vni                 | 67             | 2024-06-01 14:02:29.440412+00 |                               | {"state": "free"}                                                                                                       | V1-T1717250549462955 | integer    | t
     245 | vpc-vni                 | 84             | 2024-06-01 14:02:29.440412+00 |                               | {"state": "free"}                                                                                                       

To compare like-for-like, I have to expand the ranges defined in carbide-api-site-config.toml to determine whether they are present/equal in the database. This part wasn't difficult (I separated out the expansion of the ranges with the actual persistence to the database), but you do end up expanding the ranges as well as SELECT value from resource_pool where name = $1, e.g., "Give me all values for the resource_pool name vpc-vni" every time you want to make a comparison.

A simpler approach is to combine @ajf suggestion with detecting configuration changes (not what changed). Instead of comparing each [pool.*] with the database, separate the initial pool definitions into a new file initial-objects.toml, which is read by carbide-api on startup.

pseudocode

Create a new file `initial-objects.toml` which is read by `carbide-api` on start
Generate a hash of the contents
Check if the table "database_seed" exists with row "version = <HASH OF initial-objects.toml>" (or it could be part of resource_poo in an existing table if we don't want this to be its own)
If  hash is not present in db:
Populate the database as usual
insert <HASH> into database_seed.version
If the hash does exist in the database
Compare the hash in the db with that of `initial-objects.toml`
if there is a difference
tracing::warn!("you cannot modify initial-objects.toml after the database has been seeded"); 
if not
done

This allows continued use of `admin_grow_resource_pool` and we can look into adding others `admin_grow_network_segment` or  `admin_add_network_segment`