New Golden Config Setting Enabled Per GC Setting (replaces 847 913 936) by jeffkala · Pull Request #984 · nautobot/nautobot-app-golden-config

jeffkala · 2025-09-02T14:08:18Z

Test Plan Executed

Backups

Both GCS are using the same git repos.

1 GC Setting Only Backup enabled.
- 2 devices in scope.
2 GC settings 1 device in scope per setting
- 2 devices backup successfully
- disabled backup on 1 GCS but run GC backup on all devices
  - !image
2 GC settings backup disabled for both
- !image
2 GC settings 1 devices in both Dynamic groups make sure higher weight GC setting for that device takes precedence.

Intended

Both GCS are using the same git repos.

Make sure GC settings in the UI can't be saved if Intended is enabled and Sot agg query, Jinja repository and Jinja Template Path are not set.
1 GC Setting Only Intended enabled.
- 2 devices in scope.
2 GC settings 1 device in scope per setting
- 2 devices intended successfully
- disabled intended on 1 GCS but run GC intended on all devices
2 GC settings intended disabled for both
2 GC settings 1 devices in both Dynamic groups make sure higher weight GC setting for that device takes precedence.

Compliance

Both GCS are using the same git repos.

1 GC Setting Only compliance enabled.
- 2 devices in scope.
2 GC settings 1 device in scope per setting
- 2 devices compliance successfully
- disabled compliance on 1 GCS but run GC compliance on all devices
2 GC settings backup disabled for both
2 GC settings 1 devices in both Dynamic groups make sure higher weight GC setting for that device takes precedence.

All in One (Single)

All in One (Multiple)

Questions to Answer (ADRs)

If a device is in scope for two GC Settings and a feature is disabled on teh hightest weighted setting should it fallback to the other setting and still run? Or only use the highest weighted settings.
For the all in one jobs, what should happen if any of the jobs are not enabled? Run the ones that are? Don't run any of them and warn?

Extra Details

What do we need for each job related to repositories?

The best GoldenConfigSettings across all inscope devices. should be get_inscope_settings_from_device_qs from helpers.
Helps with quickly determining if a repo is needed for a job.
get_repo_types_for_job accompanied by inscope settings gets actual repos.
Device to settings map. This is a dict of device pk to the GoldenConfigSettings object.

From a device qs, we need to get the following:

The best GoldenConfigSettings across all inscope devices. should be get_inscope_settings_from_device_qs from helpers.
Device to settings map. This is a dict of device pk to the GoldenConfigSettings object.
Repos per inscope settings, if that specific setting is enabled based on the repos needed for the job (get_repo_types_for_job).

Other considerations for (only for compliance job, and all jobs)

If a setting is disabled, but a repo/repo path is set that means we need to do:
- Still run ensure_git_repository on the repo.
- Don't add repo to the list of repos to commit/push.

Backup job:

Repo types needed: backup_repository
Always sync repos if a device is in scope for the job that matches a GC setting.
If setting is enabled, and repo is set, and if a device is in scope for the job, then we need to commit/push only backup_repository.
We should log if a device is in the queryset, but the setting is disabled.

Intended job:

Repo types needed: jinja_repository, intended_repository
Always sync repos if a device is in scope for the job that matches a GC setting.
If setting is enabled, and repo is set, and if a device is in scope for the job, then we need to commit/push only intended_repository.
We should log if a device is in the queryset, but the setting is disabled.
- Also if templates git repo is not set, but intended is set, and setting is enabled.

Compliance job:

Repo types needed: intended_repository, backup_repository
All job:
Repo types needed: backup_repository, jinja_repository, intended_repository

jeffkala · 2025-09-02T14:14:16Z

jeffkala · 2025-09-26T20:57:22Z

The ADRS have been discussed

If a device is in scope for two GC Settings and a feature is disabled on the highest weighted setting should it fallback to the other setting and still run? Or only use the highest weighted settings.
- A: highest weight only, no fallback.
For the all in one jobs, what should happen if any of the jobs are not enabled? Run the ones that are? Don't run any of them and warn?
- A: Run whichever are enabled.

