Skip to content

Conversation

@hurricanehrndz
Copy link
Contributor

Describe your changes

Host config should only be applied when it has changed

Issue ticket number and link

Stack

Checklist

  • Is it a bug fix
  • Is a typo/documentation fix
  • Is a feature enhancement
  • It is a refactor
  • Created tests that fail without the change (if possible)

By submitting this pull request, you confirm that you have read and agree to the terms of the Contributor License Agreement.

Documentation

Select exactly one:

  • I added/updated documentation for this change
  • Documentation is not needed for this change (explain why)

Docs PR URL (required if "docs added" is checked)

Paste the PR link from https://github.com/netbirdio/docs here:

https://github.com/netbirdio/docs/pull/__

Copilot AI review requested due to automatic review settings October 24, 2025 18:53
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR optimizes DNS host configuration updates by applying changes only when the configuration has actually changed, preventing unnecessary updates when the config remains identical.

Key Changes:

  • Added hash-based change detection to compare current and new DNS host configurations
  • Introduced currentConfigHash field to track the hash of the applied configuration

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

UseStringer: true,
})
if err != nil {
log.Errorf("unable to hash the host dns configuration, got error: %s", err)
Copy link

Copilot AI Oct 24, 2025

Choose a reason for hiding this comment

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

When hashing fails, the error is logged but execution continues with hash being 0. This could cause incorrect behavior where a zero hash might spuriously match s.currentConfigHash (which is also initialized to 0), preventing the first configuration from being applied. Consider returning early or setting a flag to force application when hashing fails.

Suggested change
log.Errorf("unable to hash the host dns configuration, got error: %s", err)
log.Errorf("unable to hash the host dns configuration, got error: %s", err)
return

Copilot uses AI. Check for mistakes.
}

if s.currentConfigHash == hash {
log.Infof("not applying host config as there are no changes")
Copy link

Copilot AI Oct 24, 2025

Choose a reason for hiding this comment

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

[nitpick] Changed 'Infof' to 'Debugf' for consistency with logging levels - configuration skip events are typically debug-level information rather than info-level.

Suggested change
log.Infof("not applying host config as there are no changes")
log.Debugf("not applying host config as there are no changes")

Copilot uses AI. Check for mistakes.
@sonarqubecloud
Copy link

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.

1 participant