Skip to content

Modify reconcile to run on driver initialization#463

Merged
shaynafinocchiaro merged 7 commits into
release/v2.14.1from
usr/sf/modify-reconcile
Aug 19, 2025
Merged

Modify reconcile to run on driver initialization#463
shaynafinocchiaro merged 7 commits into
release/v2.14.1from
usr/sf/modify-reconcile

Conversation

@shaynafinocchiaro

@shaynafinocchiaro shaynafinocchiaro commented Aug 19, 2025

Copy link
Copy Markdown
Collaborator

Description

Reconcile node labels on start up if feature is not disabled.
Port over changes from #458

GitHub Issues

List the GitHub issues impacted by this PR:

GitHub Issue #
https://github.com/dell/csm/issues/1991

Checklist:

  • I have performed a self-review of my own code to ensure there are no formatting, vetting, linting, or security issues
  • I have verified that new and existing unit tests pass locally with my changes
  • I have not allowed coverage numbers to degenerate
  • I have maintained at least 90% code coverage
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • Backward compatibility is not broken

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Please also list any relevant details for your test configuration

  • Removed node labels, immediately on installing driver labels are added back

@shaynafinocchiaro shaynafinocchiaro changed the title Usr/sf/modify reconcile Modify reconcile to run on driver initialization Aug 19, 2025
Comment thread service/service.go
azReconcileInterval := r.service.getReconcileInterval()

// On first iteration, reconcile labels then start the ticker
if azReconcileInterval > 0 {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should return early if azReconcileInterval == 0.

if azReconcileInterval  == 0 {
   log.Info("Reconcile is invalid value of 0. Must be greater than 0 to enable label reconciler.")
   return nil
}

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We need the check here too or else if azReconcileInterval = 0, it will still go into the go routine below and create a ticker with interval of 0:

ticker := time.NewTicker(r.service.getReconcileInterval())

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It also follows the pattern of returning early and doesn't require use of an else statement: https://danp.net/posts/reducing-go-nesting/

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Fixed

@shaynafinocchiaro shaynafinocchiaro merged commit 3c0848b into release/v2.14.1 Aug 19, 2025
6 checks passed
@shaynafinocchiaro shaynafinocchiaro deleted the usr/sf/modify-reconcile branch August 19, 2025 20:38
@shaynafinocchiaro shaynafinocchiaro mentioned this pull request Aug 20, 2025
10 tasks
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.

4 participants