Each device routes through its highest-weighted GoldenConfigSetting and honors that Setting's per-feature enable_* flag. All in-scope Settings' repos still sync (so read-only consumers stay current); push is gated by the enable_* flag on the winning Setting. Skipped devices log E3038 naming the winning Setting and weight; jobs with no eligible devices log E3039 per feature. Plugin-level ENABLE_* settings stay as backwards-compat defaults for the new model fields. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Adds the new per-Setting enable_* flags to GoldenConfigSettingTable (colored BooleanColumn rendering plus Meta.default_columns), the filter-form sidebar, and the bulk-edit form. Centralizes E3039 emission in _log_no_eligible_devices with three branches (no devices / single device suppressed / many devices summarized) so single-device AllGoldenConfig runs no longer log redundant "no devices found" summaries. Suppresses the per-play "Device(s) with settings enabled/disabled" debug logs when the job's queryset has only one device. Picks up two pylint cleanups (E0712 on except tuple, W0108 redundant lambdas in test patches) plus a startswith fix on the _e30xx test helper. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

github-actions · 2026-05-15T03:36:21Z

Coverage report

Click to see where and how coverage changed

File	Statements	Missing	Coverage	Coverage (new stmts)	Lines missing
nautobot_golden_config
datasources.py
forms.py
jobs.py					63, 73-74, 90-99, 321-327, 329-332, 349, 353-356, 377-380, 401-404
models.py
navigation.py
tables.py
template_content.py					93-94
views.py					209, 562-563, 581-582, 606-627, 647-648, 686-687
nautobot_golden_config/api
views.py					74-77
nautobot_golden_config/nornir_plays
config_backup.py
config_deployment.py					38-44
nautobot_golden_config/utilities
config_postprocessing.py					135-136
constant.py
helper.py
Project Total

_{This report was generated by python-coverage-comment-action}

jeffkala · 2026-05-15T03:41:58Z

@nickydavey @itdependsnetworks @nkallergis I believe this is finally ready. Better late then never, but manually tested all the test cases again from the PR description.

The two ADRS in the thread were implemented from #984 (comment).

The app as a whole settings from NautobotAppConfig are used as the defaults for all the new GC settings that get created, this is for backwards compatibility.

Co-authored-by: Ken Celenza <ken@celenza.org>

jeffkala · 2026-05-19T02:26:27Z

Some new screenshots now that this is 3.x only.

jeffkala · 2026-05-19T02:41:47Z

Looks like Config Plans doesn't work as intended. Even when the feature is disabled, config plans are created. Will look at this.

Also deploys are the same, settings don't seem to take account.

jeffkala · 2026-05-19T02:53:44Z

@itdependsnetworks thoughts on these two enable_sotagg and enable_postprocessing? Should these remain app level? Or settings level like the rest are moving towards?

itdependsnetworks · 2026-05-19T03:10:03Z

    )
    gcs = {gc.id: gc for gc in models.GoldenConfigSetting.objects.all()}
-    return {device.id: gcs[device.gc_settings] for device in annotated_queryset}
+    return {device.id: gcs[device.gc_settings] for device in annotated_queryset if device.gc_settings}


why is this needed now?

I had seen a instance where if I created a device and immediately ran a GC job it seemed like a race condition existed where members were updated in the middle of a run and gc_settings didn't exist yet for that device. This was a catch for that, but I think it was largely a issue with my local environment as I've never seen it again. Will get it removed.

Side note as well @itdependsnetworks curious your thoughts on:
A config plan can outlive its device leaving any GC setting's scope, so device.gc_settings legitimately can be None from that callsite. What do we want to happen if a config plan is built while a device is in-scope for a setting and if that device gets removed or DG changes and now they shouldn't be able to create a config plan

itdependsnetworks · 2026-05-19T03:15:49Z

@jeffkala what assurance do you have that we are not introducing an N+1 issue here with settings here?

jeffkala · 2026-05-27T14:04:34Z

@jeffkala what assurance do you have that we are not introducing an N+1 issue here with settings here?

Will do some research on this.

jeffkala · 2026-05-30T15:58:57Z

With most recent commits the following UI test plan was done:

backup / intended / compliance — mixed (veos via gcs2 runs; demo-cisco-xe + vjunos E3038 via Default GCS) and
all-off (all three devices E3038 + E3039)
all_multi — backup/intended run for veos, others E3038; compliance play emits E3039
all_single_run (veos) — all features run, no skips
all_single_skip (demo-cisco-xe) — E3038 per feature, E3039 suppressed (single device)
plans_create — config plan created for demo-cisco-xe (missing hostname/ntp); gated devices E3038

jeffkala · 2026-05-30T16:09:30Z

@jeffkala what assurance do you have that we are not introducing an N+1 issue here with settings here?

Will do some research on this.

@itdependsnetworks from my research of what N+1 is I think we're safe here. I don't see multiple queries happening that would indicate this is a concern we should have.

jeffkala · 2026-06-18T17:24:49Z

@@ -0,0 +1,47 @@
+# Generated by Django 5.2.14 on 2026-05-30 03:35
+


Move to true removal of nautobot_config and just pull from there during data migration.

keep this migrations
new second migration to update from nautobot_config values

look for deprecation warning from SSoT to follow existing pattern.

jeffkala · 2026-06-18T17:32:56Z

-            .order_by("-weight")
+            # Match Meta.ordering (`-weight`, `name`) so a weight tie resolves deterministically
+            # to the lower-sorted name rather than an arbitrary DB row.
+            .order_by("-weight", "name")


Maybe implement weight being unique? To avoid this "guess" of a winner.

jeffkala · 2026-06-18T17:39:33Z

        verbose_name="Backup Path in Jinja Template Form",
        help_text="The Jinja path representation of where the backup file will be found. The variable `obj` is available as the device instance object of a given device, as is the case for all Jinja templates. e.g. `{{obj.location.name|slugify}}/{{obj.name}}.cfg`",
    )
+    enable_backup = models.BooleanField(


change all these defaults from the plugin constant to just 'true"

jeffkala · 2026-06-18T17:43:25Z

-        if ENABLE_SOTAGG and not self.sot_agg_query:
-            raise ValidationError("A GraphQL query must be defined when `ENABLE_SOTAGG` is True")


add this back.

Also important going through all these iterations now that we don't have global + overrides in a GC setting

jeffkala · 2026-06-18T17:46:41Z

+# if GoldenConfigSetting.objects.filter(enable_compliance=True).exists():
+extensions.append(ConfigComplianceDeviceCheck)
+extensions.append(ConfigComplianceLocationCheck)
+extensions.append(ConfigComplianceTenantCheck)


don't do append add to actual extensions list

jeffkala · 2026-06-18T17:47:29Z

            "golden_config": golden_config,
            "template_type": "device-configs",
-            "config_features": CONFIG_FEATURES,
+            "config_features": {


validate its using same structure as before and no html template changes are needed.

jeffkala · 2026-06-18T17:52:47Z

+        # Gate generation on each device's winning GoldenConfigSetting.enable_plan flag —
+        # devices whose winning Setting has enable_plan=False are dropped here (E3038 names
+        # the Setting and weight; E3039 fires if nothing remains).
+        self._device_qs = filter_devices_by_feature_enabled(self.logger, self._device_qs, "plan")


Provide better docs on what this is doing and where its doing it.

jeffkala · 2026-06-18T17:54:27Z

double check filters are implemented for GCS list view.

gsnider2195

Looks good to me pending feedback added in today's review

jeffkala added 3 commits August 30, 2025 20:31

backups working

18f3851

intended working

66547fd

fixed gc settings table

f1f1ac3

jeffkala changed the title ~~New Golden Config Setting Enabled Per GC Setting (replaces #913 936)~~ New Golden Config Setting Enabled Per GC Setting (replaces 847 913 936) Sep 2, 2025

working run all single job

3bd19c2

jeffkala marked this pull request as ready for review September 26, 2025 21:11

jeffkala requested review from itdependsnetworks and nkallergis as code owners September 26, 2025 21:11

This was referenced Sep 26, 2025

Continued work on GC App Settings #936

Closed

WIP | Enable / Disable Settings in UI #847

Closed

Remove conditional UI views #914

Closed

jeffkala and others added 4 commits September 26, 2025 21:25

Merge branch 'develop' into gcs-aug25

4dd4ceb

merge in develop

d6a96f0

itdependsnetworks reviewed May 16, 2026

View reviewed changes

Comment thread docs/user/app_use_cases.md Outdated

itdependsnetworks reviewed May 16, 2026

View reviewed changes

Comment thread docs/user/app_use_cases.md Outdated

itdependsnetworks reviewed May 16, 2026

View reviewed changes

Comment thread nautobot_golden_config/nornir_plays/config_backup.py Outdated

Apply suggestions from code review

423b4c2

Co-authored-by: Ken Celenza <ken@celenza.org>

fix app use case verbiage in docs

b508ece

itdependsnetworks reviewed May 19, 2026

View reviewed changes

Comment thread nautobot_golden_config/nornir_plays/config_backup.py Outdated

itdependsnetworks reviewed May 19, 2026

View reviewed changes

Comment thread nautobot_golden_config/utilities/config_postprocessing.py Outdated

itdependsnetworks reviewed May 19, 2026

View reviewed changes

A round of updates for DRY principle and docs fixes

d2515f1