Skip to content

Sync-DbaAvailabilityGroup: add -UseJobLastModified pass-through to Copy-DbaAgentJob - #10768

Merged
potatoqualitee merged 5 commits into
dataplat:developmentfrom
mrahman-DBA:Sync-DbaAvailabilityGroup_UseJobLastModified
Oct 2, 2026
Merged

potatoqualitee merged 5 commits into
dataplat:developmentfrom
mrahman-DBA:Sync-DbaAvailabilityGroup_UseJobLastModified

Conversation

@mrahman-DBA

@mrahman-DBA mrahman-DBA commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

Type of Change

  • Bug fix (non-breaking change, fixes # )
  • New feature (non-breaking change, adds functionality, fixes # )
  • Breaking change (affects multiple commands or functionality, fixes # )
  • Ran manual Pester test and has passed (Invoke-ManualPester -Path <command> -ScriptAnalyzer -Compliance)
  • Adding code coverage to existing functionality
  • Pester test is included
  • If new file reference added for test, has is been added to github.com/dataplat/appveyor-lab ?
  • Unit test is included
  • Documentation
  • Build system

Purpose

Depends on #10767 — the help text for -UseJobLastModified describes the behavior introduced there (definition comparison, in-place enabled update). Please merge #10767 first.

Sync-DbaAvailabilityGroup calls Copy-DbaAgentJob for the AgentJob step but has no way to pass -UseLastModified through. On an availability group that is synced regularly, that leaves two options for jobs that already exist on a secondary: skip them all (default), or drop and recreate every one of them (-Force). Neither keeps secondaries current without churning identical jobs on every run.

Approach

Adds a -UseJobLastModified switch that is passed through as -UseLastModified to Copy-DbaAgentJob. When set, agent jobs that already exist on a secondary are compared by definition and only updated when they differ, with the primary winning when it is not older; jobs that differ only in enabled state are updated in place.

The name follows the command's existing job-scoped switch (-DisableJobOnDestination) rather than reusing -UseLastModified, so it's clear from the parameter list that it affects the AgentJob step only and no other object type.

Commands to test

  • Sync-DbaAvailabilityGroup with and without -UseJobLastModified (jobs step only differs)

Tests

FYI: tests/Sync-DbaAvailabilityGroup.Tests.ps1 needs "UseJobLastModified" added to the expected-parameters list in the parameter validation test.

Passed through as -UseLastModified to Copy-DbaAgentJob for the AgentJob sync only; no other object type is affected. Instead of skipping every job that already exists on a secondary (or recreating all of them with -Force), jobs are compared by definition - properties, enabled state, steps and schedules - and only those that actually differ are updated, with the primary winning when it is not older than the secondary. Jobs that differ only in enabled state are updated in place.

See Copy-DbaAgentJob for the full behaviour
@andreasjordan

Copy link
Copy Markdown
Collaborator

Small, clean pass-through - thanks. Three points:

  1. Parameter test. Please add "UseJobLastModified" to the expected parameters in tests/Sync-DbaAvailabilityGroup.Tests.ps1 in this PR; otherwise the parameter validation test fails.
  2. Merge order. The switch works on development already, but the help text describes the new behaviour from Copy-DbaAgentJob: compare job definitions instead of date_modified for -UseLastModified #10767 (definition comparison, in-place enabled update). This should be merged after Copy-DbaAgentJob: compare job definitions instead of date_modified for -UseLastModified #10767 - please mention that in the description.
  3. Combination with -Force. In Copy-DbaAgentJob, -UseLastModified takes precedence over -Force, so Sync-DbaAvailabilityGroup -Force -UseJobLastModified does not overwrite jobs while -Force still applies to all other object types. One sentence in the parameter help would make that clear.

This text was created by Claude and reviewed by Andreas Jordan.

- Document that -UseJobLastModified takes precedence over -Force for jobs while -Force still applies to all other object types
- Add UseJobLastModified to the expected parameters test

- Extend the Copy-DbaAgentJob mock with -UseLastModified, which the new splat now passes, and add a pass-through test
@mrahman-DBA

Copy link
Copy Markdown
Contributor Author

The requested changes have been pushed through. Please review now

@potatoqualitee potatoqualitee left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking: the new -UseJobLastModified behavior has no required SQL-backed coverage. In ests/Sync-DbaAvailabilityGroup.Tests.ps1, the added case replaces Connect-DbaInstance, Get-DbaAgentJob, and Copy-DbaAgentJob with local functions and only asserts the captured switch. It therefore passes without executing synchronization against SQL Server. This conflicts with ests/CLAUDE.md, which requires every changed command behavior to execute the real implementation against a separately running boundary and says fabricated objects/doubles do not substitute.

Concrete gap: a real availability-group run could fail to propagate a changed job definition or preserve an identical job while this test remains green, because neither the real source/destination jobs nor Copy-DbaAgentJob run. Please add a focused integration case using real source and destination instances, invoke the real command with -UseJobLastModified (excluding unrelated object types), assert the resulting job definition/state, and clean up its fixtures. At minimum, prove that a repeated identical sync preserves the destination job ID and that a newer changed source definition propagates.

…ified

Replaces the mocked pass-through test with an integration Describe that runs the real command between InstanceCopy1 and InstanceCopy2 via -Primary/-Secondary, with every object type except AgentJob excluded:
- the first sync creates the job on the secondary
- a repeated identical sync leaves it untouched (same job_id and
  date_modified)
- a newer step change on the primary propagates to the secondary

The existing Copy-DbaAgentJob mock keeps -UseLastModified, which the command always passes. Referencing InstanceCopy* moves the file into the COPY scenario.

@potatoqualitee potatoqualitee left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the exact current head. The pass-through is wired correctly, parameter/help coverage is updated, and the prior test-coverage blocker is addressed with a real SQL-backed integration path that creates the job, proves an identical repeat leaves it untouched, and proves a newer changed step propagates. This PR still needs to land after #10767 as documented. The current Actions state is action_required, not a test failure. I found no material defect in this head.

@potatoqualitee
potatoqualitee merged commit f3280a3 into dataplat:development Oct 2, 2026
14 checks passed
@potatoqualitee

Copy link
Copy Markdown
Member

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants