Skip to content

[bug] Failed <commit> leaves candidate datastore dirty and blocks all subsequent commits (no auto-discard) #628

Description

@durd

Hi,
I found this with the help of Claude when looking into the line issue in NaC and sending an YANG-validated but device unsupported transport which caused the running datastore to become "dirty".

Let me know if I can do anything.

David

Description

Both EditConfig() and Commit() in internal/provider/helpers/netconf.go call client.Commit(ctx) and, on error, simply wrap and return the error:

// EditConfig, auto_commit=true path
if commit {
    if _, err := client.Commit(ctx); err != nil {
        return fmt.Errorf("failed to commit config: %s", FormatNetconfError(err))
    }
}
// Commit, used by iosxe_commit (device_transaction / auto_commit=false)
if _, err := client.Commit(ctx); err != nil {
    return fmt.Errorf("failed to commit config: %s", FormatNetconfError(err))
}

Neither function follows a failed <commit> with <discard-changes>. There is no DiscardChanges call anywhere in the codebase. This is shared helper code used by every generated resource (~274) plus iosxe_commit itself — not specific to any one resource.

Consequence: a <commit> can fail for reasons YANG schema validation can't catch (a value that's schema-valid but rejected by a platform-level check during candidate-to-CLI translation). When that happens, the candidate datastore is left holding the rejected content, uncommitted, indefinitely. Because <commit> commits the entire candidate, not just the change that triggered the current call, this blocks every other pending change in the same candidate too — not just the one that failed.

Reproduction (hardware-confirmed, C9200L, 17.15.4)

  1. Send an edit-config to candidate with a value that's valid per YANG schema but rejected by the platform (e.g. transport_input protocol acercon on iosxe_line, not supported on this platform). edit-config itself succeeds — schema validation passes.
  2. Attempt <commit>. Device rejects it: inconsistent value: Device refused one or more commands. Candidate is now dirty: a get-config diff between candidate and running shows the full rejected content still present in candidate, absent from running.
  3. Without discarding, add a second, entirely valid and unrelated change to candidate (e.g. a description on an unused loopback interface). edit-config for this second change succeeds on its own.
  4. Attempt <commit> again. Rejected with the same inconsistent value error — the valid, unrelated second change is blocked purely because the first change's residue is still sitting in candidate.
  5. Only an explicit out-of-band <discard-changes> clears this. Nothing in the provider does this automatically.

Root cause

EditConfig() and Commit() in internal/provider/helpers/netconf.go treat a failed client.Commit(ctx) as a terminal error to surface, not as a state requiring cleanup. No code path calls DiscardChanges after a commit failure.

Expected behavior

A failed <commit> should not leave the datastore in a state that blocks unrelated future commits. At minimum, the provider should attempt <discard-changes> after a failed commit before returning the error, so a single bad value doesn't silently block every other resource's apply in the same run. If auto-discarding is considered too aggressive to do silently, the returned error should at least explicitly tell the user the candidate is now dirty and requires a manual <discard-changes> before any further applies will succeed — right now the error message (failed to commit config: inconsistent value: ...) gives no indication that anything beyond the current resource is now blocked.

Suggested fix

In both EditConfig() and Commit(), on a client.Commit(ctx) error, call client.DiscardChanges(ctx) before returning. If the discard itself fails, wrap both errors together so the user knows manual intervention is needed rather than silently leaving candidate dirty.

Environment

  • IOS-XE 17.15.4, Catalyst C9200L-24P-4G
  • Terraform v1.15.8
  • registry.terraform.io/ciscodevnet/iosxe v1.0.0

Related

Originally investigated as a possible device_tracking-on-port-channel-specific quirk (netascode/terraform-iosxe-nac-iosxe context) — confirmed here to be unrelated to that resource entirely. The trigger was transport_input on iosxe_line purely because it was a convenient way to force a platform-level (not schema-level) commit rejection; the actual bug is in the shared commit-handling helpers, reproducible with any resource whose value can pass YANG validation but fail at commit.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions